feat: add support for OCM role creation by andclt · Pull Request #6046 · kubernetes-sigs/cluster-api-provider-aws

andclt · 2026-06-11T10:27:51Z

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

AI Usage:

Checklist:

Release note:

k8s-ci-robot · 2026-06-11T10:27:54Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2026-06-11T10:27:58Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dlipovetsky for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

k8s-ci-robot · 2026-06-11T10:28:02Z

Hi @andclt. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

robpblake · 2026-06-11T12:31:54Z

+	// Note: We do NOT perform any cleanup on deletion because OCM roles are
+	// singleton resources shared across clusters. Automated deletion could break
+	// all clusters using this role.
+	if !scope.OCMRoleConfig.DeletionTimestamp.IsZero() {


@andclt So how does a customer delete an OCM Role once created?

I think delete of the resource should unlink and delete the OCM Role? Customers will be prompted to recreate it when they try to interact with Cluster Service

As discussed on Slack, deleting the OCM role when deleting the CR instance is the right approach here since the OCM role deletion won't break running clusters - will fix it.

I believe the right behavior is to unlink then delete the ocmRole AND provide optional field eg; skipDelete to let the user skip deleting Role just delete CR.

Also we should have finalizer to handle any error happen during delete

yes I will re-add the finalizers as well, I removed them because we didn't have an actual ReconcileDelete function.

robpblake · 2026-06-11T12:33:07Z

+	// +kubebuilder:default="/"
+	Path string `json:"path,omitempty"`
+
+	// ManagedPolicies indicates whether to use AWS-managed policies (true) or customer-managed policies (false).


@andclt No need to expose this at this time. We only support customer managed policies on the OCM Role today. We can revisit if/when they move to managed policies.

sure - fixed in latest commit

robpblake · 2026-06-11T12:33:39Z

+	// Path is the IAM path for the OCM role.
+	// +optional
+	// +kubebuilder:default="/"
+	Path string `json:"path,omitempty"`


@andclt Can you validate this starts with a /?

sure - fixed in latest commit

serngawy

some comments and please add release note in the PR desc.

serngawy · 2026-06-11T15:15:08Z

+	// The final role name will be: {RolePrefix}-OCM-Role-{ExternalID}
+	// where ExternalID is the organization's external identifier from OCM.
+	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:MaxLength:=32


I would make the max 4-6 to avoid long name case

I initially wanted to keep it consistent with the ROSA CLI (which allows up to 32 chars for all role prefixes), but I see that ROSARoleConfig already limits both AccountRoleConfig and OperatorRoleConfig
prefixes to 4 characters, so I am fine to change it to 4 chars.

serngawy · 2026-06-11T15:18:10Z

+
+	// Path is the IAM path for the OCM role.
+	// +optional
+	// +kubebuilder:default="/"


no need for default as its an option

serngawy · 2026-06-11T15:20:54Z

+	OrganizationID string `json:"organizationID,omitempty"`
+
+	// Linked indicates whether the role has been successfully linked to the OCM organization.
+	Linked bool `json:"linked,omitempty"`


I don't think there is a need to show this flag. We should show error in link condition if we couldn't link it

Makes sense, the Ready condition already captures whether the role was successfully linked or not.

serngawy · 2026-06-11T15:25:32Z

+	WatchFilterValue string
+	NewStsClient     func(cloud.ScopeUsage, cloud.Session, logger.Wrapper, runtime.Object) stsiface.STSClient
+	NewOCMClient     func(ctx context.Context, scope caparosa.OCMSecretsRetriever) (caparosa.OCMClient, error)
+	Runtime          *rosacli.Runtime


this need to change as we are moving to cross account creation. The runtime need to be established with the reconciling. check my PR here

serngawy · 2026-06-11T15:28:30Z

+// +kubebuilder:resource:path=ocmroleconfigs,scope=Cluster,categories=cluster-api,shortName=ocmrole
+// +kubebuilder:storageversion
+// +kubebuilder:subresource:status
+type OCMRoleConfig struct {


lets name it as ROSAOCMRoleConfig to follow all other ROSA APIs naming. Also for all struct, types and constant let them all start with ROSA

serngawy · 2026-06-11T15:48:38Z

+	// Acquire per-organization mutex to prevent concurrent LinkOrgToRole races
+	// Multiple OCMRoleConfigs with different AWS accounts can link to the same org,
+	// causing read-modify-write races on the org label. Serialize linking per org.
+	linkMu := r.getLinkMutex(orgID)


can you elaborate more why this is needed ? how this statement " Multiple OCMRoleConfigs with different AWS accounts can link to the same org" could happen ? I think it should be; first role linked wine others will raise error cannot link.

The OCM Role is used to link an OCM Org to the AWS Account(s) that they use for cluster provisioning. An OCM org can use many AWS accounts, so it should be possible to link an OCM Role as long as there is not one already linked for the same AWS account.

This is the same functionality as offered today by the CLI in rosa create ocm-role

Yes, as Rob said, we can have multiple AWS accounts associated to the same OCM organization by defining multiple OCMRoleConfig with different IdentityRef.

serngawy · 2026-06-11T15:58:59Z

+	// Note: We do NOT perform any cleanup on deletion because OCM roles are
+	// singleton resources shared across clusters. Automated deletion could break
+	// all clusters using this role.
+	if !scope.OCMRoleConfig.DeletionTimestamp.IsZero() {


I believe the right behavior is to unlink then delete the ocmRole AND provide optional field eg; skipDelete to let the user skip deleting Role just delete CR.

serngawy · 2026-06-11T15:59:58Z

+	// Note: We do NOT perform any cleanup on deletion because OCM roles are
+	// singleton resources shared across clusters. Automated deletion could break
+	// all clusters using this role.
+	if !scope.OCMRoleConfig.DeletionTimestamp.IsZero() {


Also we should have finalizer to handle any error happen during delete

andclt · 2026-06-12T09:47:08Z

I believe the right behavior is to unlink then delete the ocmRole AND provide optional field eg; skipDelete to let the user skip deleting Role just delete CR.

@serngawy, could you share more about the optional field use case? What's the scenario where someone wants to delete the CR but keep the IAM role?

andclt · 2026-06-16T17:45:47Z

Closing in favor of: #6055
We could not update the ROSA dependency directly due to Go directive version incompatibility between ROSA and CAPA, so the logic was ported and adapted to work within CAPA's architecture.
The comments have been solved in the new PR.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 11, 2026

k8s-ci-robot requested review from faiq and serngawy June 11, 2026 10:27

k8s-ci-robot added needs-priority size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 11, 2026

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 11, 2026

andclt force-pushed the ocm-role-config-crd branch 2 times, most recently from bc4ddf2 to e72f968 Compare June 11, 2026 11:58

robpblake reviewed Jun 11, 2026

View reviewed changes

Comment thread exp/controllers/rosaocmroleconfig_controller.go

andclt force-pushed the ocm-role-config-crd branch 2 times, most recently from 390d95c to b87fe6d Compare June 11, 2026 13:59

serngawy reviewed Jun 11, 2026

View reviewed changes

feat: add support for OCM role creation

6cfa3c3

andclt force-pushed the ocm-role-config-crd branch from b87fe6d to 6cfa3c3 Compare June 12, 2026 10:43

andclt closed this Jun 16, 2026

andclt mentioned this pull request Jun 18, 2026

✨ feat: add support for OCM role creation #6055

Merged

6 tasks

Uh oh!

Conversation

andclt commented Jun 11, 2026

Uh oh!

k8s-ci-robot commented Jun 11, 2026

Uh oh!

k8s-ci-robot commented Jun 11, 2026

Uh oh!

k8s-ci-robot commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serngawy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andclt commented Jun 12, 2026

Uh oh!

andclt commented Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants