From d2f32fae1eba2b7814ce7f5162708b006c83db29 Mon Sep 17 00:00:00 2001 From: David Eads Date: Mon, 3 Aug 2026 13:27:27 -0400 Subject: [PATCH] feat: add ClusterDenyAssignment controller for Azure deny assignments Adds a controller that manages Azure deny assignments on the managed resource group for each HCP cluster. The controller ensures all required deny assignments exist with correct content, deletes stale ones, and periodically rechecks consistency with jittered recheck intervals. Deny assignments are scoped to the managed resource group and are cleaned up automatically when the resource group is deleted during cluster teardown. Co-Authored-By: Claude Opus 4.6 (1M context) --- backend/cmd/root.go | 1 + backend/pkg/app/backend.go | 56 +- .../pkg/azure/azuremockclient/mock_clients.go | 126 +++ .../azure/client/deny_assignments_client.go | 29 + .../pkg/azure/client/fpa_client_builder.go | 21 + .../azure/client/generic_resources_client.go | 29 + .../azure/client/mock_fpa_client_builder.go | 78 ++ ...uster_cluster_service_create_controller.go | 47 +- ..._cluster_service_create_controller_test.go | 76 +- ...ster_child_resources_cleanup_controller.go | 7 + .../deny_assignment_controller.go | 621 ++++++++++++ .../deny_assignment_controller_test.go | 918 ++++++++++++++++++ .../deny_assignment_definitions.go | 235 +++++ .../deny_assignment_permissions.go | 324 +++++++ backend/pkg/utils/controllerutils/util.go | 10 + internal/api/coreapi/types_cosmosdata.go | 11 + .../coreapi/types_serviceprovider_cluster.go | 34 +- internal/api/coreapi/zz_generated.deepcopy.go | 54 ++ 18 files changed, 2651 insertions(+), 26 deletions(-) create mode 100644 backend/pkg/azure/azuremockclient/mock_clients.go create mode 100644 backend/pkg/azure/client/deny_assignments_client.go create mode 100644 backend/pkg/azure/client/generic_resources_client.go create mode 100644 backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller.go create mode 100644 backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller_test.go create mode 100644 backend/pkg/controllers/cluster/denyassignments/deny_assignment_definitions.go create mode 100644 backend/pkg/controllers/cluster/denyassignments/deny_assignment_permissions.go diff --git a/backend/cmd/root.go b/backend/cmd/root.go index dcc989507da..67881f5d5ad 100644 --- a/backend/cmd/root.go +++ b/backend/cmd/root.go @@ -483,6 +483,7 @@ func (f *BackendRootCmdFlags) ToBackendOptions(ctx context.Context, cmd *cobra.C TracerProviderShutdownFunc: otelShutdown, MaestroSourceEnvironmentIdentifier: f.MaestroSourceEnvironmentIdentifier, FPAClientBuilder: fpaClientBuilder, + HasRealFPA: !f.InsecureIgnoreUserAzureManagedIdentitiesThatNeedManagedIdentitiesDataplaneAvailableAndUseMock, BackendIdentityAzureClients: backendIdentityAzureClients, BackendIdentityAzureCachedReaders: backendIdentityAzureCachedReaders, ExitOnPanic: f.ExitOnPanic, diff --git a/backend/pkg/app/backend.go b/backend/pkg/app/backend.go index 1211acccf81..59c3da37afd 100644 --- a/backend/pkg/app/backend.go +++ b/backend/pkg/app/backend.go @@ -47,6 +47,7 @@ import ( credentialrevocationdeletion "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/credentialrevocation/deletion" credentialrevocationoperations "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/credentialrevocation/operations" clusterdeletion "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/deletion" + "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/denyassignments" clusteridentity "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/identity" "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/legacycredentialrequest" clusteroperations "github.com/Azure/ARO-HCP/backend/pkg/controllers/cluster/operations" @@ -96,23 +97,27 @@ type Backend struct { } type BackendOptions struct { - AppShortDescriptionName string - AppVersion string - AzureLocation string - LeaderElectionLock resourcelock.Interface - ResourcesDBClient corecosmosstorage.ResourcesDBClient - BillingDBClient billingcosmosstorage.BillingDBClient - FleetDBClient fleetcosmosstorage.FleetDBClient - KubeApplierDBClients kubeappliercosmosstorage.KubeApplierDBClients - ClustersServiceClient ocm.ClusterServiceClientSpec - MetricsRegisterer prometheus.Registerer - MetricsGatherer prometheus.Gatherer - MetricsServerListenAddress string - MetricsServerListener net.Listener - HealthzServerListenAddress string - TracerProviderShutdownFunc func(context.Context) error - MaestroSourceEnvironmentIdentifier string - FPAClientBuilder azureclient.FirstPartyApplicationClientBuilder + AppShortDescriptionName string + AppVersion string + AzureLocation string + LeaderElectionLock resourcelock.Interface + ResourcesDBClient corecosmosstorage.ResourcesDBClient + BillingDBClient billingcosmosstorage.BillingDBClient + FleetDBClient fleetcosmosstorage.FleetDBClient + KubeApplierDBClients kubeappliercosmosstorage.KubeApplierDBClients + ClustersServiceClient ocm.ClusterServiceClientSpec + MetricsRegisterer prometheus.Registerer + MetricsGatherer prometheus.Gatherer + MetricsServerListenAddress string + MetricsServerListener net.Listener + HealthzServerListenAddress string + TracerProviderShutdownFunc func(context.Context) error + MaestroSourceEnvironmentIdentifier string + FPAClientBuilder azureclient.FirstPartyApplicationClientBuilder + // HasRealFPA indicates the backend runs against a real First Party Application rather than the + // insecure MI mock. Controllers that create Azure resources only a real FPA can create (e.g. + // deny assignments) are disabled when this is false (dev/int environments). + HasRealFPA bool BackendIdentityAzureClients *azureclient.BackendIdentityAzureClients BackendIdentityAzureCachedReaders *cachedreader.BackendIdentityAzureCachedReaders ExitOnPanic bool @@ -951,6 +956,19 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, backendInformers, ) + // The deny assignment controller creates Azure deny assignments through the FPA, which only + // exists in environments with a real First Party Application (stage/prod). Skip it entirely when + // running against the MI mock (dev/int), where deny assignments cannot be created. + var clusterDenyAssignmentController controllerutils.Controller + if b.options.HasRealFPA { + clusterDenyAssignmentController = denyassignments.NewClusterDenyAssignmentController( + utilsclock.RealClock{}, + b.options.ResourcesDBClient, + b.options.FPAClientBuilder, + backendInformers, + ) + } + clusterPendingClusterServiceIDAssignController := clustercreation.NewClusterPendingClusterServiceIDAssignController( b.options.ResourcesDBClient, backendInformers, @@ -960,6 +978,7 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, b.options.ResourcesDBClient, b.options.ClustersServiceClient, backendInformers, + b.options.HasRealFPA, ) clusterDeletionClusterServiceDeleteDispatchController := clusterdeletion.NewClusterClusterServiceDeleteDispatchController( @@ -1063,6 +1082,9 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context, go systemAdminCredentialRevocationDesiresController.Run(ctx, 20) go systemAdminCredentialRevocationCompletionController.Run(ctx, 20) go systemAdminCredentialRevocationDeletionController.Run(ctx, 20) + if clusterDenyAssignmentController != nil { + go clusterDenyAssignmentController.Run(ctx, 20) + } go clusterPendingClusterServiceIDAssignController.Run(ctx, 20) go clusterClusterServiceCreateController.Run(ctx, 20) go nodePoolClusterServiceCreateController.Run(ctx, 20) diff --git a/backend/pkg/azure/azuremockclient/mock_clients.go b/backend/pkg/azure/azuremockclient/mock_clients.go new file mode 100644 index 00000000000..7dbb7734bf5 --- /dev/null +++ b/backend/pkg/azure/azuremockclient/mock_clients.go @@ -0,0 +1,126 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package azuremockclient + +import ( + "context" + "fmt" + + azruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" + + azureclient "github.com/Azure/ARO-HCP/backend/pkg/azure/client" +) + +// DenyAssignmentsClientFunc adapts a function to the DenyAssignmentsClient.Get interface. +// Tests set GetFunc to control the response per call. +type DenyAssignmentsClientFunc struct { + GetFunc func(ctx context.Context, scope string, denyAssignmentID string, options *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) +} + +var _ azureclient.DenyAssignmentsClient = (*DenyAssignmentsClientFunc)(nil) + +func (m *DenyAssignmentsClientFunc) Get(ctx context.Context, scope string, denyAssignmentID string, options *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + if m.GetFunc != nil { + return m.GetFunc(ctx, scope, denyAssignmentID, options) + } + return armauthorization.DenyAssignmentsClientGetResponse{}, fmt.Errorf("GetFunc not set") +} + +func (m *DenyAssignmentsClientFunc) NewListForResourceGroupPager(_ string, _ *armauthorization.DenyAssignmentsClientListForResourceGroupOptions) *azruntime.Pager[armauthorization.DenyAssignmentsClientListForResourceGroupResponse] { + return nil +} + +// GenericResourcesClientFunc adapts functions to the GenericResourcesClient interface. +// Tests set the function fields to control the response. +// BeginCreateOrUpdateByID and BeginDeleteByID return a nil *Poller and an error — to simulate +// success, return (nil, nil) and the calling code will call PollUntilDone on nil. +// To avoid that, the tests should exercise paths that don't reach PollUntilDone (e.g. error paths) +// or the mock should capture the call without returning a real poller. +// +// For paths that call PollUntilDone, set CreateErr/DeleteErr to non-nil to prevent the nil-pointer dereference. +type GenericResourcesClientFunc struct { + CreateCalls []GenericResourceCreateCall + DeleteCalls []string + CreateErr error + DeleteErr error +} + +type GenericResourceCreateCall struct { + ResourceID string + APIVersion string + Resource armresources.GenericResource +} + +var _ azureclient.GenericResourcesClient = (*GenericResourcesClientFunc)(nil) + +func (m *GenericResourcesClientFunc) BeginCreateOrUpdateByID(ctx context.Context, resourceID string, apiVersion string, parameters armresources.GenericResource, options *armresources.ClientBeginCreateOrUpdateByIDOptions) (*azruntime.Poller[armresources.ClientCreateOrUpdateByIDResponse], error) { + m.CreateCalls = append(m.CreateCalls, GenericResourceCreateCall{ + ResourceID: resourceID, + APIVersion: apiVersion, + Resource: parameters, + }) + if m.CreateErr != nil { + return nil, m.CreateErr + } + return nil, fmt.Errorf("GenericResourcesClientFunc: set CreateErr to control this path; PollUntilDone cannot be called on a nil poller") +} + +func (m *GenericResourcesClientFunc) BeginDeleteByID(ctx context.Context, resourceID string, apiVersion string, options *armresources.ClientBeginDeleteByIDOptions) (*azruntime.Poller[armresources.ClientDeleteByIDResponse], error) { + m.DeleteCalls = append(m.DeleteCalls, resourceID) + if m.DeleteErr != nil { + return nil, m.DeleteErr + } + return nil, fmt.Errorf("GenericResourcesClientFunc: set DeleteErr to control this path; PollUntilDone cannot be called on a nil poller") +} + +// FirstPartyApplicationClientBuilderFunc builds mock Azure clients. +type FirstPartyApplicationClientBuilderFunc struct { + GenericResourcesClientVal azureclient.GenericResourcesClient + GenericResourcesClientErr error + DenyAssignmentsClientVal azureclient.DenyAssignmentsClient + DenyAssignmentsClientErr error +} + +var _ azureclient.FirstPartyApplicationClientBuilder = (*FirstPartyApplicationClientBuilderFunc)(nil) + +func (m *FirstPartyApplicationClientBuilderFunc) BuilderType() azureclient.FirstPartyApplicationClientBuilderType { + return azureclient.FirstPartyApplicationClientBuilderTypeValue +} + +func (m *FirstPartyApplicationClientBuilderFunc) ResourceGroupsClient(tenantID string, subscriptionID string) (azureclient.ResourceGroupsClient, error) { + return nil, fmt.Errorf("not implemented") +} + +func (m *FirstPartyApplicationClientBuilderFunc) ResourceProvidersClient(tenantID string, subscriptionID string) (azureclient.ResourceProvidersClient, error) { + return nil, fmt.Errorf("not implemented") +} + +func (m *FirstPartyApplicationClientBuilderFunc) ResourceSKUsClient(tenantID string, subscriptionID string) (azureclient.ResourceSKUsClient, error) { + return nil, fmt.Errorf("not implemented") +} + +func (m *FirstPartyApplicationClientBuilderFunc) UsageClient(tenantID string, subscriptionID string) (azureclient.UsageClient, error) { + return nil, fmt.Errorf("not implemented") +} + +func (m *FirstPartyApplicationClientBuilderFunc) GenericResourcesClient(tenantID string, subscriptionID string) (azureclient.GenericResourcesClient, error) { + return m.GenericResourcesClientVal, m.GenericResourcesClientErr +} + +func (m *FirstPartyApplicationClientBuilderFunc) DenyAssignmentsClient(tenantID string, subscriptionID string) (azureclient.DenyAssignmentsClient, error) { + return m.DenyAssignmentsClientVal, m.DenyAssignmentsClientErr +} diff --git a/backend/pkg/azure/client/deny_assignments_client.go b/backend/pkg/azure/client/deny_assignments_client.go new file mode 100644 index 00000000000..ffefcfd193a --- /dev/null +++ b/backend/pkg/azure/client/deny_assignments_client.go @@ -0,0 +1,29 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package client + +import ( + "context" + + azruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" +) + +type DenyAssignmentsClient interface { + Get(ctx context.Context, scope string, denyAssignmentID string, options *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) + NewListForResourceGroupPager(resourceGroupName string, options *armauthorization.DenyAssignmentsClientListForResourceGroupOptions) *azruntime.Pager[armauthorization.DenyAssignmentsClientListForResourceGroupResponse] +} + +var _ DenyAssignmentsClient = (*armauthorization.DenyAssignmentsClient)(nil) diff --git a/backend/pkg/azure/client/fpa_client_builder.go b/backend/pkg/azure/client/fpa_client_builder.go index e33f897eca6..fec4dd6dd34 100644 --- a/backend/pkg/azure/client/fpa_client_builder.go +++ b/backend/pkg/azure/client/fpa_client_builder.go @@ -18,6 +18,7 @@ package client import ( azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" @@ -47,6 +48,8 @@ type FirstPartyApplicationClientBuilder interface { ResourceProvidersClient(tenantID string, subscriptionID string) (ResourceProvidersClient, error) ResourceSKUsClient(tenantID string, subscriptionID string) (ResourceSKUsClient, error) UsageClient(tenantID string, subscriptionID string) (UsageClient, error) + GenericResourcesClient(tenantID string, subscriptionID string) (GenericResourcesClient, error) + DenyAssignmentsClient(tenantID string, subscriptionID string) (DenyAssignmentsClient, error) } type firstPartyApplicationClientBuilder struct { @@ -103,6 +106,24 @@ func (b *firstPartyApplicationClientBuilder) UsageClient(tenantID string, subscr return armcompute.NewUsageClient(subscriptionID, creds, b.options) } +func (b *firstPartyApplicationClientBuilder) GenericResourcesClient(tenantID string, subscriptionID string) (GenericResourcesClient, error) { + creds, err := b.fpaTokenCredRetriever.RetrieveCredential(tenantID) + if err != nil { + return nil, err + } + + return armresources.NewClient(subscriptionID, creds, b.options) +} + +func (b *firstPartyApplicationClientBuilder) DenyAssignmentsClient(tenantID string, subscriptionID string) (DenyAssignmentsClient, error) { + creds, err := b.fpaTokenCredRetriever.RetrieveCredential(tenantID) + if err != nil { + return nil, err + } + + return armauthorization.NewDenyAssignmentsClient(subscriptionID, creds, b.options) +} + func (b *firstPartyApplicationClientBuilder) BuilderType() FirstPartyApplicationClientBuilderType { return FirstPartyApplicationClientBuilderTypeValue } diff --git a/backend/pkg/azure/client/generic_resources_client.go b/backend/pkg/azure/client/generic_resources_client.go new file mode 100644 index 00000000000..fcb163f345e --- /dev/null +++ b/backend/pkg/azure/client/generic_resources_client.go @@ -0,0 +1,29 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package client + +import ( + "context" + + azruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" +) + +type GenericResourcesClient interface { + BeginCreateOrUpdateByID(ctx context.Context, resourceID string, apiVersion string, parameters armresources.GenericResource, options *armresources.ClientBeginCreateOrUpdateByIDOptions) (*azruntime.Poller[armresources.ClientCreateOrUpdateByIDResponse], error) + BeginDeleteByID(ctx context.Context, resourceID string, apiVersion string, options *armresources.ClientBeginDeleteByIDOptions) (*azruntime.Poller[armresources.ClientDeleteByIDResponse], error) +} + +var _ GenericResourcesClient = (*armresources.Client)(nil) diff --git a/backend/pkg/azure/client/mock_fpa_client_builder.go b/backend/pkg/azure/client/mock_fpa_client_builder.go index 1ce150e9fb7..16f9d82c056 100644 --- a/backend/pkg/azure/client/mock_fpa_client_builder.go +++ b/backend/pkg/azure/client/mock_fpa_client_builder.go @@ -77,6 +77,84 @@ func (c *MockFirstPartyApplicationClientBuilderBuilderTypeCall) DoAndReturn(f fu return c } +// DenyAssignmentsClient mocks base method. +func (m *MockFirstPartyApplicationClientBuilder) DenyAssignmentsClient(tenantID, subscriptionID string) (DenyAssignmentsClient, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DenyAssignmentsClient", tenantID, subscriptionID) + ret0, _ := ret[0].(DenyAssignmentsClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// DenyAssignmentsClient indicates an expected call of DenyAssignmentsClient. +func (mr *MockFirstPartyApplicationClientBuilderMockRecorder) DenyAssignmentsClient(tenantID, subscriptionID any) *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DenyAssignmentsClient", reflect.TypeOf((*MockFirstPartyApplicationClientBuilder)(nil).DenyAssignmentsClient), tenantID, subscriptionID) + return &MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall{Call: call} +} + +// MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall wrap *gomock.Call +type MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall) Return(arg0 DenyAssignmentsClient, arg1 error) *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall) Do(f func(string, string) (DenyAssignmentsClient, error)) *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall) DoAndReturn(f func(string, string) (DenyAssignmentsClient, error)) *MockFirstPartyApplicationClientBuilderDenyAssignmentsClientCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + +// GenericResourcesClient mocks base method. +func (m *MockFirstPartyApplicationClientBuilder) GenericResourcesClient(tenantID, subscriptionID string) (GenericResourcesClient, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GenericResourcesClient", tenantID, subscriptionID) + ret0, _ := ret[0].(GenericResourcesClient) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GenericResourcesClient indicates an expected call of GenericResourcesClient. +func (mr *MockFirstPartyApplicationClientBuilderMockRecorder) GenericResourcesClient(tenantID, subscriptionID any) *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenericResourcesClient", reflect.TypeOf((*MockFirstPartyApplicationClientBuilder)(nil).GenericResourcesClient), tenantID, subscriptionID) + return &MockFirstPartyApplicationClientBuilderGenericResourcesClientCall{Call: call} +} + +// MockFirstPartyApplicationClientBuilderGenericResourcesClientCall wrap *gomock.Call +type MockFirstPartyApplicationClientBuilderGenericResourcesClientCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall) Return(arg0 GenericResourcesClient, arg1 error) *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall) Do(f func(string, string) (GenericResourcesClient, error)) *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall) DoAndReturn(f func(string, string) (GenericResourcesClient, error)) *MockFirstPartyApplicationClientBuilderGenericResourcesClientCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // ResourceGroupsClient mocks base method. func (m *MockFirstPartyApplicationClientBuilder) ResourceGroupsClient(tenantID, subscriptionID string) (ResourceGroupsClient, error) { m.ctrl.T.Helper() diff --git a/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller.go b/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller.go index 2fbbd3ab154..f27fab422ae 100644 --- a/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller.go +++ b/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller.go @@ -38,6 +38,10 @@ type clusterClusterServiceCreateSyncer struct { clusterLister corelisters.ClusterLister subscriptionLister corelisters.SubscriptionLister clustersServiceClient ocm.ClusterServiceClientSpec + // denyAssignmentsEnabled mirrors whether the ClusterDenyAssignment controller runs (i.e. a real + // FPA is available). When false, cluster creation must not wait for deny assignments to be + // created, because nothing creates them. + denyAssignmentsEnabled bool } var _ controllerutils.ClusterSyncer = (*clusterClusterServiceCreateSyncer)(nil) @@ -46,14 +50,16 @@ func NewClusterClusterServiceCreateController( resourcesDBClient corecosmosstorage.ResourcesDBClient, clustersServiceClient ocm.ClusterServiceClientSpec, backendInformers coreinformers.BackendInformers, + denyAssignmentsEnabled bool, ) controllerutils.Controller { _, clusterLister := backendInformers.Clusters() _, subscriptionLister := backendInformers.Subscriptions() syncer := &clusterClusterServiceCreateSyncer{ - resourcesDBClient: resourcesDBClient, - clusterLister: clusterLister, - subscriptionLister: subscriptionLister, - clustersServiceClient: clustersServiceClient, + resourcesDBClient: resourcesDBClient, + clusterLister: clusterLister, + subscriptionLister: subscriptionLister, + clustersServiceClient: clustersServiceClient, + denyAssignmentsEnabled: denyAssignmentsEnabled, } return controllerutils.NewClusterWatchingController( @@ -115,6 +121,14 @@ func (c *clusterClusterServiceCreateSyncer) SyncOnce(ctx context.Context, key co return nil } + ready, err = c.createPreconditionDenyAssignmentsCreated(ctx, existingServiceProviderCluster) + if err != nil { + return utils.TrackError(err) + } + if !ready { + return nil + } + subscription, err := c.subscriptionLister.Get(ctx, key.SubscriptionID) if err != nil { return utils.TrackError(err) @@ -169,6 +183,31 @@ func (c *clusterClusterServiceCreateSyncer) createPreconditionDesiredVersionReso return false, nil } +// createPreconditionDenyAssignmentsCreated reports whether the ClusterDenyAssignment +// controller has finished creating all deny assignments. +// Returns (false, nil) when this controller should wait and retry. +func (c *clusterClusterServiceCreateSyncer) createPreconditionDenyAssignmentsCreated(ctx context.Context, serviceProviderCluster *coreapi.ServiceProviderCluster) (bool, error) { + logger := utils.LoggerFromContext(ctx) + + if !c.denyAssignmentsEnabled { + // Deny assignments require a real First Party Application (stage/prod). Where the FPA is not + // available (dev/int, MI mock), the ClusterDenyAssignment controller is disabled, so there is + // nothing to wait for and creation must not block on it. + return true, nil + } + + if len(serviceProviderCluster.Status.AzureResources.DenyAssignments.PendingAzureResources) == 0 && len(serviceProviderCluster.Status.AzureResources.DenyAssignments.AzureResources) > 0 { + return true, nil + } + pendingTypes := make([]string, 0, len(serviceProviderCluster.Status.AzureResources.DenyAssignments.PendingAzureResources)) + for _, denyAssignmentReference := range serviceProviderCluster.Status.AzureResources.DenyAssignments.PendingAzureResources { + pendingTypes = append(pendingTypes, denyAssignmentReference.DenyAssignmentType) + } + logger.Info("Deny assignments not yet created, waiting for ClusterDenyAssignment controller", + "pendingDenyAssignmentTypes", pendingTypes) + return false, nil +} + // findAROHCPClusterByAzureInfo returns the Cluster Service cluster whose Azure // metadata matches the given subscription, resource group, ARM resource name, // tenant ID, and managed resource group name (MRG). diff --git a/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller_test.go b/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller_test.go index 27c9dc0783e..209fb0c6ce6 100644 --- a/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller_test.go +++ b/backend/pkg/controllers/cluster/creation/cluster_cluster_service_create_controller_test.go @@ -128,6 +128,7 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) { listCluster *coreapi.HCPOpenShiftCluster // cluster seeded into the lister (nil = not found) dbCluster *coreapi.HCPOpenShiftCluster // cluster stored in the DB existingServiceProviderCluster *coreapi.ServiceProviderCluster // nil = not pre-seeded; controller get-or-creates + denyAssignmentsDisabled bool // simulates an environment without a real FPA setupMockCS func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec expectError bool verifyDB func(t *testing.T, ctx context.Context, db *corecosmosstoragetesting.MockResourcesDBClient) @@ -142,6 +143,7 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) { }), existingServiceProviderCluster: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { spc.Spec.ControlPlaneVersion.DesiredVersion = desiredVersion + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{{DenyAssignmentType: "resources-deny-assignment", DenyAssignmentResourceID: metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testManagedResourceGroup/providers/Microsoft.Authorization/denyAssignments/00000000-0000-0000-0000-000000000001"))}} }), setupMockCS: func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec { mockCS := ocm.NewMockClusterServiceClientSpec(ctrl) @@ -222,6 +224,70 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) { assert.Nil(t, cluster.ServiceProviderProperties.ClusterServiceID) }, }, + { + name: "deny assignments still pending waits without dispatching", + listCluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.ServiceProviderProperties.PendingClusterServiceID = &pendingClusterServiceID + }), + dbCluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.ServiceProviderProperties.PendingClusterServiceID = &pendingClusterServiceID + }), + existingServiceProviderCluster: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + // The desired version is resolved (that precondition passes)... + spc.Spec.ControlPlaneVersion.DesiredVersion = desiredVersion + // ...but deny assignments are still pending, so cluster creation must not dispatch yet. + spc.Status.AzureResources.DenyAssignments.PendingAzureResources = []coreapi.DenyAssignmentReference{{DenyAssignmentType: "resources-deny-assignment", DenyAssignmentResourceID: metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testManagedResourceGroup/providers/Microsoft.Authorization/denyAssignments/00000000-0000-0000-0000-000000000001"))}} + }), + setupMockCS: func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec { + // No CS calls are expected: gomock fails the test if the controller dispatches. + return ocm.NewMockClusterServiceClientSpec(ctrl) + }, + expectError: false, + verifyDB: func(t *testing.T, ctx context.Context, db *corecosmosstoragetesting.MockResourcesDBClient) { + cluster, err := db.HCPClusters(testSubscriptionID, testResourceGroupName).Get(ctx, testClusterName) + require.NoError(t, err) + assert.Nil(t, cluster.ServiceProviderProperties.ClusterServiceID, "cluster creation must not dispatch while deny assignments are pending") + }, + }, + { + name: "deny assignments disabled (no real FPA) dispatches without waiting on them", + listCluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.ServiceProviderProperties.PendingClusterServiceID = &pendingClusterServiceID + }), + dbCluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.ServiceProviderProperties.PendingClusterServiceID = &pendingClusterServiceID + }), + existingServiceProviderCluster: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + // Desired version is resolved, and NO deny assignments are tracked because the + // ClusterDenyAssignment controller is disabled in this environment. + spc.Spec.ControlPlaneVersion.DesiredVersion = desiredVersion + }), + denyAssignmentsDisabled: true, + setupMockCS: func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec { + mockCS := ocm.NewMockClusterServiceClientSpec(ctrl) + mockCS.EXPECT(). + ListClusters(gomock.Any()). + Return(ocm.NewSimpleClusterListIterator(nil, nil)) + mockCS.EXPECT(). + PostCluster(gomock.Any(), gomock.Any()). + DoAndReturn(func(_ context.Context, builder *arohcpv1alpha1.ClusterBuilder) (*arohcpv1alpha1.Cluster, error) { + csCluster, err := arohcpv1alpha1.NewCluster(). + ID(pendingClusterServiceID.ID()). + HREF(testClusterServiceIDStr). + Build() + require.NoError(t, err) + return csCluster, nil + }) + return mockCS + }, + expectError: false, + verifyDB: func(t *testing.T, ctx context.Context, db *corecosmosstoragetesting.MockResourcesDBClient) { + cluster, err := db.HCPClusters(testSubscriptionID, testResourceGroupName).Get(ctx, testClusterName) + require.NoError(t, err) + require.NotNil(t, cluster.ServiceProviderProperties.ClusterServiceID, "creation should proceed when deny assignments are disabled") + assert.Equal(t, testClusterServiceIDStr, cluster.ServiceProviderProperties.ClusterServiceID.String()) + }, + }, { name: "adopts existing Cluster Service cluster for Azure resource", listCluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { @@ -232,6 +298,7 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) { }), existingServiceProviderCluster: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { spc.Spec.ControlPlaneVersion.DesiredVersion = desiredVersion + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{{DenyAssignmentType: "resources-deny-assignment", DenyAssignmentResourceID: metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testManagedResourceGroup/providers/Microsoft.Authorization/denyAssignments/00000000-0000-0000-0000-000000000001"))}} }), setupMockCS: func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec { mockCS := ocm.NewMockClusterServiceClientSpec(ctrl) @@ -284,10 +351,11 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) { listerClusters = []*coreapi.HCPOpenShiftCluster{tt.listCluster} } syncer := &clusterClusterServiceCreateSyncer{ - resourcesDBClient: mockDB, - clusterLister: &corelistertesting.SliceClusterLister{Clusters: listerClusters}, - subscriptionLister: &corelistertesting.SliceSubscriptionLister{Subscriptions: []*coreapi.Subscription{subscription}}, - clustersServiceClient: mockCS, + resourcesDBClient: mockDB, + clusterLister: &corelistertesting.SliceClusterLister{Clusters: listerClusters}, + subscriptionLister: &corelistertesting.SliceSubscriptionLister{Subscriptions: []*coreapi.Subscription{subscription}}, + clustersServiceClient: mockCS, + denyAssignmentsEnabled: !tt.denyAssignmentsDisabled, } key := controllerutils.HCPClusterKey{ diff --git a/backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller.go b/backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller.go index 977d8a3fe65..96107ad0d7f 100644 --- a/backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller.go +++ b/backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller.go @@ -264,6 +264,13 @@ func (c *clusterChildResourcesCleanupController) extraDeleteGateShouldDeleteServ return false, utils.TrackError(fmt.Errorf("failed to get ServiceProviderCluster: %w", err)) } + // We intentionally do not gate ServiceProviderCluster cleanup on the tracked deny assignments. + // Deny assignments are scoped to the managed resource group, so Azure deletes them in cascade + // when that resource group is removed during cluster teardown; the ClusterDenyAssignment + // controller therefore does nothing on delete and never clears these references. Gating here + // would block cleanup forever. (Per Manyanda Karombi's note on + // https://github.com/Azure/ARO-HCP/pull/6269#discussion_r3656341978.) + // Check if there are any Maestro readonly bundles remaining. if len(spc.Status.MaestroReadonlyBundles) > 0 { logger.Info("waiting for cluster-scoped Maestro readonly bundles to be deleted before removing Cosmos entry", diff --git a/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller.go b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller.go new file mode 100644 index 00000000000..0f631170b74 --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller.go @@ -0,0 +1,621 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package denyassignments + +import ( + "context" + "errors" + "fmt" + "math/rand/v2" + "strings" + "time" + + "github.com/google/uuid" + + "k8s.io/apimachinery/pkg/api/equality" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilsclock "k8s.io/utils/clock" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources" + + azureclient "github.com/Azure/ARO-HCP/backend/pkg/azure/client" + "github.com/Azure/ARO-HCP/backend/pkg/utils/controllerutils" + "github.com/Azure/ARO-HCP/internal/api/coreapi" + "github.com/Azure/ARO-HCP/internal/database/cosmosstorage/corecosmosstorage" + "github.com/Azure/ARO-HCP/internal/database/cosmosstorage/cosmosstorageutils" + "github.com/Azure/ARO-HCP/internal/database/informers/coreinformers" + "github.com/Azure/ARO-HCP/internal/database/listers/corelisters" + "github.com/Azure/ARO-HCP/internal/utils" +) + +const ClusterDenyAssignmentControllerName = "ClusterDenyAssignment" + +type clusterDenyAssignmentSyncer struct { + clock utilsclock.PassiveClock + resourcesDBClient corecosmosstorage.ResourcesDBClient + clusterLister corelisters.ClusterLister + subscriptionLister corelisters.SubscriptionLister + azureFPAClientBuilder azureclient.FirstPartyApplicationClientBuilder +} + +var _ controllerutils.ClusterSyncer = (*clusterDenyAssignmentSyncer)(nil) + +func NewClusterDenyAssignmentController( + clock utilsclock.PassiveClock, + resourcesDBClient corecosmosstorage.ResourcesDBClient, + azureFPAClientBuilder azureclient.FirstPartyApplicationClientBuilder, + backendInformers coreinformers.BackendInformers, +) controllerutils.Controller { + _, clusterLister := backendInformers.Clusters() + _, subscriptionLister := backendInformers.Subscriptions() + syncer := &clusterDenyAssignmentSyncer{ + clock: clock, + resourcesDBClient: resourcesDBClient, + clusterLister: clusterLister, + subscriptionLister: subscriptionLister, + azureFPAClientBuilder: azureFPAClientBuilder, + } + + return controllerutils.NewClusterWatchingController( + ClusterDenyAssignmentControllerName, + resourcesDBClient, + backendInformers, + nil, + time.Minute, + syncer, + ) +} + +func (c *clusterDenyAssignmentSyncer) SyncOnce(ctx context.Context, key controllerutils.HCPClusterKey) error { + cluster, err := c.clusterLister.Get(ctx, key.SubscriptionID, key.ResourceGroupName, key.HCPClusterName) + if cosmosstorageutils.IsNotFoundError(err) { + return nil + } + if err != nil { + return utils.TrackError(err) + } + + // Nothing to do while the cluster is being deleted. The deny assignments are scoped to the + // managed resource group, so Azure deletes them in cascade when that resource group is removed + // during cluster teardown; there is no need to issue ARM deletions or otherwise reconcile them + // here. (Per Manyanda Karombi's note on https://github.com/Azure/ARO-HCP/pull/6269#discussion_r3656341978.) + if cluster.ServiceProviderProperties.DeletionTimestamp != nil { + return nil + } + + return c.syncDenyAssignmentUpsert(ctx, key, cluster) +} + +func (c *clusterDenyAssignmentSyncer) syncDenyAssignmentNeedsWork(cluster *coreapi.HCPOpenShiftCluster, serviceProviderCluster *coreapi.ServiceProviderCluster) bool { + if len(controllerutils.ClusterServiceIDForCluster(cluster)) == 0 { + return false + } + // Deny assignments are scoped to the managed resource group, which is derived from the + // cluster's ManagedResourceGroup name. (Status.AzureResources.ManagedResourceGroup.AzureResource + // is not populated by any controller, so it cannot be relied on here.) + if len(cluster.CustomerProperties.Platform.ManagedResourceGroup) == 0 { + return false + } + + // we need these identities to exclude them from deny assignments. + identities := cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities + if len(identities.ControlPlaneOperators) == 0 || len(identities.DataPlaneOperators) == 0 || identities.ServiceManagedIdentity == nil { + return false + } + for _, v := range identities.ControlPlaneOperators { + if v == nil { + return false + } + } + for _, v := range identities.DataPlaneOperators { + if v == nil { + return false + } + } + + if len(serviceProviderCluster.Status.AzureResources.DenyAssignments.PendingAzureResources) > 0 { + return true + } + if len(serviceProviderCluster.Status.AzureResources.DenyAssignments.AzureResources) == 0 { + return true + } + if t := serviceProviderCluster.Status.AzureResources.DenyAssignments.EarliestRecheckTime; t != nil && c.clock.Now().Before(t.Time) { + return false + } + return true +} + +func (c *clusterDenyAssignmentSyncer) syncDenyAssignmentUpsert(ctx context.Context, key controllerutils.HCPClusterKey, cluster *coreapi.HCPOpenShiftCluster) error { + logger := utils.LoggerFromContext(ctx) + + serviceProviderCluster, err := corecosmosstorage.GetOrCreateServiceProviderCluster(ctx, c.resourcesDBClient, cluster.ID) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to get or create ServiceProviderCluster: %w", err)) + } + + if !c.syncDenyAssignmentNeedsWork(cluster, serviceProviderCluster) { + return nil + } + + subscription, err := c.subscriptionLister.Get(ctx, key.SubscriptionID) + if err != nil { + return utils.TrackError(err) + } + if subscription.Properties == nil || subscription.Properties.TenantId == nil { + return utils.TrackError(fmt.Errorf("subscription %s has no tenantId", key.SubscriptionID)) + } + tenantID := *subscription.Properties.TenantId + + serviceProviderClusterCRUD := c.resourcesDBClient.ServiceProviderClusters(key.SubscriptionID, key.ResourceGroupName, key.HCPClusterName) + + // The managed resource group is the scope for the cluster's deny assignments. Build its resource + // ID from the cluster's ManagedResourceGroup name (the same source allDenyAssignmentReferences + // uses for the deny assignment resource IDs). + managedResourceGroupID, err := coreapi.ToResourceGroupResourceID(key.SubscriptionID, cluster.CustomerProperties.Platform.ManagedResourceGroup) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to build managed resource group resource ID: %w", err)) + } + + requiredDenyAssignmentReferences, err := allDenyAssignmentReferences(cluster) + if err != nil { + return utils.TrackError(err) + } + requiredDenyAssignmentReferenceByType := make(map[string]coreapi.DenyAssignmentReference, len(requiredDenyAssignmentReferences)) + for _, ref := range requiredDenyAssignmentReferences { + requiredDenyAssignmentReferenceByType[ref.DenyAssignmentType] = ref + } + denyAssignmentDefs := denyAssignmentDefinitions(cluster) + denyAssignmentDefinitionsByType := make(map[string]denyAssignmentDefinition, len(denyAssignmentDefs)) + for _, d := range denyAssignmentDefs { + denyAssignmentDefinitionsByType[d.denyAssignmentType] = d + } + + genericResourcesClient, err := c.azureFPAClientBuilder.GenericResourcesClient(tenantID, key.SubscriptionID) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to create generic resources client: %w", err)) + } + denyAssignmentsClient, err := c.azureFPAClientBuilder.DenyAssignmentsClient(tenantID, key.SubscriptionID) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to create deny assignments client: %w", err)) + } + + // Delete any deny assignments whose type is no longer required. + replacement := serviceProviderCluster.DeepCopy() + var staleDeletionErrs []error + for _, existing := range serviceProviderCluster.Status.AzureResources.DenyAssignments.AzureResources { + if _, isRequired := requiredDenyAssignmentReferenceByType[existing.DenyAssignmentType]; !isRequired { + if err := c.deleteDenyAssignment(ctx, genericResourcesClient, existing.DenyAssignmentResourceID); err != nil { + staleDeletionErrs = append(staleDeletionErrs, utils.TrackError(fmt.Errorf("failed to delete stale deny assignment %s: %w", existing.DenyAssignmentType, err))) + continue + } + logger.Info("Deleted stale deny assignment from Azure", "denyAssignmentType", existing.DenyAssignmentType) + replacement.Status.AzureResources.DenyAssignments.AzureResources = removeDenyAssignmentRef(replacement.Status.AzureResources.DenyAssignments.AzureResources, existing.DenyAssignmentType) + } + } + serviceProviderCluster, replacement, err = replaceServiceProviderClusterIfChanged(ctx, serviceProviderClusterCRUD, serviceProviderCluster, replacement, staleDeletionErrs) + if serviceProviderCluster == nil || err != nil { + return err + } + + // Ensure all existing deny assignments have correct content. + // Succeeded stay in AzureResources; failed move to pending for retry. + ensureExistingSucceeded, ensureExistingFailed, ensureExistingErr := c.ensureDenyAssignmentReferences(ctx, cluster, denyAssignmentsClient, genericResourcesClient, + managedResourceGroupID, denyAssignmentDefinitionsByType, replacement.Status.AzureResources.DenyAssignments.AzureResources) + replacement.Status.AzureResources.DenyAssignments.AzureResources = ensureExistingSucceeded + replacement.Status.AzureResources.DenyAssignments.PendingAzureResources = appendDenyAssignmentReference(replacement.Status.AzureResources.DenyAssignments.PendingAzureResources, ensureExistingFailed...) + serviceProviderCluster, replacement, err = replaceServiceProviderClusterIfChanged(ctx, serviceProviderClusterCRUD, serviceProviderCluster, replacement, []error{ensureExistingErr}) + if serviceProviderCluster == nil || err != nil { + return err + } + + // Add any required types not already in AzureResources or pending. + for _, ref := range requiredDenyAssignmentReferences { + found := false + for _, existing := range replacement.Status.AzureResources.DenyAssignments.AzureResources { + if existing.DenyAssignmentType == ref.DenyAssignmentType { + found = true + break + } + } + if !found { + replacement.Status.AzureResources.DenyAssignments.PendingAzureResources = appendDenyAssignmentReference(replacement.Status.AzureResources.DenyAssignments.PendingAzureResources, ref) + } + } + serviceProviderCluster, replacement, err = replaceServiceProviderClusterIfChanged(ctx, serviceProviderClusterCRUD, serviceProviderCluster, replacement, nil) + if serviceProviderCluster == nil || err != nil { + return err + } + + // Ensure all pending deny assignments exist in Azure with correct content. + // Succeeded move to AzureResources; failed stay in pending. + ensurePendingSucceeded, ensurePendingFailed, ensurePendingErr := c.ensureDenyAssignmentReferences(ctx, cluster, denyAssignmentsClient, genericResourcesClient, + managedResourceGroupID, denyAssignmentDefinitionsByType, replacement.Status.AzureResources.DenyAssignments.PendingAzureResources) + replacement.Status.AzureResources.DenyAssignments.AzureResources = appendDenyAssignmentReference(replacement.Status.AzureResources.DenyAssignments.AzureResources, ensurePendingSucceeded...) + replacement.Status.AzureResources.DenyAssignments.PendingAzureResources = ensurePendingFailed + serviceProviderCluster, replacement, err = replaceServiceProviderClusterIfChanged(ctx, serviceProviderClusterCRUD, serviceProviderCluster, replacement, []error{ensurePendingErr}) + if serviceProviderCluster == nil || err != nil { + return err + } + + if len(replacement.Status.AzureResources.DenyAssignments.PendingAzureResources) == 0 { + replacement.Status.AzureResources.DenyAssignments.EarliestRecheckTime = c.recheckTime() + } else { + // we're unlikely to reach here with PendingAzureResource, but if we do, running again is probably a good idea. + replacement.Status.AzureResources.DenyAssignments.EarliestRecheckTime = nil + } + _, _, err = replaceServiceProviderClusterIfChanged(ctx, serviceProviderClusterCRUD, serviceProviderCluster, replacement, nil) + return err +} + +func replaceServiceProviderClusterIfChanged( + ctx context.Context, + serviceProviderClusterCRUD cosmosstorageutils.ResourceCRUD[coreapi.ServiceProviderCluster, *coreapi.ServiceProviderCluster], + serviceProviderCluster *coreapi.ServiceProviderCluster, + replacement *coreapi.ServiceProviderCluster, + priorErrs []error, +) (*coreapi.ServiceProviderCluster, *coreapi.ServiceProviderCluster, error) { + joinedPriorErr := errors.Join(priorErrs...) + if equality.Semantic.DeepEqual(serviceProviderCluster, replacement) { + if joinedPriorErr != nil { + return nil, nil, joinedPriorErr + } + return serviceProviderCluster, replacement, nil + } + updated, err := serviceProviderClusterCRUD.Replace(ctx, replacement, nil) + if cosmosstorageutils.IsPreconditionFailedError(err) { + if joinedPriorErr != nil { + return nil, nil, joinedPriorErr + } + return nil, nil, nil + } + if err != nil { + return nil, nil, errors.Join(joinedPriorErr, utils.TrackError(fmt.Errorf("failed to replace ServiceProviderCluster: %w", err))) + } + return updated, updated.DeepCopy(), nil +} + +func (c *clusterDenyAssignmentSyncer) ensureDenyAssignmentReferences( + ctx context.Context, + cluster *coreapi.HCPOpenShiftCluster, + denyAssignmentsClient azureclient.DenyAssignmentsClient, + genericResourcesClient azureclient.GenericResourcesClient, + scope *azcorearm.ResourceID, + denyAssignmentDefinitionsByType map[string]denyAssignmentDefinition, + refs []coreapi.DenyAssignmentReference, +) (succeeded, failed []coreapi.DenyAssignmentReference, err error) { + logger := utils.LoggerFromContext(ctx) + var errs []error + + for _, ref := range refs { + definition, ok := denyAssignmentDefinitionsByType[ref.DenyAssignmentType] + if !ok { + // A pending/tracked type with no matching definition can never be reconciled and would + // otherwise keep the cluster blocked while the controller reports success. Surface it as + // an error so the sync degrades and the problem is visible. + errs = append(errs, utils.TrackError(fmt.Errorf("no definition for deny assignment type %q", ref.DenyAssignmentType))) + failed = append(failed, ref) + continue + } + + excludedIdentityResourceIDs, err := collectExcludedPrincipalIDs(cluster, definition) + if err != nil { + errs = append(errs, utils.TrackError(fmt.Errorf("failed to collect excluded identity resource IDs for %s: %w", ref.DenyAssignmentType, err))) + failed = append(failed, ref) + continue + } + + err = c.ensureDenyAssignment(ctx, cluster, denyAssignmentsClient, genericResourcesClient, + ref.DenyAssignmentResourceID, scope, excludedIdentityResourceIDs, + definition.actions, definition.notActions, definition.dataActions) + if err != nil { + errs = append(errs, utils.TrackError(fmt.Errorf("failed to ensure deny assignment %s: %w", ref.DenyAssignmentType, err))) + failed = append(failed, ref) + continue + } + + logger.Info("Ensured deny assignment", "denyAssignmentType", ref.DenyAssignmentType, "resourceID", ref.DenyAssignmentResourceID.String()) + succeeded = append(succeeded, ref) + } + + return succeeded, failed, errors.Join(errs...) +} + +func (c *clusterDenyAssignmentSyncer) ensureDenyAssignment( + ctx context.Context, + cluster *coreapi.HCPOpenShiftCluster, + denyAssignmentsClient azureclient.DenyAssignmentsClient, + genericResourcesClient azureclient.GenericResourcesClient, + resourceID *azcorearm.ResourceID, + scope *azcorearm.ResourceID, + excludedIdentityResourceIDs []*azcorearm.ResourceID, + actions []string, + notActions []string, + dataActions []string, +) error { + if notActions == nil { + notActions = []string{} + } + if dataActions == nil { + dataActions = []string{} + } + + excludedPrincipalIDs, err := resolvePrincipalIDs(cluster, excludedIdentityResourceIDs) + if err != nil { + return utils.TrackError(fmt.Errorf("failed to resolve principal IDs: %w", err)) + } + + existing, err := denyAssignmentsClient.Get(ctx, scope.String(), resourceID.Name, nil) + if err != nil && !isDenyAssignmentNotFoundError(err) { + return utils.TrackError(fmt.Errorf("failed to get deny assignment: %w", err)) + } + if err == nil && !denyAssignmentNeedsUpdate(&existing.DenyAssignment, actions, notActions, dataActions, excludedPrincipalIDs) { + return nil + } + + excludedPrincipals := make([]any, 0, len(excludedPrincipalIDs)) + for _, id := range excludedPrincipalIDs { + excludedPrincipals = append(excludedPrincipals, map[string]any{ + "id": id, + "type": "ServicePrincipal", + }) + } + + resource := armresources.GenericResource{ + Location: to.Ptr("global"), + Properties: map[string]any{ + "DenyAssignmentName": resourceID.Name, + "Permissions": []any{ + map[string]any{ + "actions": actions, + "notActions": notActions, + "dataActions": dataActions, + "notDataActions": []string{}, + }, + }, + "Scope": scope.String(), + "Principals": []any{ + map[string]any{ + "id": allPrincipalsGUID, + "type": "SystemDefined", + }, + }, + "ExcludePrincipals": excludedPrincipals, + "IsSystemProtected": true, + }, + } + + poller, err := genericResourcesClient.BeginCreateOrUpdateByID(ctx, resourceID.String(), denyAssignmentAzureAPIVersion, resource, nil) + if err != nil { + return utils.TrackError(fmt.Errorf("BeginCreateOrUpdateByID failed: %w", err)) + } + + _, err = poller.PollUntilDone(ctx, nil) + if err != nil { + return utils.TrackError(fmt.Errorf("polling deny assignment creation failed: %w", err)) + } + + return nil +} + +func isDenyAssignmentNotFoundError(err error) bool { + var azErr *azcore.ResponseError + return errors.As(err, &azErr) && azErr.ErrorCode == "DenyAssignmentNotFound" +} + +func denyAssignmentNeedsUpdate( + existing *armauthorization.DenyAssignment, + expectedActions []string, + expectedNotActions []string, + expectedDataActions []string, + expectedExcludedPrincipalIDs []string, +) bool { + if existing.Properties == nil || existing.Properties.Permissions == nil { + return true + } + if len(existing.Properties.Permissions) != 1 { + return true + } + + perm := existing.Properties.Permissions[0] + if !ptrStringSliceEqual(perm.Actions, expectedActions) { + return true + } + if !ptrStringSliceEqual(perm.NotActions, expectedNotActions) { + return true + } + if !ptrStringSliceEqual(perm.DataActions, expectedDataActions) { + return true + } + if !excludedPrincipalsEqual(existing.Properties.ExcludePrincipals, expectedExcludedPrincipalIDs) { + return true + } + return false +} + +func ptrStringSliceEqual(a []*string, b []string) bool { + if len(a) != len(b) { + return false + } + set := make(map[string]struct{}, len(b)) + for _, s := range b { + set[s] = struct{}{} + } + for _, ptr := range a { + s := "" + if ptr != nil { + s = *ptr + } + if _, ok := set[s]; !ok { + return false + } + delete(set, s) + } + return len(set) == 0 +} + +func excludedPrincipalsEqual(existing []*armauthorization.Principal, expected []string) bool { + if len(existing) != len(expected) { + return false + } + set := make(map[string]struct{}, len(expected)) + for _, id := range expected { + set[id] = struct{}{} + } + for _, p := range existing { + if p == nil || p.ID == nil { + return false + } + if _, ok := set[*p.ID]; !ok { + return false + } + delete(set, *p.ID) + } + return len(set) == 0 +} + +func (c *clusterDenyAssignmentSyncer) deleteDenyAssignment( + ctx context.Context, + client azureclient.GenericResourcesClient, + resourceID *azcorearm.ResourceID, +) error { + poller, err := client.BeginDeleteByID(ctx, resourceID.String(), denyAssignmentAzureAPIVersion, nil) + if isResourceNotFoundError(err) { + return nil + } + if err != nil { + return utils.TrackError(fmt.Errorf("BeginDeleteByID failed: %w", err)) + } + + _, err = poller.PollUntilDone(ctx, nil) + if isResourceNotFoundError(err) { + return nil + } + if err != nil { + return utils.TrackError(fmt.Errorf("polling deny assignment deletion failed: %w", err)) + } + + return nil +} + +func isResourceNotFoundError(err error) bool { + var azErr *azcore.ResponseError + return errors.As(err, &azErr) && azErr.StatusCode == 404 +} + +func (c *clusterDenyAssignmentSyncer) recheckTime() *metav1.Time { + recheckDuration := 12 * time.Hour + jitter := time.Duration(rand.Int64N(int64(recheckDuration))) + t := metav1.NewTime(c.clock.Now().Add(recheckDuration/2 + jitter)) + return &t +} + +// generateDenyAssignmentUUID deterministically derives a deny assignment's UUID exactly the way +// Cluster Service does, so both the RP and Cluster Service compute the same deny assignment IDs for +// a cluster without having to share them. It MUST stay byte-for-byte identical to Cluster Service's +// uuid.GenerateUuidV5(denyAssignmentNamespaceUuid, clusterID, suffix): a v5 (SHA-1) UUID over the +// shared namespace and the input string "$" — Cluster Service joins its salts +// suffix-first with "$". clusterID is the OCM Cluster Service cluster ID (InternalID.ClusterID()), +// and denyAssignmentType is the per-type suffix (e.g. "compute-deny-assignment"). +// +// See aro-hcp-clusters-service pkg/azure/denyassignmentcreator/deny_assignment_creator.go +// (generateDenyAssigmentId) and pkg/utils/uuid/generators.go (generateUuidV5WithSeparator). +// TestGenerateDenyAssignmentUUIDMatchesClusterService pins this equivalence. +func generateDenyAssignmentUUID(clusterID, denyAssignmentType string) string { + namespace := uuid.MustParse(denyAssignmentNamespaceUUID) + // Equivalent to Cluster Service's strings.Join([]string{denyAssignmentType, clusterID}, "$"). + return uuid.NewSHA1(namespace, []byte(denyAssignmentType+"$"+clusterID)).String() +} + +func collectExcludedPrincipalIDs(cluster *coreapi.HCPOpenShiftCluster, definition denyAssignmentDefinition) ([]*azcorearm.ResourceID, error) { + var identityResourceIDs []*azcorearm.ResourceID + + identities := cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities + + for _, operatorName := range definition.controlPlaneOperators { + resourceID, ok := identities.ControlPlaneOperators[operatorName] + if !ok || resourceID == nil { + return nil, fmt.Errorf("control plane operator %q not found in cluster identity configuration", operatorName) + } + identityResourceIDs = append(identityResourceIDs, resourceID) + } + + for _, operatorName := range definition.dataPlaneOperators { + resourceID, ok := identities.DataPlaneOperators[operatorName] + if !ok || resourceID == nil { + return nil, fmt.Errorf("data plane operator %q not found in cluster identity configuration", operatorName) + } + identityResourceIDs = append(identityResourceIDs, resourceID) + } + + if definition.includeServiceManagedID { + if identities.ServiceManagedIdentity == nil { + return nil, fmt.Errorf("service managed identity not found in cluster identity configuration") + } + identityResourceIDs = append(identityResourceIDs, identities.ServiceManagedIdentity) + } + + return identityResourceIDs, nil +} + +func resolvePrincipalIDs(cluster *coreapi.HCPOpenShiftCluster, identityResourceIDs []*azcorearm.ResourceID) ([]string, error) { + if cluster.Identity == nil { + return nil, fmt.Errorf("cluster has no identity configuration") + } + + lookup := make(map[string]string, len(cluster.Identity.UserAssignedIdentities)) + for resourceID, identity := range cluster.Identity.UserAssignedIdentities { + if identity != nil && identity.PrincipalID != nil { + lookup[strings.ToLower(resourceID)] = *identity.PrincipalID + } + } + + principalIDs := make([]string, 0, len(identityResourceIDs)) + for _, identityResourceID := range identityResourceIDs { + principalID, ok := lookup[strings.ToLower(identityResourceID.String())] + if !ok { + return nil, fmt.Errorf("principal ID not found for identity %s", identityResourceID.String()) + } + principalIDs = append(principalIDs, principalID) + } + return principalIDs, nil +} + +func appendDenyAssignmentReference(slice []coreapi.DenyAssignmentReference, refs ...coreapi.DenyAssignmentReference) []coreapi.DenyAssignmentReference { + existing := make(map[string]struct{}, len(slice)) + for _, ref := range slice { + existing[ref.DenyAssignmentType] = struct{}{} + } + for _, ref := range refs { + if _, ok := existing[ref.DenyAssignmentType]; !ok { + slice = append(slice, ref) + existing[ref.DenyAssignmentType] = struct{}{} + } + } + return slice +} + +func removeDenyAssignmentRef(slice []coreapi.DenyAssignmentReference, denyAssignmentType string) []coreapi.DenyAssignmentReference { + result := make([]coreapi.DenyAssignmentReference, 0, len(slice)) + for _, denyAssignmentReference := range slice { + if denyAssignmentReference.DenyAssignmentType != denyAssignmentType { + result = append(result, denyAssignmentReference) + } + } + return result +} diff --git a/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller_test.go b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller_test.go new file mode 100644 index 00000000000..1ca2fecdc3e --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller_test.go @@ -0,0 +1,918 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package denyassignments + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/go-logr/logr/testr" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clocktesting "k8s.io/utils/clock/testing" + "k8s.io/utils/ptr" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + azcorearm "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + + "github.com/Azure/ARO-HCP/backend/pkg/azure/azuremockclient" + "github.com/Azure/ARO-HCP/backend/pkg/utils/controllerutils" + "github.com/Azure/ARO-HCP/internal/api/coreapi" + "github.com/Azure/ARO-HCP/internal/api/metadataapi" + "github.com/Azure/ARO-HCP/internal/apitesting/coreapitesting" + "github.com/Azure/ARO-HCP/internal/database/cosmosstoragetesting/corecosmosstoragetesting" + "github.com/Azure/ARO-HCP/internal/database/listertesting/corelistertesting" + "github.com/Azure/ARO-HCP/internal/utils" +) + +const ( + testSubscriptionID = "00000000-0000-0000-0000-000000000000" + testResourceGroupName = "test-rg" + testClusterName = "test-cluster" + testTenantID = "11111111-1111-1111-1111-111111111111" + testManagedRG = "testManagedResourceGroup" +) + +func testClusterResourceID() *azcorearm.ResourceID { + return metadataapi.Must(azcorearm.ParseResourceID( + "/subscriptions/" + testSubscriptionID + + "/resourceGroups/" + testResourceGroupName + + "/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName, + )) +} + +func testManagedResourceGroupID() *azcorearm.ResourceID { + return metadataapi.Must(azcorearm.ParseResourceID( + "/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testManagedRG, + )) +} + +func testKey() controllerutils.HCPClusterKey { + return controllerutils.HCPClusterKey{ + SubscriptionID: testSubscriptionID, + ResourceGroupName: testResourceGroupName, + HCPClusterName: testClusterName, + } +} + +func testSubscription() *coreapi.Subscription { + rid := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/" + testSubscriptionID)) + return &coreapi.Subscription{ + CosmosMetadata: coreapi.CosmosMetadata{ + ResourceID: rid, + PartitionKey: strings.ToLower(rid.SubscriptionID), + }, + ResourceID: rid, + Properties: &coreapi.SubscriptionProperties{TenantId: ptr.To(testTenantID)}, + } +} + +func testIdentityResourceID(name string) *azcorearm.ResourceID { + return metadataapi.Must(azcorearm.ParseResourceID( + "/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testResourceGroupName + + "/providers/Microsoft.ManagedIdentity/userAssignedIdentities/" + name, + )) +} + +func newTestCluster(opts ...func(*coreapi.HCPOpenShiftCluster)) *coreapi.HCPOpenShiftCluster { + rid := testClusterResourceID() + cluster := coreapitesting.MinimumValidClusterTestCase() + cluster.CosmosMetadata = coreapi.CosmosMetadata{ + ResourceID: rid, + PartitionKey: strings.ToLower(rid.SubscriptionID), + } + cluster.ID = rid + cluster.Name = testClusterName + cluster.Type = rid.ResourceType.String() + cluster.ServiceProviderProperties.ClusterServiceID = nil + + csID := metadataapi.Must(metadataapi.NewInternalID("/api/aro_hcp/v1alpha1/clusters/abc123")) + cluster.ServiceProviderProperties.PendingClusterServiceID = &csID + cluster.CustomerProperties.Platform.ManagedResourceGroup = testManagedRG + + cpOps := map[string]*azcorearm.ResourceID{ + "cluster-api-azure": testIdentityResourceID("capi-azure"), + "cloud-controller-manager": testIdentityResourceID("ccm"), + "disk-csi-driver": testIdentityResourceID("disk-csi"), + "control-plane": testIdentityResourceID("control-plane"), + "image-registry": testIdentityResourceID("image-registry"), + "file-csi-driver": testIdentityResourceID("file-csi"), + "kms": testIdentityResourceID("kms"), + "ingress": testIdentityResourceID("ingress"), + "cloud-network-config": testIdentityResourceID("cloud-network-config"), + } + dpOps := map[string]*azcorearm.ResourceID{ + "image-registry": testIdentityResourceID("dp-image-registry"), + "disk-csi-driver": testIdentityResourceID("dp-disk-csi"), + "file-csi-driver": testIdentityResourceID("dp-file-csi"), + } + serviceManagedID := testIdentityResourceID("service-managed") + + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators = cpOps + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators = dpOps + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity = serviceManagedID + + allIdentities := make(map[string]*coreapi.UserAssignedIdentity) + for _, id := range cpOps { + allIdentities[strings.ToLower(id.String())] = &coreapi.UserAssignedIdentity{PrincipalID: ptr.To("principal-" + id.Name)} + } + for _, id := range dpOps { + allIdentities[strings.ToLower(id.String())] = &coreapi.UserAssignedIdentity{PrincipalID: ptr.To("principal-" + id.Name)} + } + allIdentities[strings.ToLower(serviceManagedID.String())] = &coreapi.UserAssignedIdentity{PrincipalID: ptr.To("principal-service-managed")} + cluster.Identity = &coreapi.ManagedServiceIdentity{ + UserAssignedIdentities: allIdentities, + } + + for _, opt := range opts { + opt(cluster) + } + return cluster +} + +func newTestSPC(opts ...func(*coreapi.ServiceProviderCluster)) *coreapi.ServiceProviderCluster { + resourceID := metadataapi.Must(azcorearm.ParseResourceID(fmt.Sprintf("%s/%s/%s", + testClusterResourceID().String(), + coreapi.ServiceProviderClusterResourceTypeName, + coreapi.ServiceProviderClusterResourceName, + ))) + spc := &coreapi.ServiceProviderCluster{ + CosmosMetadata: coreapi.CosmosMetadata{ResourceID: resourceID}, + Spec: coreapi.ServiceProviderClusterSpec{}, + } + spc.SetPartitionKey(testSubscriptionID) + spc.Status.AzureResources.ManagedResourceGroup.AzureResource = testManagedResourceGroupID() + for _, opt := range opts { + opt(spc) + } + return spc +} + +func denyAssignmentNotFoundError() error { + return &azcore.ResponseError{ErrorCode: "DenyAssignmentNotFound"} +} + +func resourceNotFoundError() error { + return &azcore.ResponseError{StatusCode: 404} +} + +func matchingGetResponseForAllTypes(cluster *coreapi.HCPOpenShiftCluster) func(ctx context.Context, scope string, denyAssignmentID string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + refs, _ := allDenyAssignmentReferences(cluster) + nameToType := make(map[string]string, len(refs)) + for _, ref := range refs { + nameToType[ref.DenyAssignmentResourceID.Name] = ref.DenyAssignmentType + } + + defs := denyAssignmentDefinitions(cluster) + defsByType := make(map[string]denyAssignmentDefinition, len(defs)) + for _, d := range defs { + defsByType[d.denyAssignmentType] = d + } + + return func(ctx context.Context, scope string, denyAssignmentID string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + daType, ok := nameToType[denyAssignmentID] + if !ok { + return armauthorization.DenyAssignmentsClientGetResponse{}, denyAssignmentNotFoundError() + } + def := defsByType[daType] + notActions := def.notActions + if notActions == nil { + notActions = []string{} + } + dataActions := def.dataActions + if dataActions == nil { + dataActions = []string{} + } + excludedIDs, _ := collectExcludedPrincipalIDs(cluster, def) + principalIDs, _ := resolvePrincipalIDs(cluster, excludedIDs) + excludedPrincipals := make([]*armauthorization.Principal, 0, len(principalIDs)) + for _, pid := range principalIDs { + excludedPrincipals = append(excludedPrincipals, &armauthorization.Principal{ID: ptr.To(pid)}) + } + + return armauthorization.DenyAssignmentsClientGetResponse{ + DenyAssignment: armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + { + Actions: to.SliceOfPtrs(def.actions...), + NotActions: to.SliceOfPtrs(notActions...), + DataActions: to.SliceOfPtrs(dataActions...), + }, + }, + ExcludePrincipals: excludedPrincipals, + }, + }, + }, nil + } +} + +func TestGenerateDenyAssignmentUUIDMatchesClusterService(t *testing.T) { + // referenceClusterServiceUUID replicates Cluster Service's derivation exactly + // (pkg/utils/uuid/generators.go generateUuidV5WithSeparator): a v5 UUID over the shared + // namespace and strings.Join([]string{suffix, clusterID}, "$"). If this diverges from + // generateDenyAssignmentUUID, the RP and Cluster Service would compute different deny assignment + // IDs for the same cluster and stop recognizing each other's assignments. + referenceClusterServiceUUID := func(clusterID, suffix string) string { + ns := uuid.MustParse(denyAssignmentNamespaceUUID) + return uuid.NewSHA1(ns, []byte(strings.Join([]string{suffix, clusterID}, "$"))).String() + } + + clusterIDs := []string{"2abcdef1234567890abcdef123456789", "another-cs-cluster-id"} + for _, clusterID := range clusterIDs { + for _, def := range denyAssignmentDefinitions(newTestCluster()) { + assert.Equal(t, + referenceClusterServiceUUID(clusterID, def.denyAssignmentType), + generateDenyAssignmentUUID(clusterID, def.denyAssignmentType), + "deny assignment UUID for %q must match Cluster Service's derivation", def.denyAssignmentType) + } + } + + // Golden value (computed with Cluster Service's algorithm) guards against the namespace, + // separator, or salt order changing on both sides at once. + assert.Equal(t, + "c4ff85a1-5daa-5ed4-b4e2-fdf60a7d24ad", + generateDenyAssignmentUUID("2abcdef1234567890abcdef123456789", "compute-deny-assignment"), + "deny assignment UUID derivation must not change (would desync from Cluster Service)") +} + +func TestSyncOnce(t *testing.T) { + tests := []struct { + name string + cluster *coreapi.HCPOpenShiftCluster + }{ + { + name: "cluster not found returns nil", + cluster: nil, + }, + { + name: "deletion timestamp set returns nil", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + now := metav1.Now() + c.ServiceProviderProperties.DeletionTimestamp = &now + }), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := utils.ContextWithLogger(context.Background(), testr.New(t)) + var clusters []*coreapi.HCPOpenShiftCluster + if tt.cluster != nil { + clusters = []*coreapi.HCPOpenShiftCluster{tt.cluster} + } + syncer := &clusterDenyAssignmentSyncer{ + clock: clocktesting.NewFakeClock(time.Now()), + clusterLister: &corelistertesting.SliceClusterLister{Clusters: clusters}, + } + err := syncer.SyncOnce(ctx, testKey()) + require.NoError(t, err) + }) + } +} + +func TestSyncDenyAssignmentNeedsWork(t *testing.T) { + fakeClock := clocktesting.NewFakeClock(time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)) + syncer := &clusterDenyAssignmentSyncer{clock: fakeClock} + + tests := []struct { + name string + cluster *coreapi.HCPOpenShiftCluster + spc *coreapi.ServiceProviderCluster + expected bool + }{ + { + name: "no cluster service ID", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.ServiceProviderProperties.PendingClusterServiceID = nil + c.ServiceProviderProperties.ClusterServiceID = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "no managed resource group", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.ManagedResourceGroup = "" + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "empty control plane operators", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "empty data plane operators", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "nil service managed identity", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "nil control plane operator value", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators["cluster-api-azure"] = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "nil data plane operator value", + cluster: newTestCluster(func(c *coreapi.HCPOpenShiftCluster) { + c.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators["image-registry"] = nil + }), + spc: newTestSPC(), + expected: false, + }, + { + name: "has pending deny assignments", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.PendingAzureResources = []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "some-type"}, + } + }), + expected: true, + }, + { + name: "no azure resources and no pending -- first time", + cluster: newTestCluster(), + spc: newTestSPC(), + expected: true, + }, + { + name: "azure resources present, before recheck time", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "resources-deny-assignment"}, + } + future := metav1.NewTime(fakeClock.Now().Add(1 * time.Hour)) + spc.Status.AzureResources.DenyAssignments.EarliestRecheckTime = &future + }), + expected: false, + }, + { + name: "azure resources present, past recheck time", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "resources-deny-assignment"}, + } + past := metav1.NewTime(fakeClock.Now().Add(-1 * time.Hour)) + spc.Status.AzureResources.DenyAssignments.EarliestRecheckTime = &past + }), + expected: true, + }, + { + name: "azure resources present, nil recheck time", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "resources-deny-assignment"}, + } + }), + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := syncer.syncDenyAssignmentNeedsWork(tt.cluster, tt.spc) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestSyncDenyAssignmentUpsert(t *testing.T) { + fakeClock := clocktesting.NewFakeClock(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)) + + tests := []struct { + name string + cluster *coreapi.HCPOpenShiftCluster + existingSPC *coreapi.ServiceProviderCluster + mockDenyAssignments *azuremockclient.DenyAssignmentsClientFunc + mockGenericResources *azuremockclient.GenericResourcesClientFunc + expectError bool + verify func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) + }{ + { + name: "first time initializes pending, create fails leaves them pending", + cluster: newTestCluster(), + existingSPC: newTestSPC(), + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: func(ctx context.Context, scope string, id string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + return armauthorization.DenyAssignmentsClientGetResponse{}, denyAssignmentNotFoundError() + }, + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{ + CreateErr: fmt.Errorf("simulated create failure"), + }, + expectError: true, + verify: func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) { + spc, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + assert.NotEmpty(t, spc.Status.AzureResources.DenyAssignments.PendingAzureResources, "pending should be populated after first-time initialization") + }, + }, + { + name: "removes stale deny assignment not in definitions", + cluster: newTestCluster(), + existingSPC: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = []coreapi.DenyAssignmentReference{ + { + DenyAssignmentType: "stale-type-not-in-definitions", + DenyAssignmentResourceID: metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testManagedRG + "/providers/Microsoft.Authorization/denyAssignments/stale-uuid")), + }, + } + }), + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: func(ctx context.Context, scope string, id string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + return armauthorization.DenyAssignmentsClientGetResponse{}, denyAssignmentNotFoundError() + }, + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{ + DeleteErr: resourceNotFoundError(), + CreateErr: fmt.Errorf("simulated create failure"), + }, + expectError: true, + verify: func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) { + spc, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + for _, ref := range spc.Status.AzureResources.DenyAssignments.AzureResources { + assert.NotEqual(t, "stale-type-not-in-definitions", ref.DenyAssignmentType, "stale type should have been removed") + } + }, + }, + { + name: "existing content up to date sets recheck time", + cluster: newTestCluster(), + existingSPC: func() *coreapi.ServiceProviderCluster { + cluster := newTestCluster() + refs, _ := allDenyAssignmentReferences(cluster) + return newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = refs + }) + }(), + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: matchingGetResponseForAllTypes(newTestCluster()), + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{}, + expectError: false, + verify: func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) { + spc, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + assert.NotEmpty(t, spc.Status.AzureResources.DenyAssignments.AzureResources, "AzureResources should remain populated") + assert.Empty(t, spc.Status.AzureResources.DenyAssignments.PendingAzureResources, "PendingAzureResources should be empty") + assert.NotNil(t, spc.Status.AzureResources.DenyAssignments.EarliestRecheckTime, "EarliestRecheckTime should be set") + }, + }, + { + name: "existing content mismatched triggers update attempt, failure moves to pending", + cluster: newTestCluster(), + existingSPC: func() *coreapi.ServiceProviderCluster { + cluster := newTestCluster() + refs, _ := allDenyAssignmentReferences(cluster) + return newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = refs + }) + }(), + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: func(ctx context.Context, scope string, id string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + return armauthorization.DenyAssignmentsClientGetResponse{ + DenyAssignment: armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + { + Actions: to.SliceOfPtrs("wrong-action"), + NotActions: to.SliceOfPtrs[string](), + DataActions: to.SliceOfPtrs[string](), + }, + }, + }, + }, + }, nil + }, + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{ + CreateErr: fmt.Errorf("simulated create failure"), + }, + expectError: true, + verify: func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) { + spc, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + assert.NotEmpty(t, spc.Status.AzureResources.DenyAssignments.PendingAzureResources, "failed ensure should move refs to pending") + }, + }, + { + name: "recheck time in future skips all work", + cluster: newTestCluster(), + existingSPC: func() *coreapi.ServiceProviderCluster { + cluster := newTestCluster() + refs, _ := allDenyAssignmentReferences(cluster) + future := metav1.NewTime(fakeClock.Now().Add(6 * time.Hour)) + return newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.AzureResources = refs + spc.Status.AzureResources.DenyAssignments.EarliestRecheckTime = &future + }) + }(), + mockDenyAssignments: nil, + mockGenericResources: nil, + expectError: false, + verify: func(t *testing.T, ctx context.Context, mockDB *corecosmosstoragetesting.MockResourcesDBClient) { + spc, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + assert.NotEmpty(t, spc.Status.AzureResources.DenyAssignments.AzureResources, "AzureResources should be untouched") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := utils.ContextWithLogger(context.Background(), testr.New(t)) + mockDB := corecosmosstoragetesting.NewMockResourcesDBClient() + + if tt.existingSPC != nil { + _, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Create(ctx, tt.existingSPC, nil) + require.NoError(t, err) + } + + var builder *azuremockclient.FirstPartyApplicationClientBuilderFunc + if tt.mockDenyAssignments != nil || tt.mockGenericResources != nil { + builder = &azuremockclient.FirstPartyApplicationClientBuilderFunc{ + GenericResourcesClientVal: tt.mockGenericResources, + DenyAssignmentsClientVal: tt.mockDenyAssignments, + } + } + + syncer := &clusterDenyAssignmentSyncer{ + clock: fakeClock, + resourcesDBClient: mockDB, + clusterLister: &corelistertesting.SliceClusterLister{Clusters: []*coreapi.HCPOpenShiftCluster{tt.cluster}}, + subscriptionLister: &corelistertesting.SliceSubscriptionLister{Subscriptions: []*coreapi.Subscription{testSubscription()}}, + azureFPAClientBuilder: builder, + } + + err := syncer.SyncOnce(ctx, testKey()) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + if tt.verify != nil { + tt.verify(t, ctx, mockDB) + } + }) + } +} + +func TestEnsureDenyAssignmentReferences(t *testing.T) { + cluster := newTestCluster() + defs := denyAssignmentDefinitions(cluster) + defsByType := make(map[string]denyAssignmentDefinition, len(defs)) + for _, d := range defs { + defsByType[d.denyAssignmentType] = d + } + + allRefs, err := allDenyAssignmentReferences(cluster) + require.NoError(t, err) + + var resourcesRef coreapi.DenyAssignmentReference + for _, ref := range allRefs { + if ref.DenyAssignmentType == denyAssignmentSuffixResources { + resourcesRef = ref + break + } + } + + tests := []struct { + name string + refs []coreapi.DenyAssignmentReference + defsByType map[string]denyAssignmentDefinition + mockDenyAssignments *azuremockclient.DenyAssignmentsClientFunc + mockGenericResources *azuremockclient.GenericResourcesClientFunc + expectSucceeded int + expectFailed int + expectError bool + }{ + { + name: "existing content up to date", + refs: []coreapi.DenyAssignmentReference{resourcesRef}, + defsByType: defsByType, + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: matchingGetResponseForAllTypes(cluster), + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{}, + expectSucceeded: 1, + expectFailed: 0, + expectError: false, + }, + { + name: "content mismatch triggers update attempt", + refs: []coreapi.DenyAssignmentReference{resourcesRef}, + defsByType: defsByType, + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: func(ctx context.Context, scope string, id string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + return armauthorization.DenyAssignmentsClientGetResponse{ + DenyAssignment: armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + {Actions: to.SliceOfPtrs("wrong-action"), NotActions: to.SliceOfPtrs[string](), DataActions: to.SliceOfPtrs[string]()}, + }, + }, + }, + }, nil + }, + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{CreateErr: fmt.Errorf("simulated create failure")}, + expectSucceeded: 0, + expectFailed: 1, + expectError: true, + }, + { + name: "unknown type goes to failed and surfaces an error", + refs: []coreapi.DenyAssignmentReference{ + { + DenyAssignmentType: "unknown-type", + DenyAssignmentResourceID: metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testManagedRG + "/providers/Microsoft.Authorization/denyAssignments/test-uuid")), + }, + }, + defsByType: map[string]denyAssignmentDefinition{}, + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{}, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{}, + expectSucceeded: 0, + expectFailed: 1, + expectError: true, + }, + { + name: "deny assignment not found triggers create", + refs: []coreapi.DenyAssignmentReference{resourcesRef}, + defsByType: defsByType, + mockDenyAssignments: &azuremockclient.DenyAssignmentsClientFunc{ + GetFunc: func(ctx context.Context, scope string, id string, opts *armauthorization.DenyAssignmentsClientGetOptions) (armauthorization.DenyAssignmentsClientGetResponse, error) { + return armauthorization.DenyAssignmentsClientGetResponse{}, denyAssignmentNotFoundError() + }, + }, + mockGenericResources: &azuremockclient.GenericResourcesClientFunc{CreateErr: fmt.Errorf("simulated create failure")}, + expectSucceeded: 0, + expectFailed: 1, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := utils.ContextWithLogger(context.Background(), testr.New(t)) + syncer := &clusterDenyAssignmentSyncer{clock: clocktesting.NewFakeClock(time.Now())} + + succeeded, failed, err := syncer.ensureDenyAssignmentReferences(ctx, cluster, tt.mockDenyAssignments, tt.mockGenericResources, + testManagedResourceGroupID(), tt.defsByType, tt.refs) + + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Len(t, succeeded, tt.expectSucceeded) + assert.Len(t, failed, tt.expectFailed) + }) + } +} + +func TestDeleteDenyAssignment(t *testing.T) { + rid := metadataapi.Must(azcorearm.ParseResourceID("/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testManagedRG + "/providers/Microsoft.Authorization/denyAssignments/test-uuid")) + + tests := []struct { + name string + deleteErr error + expectError bool + }{ + { + name: "resource not found is no-op", + deleteErr: resourceNotFoundError(), + expectError: false, + }, + { + name: "other error propagates", + deleteErr: fmt.Errorf("some azure error"), + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := utils.ContextWithLogger(context.Background(), testr.New(t)) + mockGenericResources := &azuremockclient.GenericResourcesClientFunc{DeleteErr: tt.deleteErr} + syncer := &clusterDenyAssignmentSyncer{clock: clocktesting.NewFakeClock(time.Now())} + + err := syncer.deleteDenyAssignment(ctx, mockGenericResources, rid) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Len(t, mockGenericResources.DeleteCalls, 1) + }) + } +} + +func TestDenyAssignmentNeedsUpdate(t *testing.T) { + actions := []string{"action1", "action2"} + notActions := []string{"notAction1"} + dataActions := []string{} + excludedPrincipalIDs := []string{"principal-1"} + + tests := []struct { + name string + existing *armauthorization.DenyAssignment + expected bool + }{ + { + name: "nil properties", + existing: &armauthorization.DenyAssignment{}, + expected: true, + }, + { + name: "matching content", + existing: &armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + {Actions: to.SliceOfPtrs(actions...), NotActions: to.SliceOfPtrs(notActions...), DataActions: to.SliceOfPtrs[string]()}, + }, + ExcludePrincipals: []*armauthorization.Principal{{ID: ptr.To("principal-1")}}, + }, + }, + expected: false, + }, + { + name: "different actions", + existing: &armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + {Actions: to.SliceOfPtrs("wrong-action"), NotActions: to.SliceOfPtrs(notActions...), DataActions: to.SliceOfPtrs[string]()}, + }, + ExcludePrincipals: []*armauthorization.Principal{{ID: ptr.To("principal-1")}}, + }, + }, + expected: true, + }, + { + name: "different excluded principals", + existing: &armauthorization.DenyAssignment{ + Properties: &armauthorization.DenyAssignmentProperties{ + Permissions: []*armauthorization.DenyAssignmentPermission{ + {Actions: to.SliceOfPtrs(actions...), NotActions: to.SliceOfPtrs(notActions...), DataActions: to.SliceOfPtrs[string]()}, + }, + ExcludePrincipals: []*armauthorization.Principal{{ID: ptr.To("wrong-principal")}}, + }, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := denyAssignmentNeedsUpdate(tt.existing, actions, notActions, dataActions, excludedPrincipalIDs) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestReplaceServiceProviderClusterIfChanged(t *testing.T) { + tests := []struct { + name string + modifyReplacement func(spc *coreapi.ServiceProviderCluster) + priorErrs []error + expectError bool + expectNilReturn bool + }{ + { + name: "no change, no errors", + modifyReplacement: nil, + priorErrs: nil, + expectError: false, + expectNilReturn: false, + }, + { + name: "with change persists", + modifyReplacement: func(spc *coreapi.ServiceProviderCluster) { + spc.Status.AzureResources.DenyAssignments.PendingAzureResources = []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "new-type"}, + } + }, + priorErrs: nil, + expectError: false, + expectNilReturn: false, + }, + { + name: "prior errors return error", + modifyReplacement: nil, + priorErrs: []error{fmt.Errorf("prior error")}, + expectError: true, + expectNilReturn: true, + }, + { + name: "nil error in slice is not treated as error", + modifyReplacement: nil, + priorErrs: []error{nil}, + expectError: false, + expectNilReturn: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := utils.ContextWithLogger(context.Background(), testr.New(t)) + mockDB := corecosmosstoragetesting.NewMockResourcesDBClient() + + spc := newTestSPC() + _, err := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName).Create(ctx, spc, nil) + require.NoError(t, err) + + spcCRUD := mockDB.ServiceProviderClusters(testSubscriptionID, testResourceGroupName, testClusterName) + original, err := spcCRUD.Get(ctx, coreapi.ServiceProviderClusterResourceName) + require.NoError(t, err) + replacement := original.DeepCopy() + + if tt.modifyReplacement != nil { + tt.modifyReplacement(replacement) + } + + returned, retReplacement, err := replaceServiceProviderClusterIfChanged(ctx, spcCRUD, original, replacement, tt.priorErrs) + if tt.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + if tt.expectNilReturn { + assert.Nil(t, returned) + assert.Nil(t, retReplacement) + } else { + assert.NotNil(t, returned) + assert.NotNil(t, retReplacement) + } + }) + } +} + +func TestAppendDenyAssignmentReference(t *testing.T) { + ref1 := coreapi.DenyAssignmentReference{DenyAssignmentType: "type-a"} + ref2 := coreapi.DenyAssignmentReference{DenyAssignmentType: "type-b"} + ref3 := coreapi.DenyAssignmentReference{DenyAssignmentType: "type-a"} + + result := appendDenyAssignmentReference(nil, ref1, ref2, ref3) + assert.Len(t, result, 2) + assert.Equal(t, "type-a", result[0].DenyAssignmentType) + assert.Equal(t, "type-b", result[1].DenyAssignmentType) + + result = appendDenyAssignmentReference([]coreapi.DenyAssignmentReference{ref1}, ref2, ref3) + assert.Len(t, result, 2) +} + +func TestRemoveDenyAssignmentRef(t *testing.T) { + refs := []coreapi.DenyAssignmentReference{ + {DenyAssignmentType: "type-a"}, + {DenyAssignmentType: "type-b"}, + {DenyAssignmentType: "type-c"}, + } + result := removeDenyAssignmentRef(refs, "type-b") + assert.Len(t, result, 2) + for _, ref := range result { + assert.NotEqual(t, "type-b", ref.DenyAssignmentType) + } +} diff --git a/backend/pkg/controllers/cluster/denyassignments/deny_assignment_definitions.go b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_definitions.go new file mode 100644 index 00000000000..9b189499b57 --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_definitions.go @@ -0,0 +1,235 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package denyassignments + +import ( + "fmt" + + "github.com/Azure/ARO-HCP/backend/pkg/utils/controllerutils" + "github.com/Azure/ARO-HCP/internal/api/coreapi" + "github.com/Azure/ARO-HCP/internal/api/metadataapi" +) + +const ( + operatorClusterAPIAzure = "cluster-api-azure" + operatorCloudControllerManager = "cloud-controller-manager" + operatorDiskCSIDriver = "disk-csi-driver" + operatorControlPlane = "control-plane" + operatorImageRegistry = "image-registry" + operatorFileCSIDriver = "file-csi-driver" + operatorKMS = "kms" + operatorIngress = "ingress" + operatorCloudNetworkConfig = "cloud-network-config" + + denyAssignmentSuffixResources = "resources-deny-assignment" + denyAssignmentSuffixDenyAllOtherRPs = "deny-all-other-rps-deny-assignment" + denyAssignmentSuffixCompute = "compute-deny-assignment" + denyAssignmentSuffixResourceHealth = "resourcehealth-deny-assignment" + denyAssignmentSuffixAPIManagement = "apimanagement-deny-assignment" + denyAssignmentSuffixStorage = "storage-deny-assignment" + denyAssignmentSuffixManagedIdentity = "managedidentity-deny-assignment" + denyAssignmentSuffixKeyVault = "keyvault-deny-assignment" + denyAssignmentSuffixContainerService = "containerservice-deny-assignment" + denyAssignmentSuffixNetworkVnetMgmt = "network-vnet-mgmt-deny-assignment" + denyAssignmentSuffixNetworkVnetRead = "network-vnet-read-deny-assignment" + denyAssignmentSuffixNetworkVnetJoin = "network-vnet-join-deny-assignment" + denyAssignmentSuffixNetworkLoadBalancing = "network-loadbalancing-deny-assignment" + denyAssignmentSuffixNetworkPrivateConn = "network-privateconn-deny-assignment" + denyAssignmentSuffixNetworkSecurityGroups = "network-securitygroups-deny-assignment" + denyAssignmentSuffixNetworkAppSecurityGroups = "network-appsecuritygroups-deny-assignment" + denyAssignmentSuffixNetworkInterfaces = "network-interfaces-deny-assignment" + denyAssignmentSuffixNetworkPoliciesServices = "network-policies-services-deny-assignment" + denyAssignmentSuffixNetworkBastionHosts = "network-bastionhosts-deny-assignment" + + denyAssignmentNamespaceUUID = "f75040b8-d8aa-4311-bda6-ba8af06db258" + denyAssignmentAzureAPIVersion = "2022-04-01" + allPrincipalsGUID = "00000000-0000-0000-0000-000000000000" +) + +type denyAssignmentDefinition struct { + denyAssignmentType string + controlPlaneOperators []string + dataPlaneOperators []string + includeServiceManagedID bool + actions []string + notActions []string + dataActions []string + conditionalKMS bool +} + +func denyAssignmentDefinitions(cluster *coreapi.HCPOpenShiftCluster) []denyAssignmentDefinition { + defs := []denyAssignmentDefinition{ + { + denyAssignmentType: denyAssignmentSuffixResources, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorControlPlane, operatorImageRegistry, operatorDiskCSIDriver}, + dataPlaneOperators: []string{operatorImageRegistry, operatorDiskCSIDriver}, + includeServiceManagedID: false, + actions: resourcesActions(), + notActions: resourcesNotActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixCompute, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorDiskCSIDriver, operatorCloudNetworkConfig}, + dataPlaneOperators: []string{operatorDiskCSIDriver}, + includeServiceManagedID: false, + actions: computeActions(), + notActions: computeNotActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixResourceHealth, + controlPlaneOperators: []string{operatorClusterAPIAzure}, + actions: resourceHealthActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixAPIManagement, + controlPlaneOperators: []string{operatorClusterAPIAzure}, + actions: apiManagementActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixStorage, + controlPlaneOperators: []string{operatorImageRegistry, operatorFileCSIDriver}, + dataPlaneOperators: []string{operatorImageRegistry, operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: storageActions(), + dataActions: storageDataActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixManagedIdentity, + controlPlaneOperators: []string{operatorControlPlane, operatorDiskCSIDriver}, + dataPlaneOperators: []string{operatorDiskCSIDriver}, + includeServiceManagedID: true, + actions: managedIdentityActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixKeyVault, + controlPlaneOperators: []string{operatorDiskCSIDriver}, + dataPlaneOperators: []string{operatorDiskCSIDriver}, + actions: keyVaultActions(), + dataActions: keyVaultDataActions(), + conditionalKMS: true, + }, + { + denyAssignmentType: denyAssignmentSuffixContainerService, + controlPlaneOperators: []string{operatorClusterAPIAzure}, + actions: containerServiceActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkVnetMgmt, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorFileCSIDriver, operatorCloudControllerManager}, + dataPlaneOperators: []string{operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: networkVirtualNetworksManagementActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkVnetRead, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorControlPlane, operatorImageRegistry, operatorIngress, operatorFileCSIDriver, operatorCloudNetworkConfig}, + dataPlaneOperators: []string{operatorImageRegistry, operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: networkVirtualNetworksReadActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkVnetJoin, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorImageRegistry, operatorIngress, operatorCloudNetworkConfig, operatorDiskCSIDriver, operatorFileCSIDriver}, + dataPlaneOperators: []string{operatorImageRegistry, operatorFileCSIDriver, operatorDiskCSIDriver}, + actions: networkVirtualNetworksJoinActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkLoadBalancing, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorControlPlane, operatorCloudNetworkConfig, operatorDiskCSIDriver, operatorFileCSIDriver}, + dataPlaneOperators: []string{operatorDiskCSIDriver, operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: networkLoadBalancingPublicIPAndRouteTablesActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkPrivateConn, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorImageRegistry, operatorIngress, operatorFileCSIDriver, operatorCloudControllerManager}, + dataPlaneOperators: []string{operatorImageRegistry, operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: networkPrivateConnectivityActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkSecurityGroups, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorControlPlane, operatorDiskCSIDriver, operatorFileCSIDriver}, + dataPlaneOperators: []string{operatorDiskCSIDriver, operatorFileCSIDriver}, + includeServiceManagedID: true, + actions: networkSecurityGroupsAndNatGatewaysActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkAppSecurityGroups, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorControlPlane, operatorDiskCSIDriver}, + dataPlaneOperators: []string{operatorDiskCSIDriver}, + actions: applicationSecurityGroupsActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkInterfaces, + controlPlaneOperators: []string{operatorClusterAPIAzure, operatorCloudControllerManager, operatorControlPlane, operatorImageRegistry, operatorCloudNetworkConfig, operatorDiskCSIDriver}, + dataPlaneOperators: []string{operatorImageRegistry, operatorDiskCSIDriver}, + actions: networkInterfacesActions(), + notActions: networkInterfacesNotActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkPoliciesServices, + controlPlaneOperators: []string{operatorFileCSIDriver, operatorCloudControllerManager}, + dataPlaneOperators: []string{operatorFileCSIDriver}, + actions: networkPoliciesAndServicesActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixNetworkBastionHosts, + controlPlaneOperators: []string{operatorClusterAPIAzure}, + actions: bastionHostsActions(), + }, + { + denyAssignmentType: denyAssignmentSuffixDenyAllOtherRPs, + actions: denyAllOtherRPsActions(), + notActions: denyAllOtherRPsNotActions(), + }, + } + + // For KeyVault, conditionally add KMS operator exclusion + for i := range defs { + if defs[i].conditionalKMS && isKMSEncryptionEnabled(cluster) { + defs[i].controlPlaneOperators = append(defs[i].controlPlaneOperators, operatorKMS) + } + } + + return defs +} + +func allDenyAssignmentReferences(cluster *coreapi.HCPOpenShiftCluster) ([]coreapi.DenyAssignmentReference, error) { + defs := denyAssignmentDefinitions(cluster) + csClusterID := controllerutils.ClusterServiceIDForCluster(cluster) + subscriptionID := cluster.ID.SubscriptionID + managedResourceGroup := cluster.CustomerProperties.Platform.ManagedResourceGroup + + denyAssignmentReferences := make([]coreapi.DenyAssignmentReference, 0, len(defs)) + for _, d := range defs { + daUUID := generateDenyAssignmentUUID(csClusterID, d.denyAssignmentType) + azureResourceID, err := coreapi.ToDenyAssignmentResourceID(subscriptionID, managedResourceGroup, daUUID) + if err != nil { + return nil, fmt.Errorf("failed to build deny assignment resource ID for %s: %w", d.denyAssignmentType, err) + } + denyAssignmentReferences = append(denyAssignmentReferences, coreapi.DenyAssignmentReference{ + DenyAssignmentType: d.denyAssignmentType, + DenyAssignmentResourceID: azureResourceID, + }) + } + return denyAssignmentReferences, nil +} + +func isKMSEncryptionEnabled(cluster *coreapi.HCPOpenShiftCluster) bool { + return cluster.CustomerProperties.Etcd.DataEncryption.KeyManagementMode == metadataapi.EtcdDataEncryptionKeyManagementModeTypeCustomerManaged && + cluster.CustomerProperties.Etcd.DataEncryption.CustomerManaged != nil && + cluster.CustomerProperties.Etcd.DataEncryption.CustomerManaged.Kms != nil +} diff --git a/backend/pkg/controllers/cluster/denyassignments/deny_assignment_permissions.go b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_permissions.go new file mode 100644 index 00000000000..84259d84072 --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_permissions.go @@ -0,0 +1,324 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package denyassignments + +func resourcesActions() []string { + return []string{ + "Microsoft.Resources/subscriptions/resourceGroups/delete", + "Microsoft.Resources/subscriptions/resourceGroups/read", + "Microsoft.Resources/subscriptions/resourceGroups/write", + "Microsoft.Resources/deployments/delete", + "Microsoft.Resources/deployments/write", + } +} + +func resourcesNotActions() []string { + return []string{ + "Microsoft.Resources/tags/*", + } +} + +func computeActions() []string { + return []string{ + "Microsoft.Compute/availabilitySets/delete", + "Microsoft.Compute/availabilitySets/write", + "Microsoft.Compute/disks/beginGetAccess/action", + "Microsoft.Compute/disks/delete", + "Microsoft.Compute/disks/endGetAccess/action", + "Microsoft.Compute/disks/write", + "Microsoft.Compute/images/delete", + "Microsoft.Compute/images/write", + "Microsoft.Compute/snapshots/beginGetAccess/action", + "Microsoft.Compute/snapshots/delete", + "Microsoft.Compute/snapshots/endGetAccess/action", + "Microsoft.Compute/snapshots/write", + "Microsoft.Compute/availabilitySets/read", + "Microsoft.Compute/diskEncryptionSets/read", + "Microsoft.Compute/disks/read", + "Microsoft.Compute/locations/DiskOperations/read", + "Microsoft.Compute/locations/operations/read", + "Microsoft.Compute/snapshots/read", + "Microsoft.Compute/virtualMachineScaleSets/read", + "Microsoft.Compute/virtualMachineScaleSets/virtualMachines/read", + "Microsoft.Compute/virtualMachineScaleSets/virtualMachines/write", + "Microsoft.Compute/virtualMachines/delete", + "Microsoft.Compute/virtualMachines/read", + "Microsoft.Compute/virtualMachines/write", + } +} + +func computeNotActions() []string { + return []string{ + "Microsoft.Compute/disks/beginGetAccess/action", + "Microsoft.Compute/disks/endGetAccess/action", + "Microsoft.Compute/disks/write", + "Microsoft.Compute/snapshots/beginGetAccess/action", + "Microsoft.Compute/snapshots/delete", + "Microsoft.Compute/snapshots/endGetAccess/action", + "Microsoft.Compute/snapshots/write", + } +} + +func resourceHealthActions() []string { + return []string{ + "Microsoft.ResourceHealth/events/action", + } +} + +func apiManagementActions() []string { + return []string{ + "Microsoft.ApiManagement/service/groups/delete", + "Microsoft.ApiManagement/service/groups/read", + "Microsoft.ApiManagement/service/groups/write", + "Microsoft.ApiManagement/service/workspaces/tags/read", + "Microsoft.ApiManagement/service/workspaces/tags/write", + } +} + +func storageActions() []string { + return []string{ + "Microsoft.Storage/storageAccounts/read", + "Microsoft.Storage/storageAccounts/write", + "Microsoft.Storage/storageAccounts/delete", + "Microsoft.Storage/storageAccounts/listKeys/action", + "Microsoft.Storage/storageAccounts/regeneratekey/action", + "Microsoft.Storage/storageAccounts/blobServices/read", + "Microsoft.Storage/storageAccounts/blobServices/write", + "Microsoft.Storage/storageAccounts/blobServices/containers/delete", + "Microsoft.Storage/storageAccounts/blobServices/containers/read", + "Microsoft.Storage/storageAccounts/blobServices/containers/write", + "Microsoft.Storage/storageAccounts/blobServices/generateUserDelegationKey/action", + "Microsoft.Storage/storageAccounts/fileServices/read", + "Microsoft.Storage/storageAccounts/fileServices/write", + "Microsoft.Storage/storageAccounts/fileServices/shares/read", + "Microsoft.Storage/storageAccounts/fileServices/shares/write", + "Microsoft.Storage/storageAccounts/fileServices/shares/delete", + "Microsoft.Storage/storageAccounts/PrivateEndpointConnectionsApproval/action", + "Microsoft.Storage/operations/read", + } +} + +func storageDataActions() []string { + return []string{ + "Microsoft.Storage/storageAccounts/blobServices/containers/blobs/read", + "Microsoft.Storage/storageAccounts/blobServices/containers/blobs/write", + "Microsoft.Storage/storageAccounts/blobServices/containers/blobs/delete", + "Microsoft.Storage/storageAccounts/blobServices/containers/blobs/add/action", + "Microsoft.Storage/storageAccounts/blobServices/containers/blobs/move/action", + "Microsoft.Storage/storageAccounts/fileServices/fileshares/files/read", + "Microsoft.Storage/storageAccounts/fileServices/fileshares/files/write", + "Microsoft.Storage/storageAccounts/fileServices/fileshares/files/delete", + } +} + +func managedIdentityActions() []string { + return []string{ + "Microsoft.ManagedIdentity/userAssignedIdentities/assign/action", + "Microsoft.ManagedIdentity/userAssignedIdentities/read", + "Microsoft.ManagedIdentity/userAssignedIdentities/write", + "Microsoft.ManagedIdentity/userAssignedIdentities/delete", + "Microsoft.ManagedIdentity/userAssignedIdentities/federatedIdentityCredentials/read", + "Microsoft.ManagedIdentity/userAssignedIdentities/federatedIdentityCredentials/write", + "Microsoft.ManagedIdentity/userAssignedIdentities/federatedIdentityCredentials/delete", + } +} + +func keyVaultActions() []string { + return []string{ + "Microsoft.KeyVault/vaults/deploy/action", + } +} + +func keyVaultDataActions() []string { + return []string{ + "Microsoft.KeyVault/vaults/keys/read", + "Microsoft.KeyVault/vaults/keys/update/action", + "Microsoft.KeyVault/vaults/keys/backup/action", + "Microsoft.KeyVault/vaults/keys/encrypt/action", + "Microsoft.KeyVault/vaults/keys/decrypt/action", + "Microsoft.KeyVault/vaults/keys/wrap/action", + "Microsoft.KeyVault/vaults/keys/unwrap/action", + "Microsoft.KeyVault/vaults/keys/sign/action", + "Microsoft.KeyVault/vaults/keys/verify/action", + } +} + +func containerServiceActions() []string { + return []string{ + "Microsoft.ContainerService/managedClusters/agentPools/write", + "Microsoft.ContainerService/managedClusters/delete", + "Microsoft.ContainerService/managedClusters/write", + } +} + +func networkVirtualNetworksManagementActions() []string { + return []string{ + "Microsoft.Network/virtualNetworks/delete", + "Microsoft.Network/virtualNetworks/write", + "Microsoft.Network/virtualNetworks/subnets/delete", + "Microsoft.Network/virtualNetworks/subnets/write", + } +} + +func networkVirtualNetworksReadActions() []string { + return []string{ + "Microsoft.Network/virtualNetworks/read", + "Microsoft.Network/virtualNetworks/subnets/read", + "Microsoft.Network/virtualNetworks/virtualNetworkPeerings/read", + } +} + +func networkVirtualNetworksJoinActions() []string { + return []string{ + "Microsoft.Network/virtualNetworks/join/action", + "Microsoft.Network/virtualNetworks/subnets/join/action", + } +} + +func networkLoadBalancingPublicIPAndRouteTablesActions() []string { + return []string{ + "Microsoft.Network/loadBalancers/inboundNATRules/join/action", + "Microsoft.Network/loadBalancers/loadBalancingRules/read", + "Microsoft.Network/loadBalancers/read", + "Microsoft.Network/loadBalancers/write", + "Microsoft.Network/loadBalancers/delete", + "Microsoft.Network/loadBalancers/backendAddressPools/join/action", + "Microsoft.Network/loadBalancers/backendAddressPools/read", + "Microsoft.Network/loadBalancers/backendAddressPools/write", + "Microsoft.Network/loadBalancers/frontendIPConfigurations/join/action", + "Microsoft.Network/loadBalancers/inboundNatRules/join/action", + "Microsoft.Network/loadBalancers/probes/join/action", + "Microsoft.Network/virtualNetworks/joinLoadBalancer/action", + "Microsoft.Network/publicIPAddresses/read", + "Microsoft.Network/publicIPAddresses/write", + "Microsoft.Network/publicIPAddresses/delete", + "Microsoft.Network/publicIPAddresses/join/action", + "Microsoft.Network/publicIPPrefixes/join/action", + "Microsoft.Network/routeTables/read", + "Microsoft.Network/routeTables/write", + "Microsoft.Network/routeTables/delete", + "Microsoft.Network/routeTables/join/action", + } +} + +func networkPrivateConnectivityActions() []string { + return []string{ + "Microsoft.Network/privatelinkservices/delete", + "Microsoft.Network/privatelinkservices/read", + "Microsoft.Network/privatelinkservices/write", + "Microsoft.Network/privateEndpoints/read", + "Microsoft.Network/privateEndpoints/write", + "Microsoft.Network/privateEndpoints/delete", + "Microsoft.Network/privateDnsOperationStatuses/read", + "Microsoft.Network/privateDnsZones/join/action", + "Microsoft.Network/privateDnsZones/read", + "Microsoft.Network/privateDnsZones/virtualNetworkLinks/read", + "Microsoft.Network/privateEndpoints/privateDnsZoneGroups/read", + "Microsoft.Network/privateEndpoints/privateDnsZoneGroups/write", + "Microsoft.Network/privateDnsZones/write", + "Microsoft.Network/privateDnsZones/delete", + "Microsoft.Network/privateDnsZones/A/write", + "Microsoft.Network/privateDnsZones/A/delete", + "Microsoft.Network/privateDnsZones/virtualNetworkLinks/write", + "Microsoft.Network/privateDnsZones/virtualNetworkLinks/delete", + "Microsoft.Network/dnsZones/write", + "Microsoft.Network/dnsZones/delete", + "Microsoft.Network/dnsZones/A/write", + "Microsoft.Network/dnsZones/A/delete", + "Microsoft.Network/locations/operations/read", + } +} + +func networkSecurityGroupsAndNatGatewaysActions() []string { + return []string{ + "Microsoft.Network/networkSecurityGroups/read", + "Microsoft.Network/networkSecurityGroups/write", + "Microsoft.Network/networkSecurityGroups/delete", + "Microsoft.Network/networkSecurityGroups/join/action", + "Microsoft.Network/natGateways/join/action", + "Microsoft.Network/natGateways/read", + } +} + +func applicationSecurityGroupsActions() []string { + return []string{ + "Microsoft.Network/applicationSecurityGroups/read", + "Microsoft.Network/applicationSecurityGroups/write", + "Microsoft.Network/applicationSecurityGroups/delete", + "Microsoft.Network/applicationSecurityGroups/joinNetworkSecurityRule/action", + "Microsoft.Network/applicationSecurityGroups/joinIpConfiguration/action", + } +} + +func networkInterfacesActions() []string { + return []string{ + "Microsoft.Network/networkInterfaces/read", + "Microsoft.Network/networkInterfaces/write", + "Microsoft.Network/networkInterfaces/delete", + "Microsoft.Network/networkInterfaces/join/action", + "Microsoft.Network/networkInterfaces/loadBalancers/read", + "Microsoft.Network/networkInterfaces/effectiveRouteTable/action", + } +} + +func networkInterfacesNotActions() []string { + return []string{ + "Microsoft.Network/networkInterfaces/effectiveRouteTable/action", + "Microsoft.Network/networkSecurityGroups/join/action", + } +} + +func networkPoliciesAndServicesActions() []string { + return []string{ + "Microsoft.Network/serviceEndpointPolicies/read", + "Microsoft.Network/serviceEndpointPolicies/write", + "Microsoft.Network/serviceEndpointPolicies/delete", + "Microsoft.Network/serviceEndpointPolicies/join/action", + "Microsoft.Network/networkIntentPolicies/join/action", + "Microsoft.Network/networkManagers/ipamPools/associateResourcesToPool/action", + } +} + +func bastionHostsActions() []string { + return []string{ + "Microsoft.Network/bastionHosts/write", + "Microsoft.Network/bastionHosts/delete", + } +} + +func denyAllOtherRPsActions() []string { + return []string{ + "*/action", + "*/delete", + "*/write", + } +} + +func denyAllOtherRPsNotActions() []string { + return []string{ + "Microsoft.Resources/*", + "Microsoft.Compute/*", + "Microsoft.Storage/*", + "Microsoft.Network/*", + "Microsoft.ManagedIdentity/*", + "Microsoft.KeyVault/*", + "Microsoft.Authorization/*", + "Microsoft.ContainerService/*", + "Microsoft.ResourceHealth/*", + "Microsoft.ApiManagement/*", + "Microsoft.Insights/*", + "Microsoft.PolicyInsights/*", + } +} diff --git a/backend/pkg/utils/controllerutils/util.go b/backend/pkg/utils/controllerutils/util.go index f28a6a17b80..ddab17b32af 100644 --- a/backend/pkg/utils/controllerutils/util.go +++ b/backend/pkg/utils/controllerutils/util.go @@ -428,3 +428,13 @@ func WriteController(ctx context.Context, controllerCRUD cosmosstorageutils.Reso } return nil } + +func ClusterServiceIDForCluster(cluster *coreapi.HCPOpenShiftCluster) string { + if cluster.ServiceProviderProperties.PendingClusterServiceID != nil { + return cluster.ServiceProviderProperties.PendingClusterServiceID.ClusterID() + } + if cluster.ServiceProviderProperties.ClusterServiceID != nil { + return cluster.ServiceProviderProperties.ClusterServiceID.ClusterID() + } + return "" +} diff --git a/internal/api/coreapi/types_cosmosdata.go b/internal/api/coreapi/types_cosmosdata.go index b378a2133dd..29dcf54fc88 100644 --- a/internal/api/coreapi/types_cosmosdata.go +++ b/internal/api/coreapi/types_cosmosdata.go @@ -116,6 +116,17 @@ func ToSystemAdminCredentialRevocationResourceID(subscriptionName, resourceGroup return azcorearm.ParseResourceID(ToSystemAdminCredentialRevocationResourceIDString(subscriptionName, resourceGroupName, clusterName, revocationName)) } +func ToDenyAssignmentResourceID(subscriptionID, resourceGroupName, denyAssignmentName string) (*azcorearm.ResourceID, error) { + return azcorearm.ParseResourceID(ToDenyAssignmentResourceIDString(subscriptionID, resourceGroupName, denyAssignmentName)) +} + +func ToDenyAssignmentResourceIDString(subscriptionID, resourceGroupName, denyAssignmentName string) string { + return strings.ToLower(path.Join( + ToResourceGroupResourceIDString(subscriptionID, resourceGroupName), + "providers", "Microsoft.Authorization", "denyAssignments", denyAssignmentName, + )) +} + func ToServiceProviderNodePoolResourceIDString(subscriptionName, resourceGroupName, clusterName, nodePoolName string) string { return strings.ToLower(path.Join( ToNodePoolResourceIDString(subscriptionName, resourceGroupName, clusterName, nodePoolName), diff --git a/internal/api/coreapi/types_serviceprovider_cluster.go b/internal/api/coreapi/types_serviceprovider_cluster.go index 15c4104511e..32c5fe5692a 100644 --- a/internal/api/coreapi/types_serviceprovider_cluster.go +++ b/internal/api/coreapi/types_serviceprovider_cluster.go @@ -399,7 +399,7 @@ type ServiceProviderClusterDataPlaneOperatorManagedIdentity struct { // AzureResources groups the Azure resource references associated with a cluster. type AzureResources struct { // DenyAssignments tracks the deny assignments applied to the cluster's resources. - DenyAssignments AzureMultiReference `json:"denyAssignments,omitempty"` + DenyAssignments DenyAssignmentReferences `json:"denyAssignments,omitempty"` // ManagedResourceGroup tracks the managed resource group for the cluster. ManagedResourceGroup AzureReference `json:"managedResourceGroup,omitempty"` } @@ -442,6 +442,38 @@ type AzureReference struct { EarliestRecheckTime *metav1.Time `json:"earliestRecheckTime,omitempty"` } +type DenyAssignmentReferences struct { + // PendingAzureResources contains resource IDs that have been requested but + // not yet confirmed to exist in Azure. + // Written by: ClusterDenyAssignment + PendingAzureResources []DenyAssignmentReference `json:"pendingDenyAssignments,omitempty"` + // AzureResources contains resource IDs that have been confirmed to exist in Azure. + // Written by: ClusterDenyAssignment + AzureResources []DenyAssignmentReference `json:"denyAssignments,omitempty"` + // EarliestRecheckTime is the earliest time at which the controller should + // re-check the pending resources. Nil means recheck immediately. + // This allows for controllers to avoid repeatedly hitting an Azure API to recheck that the desired state is true. + // Controllers should set this field with substantial jitter: without another concern, jitter of 50% is considered normal + // so that any storms are quickly dissipated. + // Additionally, long recheck times are recommended for resources outside of their active phases. Order of at least + // six hours is, with durations up to 24 hours considered normal. + // Written by: ClusterDenyAssignment + EarliestRecheckTime *metav1.Time `json:"earliestRecheckTime,omitempty"` +} + +// DenyAssignmentReference identifies a single Azure deny assignment. +// +k8s:deepcopy-gen=true +type DenyAssignmentReference struct { + // DenyAssignmentType identifies the category of deny assignment (e.g. "resources-deny-assignment"). + // Used as a suffix when generating the deterministic deny assignment UUID. + // Written by: ClusterDenyAssignment + DenyAssignmentType string `json:"denyAssignmentType"` + // DenyAssignmentResourceID is the full Azure resource ID of the deny assignment, + // e.g. "/subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.Authorization/denyAssignments/{uuid}". + // Written by: ClusterDenyAssignment + DenyAssignmentResourceID *azcorearm.ResourceID `json:"denyAssignmentResourceID"` +} + // ServiceProviderClusterStatusVersion contains the actual version information. type ServiceProviderClusterStatusVersion struct { // ActiveVersions is an array of versions currently active in the control plane, ordered with the most recent first. diff --git a/internal/api/coreapi/zz_generated.deepcopy.go b/internal/api/coreapi/zz_generated.deepcopy.go index 3adb35cc8b0..8490e432f17 100644 --- a/internal/api/coreapi/zz_generated.deepcopy.go +++ b/internal/api/coreapi/zz_generated.deepcopy.go @@ -478,6 +478,60 @@ func (in *CustomerPlatformProfile) DeepCopy() *CustomerPlatformProfile { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DenyAssignmentReference) DeepCopyInto(out *DenyAssignmentReference) { + *out = *in + if in.DenyAssignmentResourceID != nil { + in, out := &in.DenyAssignmentResourceID, &out.DenyAssignmentResourceID + *out = DeepCopyResourceID(*in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DenyAssignmentReference. +func (in *DenyAssignmentReference) DeepCopy() *DenyAssignmentReference { + if in == nil { + return nil + } + out := new(DenyAssignmentReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DenyAssignmentReferences) DeepCopyInto(out *DenyAssignmentReferences) { + *out = *in + if in.PendingAzureResources != nil { + in, out := &in.PendingAzureResources, &out.PendingAzureResources + *out = make([]DenyAssignmentReference, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.AzureResources != nil { + in, out := &in.AzureResources, &out.AzureResources + *out = make([]DenyAssignmentReference, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.EarliestRecheckTime != nil { + in, out := &in.EarliestRecheckTime, &out.EarliestRecheckTime + *out = (*in).DeepCopy() + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DenyAssignmentReferences. +func (in *DenyAssignmentReferences) DeepCopy() *DenyAssignmentReferences { + if in == nil { + return nil + } + out := new(DenyAssignmentReferences) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DeploymentPreflight) DeepCopyInto(out *DeploymentPreflight) { *out = *in