api: add ingress visibility to ARM API and wire to Cluster Service by zgalor · Pull Request #5643 · Azure/ARO-HCP

zgalor · 2026-06-14T11:53:44Z

ARO-27757

Summary

Adds IngressProfile with visibility field (Public/Private) to the v2026-06-30-preview ARM API
Wires ingress visibility through to Cluster Service ingresses[0].listening (external/internal) during cluster creation
Field is immutable after creation and defaults to Public, matching the existing API visibility pattern

Changes

TypeSpec: Added IngressProfile model with @added(Versions.v2026_06_30_preview), updated Visibility union descriptions
Generated code: OpenAPI spec for v2026-06-30-preview, Go models, serde (auto-generated)
Internal types: Added CustomerIngressProfile struct, defaults in NewDefaultHCPOpenShiftCluster and EnsureDefaults
Versioned API (v20260630preview): Added newIngressProfile/normalizeIngress, wired into ConvertToExternal/ConvertToInternal/SetDefaults
Cross-version preservation: Added ingress to preserveUnknownClusterFields in both v20240610preview and v20251223preview
Validation: Added validateCustomerIngressProfile with required, immutable, and enum checks
CS conversion: Reuses convertVisibilityToListening to build Ingresses(NewIngressList().Items(NewIngress().Default(true).Listening(...))) in BuildCSCluster
Tests: Private ingress conversion test, ingress immutability validation test, defaults consistency tests (canonical↔constructor↔versioned), pre-existing data test, fuzz roundtrip exclusions for older API versions

Test plan

🤖 Generated with Claude Code

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds support for configuring cluster Ingress visibility for HCP OpenShift clusters across internal types, validation, API version conversions, and OCM cluster build output.

Changes:

Introduces CustomerIngressProfile / IngressProfile with visibility and defaulting behavior.
Adds validation for ingress visibility (required, enum, immutable on update) and updates unit tests.
Plumbs ingress visibility through OCM conversion and updates OpenAPI/spec + example payloads.

Reviewed changes

Copilot reviewed 21 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/validation/validate_cluster.go	Adds ingress validation hook and implements `validateCustomerIngressProfile`.
internal/validation/hcpopenshiftcluster_test.go	Extends required-field validation test coverage for ingress visibility.
internal/ocm/convert.go	Maps ingress visibility to OCM “Ingresses[].Listening”.
internal/ocm/convert_test.go	Updates expected OCM cluster builders to include default ingress listening.
internal/api/v20251223preview/hcpopenshiftclusters_methods.go	Defaults, (de)normalization, and versioned shape for ingress profile.
internal/api/v20240610preview/hcpopenshiftclusters_methods.go	Preserves new ingress field across older API version conversions.
internal/api/v20240610preview/conversion_fuzz_test.go	Adjusts fuzz round-trip behavior to account for new ingress field.
internal/api/types_cluster.go	Adds internal `CustomerIngressProfile` and defaults in constructors/EnsureDefaults.
api/.../preview/2025-12-23-preview/openapi.json	Adds `IngressProfile` definition + property, updates Visibility enum descriptions.
api/.../preview/2024-06-10-preview/openapi.json	Updates Visibility enum descriptions.
api/.../hcpCluster-models.tsp	Adds ingress to the TypeSpec model and refines Visibility doc text.
api/.../examples/*/2025-12-23-preview/.json	Updates example payloads to include `properties.ingress.visibility`.

+		ingressListening, err := convertVisibilityToListening(hcpCluster.CustomerProperties.Ingress.Visibility)
+		if err != nil {
+			return nil, nil, err
+		}
+		clusterBuilder.Ingresses(arohcpv1alpha1.NewIngressList().Items(
+			arohcpv1alpha1.NewIngress().Default(true).Listening(ingressListening),
+		))


mbarnes · 2026-06-15T13:11:25Z

        },
+        "ingress": {
+          "$ref": "#/definitions/IngressProfile",
+          "description": "Shows the cluster ingress profile",


Agree with the suggestion.

Copilot

Pull request overview

Copilot reviewed 22 out of 26 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

+        "ingress": {
+          "$ref": "#/definitions/IngressProfile",
+          "description": "Shows the cluster ingress profile",
+          "x-ms-mutability": [
+            "read",
+            "create"
+          ]


Copilot

Pull request overview

Copilot reviewed 115 out of 119 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

mbarnes · 2026-06-15T13:23:45Z

+	if obj.Properties.Ingress == nil {
+		obj.Properties.Ingress = &generated.IngressProfile{}
+	}
+	if obj.Properties.Ingress.Visibility == nil {
+		obj.Properties.Ingress.Visibility = ptr.To(generated.VisibilityPublic)
+	}


I'm less familiar with patching behavior than I used to be, but this is following the same pattern as APIProfile. I don't see a problem.

        },
+        "ingress": {
+          "$ref": "#/definitions/IngressProfile",
+          "description": "Shows the cluster ingress profile",


Copilot

Pull request overview

Copilot reviewed 38 out of 42 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

+    "IngressProfile": {
+      "type": "object",
+      "description": "Information about the Ingress of a cluster.",
+      "properties": {
+        "visibility": {
+          "type": "string",
+          "description": "The internet visibility of the OpenShift Ingress.",
+          "default": "Public",
+          "enum": [
+            "Public",
+            "Private"
+          ],
+          "x-ms-enum": {
+            "name": "Visibility",
+            "modelAsString": true,
+            "values": [
+              {
+                "name": "Public",
+                "value": "Public",
+                "description": "The endpoint is visible from the internet."
+              },
+              {
+                "name": "Private",
+                "value": "Private",
+                "description": "The endpoint is not visible from the internet."
+              }
+            ]
+          },
+          "x-ms-mutability": [
+            "read",
+            "create"
+          ]
+        }
+      }
+    },


zgalor · 2026-06-15T11:12:37Z

Note: The e2e-parallel failure is expected — this PR sends the ingresses field to Cluster Service on cluster creation, but CS doesn't accept it yet. This depends on CS MR !353 (ARO-26911) being merged and deployed to the test environment.

miguelsorianod · 2026-06-15T11:26:47Z

            "visibility": "Public",
            "url": "https://api.test.shrd.usw3test.hcp.osadev.cloud:443"
          },
+          "ingress": {


Does this control the default ingress visibility, or the visibility of all ingresses created over time? Depending on the answer, the naming can be confusing.

Are we always going to offer a default ingress, or is it possible or being discussed that that will be on end-users responsibility?

This controls the default ingress visibility - the single default OpenShift ingress that's created with the cluster. It maps to CS's ingresses[0].default=true with the corresponding listening method (external/internal).

ARO HCP clusters have a single ingress configuration exposed through the ARM API. The field is immutable after creation. The naming follows the existing ApiProfile pattern (singular) since it represents a single cluster-level configuration, not a collection.

Should we call it defaultIngress then?

I don't think we should. I think ARO HCP doesn't have the concept of default ingress.
@bennerv @mbarnes keep me honest here

Isn't default ingress an OpenShift concept (https://github.com/openshift/cluster-ingress-operator#installing) ?

zgalor · 2026-06-15T12:16:09Z

@bennerv @mbarnes PTAL

mbarnes

Two issues:

This should be added to the 2026-06-30-preview version, not 2025-12-23-preview, although I realize that's not a fair ask at this moment since we haven't yet merged the scaffolding for the new version yet.
Either way, we'll need some new test cases for the new "public" ingress default in /internal/database/convert_defaults_consistency_test.go and I think a couple of the integration tests. There's a DDR in-repo about default handling across API versions, with a detailed checklist for new fields.

mbarnes · 2026-06-15T13:11:25Z

        },
+        "ingress": {
+          "$ref": "#/definitions/IngressProfile",
+          "description": "Shows the cluster ingress profile",


Agree with the suggestion.

mbarnes · 2026-06-15T13:23:45Z

+	if obj.Properties.Ingress == nil {
+		obj.Properties.Ingress = &generated.IngressProfile{}
+	}
+	if obj.Properties.Ingress.Visibility == nil {
+		obj.Properties.Ingress.Visibility = ptr.To(generated.VisibilityPublic)
+	}


I'm less familiar with patching behavior than I used to be, but this is following the same pattern as APIProfile. I don't see a problem.

Add IngressProfile with a visibility field (Public/Private) to the v2025-12-23-preview ARM API, allowing customers to configure private ingress at cluster creation time. The field is immutable after creation and defaults to Public. The ingress visibility is converted to CS ingresses[0].listening (external/internal) and sent during cluster creation, reusing the existing convertVisibilityToListening mapping. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

openshift-ci · 2026-06-17T08:40:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zgalor
Once this PR has been reviewed and has the lgtm label, please assign geoberle for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Copilot reviewed 40 out of 44 changed files in this pull request and generated no new comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

Regenerate v2024 and v2025 openapi.json to pick up the updated Visibility description ("endpoint" instead of "API server"). Fix APIProfile field ordering in v2026 generated models to match CI's autorest output. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Add v2026→v2024 and v2026→v2025 cross-version PUT and PATCH tests to verify that ingress visibility (a v2026-only field) is preserved when a cluster is updated via older API versions. Also add ingress to the v2026 cluster create payload with visibility=Private. Addresses DDR checklist item 6f from docs/api-version-defaults-and-storage.md. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Copilot

Pull request overview

Copilot reviewed 43 out of 47 changed files in this pull request and generated 2 comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

Comments suppressed due to low confidence (6)

test-integration/frontend/cross_version_roundtrip_test.go:1

These 4 new integration tests each create a fresh v2026 cluster, which can significantly increase CI time and flakiness for the integration suite. Consider reducing cluster creates by combining PUT+PATCH assertions per version pair (create once, snapshot before, run both mutations sequentially with separate comparisons) or by reusing a single created cluster across subtests where isolation isn’t required.
test-integration/frontend/cross_version_roundtrip_test.go:1
These 4 new integration tests each create a fresh v2026 cluster, which can significantly increase CI time and flakiness for the integration suite. Consider reducing cluster creates by combining PUT+PATCH assertions per version pair (create once, snapshot before, run both mutations sequentially with separate comparisons) or by reusing a single created cluster across subtests where isolation isn’t required.
test-integration/frontend/cross_version_roundtrip_test.go:1
These 4 new integration tests each create a fresh v2026 cluster, which can significantly increase CI time and flakiness for the integration suite. Consider reducing cluster creates by combining PUT+PATCH assertions per version pair (create once, snapshot before, run both mutations sequentially with separate comparisons) or by reusing a single created cluster across subtests where isolation isn’t required.
test-integration/frontend/cross_version_roundtrip_test.go:1
These 4 new integration tests each create a fresh v2026 cluster, which can significantly increase CI time and flakiness for the integration suite. Consider reducing cluster creates by combining PUT+PATCH assertions per version pair (create once, snapshot before, run both mutations sequentially with separate comparisons) or by reusing a single created cluster across subtests where isolation isn’t required.
test-integration/frontend/cross_version_roundtrip_test.go:1
The new cross-version cluster tests are largely copy/paste (v2024 vs v2025, PUT vs PATCH). This repetition makes it easy for future version additions to drift (e.g., different logging, missed steps). Consider factoring the shared flow into a helper that takes (roundTripVersion, method PUT/PATCH, mutation body) and uses table-driven cases to generate the test permutations.
test-integration/frontend/cross_version_roundtrip_test.go:1
The new cross-version cluster tests are largely copy/paste (v2024 vs v2025, PUT vs PATCH). This repetition makes it easy for future version additions to drift (e.g., different logging, missed steps). Consider factoring the shared flow into a helper that takes (roundTripVersion, method PUT/PATCH, mutation body) and uses table-driven cases to generate the test permutations.

+  /** Shows the cluster ingress profile */
+  @added(Versions.v2026_06_30_preview)
+  @visibility(Lifecycle.Read, Lifecycle.Create)
+  ingress?: IngressProfile;


+/** Information about the Ingress of a cluster. */
+@added(Versions.v2026_06_30_preview)
+model IngressProfile {
+  /** The internet visibility of the OpenShift Ingress. */
+  @added(Versions.v2026_06_30_preview)
+  @visibility(Lifecycle.Read, Lifecycle.Create)
+  visibility?: Visibility = Visibility.Public;
+}


Change "Shows the cluster ingress profile" to "The cluster ingress configuration" since this is a configurable property (Read+Create), not a status field. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

zgalor · 2026-06-17T12:10:43Z

@mbarnes, I addressed your comments.

Moved the ned field to 2026-06-30-preview version and added integration tests according to the DDR you shared. Note that some e2e might fail until the updated CS is rolled out.

Please take another look

mbarnes · 2026-06-17T12:36:49Z

For the ci/prow/api-validation error, it looks like you might just need to run make -C api generate again.

Overall LGTM. I'll give formal approval once api-validation is fixed.

Copilot

Pull request overview

Copilot reviewed 43 out of 47 changed files in this pull request and generated 3 comments.

Files not reviewed (1)

internal/api/zz_generated.deepcopy.go: Generated file

Comments suppressed due to low confidence (1)

test-integration/frontend/cross_version_roundtrip_test.go:1

These new cross-version cluster tests are highly repetitive (only the 'older version' and operation differ). Consider consolidating into a small table-driven helper (e.g., parameters: olderVersion, method PUT/PATCH, createVersion=v2026, expectedPreservedFields) to reduce duplication and make it easier to extend coverage for future API versions.

 	if obj.Properties.API.Visibility == nil {
 		obj.Properties.API.Visibility = ptr.To(generated.VisibilityPublic)
 	}
+	if obj.Properties.Ingress == nil {
+		obj.Properties.Ingress = &generated.IngressProfile{}
+	}
+	if obj.Properties.Ingress.Visibility == nil {
+		obj.Properties.Ingress.Visibility = ptr.To(generated.VisibilityPublic)
+	}


+func normalizeIngress(p *generated.IngressProfile, out *api.CustomerIngressProfile) {
+	out.Visibility = api.Visibility(api.Deref(p.Visibility))
+}


+	if obj.Properties.Ingress == nil {
+		obj.Properties.Ingress = &generated.IngressProfile{}
+	}


Sync the comment in generated models.go with the TypeSpec description change ("The cluster ingress configuration"). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

openshift-ci · 2026-06-17T15:50:34Z

@zgalor: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-parallel	`57891df`	link	true	`/test e2e-parallel`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copilot AI review requested due to automatic review settings June 14, 2026 11:53

openshift-ci Bot requested review from mbarnes and miguelsorianod June 14, 2026 11:54

Copilot AI reviewed Jun 14, 2026

View reviewed changes

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch from 0288fe3 to f9d01c6 Compare June 14, 2026 12:00

Copilot AI review requested due to automatic review settings June 14, 2026 14:30

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch from f9d01c6 to d5a3a6c Compare June 14, 2026 14:30

Copilot AI reviewed Jun 14, 2026

View reviewed changes

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch 2 times, most recently from e80486f to 4acc332 Compare June 15, 2026 06:20

Copilot AI review requested due to automatic review settings June 15, 2026 06:20

Copilot AI reviewed Jun 15, 2026

View reviewed changes

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch 2 times, most recently from e0bde11 to 5bd837f Compare June 15, 2026 08:35

Copilot AI review requested due to automatic review settings June 15, 2026 08:35

Copilot AI reviewed Jun 15, 2026

View reviewed changes

miguelsorianod reviewed Jun 15, 2026

View reviewed changes

mbarnes requested changes Jun 15, 2026

View reviewed changes

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch from 5bd837f to 68fcfbb Compare June 16, 2026 13:01

Copilot AI review requested due to automatic review settings June 17, 2026 08:40

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch from 68fcfbb to cc01099 Compare June 17, 2026 08:40

Copilot AI reviewed Jun 17, 2026

View reviewed changes

zgalor and others added 2 commits June 17, 2026 12:46

Copilot AI review requested due to automatic review settings June 17, 2026 11:22

Copilot AI reviewed Jun 17, 2026

View reviewed changes

fix: improve ingress property description in TypeSpec

533b138

Change "Shows the cluster ingress profile" to "The cluster ingress configuration" since this is a configurable property (Read+Create), not a status field. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Copilot AI review requested due to automatic review settings June 17, 2026 14:40

Copilot AI reviewed Jun 17, 2026

View reviewed changes

fix: update ingress description in generated models

57891df

Sync the comment in generated models.go with the TypeSpec description change ("The cluster ingress configuration"). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

zgalor force-pushed the zgalor/aro-26912-private-ingress-frontend branch from 6938933 to 57891df Compare June 17, 2026 14:45

Conversation

zgalor commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

zgalor commented Jun 15, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miguelsorianod Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zgalor commented Jun 15, 2026

Uh oh!

mbarnes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openshift-ci Bot commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

zgalor commented Jun 17, 2026

Uh oh!

mbarnes commented Jun 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

openshift-ci Bot commented Jun 17, 2026

Uh oh!

zgalor commented Jun 14, 2026 •

edited

Loading

zgalor commented Jun 15, 2026 •

edited by openshift-ci Bot

Loading

miguelsorianod Jun 15, 2026 •

edited

Loading