diff --git a/frontend/pkg/frontend/cluster.go b/frontend/pkg/frontend/cluster.go index 3652e88b648..ee43f4a04a8 100644 --- a/frontend/pkg/frontend/cluster.go +++ b/frontend/pkg/frontend/cluster.go @@ -266,6 +266,32 @@ func (f *Frontend) newClusterAdmissionContext(ctx context.Context, op operation. OriginalCluster: originalCluster.DeepCopy(), } + 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 { + return nil, fmt.Errorf("cannot list clusters for cluster admission: %w", err) + } + return admissionContext, nil + } + if op.Type != operation.Update { return admissionContext, nil } diff --git a/frontend/pkg/frontend/frontend_test.go b/frontend/pkg/frontend/frontend_test.go index bb8ed108fc7..67d8f1bde05 100644 --- a/frontend/pkg/frontend/frontend_test.go +++ b/frontend/pkg/frontend/frontend_test.go @@ -435,6 +435,22 @@ func TestDeploymentPreflight(t *testing.T) { "subnetId": api.TestSubnetResourceID, "networkSecurityGroupId": api.TestNetworkSecurityGroupResourceID, }, + "etcd": map[string]any{ + "dataEncryption": map[string]any{ + "keyManagementMode": "CustomerManaged", + "customerManaged": map[string]any{ + "encryptionType": "KMS", + "kms": map[string]any{ + "visibility": "Public", + "activeKey": map[string]any{ + "name": "test-key", + "vaultName": "test-vault", + "version": "test-version", + }, + }, + }, + }, + }, }, }, expectStatus: arm.DeploymentPreflightStatusSucceeded, @@ -475,6 +491,9 @@ func TestDeploymentPreflight(t *testing.T) { {message: "Unsupported value: \"invisible\": supported values: \"Private\", \"Public\"", target: "properties.api.visibility"}, {message: "Required value", target: "properties.platform.subnetId"}, {message: "Required value", target: "properties.platform.networkSecurityGroupId"}, + {message: "Unsupported value: \"PlatformManaged\": supported values: \"CustomerManaged\"", target: "properties.etcd.dataEncryption.keyManagementMode"}, + {message: "Required value", target: "properties.platform.subnetId"}, + {message: "Required value", target: "properties.platform.networkSecurityGroupId"}, }, }, { diff --git a/internal/admission/admit_cluster.go b/internal/admission/admit_cluster.go index 4fe23ed6c3e..236f5e13c24 100644 --- a/internal/admission/admit_cluster.go +++ b/internal/admission/admit_cluster.go @@ -53,6 +53,16 @@ type ClusterAdmissionContext struct { // 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 + // (not including the current cluster being admitted), used + // for cross-cluster platform resource uniqueness on CREATE. + // The list is empty on UPDATE. + SubscriptionClusters []*api.HCPOpenShiftCluster + // SubscriptionNodePools lists node pool documents under SubscriptionClusters, + // 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 } // ClusterAdmissionNodePool is a single node pool plus its prefetched service @@ -222,6 +232,143 @@ func admitClusterCustomerProperties(ctx context.Context, admissionContext *Clust errs := field.ErrorList{} errs = append(errs, admitClusterVersionProfile(ctx, admissionContext, op, fldPath.Child("version"), &newObj.Version, safe.Field(oldObj, validation.ToClusterCustomerPropertiesVersion))...) + errs = append(errs, admitClusterPlatform(ctx, admissionContext, op, fldPath.Child("platform"), &newObj.Platform)...) + + return errs +} + +func admitClusterPlatform(ctx context.Context, admissionContext *ClusterAdmissionContext, op operation.Operation, fldPath *field.Path, newObj *api.CustomerPlatformProfile) field.ErrorList { + errs := field.ErrorList{} + + 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)...) + return errs +} + +// admitClusterManagedResourceGroupName ensures the managed resource group name +// is unique within the subscription on CREATE. +// +// 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 { + return nil + } + + if admissionContext.OriginalCluster == nil { + return field.ErrorList{field.InternalError(fldPath, errors.New("original cluster is required for admission"))} + } + + mrgPath := fldPath.Child("managedResourceGroup") + if len(newObj.ManagedResourceGroup) == 0 { + return field.ErrorList{field.Required(mrgPath, "")} + } + + subscriptionID := admissionContext.OriginalCluster.ID.SubscriptionID + var errs field.ErrorList + + for _, existing := range admissionContext.SubscriptionClusters { + if strings.EqualFold(newObj.ManagedResourceGroup, existing.CustomerProperties.Platform.ManagedResourceGroup) { + errs = append(errs, field.Invalid( + mrgPath, + newObj.ManagedResourceGroup, + fmt.Sprintf("Cluster with managed resource group name '%s' in subscription '%s' "+ + "already exists, please provide a unique managed resource group name", + newObj.ManagedResourceGroup, subscriptionID), + )) + break + } + } + + return errs +} + +// admitClusterSubnetResourceID ensures that the subnet ID is not already in use by any other +// cluster or node pool within the same subscription when creating a new cluster. +// +// Best-effort only: compares against SubscriptionClusters and SubscriptionNodePools +// 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 { + return nil + } + + subnetPath := fldPath.Child("subnetId") + if newObj.SubnetID == nil { + return field.ErrorList{field.Required(subnetPath, "")} + } + subnetID := newObj.SubnetID.String() + var errs field.ErrorList + + for _, existing := range admissionContext.SubscriptionClusters { + existingSubnet := existing.CustomerProperties.Platform.SubnetID + if existingSubnet == nil { + errs = append(errs, field.InternalError(subnetPath, errors.New("existing cluster is missing subnetId"))) + continue + } + 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 + } + } + + for _, nodePool := range admissionContext.SubscriptionNodePools { + nodePoolSubnet := nodePool.Properties.Platform.SubnetID + if nodePoolSubnet == nil { + errs = append(errs, field.InternalError(subnetPath, errors.New("existing node pool is missing subnetId"))) + continue + } + 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 + } + } + + return errs +} + +// admitClusterNetworkSecurityGroupResourceID ensures that the network security group ID is not already in use by any other +// cluster within the same subscription when creating a new cluster. +// +// 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 { + return nil + } + + nsgPath := fldPath.Child("networkSecurityGroupId") + if newObj.NetworkSecurityGroupID == nil { + return field.ErrorList{field.Required(nsgPath, "")} + } + nsgID := newObj.NetworkSecurityGroupID.String() + var errs field.ErrorList + + for _, existing := range admissionContext.SubscriptionClusters { + existingNSG := existing.CustomerProperties.Platform.NetworkSecurityGroupID + if existingNSG == nil { + errs = append(errs, field.InternalError(nsgPath, errors.New("existing cluster is missing networkSecurityGroupId"))) + continue + } + 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 + } + } return errs } diff --git a/internal/admission/admit_cluster_test.go b/internal/admission/admit_cluster_test.go index c48afc1f3c7..c740f299fc1 100644 --- a/internal/admission/admit_cluster_test.go +++ b/internal/admission/admit_cluster_test.go @@ -684,3 +684,167 @@ func TestAdmitCluster_Update(t *testing.T) { }) } } + +func TestAdmitCluster_PlatformResourceIDs(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + const subscriptionID = "6b690bec-0c16-4ecb-8f67-781caf40bba7" + + subnetID := api.Must(azcorearm.ParseResourceID( + "/subscriptions/" + subscriptionID + "/resourceGroups/customer/providers/Microsoft.Network/virtualNetworks/vnet/subnets/cluster-subnet")) + otherSubnetID := api.Must(azcorearm.ParseResourceID( + "/subscriptions/" + subscriptionID + "/resourceGroups/other/providers/Microsoft.Network/virtualNetworks/vnet/subnets/other")) + nsgID := api.Must(azcorearm.ParseResourceID( + "/subscriptions/" + subscriptionID + "/resourceGroups/customer/providers/Microsoft.Network/networkSecurityGroups/cluster-nsg")) + otherNsgID := api.Must(azcorearm.ParseResourceID( + "/subscriptions/" + subscriptionID + "/resourceGroups/other/providers/Microsoft.Network/networkSecurityGroups/other-nsg")) + + makeCluster := func(name, managedResourceGroup string, subnet, nsg *azcorearm.ResourceID) *api.HCPOpenShiftCluster { + resourceID := api.Must(azcorearm.ParseResourceID(fmt.Sprintf( + "/subscriptions/%s/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/%s", + subscriptionID, name))) + return &api.HCPOpenShiftCluster{ + TrackedResource: arm.NewTrackedResource(resourceID, "eastus"), + CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ + Platform: api.CustomerPlatformProfile{ + ManagedResourceGroup: managedResourceGroup, + SubnetID: subnet, + NetworkSecurityGroupID: nsg, + }, + }, + } + } + + makeNodePool := func(clusterName, nodePoolName string, subnet *azcorearm.ResourceID) *api.HCPOpenShiftClusterNodePool { + resourceID := api.Must(azcorearm.ParseResourceID(fmt.Sprintf( + "/subscriptions/%s/resourceGroups/rg/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/%s/hcpOpenShiftClusterNodePools/%s", + subscriptionID, clusterName, nodePoolName))) + return &api.HCPOpenShiftClusterNodePool{ + TrackedResource: arm.NewTrackedResource(resourceID, "eastus"), + Properties: api.HCPOpenShiftClusterNodePoolProperties{ + Platform: api.NodePoolPlatformProfile{ + SubnetID: subnet, + }, + }, + } + } + + tests := []struct { + name string + subscriptionClusters []*api.HCPOpenShiftCluster + subscriptionNodePools []*api.HCPOpenShiftClusterNodePool + newCluster *api.HCPOpenShiftCluster + expectErrors []utils.ExpectedError + }{ + { + name: "create with empty subscription clusters", + subscriptionClusters: nil, + newCluster: makeCluster("new-cluster", "mrg-new", subnetID, nsgID), + expectErrors: []utils.ExpectedError{}, + }, + { + name: "create rejects duplicate subnet", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", subnetID, nsgID), + }, + newCluster: makeCluster("new-cluster", "mrg-new", subnetID, otherNsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.subnetId", Message: "already in use by another cluster"}, + }, + }, + { + name: "create rejects duplicate network security group", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", subnetID, nsgID), + }, + newCluster: makeCluster("new-cluster", "mrg-new", otherSubnetID, nsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.networkSecurityGroupId", Message: "already in use by another cluster"}, + }, + }, + { + name: "create rejects duplicate managed resource group", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "shared-mrg", subnetID, nsgID), + }, + newCluster: makeCluster("new-cluster", "shared-mrg", otherSubnetID, otherNsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.managedResourceGroup", Message: "please provide a unique managed resource group name"}, + }, + }, + { + name: "create rejects duplicate subnet used by node pool", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", otherSubnetID, nsgID), + }, + subscriptionNodePools: []*api.HCPOpenShiftClusterNodePool{ + makeNodePool("existing-cluster", "workers", subnetID), + }, + newCluster: makeCluster("new-cluster", "mrg-new", subnetID, otherNsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.subnetId", Message: "already in use by another cluster"}, + }, + }, + { + name: "create allows distinct platform values", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", subnetID, nsgID), + }, + newCluster: makeCluster("new-cluster", "mrg-new", otherSubnetID, otherNsgID), + expectErrors: []utils.ExpectedError{}, + }, + { + name: "create with nil new platform resource IDs returns required errors", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", subnetID, nsgID), + }, + newCluster: makeCluster("new-cluster", "mrg-new", nil, nil), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.subnetId", Message: "Required value"}, + {FieldPath: "properties.platform.networkSecurityGroupId", Message: "Required value"}, + }, + }, + { + name: "create with existing cluster missing platform resource IDs returns internal errors", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", nil, nil), + }, + newCluster: makeCluster("new-cluster", "mrg-new", subnetID, nsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.subnetId", Message: "existing cluster is missing subnetId"}, + {FieldPath: "properties.platform.networkSecurityGroupId", Message: "existing cluster is missing networkSecurityGroupId"}, + }, + }, + { + name: "create with existing node pool missing subnet returns internal error", + subscriptionClusters: []*api.HCPOpenShiftCluster{ + makeCluster("existing-cluster", "mrg-existing", otherSubnetID, nsgID), + }, + subscriptionNodePools: []*api.HCPOpenShiftClusterNodePool{ + makeNodePool("existing-cluster", "workers", nil), + }, + newCluster: makeCluster("new-cluster", "mrg-new", subnetID, otherNsgID), + expectErrors: []utils.ExpectedError{ + {FieldPath: "properties.platform.subnetId", Message: "existing node pool is missing subnetId"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + admissionContext := &ClusterAdmissionContext{ + OriginalCluster: tt.newCluster.DeepCopy(), + SubscriptionClusters: tt.subscriptionClusters, + SubscriptionNodePools: tt.subscriptionNodePools, + } + + errs := AdmitCluster(ctx, admissionContext, operation.Operation{Type: operation.Create}, tt.newCluster, nil) + + utils.VerifyErrorsMatch(t, tt.expectErrors, errs) + }) + } +} diff --git a/internal/api/enums.go b/internal/api/enums.go index 02dc383bf3d..9fd63d427a6 100644 --- a/internal/api/enums.go +++ b/internal/api/enums.go @@ -164,13 +164,17 @@ const ( // 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, ) ) diff --git a/internal/api/testhelpers.go b/internal/api/testhelpers.go index c4605bebf51..4c6cb921acf 100644 --- a/internal/api/testhelpers.go +++ b/internal/api/testhelpers.go @@ -83,6 +83,19 @@ func MinimumValidClusterTestCase() *HCPOpenShiftCluster { resource.CustomerProperties.Platform.VnetIntegrationSubnetID = Must(azcorearm.ParseResourceID(TestVnetIntegrationSubnetResourceID)) resource.CustomerProperties.Platform.NetworkSecurityGroupID = Must(azcorearm.ParseResourceID(TestNetworkSecurityGroupResourceID)) resource.ServiceProviderProperties.ManagedIdentitiesDataPlaneIdentityURL = TestManagedIdentitiesDataPlaneIdentityURL + // PlatformManaged etcd encryption is not currently supported; require CustomerManaged for a valid cluster. + resource.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + resource.CustomerProperties.Etcd.DataEncryption.CustomerManaged = &CustomerManagedEncryptionProfile{ + EncryptionType: CustomerManagedEncryptionTypeKMS, + Kms: &KmsEncryptionProfile{ + Visibility: KeyVaultVisibilityPublic, + ActiveKey: KmsKey{ + Name: "test-key", + VaultName: "test-vault", + Version: "test-version", + }, + }, + } // Add required systemData fields createdAt := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) resource.SystemData = &arm.SystemData{ diff --git a/internal/api/types_cluster.go b/internal/api/types_cluster.go index 31f06cef0c8..35913cd5e00 100644 --- a/internal/api/types_cluster.go +++ b/internal/api/types_cluster.go @@ -292,7 +292,10 @@ func NewDefaultHCPOpenShiftCluster(resourceID *azcorearm.ResourceID, azureLocati MaxNodeProvisionTimeSeconds: DefaultClusterMaxNodeProvisionTimeSeconds, PodPriorityThreshold: DefaultClusterPodPriorityThreshold, }, - //Even though PlatformManaged Mode is currently not supported by CS . This is the default value . + // PlatformManaged is still the default for absent values (EnsureDefaults / Cosmos + // documents), but it is rejected by ValidEtcdDataEncryptionKeyManagementModeType + // until platform-managed etcd encryption is supported. CustomerManaged must be set + // for a create request to succeed. Etcd: EtcdProfile{ DataEncryption: EtcdDataEncryptionProfile{ KeyManagementMode: EtcdDataEncryptionKeyManagementModeTypePlatformManaged, diff --git a/internal/ocm/convert_test.go b/internal/ocm/convert_test.go index 530a65846d9..2693ffba18b 100644 --- a/internal/ocm/convert_test.go +++ b/internal/ocm/convert_test.go @@ -225,7 +225,18 @@ func ocmClusterDefaults(azureLocation string) *arohcpv1alpha1.ClusterBuilder { Azure(arohcpv1alpha1.NewAzure(). EtcdEncryption(arohcpv1alpha1.NewAzureEtcdEncryption(). DataEncryption(arohcpv1alpha1.NewAzureEtcdDataEncryption(). - KeyManagementMode(csKeyManagementModePlatformManaged))). + KeyManagementMode(csKeyManagementModeCustomerManaged). + CustomerManaged(arohcpv1alpha1.NewAzureEtcdDataEncryptionCustomerManaged(). + EncryptionType("kms"). + Kms(arohcpv1alpha1.NewAzureKmsEncryption(). + Visibility(arohcpv1alpha1.AzureKmsEncryptionVisibilityPublic). + ActiveKey(arohcpv1alpha1.NewAzureKmsKey(). + KeyName("test-key"). + KeyVaultName("test-vault"). + KeyVersion("test-version"), + ), + ), + ))). ManagedResourceGroupName(api.TestManagedResourceGroupName). NetworkSecurityGroupResourceID(api.TestNetworkSecurityGroupResourceID). NodesOutboundConnectivity(arohcpv1alpha1.NewAzureNodesOutboundConnectivity(). diff --git a/internal/validation/hcpopenshiftcluster_test.go b/internal/validation/hcpopenshiftcluster_test.go index 8810a9c5e6a..f04d2f26087 100644 --- a/internal/validation/hcpopenshiftcluster_test.go +++ b/internal/validation/hcpopenshiftcluster_test.go @@ -208,6 +208,10 @@ func TestClusterRequired(t *testing.T) { Message: "Required value", FieldPath: "customerProperties.platform.networkSecurityGroupId", }, + { + Message: "Unsupported value", + FieldPath: "customerProperties.etcd.dataEncryption.keyManagementMode", + }, { Message: "Required value", FieldPath: "serviceProviderProperties.managedIdentitiesDataPlaneIdentityURL", @@ -634,15 +638,12 @@ func TestClusterValidate(t *testing.T) { }, { name: "Customer managed ETCD key management mode requires CustomerManaged fields", - tweaks: &api.HCPOpenShiftCluster{ - CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ - Etcd: api.EtcdProfile{ - DataEncryption: api.EtcdDataEncryptionProfile{ - KeyManagementMode: api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged, - }, - }, - }, - }, + resource: func() *api.HCPOpenShiftCluster { + c := api.MinimumValidClusterTestCase() + c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = nil + return c + }(), expectErrors: []utils.ExpectedError{ { Message: "must be specified when `keyManagementMode` is \"CustomerManaged\"", @@ -651,18 +652,18 @@ func TestClusterValidate(t *testing.T) { }, }, { - name: "Platform managed ETCD key management mode excludes CustomerManaged fields", - tweaks: &api.HCPOpenShiftCluster{ - CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ - Etcd: api.EtcdProfile{ - DataEncryption: api.EtcdDataEncryptionProfile{ - KeyManagementMode: api.EtcdDataEncryptionKeyManagementModeTypePlatformManaged, - CustomerManaged: &api.CustomerManagedEncryptionProfile{}, - }, - }, - }, - }, + name: "Platform managed ETCD key management mode is not supported", + resource: func() *api.HCPOpenShiftCluster { + c := api.MinimumValidClusterTestCase() + c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypePlatformManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = &api.CustomerManagedEncryptionProfile{} + return c + }(), expectErrors: []utils.ExpectedError{ + { + Message: "Unsupported value", + FieldPath: "customerProperties.etcd.dataEncryption.keyManagementMode", + }, { Message: "may only be specified when `keyManagementMode` is \"CustomerManaged\"", FieldPath: "customerProperties.etcd.dataEncryption.customerManaged", @@ -675,18 +676,14 @@ func TestClusterValidate(t *testing.T) { }, { name: "Customer managed Key Management Service (KMS) requires Kms fields", - tweaks: &api.HCPOpenShiftCluster{ - CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ - Etcd: api.EtcdProfile{ - DataEncryption: api.EtcdDataEncryptionProfile{ - KeyManagementMode: api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged, - CustomerManaged: &api.CustomerManagedEncryptionProfile{ - EncryptionType: api.CustomerManagedEncryptionTypeKMS, - }, - }, - }, - }, - }, + resource: func() *api.HCPOpenShiftCluster { + c := api.MinimumValidClusterTestCase() + c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = &api.CustomerManagedEncryptionProfile{ + EncryptionType: api.CustomerManagedEncryptionTypeKMS, + } + return c + }(), expectErrors: []utils.ExpectedError{ { Message: "must be specified when `encryptionType` is \"KMS\"", @@ -697,19 +694,15 @@ func TestClusterValidate(t *testing.T) { { // FIXME Use a valid alternate EncryptionType once we have one. name: "Alternate customer managed ETCD encyption type excludes Kms fields", - tweaks: &api.HCPOpenShiftCluster{ - CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ - Etcd: api.EtcdProfile{ - DataEncryption: api.EtcdDataEncryptionProfile{ - KeyManagementMode: api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged, - CustomerManaged: &api.CustomerManagedEncryptionProfile{ - EncryptionType: "Alternate", - Kms: &api.KmsEncryptionProfile{}, - }, - }, - }, - }, - }, + resource: func() *api.HCPOpenShiftCluster { + c := api.MinimumValidClusterTestCase() + c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = &api.CustomerManagedEncryptionProfile{ + EncryptionType: "Alternate", + Kms: &api.KmsEncryptionProfile{}, + } + return c + }(), expectErrors: []utils.ExpectedError{ { Message: "supported values: \"KMS\"", @@ -806,6 +799,7 @@ func TestClusterValidate(t *testing.T) { // Use a different resource group name to avoid a subnet ID error. SubnetID: api.Must(azcorearm.ParseResourceID(path.Join("/subscriptions", api.TestSubscriptionID, "resourceGroups", "anotherResourceGroup", "providers", "Microsoft.Network", "virtualNetworks", api.TestVirtualNetworkName, "subnets", api.TestSubnetName))), VnetIntegrationSubnetID: api.Must(azcorearm.ParseResourceID(path.Join("/subscriptions", api.TestSubscriptionID, "resourceGroups", "anotherResourceGroup", "providers", "Microsoft.Network", "virtualNetworks", api.TestVirtualNetworkName, "subnets", api.TestVnetIntegrationSubnetName))), + NetworkSecurityGroupID: api.Must(azcorearm.ParseResourceID(path.Join("/subscriptions", api.TestSubscriptionID, "resourceGroups", "anotherResourceGroup", "providers", "Microsoft.Network", "networkSecurityGroups", api.TestNetworkSecurityGroupName))), }, }, }, @@ -818,15 +812,13 @@ func TestClusterValidate(t *testing.T) { }, { name: "Cluster with invalid subnet ID", - tweaks: &api.HCPOpenShiftCluster{ - CustomerProperties: api.HCPOpenShiftClusterCustomerProperties{ - Platform: api.CustomerPlatformProfile{ - ManagedResourceGroup: "MRG", - SubnetID: api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/MRG/providers/Microsoft.Network/virtualNetworks/testVirtualNetwork/subnets/testSubnet")), - VnetIntegrationSubnetID: api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/anotherResourceGroup/providers/Microsoft.Network/virtualNetworks/testVirtualNetwork/subnets/testVnetIntegrationSubnet")), - }, - }, - }, + resource: func() *api.HCPOpenShiftCluster { + c := api.MinimumValidClusterTestCase() + c.CustomerProperties.Platform.ManagedResourceGroup = "MRG" + c.CustomerProperties.Platform.SubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/MRG/providers/Microsoft.Network/virtualNetworks/testVirtualNetwork/subnets/testSubnet")) + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/anotherResourceGroup/providers/Microsoft.Network/virtualNetworks/testVirtualNetwork/subnets/testVnetIntegrationSubnet")) + return c + }(), expectErrors: []utils.ExpectedError{ { Message: "must not be the same resource group name: \"MRG\"", @@ -836,6 +828,10 @@ func TestClusterValidate(t *testing.T) { Message: "must be in the same Azure subscription: \"11111111-1111-1111-1111-111111111111\"", FieldPath: "customerProperties.platform.subnetId", }, + { + Message: "must belong to the same VNet as subnetId", + FieldPath: "customerProperties.platform.vnetIntegrationSubnetId", + }, { Message: "must be in the same Azure subscription: \"11111111-1111-1111-1111-111111111111\"", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId", @@ -927,6 +923,14 @@ func TestClusterValidate(t *testing.T) { }, }, expectErrors: []utils.ExpectedError{ + { + Message: "must be unique within the cluster", + FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.controlPlaneOperators", + }, + { + Message: "must be unique within the cluster", + FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.serviceManagedIdentity", + }, { Message: "identity is used multiple times", FieldPath: "identity.userAssignedIdentities", diff --git a/internal/validation/validate_cluster.go b/internal/validation/validate_cluster.go index b431c5bd6ff..b01b4c0ce83 100644 --- a/internal/validation/validate_cluster.go +++ b/internal/validation/validate_cluster.go @@ -17,7 +17,9 @@ package validation import ( "context" "fmt" + "maps" "net" + "slices" "strings" "github.com/blang/semver/v4" @@ -173,8 +175,12 @@ func validateResourceIDsAgainstClusterID(ctx context.Context, op operation.Opera // Validate that managed resource group is different from cluster resource group errs = append(errs, DifferentResourceGroupName(ctx, op, field.NewPath("customerProperties", "platform", "managedResourceGroup"), &newCluster.CustomerProperties.Platform.ManagedResourceGroup, nil, newCluster.ID.ResourceGroupName)...) + // NOTE: Our admission logic expects that the subnet and network security group are in the same subscription as the cluster. + // If these validations are removed, the admission logic should also be updated. errs = append(errs, SameSubscription(ctx, op, field.NewPath("customerProperties", "platform", "subnetId"), newCluster.CustomerProperties.Platform.SubnetID, nil, newCluster.ID.SubscriptionID)...) errs = append(errs, DifferentResourceGroupNameFromResourceID(ctx, op, field.NewPath("customerProperties", "platform", "subnetId"), newCluster.CustomerProperties.Platform.SubnetID, nil, newCluster.CustomerProperties.Platform.ManagedResourceGroup)...) + errs = append(errs, SameSubscription(ctx, op, field.NewPath("customerProperties", "platform", "networkSecurityGroupId"), newCluster.CustomerProperties.Platform.NetworkSecurityGroupID, nil, newCluster.ID.SubscriptionID)...) + errs = append(errs, DifferentResourceGroupNameFromResourceID(ctx, op, field.NewPath("customerProperties", "platform", "networkSecurityGroupId"), newCluster.CustomerProperties.Platform.NetworkSecurityGroupID, nil, newCluster.CustomerProperties.Platform.ManagedResourceGroup)...) errs = append(errs, SameSubscription(ctx, op, field.NewPath("customerProperties", "platform", "vnetIntegrationSubnetId"), newCluster.CustomerProperties.Platform.VnetIntegrationSubnetID, nil, newCluster.ID.SubscriptionID)...) for operatorName, operatorIdentity := range newCluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators { @@ -622,6 +628,7 @@ func validateCustomerPlatformProfile(ctx context.Context, op operation.Operation errs = append(errs, RestrictedResourceIDWithResourceGroup(ctx, op, fldPath.Child("vnetIntegrationSubnetId"), newObj.VnetIntegrationSubnetID, safe.Field(oldObj, toPlatformVnetIntegrationSubnetID), "Microsoft.Network/virtualNetworks/subnets")...) errs = append(errs, DifferentResourceGroupNameFromResourceID(ctx, op, fldPath.Child("vnetIntegrationSubnetId"), newObj.VnetIntegrationSubnetID, nil, newObj.ManagedResourceGroup)...) // SameSubscription is validated in validateResourceIDsAgainstClusterID against cluster subscription + errs = append(errs, SameVirtualNetwork(ctx, op, fldPath.Child("vnetIntegrationSubnetId"), newObj.VnetIntegrationSubnetID, nil, newObj.SubnetID)...) } //OutboundType OutboundType `json:"outboundType,omitempty"` @@ -633,6 +640,9 @@ func validateCustomerPlatformProfile(ctx context.Context, op operation.Operation errs = append(errs, validate.RequiredPointer(ctx, op, fldPath.Child("networkSecurityGroupId"), newObj.NetworkSecurityGroupID, safe.Field(oldObj, toPlatformNetworkSecurityGroupID))...) errs = append(errs, immutableByReflect(ctx, op, fldPath.Child("networkSecurityGroupId"), newObj.NetworkSecurityGroupID, safe.Field(oldObj, toPlatformNetworkSecurityGroupID))...) errs = append(errs, RestrictedResourceIDWithResourceGroup(ctx, op, fldPath.Child("networkSecurityGroupId"), newObj.NetworkSecurityGroupID, safe.Field(oldObj, toPlatformNetworkSecurityGroupID), "Microsoft.Network/networkSecurityGroups")...) + // Note: SameSubscription and DifferentResourceGroupNameFromResourceID for + // networkSecurityGroupId are performed at the cluster peer-field level in + // validateResourceIDsAgainstClusterID. //OperatorsAuthentication OperatorsAuthenticationProfile `json:"operatorsAuthentication,omitempty"` errs = append(errs, immutableByReflect(ctx, op, fldPath.Child("operatorsAuthentication"), &newObj.OperatorsAuthentication, safe.Field(oldObj, toPlatformOperatorsAuthentication))...) @@ -726,6 +736,44 @@ func validateUserAssignedIdentitiesProfile(ctx context.Context, op operation.Ope errs = append(errs, immutableByReflect(ctx, op, fldPath.Child("serviceManagedIdentity"), newObj.ServiceManagedIdentity, safe.Field(oldObj, toUserAssignedIdentitiesServiceManagedIdentity))...) errs = append(errs, RestrictedResourceIDWithResourceGroup(ctx, op, fldPath.Child("serviceManagedIdentity"), newObj.ServiceManagedIdentity, safe.Field(oldObj, toUserAssignedIdentitiesServiceManagedIdentity), "Microsoft.ManagedIdentity/userAssignedIdentities")...) + // Managed identity resource IDs must be unique across control-plane operators, + // data-plane operators, and the service managed identity within a cluster + errs = append(errs, validateManagedIdentitiesUniqueWithinCluster(fldPath, newObj)...) + + return errs +} + +// validateManagedIdentitiesUniqueWithinCluster ensures that each managed identity +// resource ID used by control-plane operators, data-plane operators, or the +// service managed identity appears at most once within the cluster. +func validateManagedIdentitiesUniqueWithinCluster(fldPath *field.Path, newObj *api.UserAssignedIdentitiesProfile) field.ErrorList { + observed := map[string]*field.Path{} + var errs field.ErrorList + + record := func(identity *azcorearm.ResourceID, identityPath *field.Path) { + if identity == nil { + return + } + key := strings.ToLower(identity.String()) + if _, ok := observed[key]; ok { + errs = append(errs, field.Invalid( + identityPath, + identity.String(), + fmt.Sprintf("managed identity with resource id '%s' must be unique within the cluster", identity.String()), + )) + return + } + observed[key] = identityPath + } + + for _, operatorName := range slices.Sorted(maps.Keys(newObj.ControlPlaneOperators)) { + record(newObj.ControlPlaneOperators[operatorName], fldPath.Child("controlPlaneOperators").Key(operatorName)) + } + for _, operatorName := range slices.Sorted(maps.Keys(newObj.DataPlaneOperators)) { + record(newObj.DataPlaneOperators[operatorName], fldPath.Child("dataPlaneOperators").Key(operatorName)) + } + record(newObj.ServiceManagedIdentity, fldPath.Child("serviceManagedIdentity")) + return errs } diff --git a/internal/validation/validate_cluster_comprehensive_test.go b/internal/validation/validate_cluster_comprehensive_test.go index b73b9db4da3..cf81fdbc358 100644 --- a/internal/validation/validate_cluster_comprehensive_test.go +++ b/internal/validation/validate_cluster_comprehensive_test.go @@ -369,7 +369,7 @@ func TestValidateClusterCreate(t *testing.T) { name: "wrong NSG resource type - create", cluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Platform.NetworkSecurityGroupID = api.Must(azcorearm.ParseResourceID("/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet")) + c.CustomerProperties.Platform.NetworkSecurityGroupID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet")) return c }(), expectErrors: []utils.ExpectedError{ @@ -392,6 +392,19 @@ func TestValidateClusterCreate(t *testing.T) { cluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = "InvalidMode" + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = nil + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "Unsupported value", FieldPath: "customerProperties.etcd.dataEncryption.keyManagementMode"}, + }, + }, + { + name: "platform managed etcd encryption is not supported - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypePlatformManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = nil return c }(), expectErrors: []utils.ExpectedError{ @@ -670,6 +683,46 @@ func TestValidateClusterCreate(t *testing.T) { return c }(), expectErrors: []utils.ExpectedError{ + {Message: "must be unique within the cluster", FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.controlPlaneOperators"}, + {Message: "identity is used multiple times", FieldPath: "identity.userAssignedIdentities[/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/shared-identity]"}, + }, + }, + { + name: "duplicate managed identity across data plane operators - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + sharedIdentityID := "/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/shared-dataplane-identity" + sharedIdentity := api.Must(azcorearm.ParseResourceID(sharedIdentityID)) + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators = map[string]*azcorearm.ResourceID{ + "dataplane-operator-1": sharedIdentity, + "dataplane-operator-2": sharedIdentity, + } + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "must be unique within the cluster", FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.dataPlaneOperators"}, + }, + }, + { + name: "duplicate managed identity between control plane and service managed identity - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + identityID := "/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/shared-identity" + identityResourceID := api.Must(azcorearm.ParseResourceID(identityID)) + c.Identity = &arm.ManagedServiceIdentity{ + Type: arm.ManagedServiceIdentityTypeUserAssigned, + UserAssignedIdentities: map[string]*arm.UserAssignedIdentity{ + identityID: {}, + }, + } + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators = map[string]*azcorearm.ResourceID{ + "test-operator": identityResourceID, + } + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity = identityResourceID + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "must be unique within the cluster", FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.serviceManagedIdentity"}, {Message: "identity is used multiple times", FieldPath: "identity.userAssignedIdentities[/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.ManagedIdentity/userAssignedIdentities/shared-identity]"}, }, }, @@ -744,6 +797,7 @@ func TestValidateClusterCreate(t *testing.T) { }(), expectErrors: []utils.ExpectedError{ {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.subnetId"}, + {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.networkSecurityGroupId"}, {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.managedResourceGroup"}, {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.operatorsAuthentication.userAssignedIdentities.controlPlaneOperators[test-operator]"}, @@ -758,9 +812,32 @@ func TestValidateClusterCreate(t *testing.T) { return c }(), expectErrors: []utils.ExpectedError{ + {Message: "must belong to the same VNet as subnetId", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, {Message: "must be in the same Azure subscription", FieldPath: "customerProperties.platform.subnetId"}, }, }, + { + name: "network security group in different subscription - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + c.CustomerProperties.Platform.NetworkSecurityGroupID = api.Must(azcorearm.ParseResourceID("/subscriptions/different-sub/resourceGroups/test-rg/providers/Microsoft.Network/networkSecurityGroups/test-nsg")) + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "must be in the same Azure subscription", FieldPath: "customerProperties.platform.networkSecurityGroupId"}, + }, + }, + { + name: "network security group in managed resource group - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + c.CustomerProperties.Platform.NetworkSecurityGroupID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/managed-rg/providers/Microsoft.Network/networkSecurityGroups/test-nsg")) + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.networkSecurityGroupId"}, + }, + }, { name: "vnet integration subnet is optional - create", cluster: func() *api.HCPOpenShiftCluster { @@ -788,6 +865,7 @@ func TestValidateClusterCreate(t *testing.T) { return c }(), expectErrors: []utils.ExpectedError{ + {Message: "must belong to the same VNet as subnetId", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, {Message: "must be in the same Azure subscription", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, }, }, @@ -801,6 +879,18 @@ func TestValidateClusterCreate(t *testing.T) { }(), expectErrors: []utils.ExpectedError{ {Message: "must not be the same resource group name", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, + {Message: "must belong to the same VNet as subnetId", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, + }, + }, + { + name: "vnet integration subnet in different VNet - create", + cluster: func() *api.HCPOpenShiftCluster { + c := createValidCluster() + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.Network/virtualNetworks/other-vnet/subnets/test-vnet-integration-subnet")) + return c + }(), + expectErrors: []utils.ExpectedError{ + {Message: "must belong to the same VNet as subnetId", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, }, }, { @@ -1595,6 +1685,7 @@ func TestValidateClusterUpdate(t *testing.T) { expectErrors: []utils.ExpectedError{ {Message: "field is immutable", FieldPath: "customerProperties.platform"}, {Message: "field is immutable", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, + {Message: "must belong to the same VNet as subnetId", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, {Message: "must be in the same Azure subscription", FieldPath: "customerProperties.platform.vnetIntegrationSubnetId"}, }, }, @@ -1607,7 +1698,7 @@ func TestValidateClusterUpdate(t *testing.T) { }(), oldCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/old-vnet-integration-subnet")) + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/old-vnet-integration-subnet")) return c }(), expectErrors: []utils.ExpectedError{ @@ -1619,12 +1710,12 @@ func TestValidateClusterUpdate(t *testing.T) { name: "valid vnet integration subnet unchanged - update", newCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/vnet-integration-subnet")) + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/vnet-integration-subnet")) return c }(), oldCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/vnet-integration-subnet")) + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/vnet-integration-subnet")) return c }(), }, @@ -1645,7 +1736,7 @@ func TestValidateClusterUpdate(t *testing.T) { name: "cannot add vnet integration subnet on update - update", newCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/test-rg/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/new-vnet-integration-subnet")) + c.CustomerProperties.Platform.VnetIntegrationSubnetID = api.Must(azcorearm.ParseResourceID("/subscriptions/0465bc32-c654-41b8-8d87-9815d7abe8f6/resourceGroups/some-resource-group/providers/Microsoft.Network/virtualNetworks/test-vnet/subnets/new-vnet-integration-subnet")) return c }(), oldCluster: func() *api.HCPOpenShiftCluster { @@ -1663,17 +1754,17 @@ func TestValidateClusterUpdate(t *testing.T) { newCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + c.CustomerProperties.Etcd.DataEncryption.CustomerManaged = nil return c }(), oldCluster: func() *api.HCPOpenShiftCluster { c := createValidCluster() - c.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypePlatformManaged return c }(), expectErrors: []utils.ExpectedError{ {Message: "field is immutable", FieldPath: "customerProperties.etcd"}, {Message: "field is immutable", FieldPath: "customerProperties.etcd.dataEncryption"}, - {Message: "field is immutable", FieldPath: "customerProperties.etcd.dataEncryption.keyManagementMode"}, + {Message: "field is immutable", FieldPath: "customerProperties.etcd.dataEncryption.customerManaged"}, {Message: "must be specified when `keyManagementMode` is \"CustomerManaged\"", FieldPath: "customerProperties.etcd.dataEncryption.customerManaged"}, }, }, @@ -2454,5 +2545,19 @@ func createValidCluster() *api.HCPOpenShiftCluster { cluster.ServiceProviderProperties.ManagedIdentitiesDataPlaneIdentityURL = api.TestManagedIdentitiesDataPlaneIdentityURL + // PlatformManaged etcd encryption is not currently supported; require CustomerManaged for a valid cluster. + cluster.CustomerProperties.Etcd.DataEncryption.KeyManagementMode = api.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged + cluster.CustomerProperties.Etcd.DataEncryption.CustomerManaged = &api.CustomerManagedEncryptionProfile{ + EncryptionType: api.CustomerManagedEncryptionTypeKMS, + Kms: &api.KmsEncryptionProfile{ + Visibility: api.KeyVaultVisibilityPublic, + ActiveKey: api.KmsKey{ + Name: "test-key", + VaultName: "test-vault", + Version: "test-version", + }, + }, + } + return cluster } diff --git a/internal/validation/validators.go b/internal/validation/validators.go index 23042c74c99..0ba51f458dd 100644 --- a/internal/validation/validators.go +++ b/internal/validation/validators.go @@ -658,6 +658,24 @@ func SameSubscription(ctx context.Context, op operation.Operation, fldPath *fiel return nil } +// SameVirtualNetwork checks that value and otherSubnet belong to the same Azure VNet. +// Both must be subnet resource IDs whose Parent is the VNet. +func SameVirtualNetwork(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *azcorearm.ResourceID, otherSubnet *azcorearm.ResourceID) field.ErrorList { + if value == nil || otherSubnet == nil || value.Parent == nil || otherSubnet.Parent == nil { + return nil + } + + if !strings.EqualFold(value.Parent.String(), otherSubnet.Parent.String()) { + return field.ErrorList{field.Invalid( + fldPath, + value.String(), + fmt.Sprintf("must belong to the same VNet as subnetId '%s'", otherSubnet.String()), + )} + } + + return nil +} + // ResourceID // 1. has subscription // 2. has name diff --git a/test-integration/frontend/artifacts/FrontendCRUD/Cluster/experimental-features-no-afec/04-httpCreate-cluster-with-4.19/create-with-4.19.json b/test-integration/frontend/artifacts/FrontendCRUD/Cluster/experimental-features-no-afec/04-httpCreate-cluster-with-4.19/create-with-4.19.json index f2f6e196386..a7255f312dc 100644 --- a/test-integration/frontend/artifacts/FrontendCRUD/Cluster/experimental-features-no-afec/04-httpCreate-cluster-with-4.19/create-with-4.19.json +++ b/test-integration/frontend/artifacts/FrontendCRUD/Cluster/experimental-features-no-afec/04-httpCreate-cluster-with-4.19/create-with-4.19.json @@ -36,10 +36,10 @@ "serviceCidr": "172.30.0.0/16" }, "platform": { - "managedResourceGroup": "managed-resource-group-name", - "networkSecurityGroupId": "/subscriptions/a1b2c3d4-0000-0000-0000-afec00000000/resourceGroups/bar/providers/Microsoft.Network/networkSecurityGroups/nsg", + "managedResourceGroup": "managed-resource-group-create-with-419", + "networkSecurityGroupId": "/subscriptions/a1b2c3d4-0000-0000-0000-afec00000000/resourceGroups/bar/providers/Microsoft.Network/networkSecurityGroups/nsg-create-with-419", "outboundType": "LoadBalancer", - "subnetId": "/subscriptions/a1b2c3d4-0000-0000-0000-afec00000000/resourceGroups/bar/providers/Microsoft.Network/virtualNetworks/vnet/subnets/subnet" + "subnetId": "/subscriptions/a1b2c3d4-0000-0000-0000-afec00000000/resourceGroups/bar/providers/Microsoft.Network/virtualNetworks/vnet/subnets/subnet-create-with-419" }, "version": { "channelGroup": "stable", diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2024-06-10-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2024-06-10-preview.json index bd1288e63df..28b8af710d1 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2024-06-10-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2024-06-10-preview.json @@ -19,7 +19,17 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "vaultName": "vc-key-vault", + "version": "2024-12-01-preview" + } + } + }, + "keyManagementMode": "CustomerManaged" } }, "network": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2025-12-23-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2025-12-23-preview.json index 800d8e6e474..7feb73377d2 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2025-12-23-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2025-12-23-preview.json @@ -19,7 +19,18 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "network": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2026-06-30-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2026-06-30-preview.json index 8de0bc6f50c..ee266755f1d 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2026-06-30-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/get/2026-06-30-preview.json @@ -19,7 +19,18 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "ingress": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2024-06-10-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2024-06-10-preview.json index bd1288e63df..28b8af710d1 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2024-06-10-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2024-06-10-preview.json @@ -19,7 +19,17 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "vaultName": "vc-key-vault", + "version": "2024-12-01-preview" + } + } + }, + "keyManagementMode": "CustomerManaged" } }, "network": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2025-12-23-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2025-12-23-preview.json index 800d8e6e474..7feb73377d2 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2025-12-23-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2025-12-23-preview.json @@ -19,7 +19,18 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "network": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2026-06-30-preview.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2026-06-30-preview.json index 8de0bc6f50c..ee266755f1d 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2026-06-30-preview.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/expected/list/2026-06-30-preview.json @@ -19,7 +19,18 @@ }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "ingress": { diff --git a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/request.json b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/request.json index 8f9cbc2636c..a9503735688 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/request.json +++ b/test-integration/frontend/artifacts/VersionCompliance/Cluster/basic-cluster/request.json @@ -8,6 +8,21 @@ "api": { "visibility": "Public" }, + "etcd": { + "dataEncryption": { + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "vaultName": "vc-key-vault", + "version": "2024-12-01-preview" + } + } + }, + "keyManagementMode": "CustomerManaged" + } + }, "network": { "hostPrefix": 23, "machineCidr": "10.0.0.0/16", diff --git a/test-integration/frontend/artifacts/VersionCompliance/NodePool/basic-nodepool/cluster.json b/test-integration/frontend/artifacts/VersionCompliance/NodePool/basic-nodepool/cluster.json index dace7620d82..205c7f37aa6 100644 --- a/test-integration/frontend/artifacts/VersionCompliance/NodePool/basic-nodepool/cluster.json +++ b/test-integration/frontend/artifacts/VersionCompliance/NodePool/basic-nodepool/cluster.json @@ -8,6 +8,21 @@ "api": { "visibility": "Public" }, + "etcd": { + "dataEncryption": { + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "vaultName": "vc-key-vault", + "version": "2024-12-01-preview" + } + } + }, + "keyManagementMode": "CustomerManaged" + } + }, "network": { "hostPrefix": 23, "machineCidr": "10.0.0.0/16", diff --git a/test-integration/frontend/cross_version_roundtrip_test.go b/test-integration/frontend/cross_version_roundtrip_test.go index 7c2bbdba7c6..9b276f1df75 100644 --- a/test-integration/frontend/cross_version_roundtrip_test.go +++ b/test-integration/frontend/cross_version_roundtrip_test.go @@ -198,7 +198,17 @@ func clusterCreatePayload(clusterName, apiVersion string) []byte { }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "vaultName": "vc-key-vault", + "version": "2024-12-01-preview" + } + } + }, + "keyManagementMode": "CustomerManaged" } }, "network": { @@ -248,7 +258,18 @@ func clusterCreatePayload(clusterName, apiVersion string) []byte { }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "nodeDrainTimeoutMinutes": 15, @@ -300,7 +321,18 @@ func clusterCreatePayload(clusterName, apiVersion string) []byte { }, "etcd": { "dataEncryption": { - "keyManagementMode": "PlatformManaged" + "customerManaged": { + "encryptionType": "KMS", + "kms": { + "activeKey": { + "name": "vc-encryption-key", + "version": "2024-12-01-preview" + }, + "vaultName": "vc-key-vault", + "visibility": "Public" + } + }, + "keyManagementMode": "CustomerManaged" } }, "ingress": {