feat: add ClusterDenyAssignment controller for Azure deny assignments - #6269
feat: add ClusterDenyAssignment controller for Azure deny assignments#6269deads2k wants to merge 13 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new backend reconciliation loop to enforce least-privilege by creating Azure Microsoft.Authorization/denyAssignments on the managed resource group during cluster creation and removing them during cluster deletion, with progress/state tracked on ServiceProviderCluster.Status.
Changes:
- Introduces
ClusterDenyAssignmentcontroller and deny-assignment definition/permissions catalog to create/delete deterministic deny assignments. - Extends
ServiceProviderClusterStatusto persistPendingDenyAssignmentsandDenyAssignments, plus helper functions to build deny-assignment resource IDs. - Adds lifecycle gates: cluster-service create waits for deny assignments, and ServiceProviderCluster deletion is blocked until deny assignments are removed; wires new Azure clients and controller into backend startup.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/api/zz_generated.deepcopy.go | Adds deepcopy support for the new deny-assignment reference type and new ServiceProviderClusterStatus fields. |
| internal/api/types_serviceprovider_cluster.go | Adds PendingDenyAssignments/DenyAssignments to SPC status and defines DenyAssignmentReference. |
| internal/api/types_cosmosdata.go | Adds helpers to construct/parse deny assignment ARM resource IDs. |
| backend/pkg/controllers/denyassignments/deny_assignment_permissions.go | Defines the action/notAction/dataAction catalogs used in deny assignment permissions. |
| backend/pkg/controllers/denyassignments/deny_assignment_definitions.go | Defines deny assignment “types” and which operator identities they exclude, plus deterministic ID generation inputs. |
| backend/pkg/controllers/denyassignments/deny_assignment_controller.go | Implements the create/delete reconciliation logic and persistence of pending/created deny assignments. |
| backend/pkg/controllers/clusterdeletion/cluster_child_resources_cleanup_controller.go | Blocks ServiceProviderCluster deletion while deny assignments are still tracked as pending/created. |
| backend/pkg/controllers/clustercreation/cluster_cluster_service_create_controller.go | Adds a precondition gate requiring deny assignments to be created before cluster-service create dispatch. |
| backend/pkg/controllers/clustercreation/cluster_cluster_service_create_controller_test.go | Updates existing tests to satisfy the new deny-assignment precondition gate. |
| backend/pkg/azure/client/generic_resources_client.go | Adds a minimal interface wrapper for armresources.Client generic CRUD by ID (used for deny assignments). |
| backend/pkg/azure/client/fpa_client_builder.go | Extends FPA client builder to construct generic resources + deny assignments clients. |
| backend/pkg/azure/client/deny_assignments_client.go | Adds a minimal interface wrapper for armauthorization.DenyAssignmentsClient (used for GET). |
| backend/pkg/app/backend.go | Registers and runs the new deny assignment controller under backend leader election. |
Files not reviewed (1)
- internal/api/zz_generated.deepcopy.go: Generated file
Comments suppressed due to low confidence (2)
backend/pkg/controllers/denyassignments/deny_assignment_controller.go:304
- ensureDenyAssignment dereferences resourceID and scope without nil checks (resourceID.Name / scope.String()). If a stored ServiceProviderCluster contains a DenyAssignmentReference with a nil DenyAssignmentResourceID, this will panic the controller.
Add explicit nil checks and return a tracked error instead of panicking.
if notActions == nil {
notActions = []string{}
}
if dataActions == nil {
dataActions = []string{}
}
excludedPrincipalIDs, err := resolvePrincipalIDs(cluster, excludedIdentityResourceIDs)
if err != nil {
return utils.TrackError(fmt.Errorf("failed to resolve principal IDs: %w", err))
}
existing, err := denyAssignmentsClient.Get(ctx, scope.String(), resourceID.Name, nil)
backend/pkg/controllers/denyassignments/deny_assignment_controller.go:439
- deleteDenyAssignment calls resourceID.String() without checking for nil. A nil DenyAssignmentResourceID in persisted state will panic during cluster deletion cleanup.
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) {
| definition, ok := defByType[denyAssignmentReference.DenyAssignmentType] | ||
| if !ok { | ||
| logger.Error(nil, "Skipping unknown deny assignment type", "denyAssignmentType", denyAssignmentReference.DenyAssignmentType) | ||
| continue | ||
| } |
| ready, err = c.createPreconditionDenyAssignmentsCreated(ctx, existingServiceProviderCluster) | ||
| if err != nil { | ||
| return utils.TrackError(err) | ||
| } | ||
| if !ready { | ||
| return nil | ||
| } |
| pendingTypes = append(pendingTypes, denyAssignmentReference.DenyAssignmentType) | ||
| } | ||
| logger.Info("Deny assignments not yet created, waiting for ClusterDenyAssignment controller", | ||
| "pendingDenyAssignmentTypes", pendingTypes) |
There was a problem hiding this comment.
How useful is it to log the types we're waiting on? Could we also just give a number?
| dbCluster: newTestCluster(), | ||
| existingServiceProviderCluster: newTestSPC(func(spc *api.ServiceProviderCluster) { | ||
| spc.Spec.ControlPlaneVersion.DesiredVersion = desiredVersion | ||
| spc.Status.DenyAssignments = []api.DenyAssignmentReference{{DenyAssignmentType: "resources-deny-assignment", DenyAssignmentResourceID: api.Must(azcorearm.ParseResourceID("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/testManagedResourceGroup/providers/Microsoft.Authorization/denyAssignments/00000000-0000-0000-0000-000000000001"))}} |
There was a problem hiding this comment.
I suggest adding a new test where desiredVersion is set but deny assignments are still pending
| "github.com/Azure/ARO-HCP/internal/api" | ||
| ) | ||
|
|
||
| const ( |
There was a problem hiding this comment.
we're moving to a common single definition now that we have support for more excluded principals (Massimo can confirm if it's rolled out completely or not).
We should update this too only leverage a single deny assignment instead of multiple and hold the PR on massimo's confirmation it's rolled out to all regions we're deployed in.
PR should just lay down a single deny assignment which looks like: https://github.com/openshift-online/aro-hcp-clusters-service/blob/master/pkg/azure/denyassignmentcreator/deny_assignment_creator.go#L245-L303. We're missing a few permissions from there, so we'll see the following additional permissions shortly:
"Microsoft.Network/dnszones/CAA/write",
"Microsoft.Network/dnszones/TXT/write",
"Microsoft.Network/dnszones/TXT/delete",
"Microsoft.Compute/virtualMachines/retrieveBootDiagnosticsData/action",
and the removal of Microsoft.Authorization/roleAssignments/read since read permissions are already excluded.
There was a problem hiding this comment.
If instead we want 1:1 parity between CS and what's here, we need to introduce a deny assignment similar to what is mentioned above ^ since that one doesn't appear in this PR.
Today CS lays down all the ones mentioned + the one above if the excluded principal limit is large enough.
There was a problem hiding this comment.
the one above if the excluded principal limit is large enough.
+1; we should create the complete one as well to align with CS
There was a problem hiding this comment.
we're moving to a common single definition now that we have support for more excluded principals (Massimo can confirm if it's rolled out completely or not).
Just checked. The ExcludedPrincipal Limit has been increased across all regions (see this kusto query )
| return utils.TrackError(fmt.Errorf("BeginDeleteByID failed: %w", err)) | ||
| } | ||
|
|
||
| _, err = poller.PollUntilDone(ctx, nil) |
There was a problem hiding this comment.
Do we want to block here or somehow signal "reenqueue and retry after a while" ?
There was a problem hiding this comment.
This exceeds my azure knowledge. Are these fast or so slow?
| return utils.TrackError(fmt.Errorf("BeginCreateOrUpdateByID failed: %w", err)) | ||
| } | ||
|
|
||
| _, err = poller.PollUntilDone(ctx, nil) |
There was a problem hiding this comment.
as a note, if this ends up in a failed terminal state we would be erroring and retrying indefinitely
There was a problem hiding this comment.
as a note, if this ends up in a failed terminal state we would be erroring and retrying indefinitely
I bet it's an azure client gap on my part again. What is supposed to happen?
| serviceProviderCluster.Status.DenyAssignments = append(serviceProviderCluster.Status.DenyAssignments, denyAssignmentReference) | ||
| serviceProviderCluster.Status.PendingDenyAssignments = removeDenyAssignmentRef(serviceProviderCluster.Status.PendingDenyAssignments, denyAssignmentReference.DenyAssignmentType) | ||
| } | ||
| _, err = serviceProviderClusterCRUD.Replace(ctx, serviceProviderCluster, nil) |
There was a problem hiding this comment.
an alternative is to deepcopy and do semantic deepequal like in other controllers, to only replace when they differ
| return utils.TrackError(err) | ||
| } | ||
|
|
||
| if cluster.ServiceProviderProperties.DeletionTimestamp != nil { |
There was a problem hiding this comment.
There is no need to issue an arm request to delete a deny assingement when the cluster is being deleted.
That's because the MRG is going to be deleted and the corresponding deny assignment will be deleted in cascade.
| return len(serviceProviderCluster.Status.DenyAssignments) > 0 || len(serviceProviderCluster.Status.PendingDenyAssignments) > 0 | ||
| } | ||
|
|
||
| func (c *clusterDenyAssignmentSyncer) creationNeedsWork(cluster *api.HCPOpenShiftCluster, serviceProviderCluster *api.ServiceProviderCluster) bool { |
There was a problem hiding this comment.
There is no point in doing the work if:
- The MRG doesn't exist
- or the principals ids collected here https://github.com/deads2k/ARO-HCP/blob/4ddb416b6d93b38a61890b264e90492d3db75551/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L467 are not there
Let's add at least the second part in the creationNeedsWork check.
There was a problem hiding this comment.
- or the principals ids collected here https://github.com/deads2k/ARO-HCP/blob/4ddb416b6d93b38a61890b264e90492d3db75551/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L467 are not there
Let's add at least the second part in the
creationNeedsWorkcheck.
It's a little weird to fanout and I bet it ages poorly as the list of definitions changes. You sure or shall we not add to pending when there isn't an ID for it?
There was a problem hiding this comment.
During early cluster creation phases, it is expected for the principalID to not be there up until when they are reconciled;
If we don't gate the controller for their existence, when collecting them we'll return an error https://github.com/deads2k/ARO-HCP/blob/4ddb416b6d93b38a61890b264e90492d3db75551/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L467 causing undesired retries due to https://github.com/deads2k/ARO-HCP/blob/4ddb416b6d93b38a61890b264e90492d3db75551/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L189
or shall we not add to pending when there isn't an ID for it?
Is the idea that we sync but only add the deny assignment to the pending list and not return an error? If so, that can work too.
There was a problem hiding this comment.
Ok, you've phrased it as, "don't have principleIDs for deny assignment", but do you actually mean:
cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ControlPlaneOperators and ``cluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.DataPlaneOperatorsandcluster.CustomerProperties.Platform.OperatorsAuthentication.UserAssignedIdentities.ServiceManagedIdentity` have values?
There was a problem hiding this comment.
Sorry for the confusion in the phrasing.
Concretely, I meant that we should only trigger the creation of the deny assignment when we know that the principal IDs of the operator + smi have been retrieved so that we are able to resolve the principalIDs of each operator identity (data plane and control plane) + the service manage identity in this piece of code https://github.com/deads2k/ARO-HCP/blob/41353414c6de01f1704ed83529dee8bb40c5caec/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L549
This line https://github.com/deads2k/ARO-HCP/blob/41353414c6de01f1704ed83529dee8bb40c5caec/backend/pkg/controllers/denyassignments/deny_assignment_controller.go#L565 will return an error each all the time even early on during provisioning phases where the principalIDs have not been retrieved from Azure yet.
In practice it is okay, since eventually they'll be reconciled, but the error is likely to trigger the BackendControllerRetryHotLoop when a cluster is being created in Stage/Prod envs where we can create deny assignments
| return len(serviceProviderCluster.Status.DenyAssignments) > 0 || len(serviceProviderCluster.Status.PendingDenyAssignments) > 0 | ||
| } | ||
|
|
||
| func (c *clusterDenyAssignmentSyncer) creationNeedsWork(cluster *api.HCPOpenShiftCluster, serviceProviderCluster *api.ServiceProviderCluster) bool { |
There was a problem hiding this comment.
The controller can only be run in environment where deny assignment can be created i.e stage & prod. Lower envs, this controller needs to be disabled
There was a problem hiding this comment.
I give detail about that here: #6269 (comment)
| go dispatchRequestCredentialController.Run(ctx, 20) | ||
| go dispatchRevokeCredentialsController.Run(ctx, 20) | ||
| go clusterClusterServiceCreateController.Run(ctx, 20) | ||
| go clusterDenyAssignmentController.Run(ctx, 20) |
| @@ -0,0 +1,529 @@ | |||
| // Copyright 2026 Microsoft Corporation | |||
There was a problem hiding this comment.
My understanding is that deny assignments can only be created in environments where a real FPA exists. On environments where it doesn't exist (like dev, integration) they can't be created because only a real FPA can create them. This would impact the execution of this controller (as well as preconditions . What signals to backend whether a real FPA is being used or not is the CLI flag --insecure-ignore-user-azure-managed-identities-that-need-managed-identities-dataplane-available-and-use-mock (backend/cmd/root.go).
I don't recall the details on whether attempting to create a denyassignment fails with an error, fails silently or simply creates the resource with no effect. If we are interested on this detail we should find it out.
| ) | ||
| } | ||
|
|
||
| func (c *clusterDenyAssignmentSyncer) deletionNeedsWork(serviceProviderCluster *api.ServiceProviderCluster) bool { |
There was a problem hiding this comment.
Here I think we should not proceed with deletion if CSID is still set:
if cluster.ServiceProviderProperties.ClusterServiceID != nil && len(cluster.ServiceProviderProperties.ClusterServiceID.String()) > 0 {
return false
}
CS has an ordered and coordinated delete process. For example, we don't delete the managed resource group until the deny assignments within it have been deleted. Or another example being we don't delete the managed resource group until the cluster has been fully deleted from the management cluster side, because there are resources within the managed resourcegroup that were created as part of the creation of the cluster in the management cluster side
chore: render digests using materialize
…create Introduce a new controller that assigns a PendingClusterServiceID (using ksuid-based /api/aro_hcp/v1alpha1/clusters/<uid>) on clusters that have neither PendingClusterServiceID nor ClusterServiceID set. The ClusterClusterServiceCreate controller now requires PendingClusterServiceID to be present before it will act, and uses its ClusterID() as the cluster-service instance ID instead of generating a new UID inline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a backend controller that creates Azure deny assignments on the managed resource group before cluster-service create and deletes them during cluster deletion. The controller tracks state via PendingDenyAssignments and DenyAssignments on ServiceProviderClusterStatus. The deny assignment permissions catalog is ported from clusters-service, covering 19 resource-provider-specific deny assignments that enforce least-privilege access for OpenShift operator managed identities. Key behaviors: - Gates ClusterClusterServiceCreate on all deny assignments being created - Blocks ServiceProviderCluster deletion while deny assignments remain - Uses ensureDenyAssignment (read-compare-update) for idempotency - Handles already-deleted assignments gracefully (404 → no-op) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The deny assignment controller now requires either PendingClusterServiceID or ClusterServiceID to be set before creating deny assignments. The .ClusterID() from whichever is available is used to generate deterministic deny assignment UUIDs, matching the clusters-service logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nments The deny assignment controller now waits for the managed resource group to be confirmed in Azure before attempting to create deny assignments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After all deny assignments reach stable state (all created, none pending), set EarliestRecheckTime to 12h + 50% jitter (6h-18h range). Add syncConsistencyCheck that runs when EarliestRecheckTime has passed. It verifies the expected set of deny assignments matches what is tracked in the ServiceProviderCluster and checks that each still exists in Azure. Stale types are removed, missing-from-Azure are moved to pending, and newly defined types are added to pending. When drift is detected, EarliestRecheckTime is cleared so syncCreation picks up immediately. When everything is consistent, EarliestRecheckTime is set again. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a deny assignment type is no longer in the expected set, actually delete it from Azure before removing it from the tracked AzureResources. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4ddb416 to
3768270
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
working on testing, but I think I got the other feedback fixed |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 45 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (11)
internal/api/types_serviceprovider_cluster.go:257
- New Cosmos-persisted fields should include
// Written by:annotations for leaf fields. Add the writer annotation for AzureResources.
internal/api/types_serviceprovider_cluster.go:265 - New Cosmos-persisted fields should include
// Written by:annotations for leaf fields. Add the writer annotation for EarliestRecheckTime.
internal/api/types_serviceprovider_cluster.go:276 - New Cosmos-persisted fields should include
// Written by:annotations for leaf fields. Add the writer annotation for DenyAssignmentResourceID.
backend/pkg/controllers/denyassignments/deny_assignment_controller.go:97 - This controller exits early when DeletionTimestamp is set, but the PR description states deny assignments are deleted during cluster deletion. Either implement the deletion path here (or elsewhere) or update the PR description so behavior matches reality.
// Deny assignments are scoped to the managed resource group and are
// cleaned up automatically when the managed resource group is deleted.
if cluster.ServiceProviderProperties.DeletionTimestamp != nil {
return nil
}
internal/api/types_serviceprovider_cluster.go:255
- New Cosmos-persisted fields should include
// Written by:annotations for leaf fields. Add the writer annotation for PendingAzureResources.
This issue also appears in the following locations of the same file:
- line 256
- line 263
internal/api/types_serviceprovider_cluster.go:273
- New Cosmos-persisted fields should include
// Written by:annotations for leaf fields. Add the writer annotation for DenyAssignmentType.
This issue also appears on line 274 of the same file.
internal/api/types_serviceprovider_cluster.go:210
- Per the repo’s Cosmos DB documentation conventions (see docs/cosmos-data-flow.md), changes to Cosmos-persisted API types should be reflected in docs/cosmos-data-flow.md. This PR adds/changes persisted deny-assignment tracking fields but does not update the data-flow doc.
internal/ocm/csuid.go:26 - The comment describes KSUIDs as “20 bytes of random data”. KSUIDs include a timestamp component, so this is misleading; please update the doc comment to match what is actually generated.
config/config.yaml:162 - This PR updates several image digests/Helm chart sources alongside the new deny-assignment controller. CONTRIBUTING.md requires “one PR per task” and to not mix features with unrelated changes; consider splitting the image bumps into a separate PR to keep review scope focused.
arobit:
forwarder:
image:
registry: mcr.microsoft.com
repository: oss/v2/fluent/fluent-bit
digest: sha256:c81f949228a0d446aea4966d8da39cfcd63f17c69a5eae143ce8264ddddc5777 # v5.0.4 (2026-07-28 19:40)
resources:
backend/pkg/controllers/denyassignments/deny_assignment_controller.go:62
- The new deny-assignment controller is substantial and introduces non-trivial Azure API interactions, but there are no unit tests for it in this package. The repo has broad controller test coverage (e.g. backend/pkg/controllers/clustercreation/*_test.go), so adding focused tests for ensure/update/delete decisions (with mocked Azure clients) would reduce regression risk.
func NewClusterDenyAssignmentController(
clock utilsclock.PassiveClock,
resourcesDBClient database.ResourcesDBClient,
azureFPAClientBuilder azureclient.FirstPartyApplicationClientBuilder,
backendInformers informers.BackendInformers,
internal/api/types_serviceprovider_cluster.go:209
- New Cosmos-persisted fields should follow the established
// Written by:annotations for leaf fields (per repo conventions). Add a writer annotation for this DenyAssignments field.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 47 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
internal/api/types_serviceprovider_cluster.go:266
- New Cosmos-persisted fields in ServiceProviderClusterStatus should include per-leaf "Written by:" annotations (per the repo's Cosmos writer-annotation convention). DenyAssignmentReferences currently has none, which makes it hard to audit which controller mutates these fields and will drift from docs/cosmos-data-flow.md.
internal/api/types_serviceprovider_cluster.go:277 - DenyAssignmentReference is stored in Cosmos but its leaf fields are missing "Written by:" annotations, which the rest of this file uses to keep Cosmos writer ownership clear.
config/config.yaml:177 - This PR includes multiple image digest bumps and rendered config updates (e.g. fluent-bit/mdsd/hypershift/velero) that are not described in the PR summary and are unrelated to adding the ClusterDenyAssignment controller. CONTRIBUTING.md requires keeping PRs focused (one task per PR); consider splitting these bumps into a separate PR so review/rollback is safer.
arobit:
forwarder:
image:
registry: mcr.microsoft.com
repository: oss/v2/fluent/fluent-bit
digest: sha256:c81f949228a0d446aea4966d8da39cfcd63f17c69a5eae143ce8264ddddc5777 # v5.0.4 (2026-07-28 19:40)
resources:
requests:
cpu: 100m
memory: 256Mi
limits:
memory: 1248Mi
kusto:
enabled: false
buffering: true
environmentName: "{{ .ctx.environment }}"
mdsd:
enabled: true
image:
registry: mcr.microsoft.com
repository: geneva/distroless/mdsd
digest: sha256:5c0159feac3312c447b10c840558a5db6591f78c498cf9150e010be9fa860a20 # 1.43.0-20260728-3 (2026-07-28 17:38)
| // Deny assignments are scoped to the managed resource group and are | ||
| // cleaned up automatically when the managed resource group is deleted. | ||
| if cluster.ServiceProviderProperties.DeletionTimestamp != nil { | ||
| return nil | ||
| } | ||
|
|
||
| return c.syncDenyAssignmentUpsert(ctx, key, cluster) |
| if serviceProviderCluster.Status.AzureResources.ManagedResourceGroup.AzureResource == nil { | ||
| return false | ||
| } |
| managedResourceGroupID := serviceProviderCluster.Status.AzureResources.ManagedResourceGroup.AzureResource | ||
|
|
|
@deads2k: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| } | ||
|
|
||
| lookup := make(map[string]string, len(cluster.Identity.UserAssignedIdentities)) | ||
| for resourceID, identity := range cluster.Identity.UserAssignedIdentities { |
There was a problem hiding this comment.
The cluster.Identity.UserAssignedIdentities field only contain the control plane identities + the service managed identities; the data plane identities will be missing;
We need a way to track their principal ids somewhere in the RP so that we can use them;
or alternatively, retrieve them via a call to ARM using the credentials of the ServiceManagedIdentity (that's how CS does it https://github.com/openshift-online/aro-hcp-clusters-service/blob/master/pkg/clusterprovisioner/acm/aro/fetch_and_persist_data_plane_managed_identities_client_ids_and_principal_ids_step.go)
Add a backend controller that creates Azure deny assignments on the managed resource group before cluster-service create and deletes them during cluster deletion. The controller tracks state via PendingDenyAssignments and DenyAssignments on ServiceProviderClusterStatus.
The deny assignment permissions catalog is ported from clusters-service, covering 19 resource-provider-specific deny assignments that enforce least-privilege access for OpenShift operator managed identities.
Key behaviors:
/hold
cluster service needs fixes before we can do this.