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..15e81e5fade 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 Chitimbo'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..f07e8739278 --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller.go @@ -0,0 +1,646 @@ +// 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 Chitimbo'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 + } + } + + // Excluded principal IDs are resolved from the identities the MSI and data-plane identity + // controllers mirror onto the ServiceProviderCluster. If those maps are empty, resolution can + // only fail, so there is nothing to do yet; wait for the resolution controllers to populate them. + if len(serviceProviderCluster.Status.MSIManagedIdentities.ControlPlaneOperatorsIdentities) == 0 { + return false + } + if len(serviceProviderCluster.Status.DataPlaneOperatorsManagedIdentities.Identities) == 0 { + 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, replacement, 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, replacement, 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, + serviceProviderCluster *coreapi.ServiceProviderCluster, + 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, serviceProviderCluster, 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, + serviceProviderCluster *coreapi.ServiceProviderCluster, + 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(serviceProviderCluster, 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(serviceProviderCluster *coreapi.ServiceProviderCluster, identityResourceIDs []*azcorearm.ResourceID) ([]string, error) { + principalIDs := make([]string, 0, len(identityResourceIDs)) + for _, identityResourceID := range identityResourceIDs { + principalID, err := resolvePrincipalID(serviceProviderCluster, identityResourceID) + if err != nil { + return nil, utils.TrackError(err) + } + principalIDs = append(principalIDs, principalID) + } + return principalIDs, nil +} + +// resolvePrincipalID returns the resolved Azure principal ID for a single managed-identity resource +// ID, looked up among the identities the backend has mirrored onto the ServiceProviderCluster: +// the control plane operators and the service managed identity (MSIManagedIdentities), and the data +// plane operators (DataPlaneOperatorsManagedIdentities). The maps are keyed by the fully lowercased +// resource ID. It returns an error when the identity is not tracked yet or its principal ID has not +// been resolved (nil/empty), so the caller waits and retries rather than writing a deny assignment +// with a missing exclusion. +func resolvePrincipalID(serviceProviderCluster *coreapi.ServiceProviderCluster, identityResourceID *azcorearm.ResourceID) (string, error) { + key := strings.ToLower(identityResourceID.String()) + + msi := serviceProviderCluster.Status.MSIManagedIdentities + if identity := msi.ControlPlaneOperatorsIdentities[key]; identity != nil && identity.PrincipalID != nil && len(*identity.PrincipalID) > 0 { + return *identity.PrincipalID, nil + } + if smi := msi.ServiceManagedIdentity; smi != nil && smi.ResourceID != nil && + strings.ToLower(smi.ResourceID.String()) == key && smi.PrincipalID != nil && len(*smi.PrincipalID) > 0 { + return *smi.PrincipalID, nil + } + if identity := serviceProviderCluster.Status.DataPlaneOperatorsManagedIdentities.Identities[key]; identity != nil && identity.PrincipalID != nil && len(*identity.PrincipalID) > 0 { + return *identity.PrincipalID, nil + } + + return "", fmt.Errorf("no resolved principal ID for identity %s on the ServiceProviderCluster", identityResourceID.String()) +} + +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..239adf65f97 --- /dev/null +++ b/backend/pkg/controllers/cluster/denyassignments/deny_assignment_controller_test.go @@ -0,0 +1,1058 @@ +// 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, dpOps, serviceManagedID := testClusterIdentities() + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators = cpOps + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperators = dpOps + cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity = serviceManagedID + + for _, opt := range opts { + opt(cluster) + } + return cluster +} + +// testClusterIdentities returns the control plane operator, data plane operator, and service +// managed identity resource IDs used across the deny-assignment tests. newTestCluster wires these +// into the cluster's operator configuration; newTestSPC mirrors them (with resolved principal IDs) +// onto the ServiceProviderCluster the way the identity-resolution controllers do in production. +func testClusterIdentities() (cpOps, dpOps map[string]*azcorearm.ResourceID, serviceManagedID *azcorearm.ResourceID) { + 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") + return cpOps, dpOps, serviceManagedID +} + +// testPrincipalID returns the deterministic principal ID the tests expect for a given managed +// identity resource ID. It is the value newTestSPC records on the ServiceProviderCluster and the +// value resolvePrincipalID must return for that identity. +func testPrincipalID(id *azcorearm.ResourceID) string { + return "principal-" + id.Name +} + +// seedResolvedIdentities mirrors the cluster's managed identities onto the ServiceProviderCluster +// status the same way the MSI and data-plane identity-resolution controllers do in production, so +// resolvePrincipalID can find a resolved principal ID for every excluded identity. +func seedResolvedIdentities(spc *coreapi.ServiceProviderCluster) { + cpOps, dpOps, serviceManagedID := testClusterIdentities() + + cpIdentities := make(map[string]*coreapi.ServiceProviderClusterControlPlaneOperatorIdentity, len(cpOps)) + for _, id := range cpOps { + cpIdentities[strings.ToLower(id.String())] = &coreapi.ServiceProviderClusterControlPlaneOperatorIdentity{ + ResourceID: id, + PrincipalID: ptr.To(testPrincipalID(id)), + } + } + spc.Status.MSIManagedIdentities.ControlPlaneOperatorsIdentities = cpIdentities + spc.Status.MSIManagedIdentities.ServiceManagedIdentity = &coreapi.ServiceProviderClusterServiceManagedIdentity{ + ResourceID: serviceManagedID, + PrincipalID: ptr.To(testPrincipalID(serviceManagedID)), + } + + dpIdentities := make(map[string]*coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity, len(dpOps)) + for _, id := range dpOps { + dpIdentities[strings.ToLower(id.String())] = &coreapi.ServiceProviderClusterDataPlaneOperatorManagedIdentity{ + ResourceID: id, + PrincipalID: ptr.To(testPrincipalID(id)), + } + } + spc.Status.DataPlaneOperatorsManagedIdentities.Identities = dpIdentities +} + +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() + seedResolvedIdentities(spc) + 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, spc *coreapi.ServiceProviderCluster) 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(spc, 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 TestResolvePrincipalID(t *testing.T) { + cpOps, dpOps, serviceManagedID := testClusterIdentities() + controlPlaneID := cpOps["cluster-api-azure"] + dataPlaneID := dpOps["image-registry"] + untrackedID := testIdentityResourceID("not-mirrored") + + tests := []struct { + name string + spc *coreapi.ServiceProviderCluster + identity *azcorearm.ResourceID + expected string + expectErr bool + errorContains string + }{ + { + name: "control plane operator identity resolves", + spc: newTestSPC(), + identity: controlPlaneID, + expected: testPrincipalID(controlPlaneID), + }, + { + name: "service managed identity resolves", + spc: newTestSPC(), + identity: serviceManagedID, + expected: testPrincipalID(serviceManagedID), + }, + { + name: "data plane operator identity resolves", + spc: newTestSPC(), + identity: dataPlaneID, + expected: testPrincipalID(dataPlaneID), + }, + { + name: "lookup is case-insensitive on the resource ID", + spc: newTestSPC(), + identity: metadataapi.Must(azcorearm.ParseResourceID(strings.ToUpper( + "/subscriptions/" + testSubscriptionID + "/resourceGroups/" + testResourceGroupName + + "/providers/Microsoft.ManagedIdentity/userAssignedIdentities/capi-azure"))), + expected: testPrincipalID(controlPlaneID), + }, + { + name: "identity not mirrored onto the SPC returns an error", + spc: newTestSPC(), + identity: untrackedID, + expectErr: true, + errorContains: "no resolved principal ID", + }, + { + name: "control plane identity with unresolved principal ID returns an error", + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.MSIManagedIdentities.ControlPlaneOperatorsIdentities[strings.ToLower(controlPlaneID.String())].PrincipalID = nil + }), + identity: controlPlaneID, + expectErr: true, + errorContains: "no resolved principal ID", + }, + { + name: "data plane identity with empty principal ID returns an error", + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.DataPlaneOperatorsManagedIdentities.Identities[strings.ToLower(dataPlaneID.String())].PrincipalID = ptr.To("") + }), + identity: dataPlaneID, + expectErr: true, + errorContains: "no resolved principal ID", + }, + { + name: "service managed identity with mismatched resource ID returns an error", + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.MSIManagedIdentities.ServiceManagedIdentity.ResourceID = testIdentityResourceID("some-other-identity") + }), + identity: serviceManagedID, + expectErr: true, + errorContains: "no resolved principal ID", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + principalID, err := resolvePrincipalID(tt.spc, tt.identity) + if tt.expectErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errorContains, "unexpected error message") + return + } + require.NoError(t, err) + assert.Equal(t, tt.expected, principalID, "unexpected principal ID") + }) + } +} + +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: "control plane operator identities not yet mirrored onto the SPC", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.MSIManagedIdentities.ControlPlaneOperatorsIdentities = nil + }), + expected: false, + }, + { + name: "data plane operator identities not yet mirrored onto the SPC", + cluster: newTestCluster(), + spc: newTestSPC(func(spc *coreapi.ServiceProviderCluster) { + spc.Status.DataPlaneOperatorsManagedIdentities.Identities = nil + }), + 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(), newTestSPC()), + }, + 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() + spc := newTestSPC() + 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, spc), + }, + 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, spc, 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