Skip to content

feat(frontend): reject duplicate subnet, NSG, and MRG on cluster create - #6004

Closed
Jakob Gray (JakobGray) wants to merge 4 commits into
Azure:mainfrom
JakobGray:cluster-install-admission
Closed

Jakob Gray (JakobGray) wants to merge 4 commits into
Azure:mainfrom
JakobGray:cluster-install-admission

Conversation

@JakobGray

Copy link
Copy Markdown
Collaborator

What

  • Prefetch subscription clusters and node pools in newClusterAdmissionContext
  • Add admitClusterManagedResourceGroupName, admitClusterSubnetResourceID, and admitClusterNetworkSecurityGroupResourceID (CREATE only)
  • Add NSG same-subscription and not-in-MRG rules to static validation (parity with validateAroHcpClusterNetworkSecurityGroupResourceId)

This change closes the following gaps in validation during cluster create between CS and ARO-HCP:

  • Duplicate MRG in subscription
  • Duplicate subnet on another cluster
  • Duplicate subnet on another cluster’s node pool
  • Duplicate NSG on another cluster
  • NSG in wrong subscription
  • NSG in managed RG

These mirror checks done in CS before a cluster create is accepted:

  • validateAroHcpClusterNetworkSecurityGroupResourceId
  • validateManagedResourceGroupUniqueness
  • validateSubnetUniqueness
  • validateNsgUniqueness

These checks are best-effort. Race conditions are possible for clusters admitted at the same time.

Why

Cluster Service no longer returns synchronous 400s for platform uniqueness checks after async CS create migration. Move those checks to the frontend so invalid cluster PUTs fail at admission time with the same error messages CS used in performSpecValidation.

Testing

Added unit tests for new functions. Modified integration tests where new errors were occurring because they are now being caught by the admission validation. Tested against existing E2E tests around subnet/NSG/MRG reuse between clusters (test/e2e/cluster_nsg_subnet_reuse.go and test/e2e/clusters_sharing_resgroup.go)

Special notes for your reviewer

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes)
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge

If E2E tests are included:

  • E2E tests follow Principles of Good E2E Test Case Design
  • If new E2E use case is covered (via a new test or new check/verifier),
    demonstrate that the test is able to detect a defect/error and fail with
    proper error message and logs which communicates nature of the problem.

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JakobGray
Once this PR has been reviewed and has the lgtm label, please assign deads2k 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:

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

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR moves several platform “uniqueness” and NSG placement checks (previously enforced by Cluster Service) into the frontend’s admission/static validation so invalid cluster creates fail early with consistent error messages.

Changes:

  • Prefetch subscription-scoped clusters and node pools during cluster CREATE admission and add best-effort checks rejecting duplicate managed RG, subnet, and NSG usage across clusters/node pools.
  • Extend static validation so networkSecurityGroupId must be in the same subscription and must not be in the cluster’s managed resource group.
  • Update unit/integration tests and test artifacts to reflect newly enforced CREATE-time validation behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test-integration/frontend/artifacts/FrontendCRUD/Cluster/experimental-features-no-afec/04-httpCreate-cluster-with-4.19/create-with-4.19.json Updates test create payload resource IDs to avoid conflicts with new admission checks.
internal/validation/validate_cluster.go Adds NSG “same subscription” and “not in managed RG” checks at the cluster-level validation layer.
internal/validation/validate_cluster_comprehensive_test.go Adds/adjusts validation test coverage for new NSG subscription/RG constraints.
internal/validation/hcpopenshiftcluster_test.go Updates test cluster setup to include an NSG ID consistent with the new rules.
internal/admission/admit_cluster.go Introduces CREATE-only cross-cluster/nodepool uniqueness admission checks for MRG/subnet/NSG.
internal/admission/admit_cluster_test.go Adds unit tests verifying the new CREATE-only admission checks.
frontend/pkg/frontend/cluster.go Prefetches subscription clusters and their node pools to populate the admission context on CREATE.

Comment thread internal/admission/admit_cluster.go
Comment on lines +299 to +300
nodePoolSubnet := nodePool.Properties.Platform.SubnetID
if strings.EqualFold(subnetID, nodePoolSubnet.String()) {
Comment thread internal/admission/admit_cluster.go Outdated
Comment thread internal/admission/admit_cluster.go
Comment thread internal/admission/admit_cluster.go Outdated
Comment on lines +282 to +284
subnetPath := fldPath.Child("subnetId")
subnetID := newObj.SubnetID.String()
var errs field.ErrorList
Comment on lines +324 to +326
nsgPath := fldPath.Child("networkSecurityGroupId")
nsgID := newObj.NetworkSecurityGroupID.String()
var errs field.ErrorList
Copilot AI review requested due to automatic review settings July 10, 2026 04:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Comment thread internal/admission/admit_cluster.go
Comment on lines +299 to +300
nodePoolSubnet := nodePool.Properties.Platform.SubnetID
if strings.EqualFold(subnetID, nodePoolSubnet.String()) {
Comment thread internal/admission/admit_cluster.go
Comment thread internal/admission/admit_cluster.go Outdated
Comment thread internal/admission/admit_cluster.go
Copilot AI review requested due to automatic review settings July 10, 2026 05:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +286 to +296
for _, existing := range admissionContext.SubscriptionClusters {
existingSubnet := existing.CustomerProperties.Platform.SubnetID
if strings.EqualFold(subnetID, existingSubnet.String()) {
errs = append(errs, field.Invalid(
subnetPath,
subnetID,
fmt.Sprintf("Subnet '%s' is already in use by another cluster", subnetID),
))
break
}
}
Comment on lines +298 to +308
for _, nodePool := range admissionContext.SubscriptionNodePools {
nodePoolSubnet := nodePool.Properties.Platform.SubnetID
if strings.EqualFold(subnetID, nodePoolSubnet.String()) {
errs = append(errs, field.Invalid(
subnetPath,
subnetID,
fmt.Sprintf("Subnet '%s' is already in use by another cluster", subnetID),
))
break
}
}
Comment on lines +327 to +337
for _, existing := range admissionContext.SubscriptionClusters {
existingNSG := existing.CustomerProperties.Platform.NetworkSecurityGroupID
if strings.EqualFold(nsgID, existingNSG.String()) {
errs = append(errs, field.Invalid(
nsgPath,
nsgID,
fmt.Sprintf("Network Security Group '%s' is already in use by another cluster", nsgID),
))
break
}
}
Comment thread internal/admission/admit_cluster.go Outdated
// ClusterNodePools is the list of node pools belonging to the cluster, used
// for minor-version skew checks against the desired cluster version.
ClusterNodePools []ClusterAdmissionNodePool
// SubscriptionClusters lists cluster documents in the same subscription, used

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document whether it includes the cluster being processed itself or not

Comment thread internal/admission/admit_cluster.go Outdated
// used to ensure a cluster subnet is not already assigned to another cluster's
// node pool on CREATE.
// The list is empty on UPDATE.
SubscriptionNodePools []*api.HCPOpenShiftClusterNodePool

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check this? don't we check that for nodepools the subnet must be part of the VNet of the cluster?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replicates the ValidateSubnetNotUsedByAnotherClusterNodePools() validation in CS. Node pool subnet must be in the same VNet as its parent cluster, but VNet sharing across clusters is not prevented

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding subnets, does the following summarize the behavior?

in CS:

  • We allow reusing VNets between clusters
  • We do not allow reusing Subnets between clusters
  • We enforce that the node pool subnets must belong to the parent cluster VNet
  • We allow reusing Subnets between Node Pools within the same cluster
  • We do not allow reusing Subnets between Node Pools across clusters

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That all sounds correct

Comment thread internal/admission/admit_cluster.go Outdated
// prefetched before admission runs. Concurrent creates (or a create racing with a
// node pool create) using the same subnet can both succeed.
func admitClusterSubnetResourceID(_ context.Context, admissionContext *ClusterAdmissionContext, op operation.Operation, fldPath *field.Path, newObj *api.CustomerPlatformProfile) field.ErrorList {
if op.Type != operation.Create || newObj.SubnetID == nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can newObj.SubnetID be nil at this point? if not, remove the check

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a regular end-to-end flow it can be expected that the field is set and caught further upstream. In unit or integration tests it could be that a cluster is defined that is not fully formed because it is isolating focus on something else. If the subnet isn't set it is safe to say there is no uniqueness concern and the validation can return early.

@miguelsorianod Miguel Soriano (miguelsorianod) Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't condition the logic to unit tests/integration tests

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we run admission even when we have validation failures. Because of this, this needs a check to avoid panicking. To be safe we can return an error if the assumption of the subnet being set is broken.

Comment thread internal/admission/admit_cluster.go Outdated
// Best-effort only: compares against SubscriptionClusters prefetched before
// admission runs. Concurrent creates with the same MRG name can both succeed.
func admitClusterManagedResourceGroupName(_ context.Context, admissionContext *ClusterAdmissionContext, op operation.Operation, fldPath *field.Path, newObj *api.CustomerPlatformProfile) field.ErrorList {
if op.Type != operation.Create || len(newObj.ManagedResourceGroup) == 0 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can managedresourcegroup be nil at this point? if not, remove the check

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #6004 (comment)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't condition the logic to unit tests/integration tests

Comment thread internal/admission/admit_cluster.go Outdated
// Best-effort only: compares against SubscriptionClusters prefetched before
// admission runs. Concurrent creates with the same NSG can both succeed.
func admitClusterNetworkSecurityGroupResourceID(_ context.Context, admissionContext *ClusterAdmissionContext, op operation.Operation, fldPath *field.Path, newObj *api.CustomerPlatformProfile) field.ErrorList {
if op.Type != operation.Create || newObj.NetworkSecurityGroupID == nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can newObj.SubnetID be nil at this point? if not, remove the check

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #6004 (comment)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't condition the logic to unit tests/integration tests

Copilot AI review requested due to automatic review settings July 13, 2026 18:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comment thread internal/admission/admit_cluster.go Outdated
Comment on lines +243 to +245
errs = append(errs, admitClusterManagedResourceGroupName(ctx, admissionContext, op, fldPath, &newObj)...)
errs = append(errs, admitClusterSubnetResourceID(ctx, admissionContext, op, fldPath, &newObj)...)
errs = append(errs, admitClusterNetworkSecurityGroupResourceID(ctx, admissionContext, op, fldPath, &newObj)...)
Comment on lines +293 to +303
for _, existing := range admissionContext.SubscriptionClusters {
existingSubnet := existing.CustomerProperties.Platform.SubnetID
if strings.EqualFold(subnetID, existingSubnet.String()) {
errs = append(errs, field.Invalid(
subnetPath,
subnetID,
fmt.Sprintf("Subnet '%s' is already in use by another cluster", subnetID),
))
break
}
}
Comment on lines +305 to +315
for _, nodePool := range admissionContext.SubscriptionNodePools {
nodePoolSubnet := nodePool.Properties.Platform.SubnetID
if strings.EqualFold(subnetID, nodePoolSubnet.String()) {
errs = append(errs, field.Invalid(
subnetPath,
subnetID,
fmt.Sprintf("Subnet '%s' is already in use by another cluster", subnetID),
))
break
}
}
Comment on lines +334 to +344
for _, existing := range admissionContext.SubscriptionClusters {
existingNSG := existing.CustomerProperties.Platform.NetworkSecurityGroupID
if strings.EqualFold(nsgID, existingNSG.String()) {
errs = append(errs, field.Invalid(
nsgPath,
nsgID,
fmt.Sprintf("Network Security Group '%s' is already in use by another cluster", nsgID),
))
break
}
}
Copilot AI review requested due to automatic review settings July 13, 2026 19:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment on lines +827 to +830
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

Copilot AI review requested due to automatic review settings July 14, 2026 15:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

Comment thread internal/validation/validators.go Outdated
return field.ErrorList{field.Invalid(
fldPath,
value.String(),
fmt.Sprintf("must belong to the same VNet as subnetId '%s'", otherSubnet.Parent.String()),
Comment on lines +263 to +266
mrgPath := fldPath.Child("managedResourceGroup")
if len(newObj.ManagedResourceGroup) == 0 {
return field.ErrorList{field.InternalError(mrgPath, errors.New("managedResourceGroup must be set"))}
}
Comment on lines +298 to +301
subnetPath := fldPath.Child("subnetId")
if newObj.SubnetID == nil {
return field.ErrorList{field.InternalError(subnetPath, errors.New("subnetId must be set"))}
}
Comment on lines +306 to +310
existingSubnet := existing.CustomerProperties.Platform.SubnetID
if existingSubnet == nil {
errs = append(errs, field.InternalError(subnetPath, errors.New("existing cluster is missing subnetId")))
continue
}
Comment on lines +322 to +326
nodePoolSubnet := nodePool.Properties.Platform.SubnetID
if nodePoolSubnet == nil {
errs = append(errs, field.InternalError(subnetPath, errors.New("existing node pool is missing subnetId")))
continue
}
Comment on lines +350 to +353
nsgPath := fldPath.Child("networkSecurityGroupId")
if newObj.NetworkSecurityGroupID == nil {
return field.ErrorList{field.InternalError(nsgPath, errors.New("networkSecurityGroupId must be set"))}
}
Comment on lines +358 to +362
existingNSG := existing.CustomerProperties.Platform.NetworkSecurityGroupID
if existingNSG == nil {
errs = append(errs, field.InternalError(nsgPath, errors.New("existing cluster is missing networkSecurityGroupId")))
continue
}
Comment on lines +269 to +288
if op.Type == operation.Create {
subscriptionID := originalCluster.ID.SubscriptionID
clusterIterator, err := f.resourcesDBClient.HCPClusters(subscriptionID, "").List(ctx, nil)
if err != nil {
return nil, fmt.Errorf("cannot list clusters for cluster admission: %w", err)
}
for _, cluster := range clusterIterator.Items(ctx) {
admissionContext.SubscriptionClusters = append(admissionContext.SubscriptionClusters, cluster)

nodePoolIterator, err := f.resourcesDBClient.HCPClusters(subscriptionID, cluster.ID.ResourceGroupName).NodePools(cluster.ID.Name).List(ctx, nil)
if err != nil {
return nil, fmt.Errorf("cannot list node pools for cluster admission: %w", err)
}
for _, nodePool := range nodePoolIterator.Items(ctx) {
admissionContext.SubscriptionNodePools = append(admissionContext.SubscriptionNodePools, nodePool)
}
if err := nodePoolIterator.GetError(); err != nil {
return nil, fmt.Errorf("cannot list node pools for cluster admission: %w", err)
}
}
Comment thread internal/api/enums.go
Comment thread internal/api/enums.go Outdated
// EtcdDataEncryptionKeyManagementModeTypeCustomerManaged - Customer managed encryption key management mode type.
EtcdDataEncryptionKeyManagementModeTypeCustomerManaged EtcdDataEncryptionKeyManagementModeType = "CustomerManaged"
// EtcdDataEncryptionKeyManagementModeTypePlatformManaged - Platform managed encryption key management mode type.
// Not currently supported by Cluster Service; left defined so EnsureDefaults / Cosmos defaults keep

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to mention "by Cluster Service". Our service in general doesn't support it.

Comment thread internal/validation/validate_cluster.go Outdated
observed[key] = identityPath
}

for operatorName, identity := range newObj.ControlPlaneOperators {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without sorting, are the tests deterministic?

Jakob Gray (JakobGray) and others added 3 commits July 14, 2026 12:55
Cluster Service no longer returns synchronous 400s for platform
uniqueness checks after async CS create migration. Move those checks to
the frontend so invalid cluster PUTs fail at admission time with the same
error messages CS used in performSpecValidation. These checks are best-effort
and may not catch concurrent creates.

- Prefetch subscription clusters and node pools in newClusterAdmissionContext
- Add admitClusterManagedResourceGroupName, admitClusterSubnetResourceID,
  and admitClusterNetworkSecurityGroupResourceID (CREATE only)
- Add NSG same-subscription and not-in-MRG rules to static validation
  (parity with validateAroHcpClusterNetworkSecurityGroupResourceId)

Co-authored-by: Cursor <cursoragent@cursor.com>
Mirror CS validateAroHcpSwiftSubnetSameVnet: vnetIntegrationSubnetId must
use the same VNet as subnetId.

Co-authored-by: Cursor <cursoragent@cursor.com>
Mirror CS validateAzureOperatorsAuthenticationManagedIdentitiesUniqueWithinCluster
across service, control-plane, and data-plane operator identities.
Copilot AI review requested due to automatic review settings July 14, 2026 17:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +298 to +301
subnetPath := fldPath.Child("subnetId")
if newObj.SubnetID == nil {
return field.ErrorList{field.Required(subnetPath, "")}
}
Comment on lines +350 to +353
nsgPath := fldPath.Child("networkSecurityGroupId")
if newObj.NetworkSecurityGroupID == nil {
return field.ErrorList{field.Required(nsgPath, "")}
}
Comment on lines +269 to +289
if op.Type == operation.Create {
subscriptionID := originalCluster.ID.SubscriptionID
clusterIterator, err := f.resourcesDBClient.HCPClusters(subscriptionID, "").List(ctx, nil)
if err != nil {
return nil, fmt.Errorf("cannot list clusters for cluster admission: %w", err)
}
for _, cluster := range clusterIterator.Items(ctx) {
admissionContext.SubscriptionClusters = append(admissionContext.SubscriptionClusters, cluster)

nodePoolIterator, err := f.resourcesDBClient.HCPClusters(subscriptionID, cluster.ID.ResourceGroupName).NodePools(cluster.ID.Name).List(ctx, nil)
if err != nil {
return nil, fmt.Errorf("cannot list node pools for cluster admission: %w", err)
}
for _, nodePool := range nodePoolIterator.Items(ctx) {
admissionContext.SubscriptionNodePools = append(admissionContext.SubscriptionNodePools, nodePool)
}
if err := nodePoolIterator.GetError(); err != nil {
return nil, fmt.Errorf("cannot list node pools for cluster admission: %w", err)
}
}
if err := clusterIterator.GetError(); err != nil {
Comment thread internal/api/enums.go
Comment on lines 164 to 178
// EtcdDataEncryptionKeyManagementModeTypeCustomerManaged - Customer managed encryption key management mode type.
EtcdDataEncryptionKeyManagementModeTypeCustomerManaged EtcdDataEncryptionKeyManagementModeType = "CustomerManaged"
// EtcdDataEncryptionKeyManagementModeTypePlatformManaged - Platform managed encryption key management mode type.
// Not currently supported; left defined so EnsureDefaults / Cosmos defaults keep
// filling the historic value, but excluded from ValidEtcdDataEncryptionKeyManagementModeType until
// platform-managed etcd encryption is supported.
EtcdDataEncryptionKeyManagementModeTypePlatformManaged EtcdDataEncryptionKeyManagementModeType = "PlatformManaged"
)

var (
ValidEtcdDataEncryptionKeyManagementModeType = sets.New[EtcdDataEncryptionKeyManagementModeType](
EtcdDataEncryptionKeyManagementModeTypeCustomerManaged,
EtcdDataEncryptionKeyManagementModeTypePlatformManaged,
// TODO: re-enable once platform-managed etcd encryption is supported.
// EtcdDataEncryptionKeyManagementModeTypePlatformManaged,
)
Comment on lines +259 to +268
"etcd": {
"dataEncryption": {
"keyManagementMode": "PlatformManaged"
"customerManaged": {
"encryptionType": "KMS",
"kms": {
"activeKey": {
"name": "vc-encryption-key",
"vaultName": "vc-key-vault",
"version": "2024-12-01-preview"
}
Comment on lines +321 to +330
"etcd": {
"dataEncryption": {
"keyManagementMode": "PlatformManaged"
"customerManaged": {
"encryptionType": "KMS",
"kms": {
"activeKey": {
"name": "vc-encryption-key",
"vaultName": "vc-key-vault",
"version": "2024-12-01-preview"
}
Comment thread internal/admission/admit_cluster_test.go
Align with CS validateAroHcpEtcdEncryptionDataEncryptionKeyManagementMode:
only CustomerManaged is accepted until platform-managed etcd encryption is supported.
Copilot AI review requested due to automatic review settings July 14, 2026 17:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment on lines +298 to +301
subnetPath := fldPath.Child("subnetId")
if newObj.SubnetID == nil {
return field.ErrorList{field.Required(subnetPath, "")}
}
Comment on lines +350 to +353
nsgPath := fldPath.Child("networkSecurityGroupId")
if newObj.NetworkSecurityGroupID == nil {
return field.ErrorList{field.Required(nsgPath, "")}
}
Comment on lines +263 to +266
mrgPath := fldPath.Child("managedResourceGroup")
if len(newObj.ManagedResourceGroup) == 0 {
return field.ErrorList{field.Required(mrgPath, "")}
}
@JakobGray

Copy link
Copy Markdown
Collaborator Author

/test integration

@JakobGray

Copy link
Copy Markdown
Collaborator Author

/hold These changes will likely be incorporated as a part of #6082

@openshift-ci

openshift-ci Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

PR needs rebase.

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.

@JakobGray

Copy link
Copy Markdown
Collaborator Author

Work merged in #6121

@miguelsorianod

Copy link
Copy Markdown
Collaborator

The same code has been merged in a unified PR with other changes, in #6121.

Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants