CNTRLPLANE-2205: feat(aws): add shared-role support for IAM operations

[sjenning]

Add shared-role flag to create/destroy IAM commands to support creating a single shared IAM role with all component policies instead of individual roles for each component. This reduces the number of IAM roles from 7+ to 1, reducing AWS API load and Roles quota issues.

Changes:

• Add SharedRole field to CreateIAMOptions and DestroyIAMOptions
• Add --shared-role flag to "hypershift create iam aws" command
• Add --shared-role flag to "hypershift destroy iam aws" command
• Implement CreateSharedOIDCRole to create single role with multiple inline policies
• Refactor DestroyOIDCRole to dynamically list and delete all inline policies, supporting both shared and individual roles
• Fix credential secret creation bug: change from map to slice to support multiple secrets with same ARN when using shared roles

The shared role includes all service accounts in its trust policy and contains separate inline policies for each component (ingress, image-registry, ebs-csi, cloud-controller, node-pool, control-plane-operator, network, karpenter, kms-provider).

---

Note

Adds a --shared-role option to create/destroy IAM paths to use a single aggregated IAM role, updates deletion logic accordingly, and fixes credential secret syncing for shared ARNs.

• IAM:
  • Shared role support: Implement CreateSharedOIDCRole in cmd/infra/aws/iam.go to create one role (<infra-id>-shared-role) with combined trust policy and multiple inline policies; sets all role ARNs to the shared ARN.
  • Destroy logic: In cmd/infra/aws/destroy_iam.go, add SharedRole handling to delete the shared role or individual roles; refactor DestroyOIDCRole to list/delete all inline policies dynamically and detach managed policies.
• CLI/Options:
  • Add SharedRole field and --shared-role flag across create/destroy commands and option structs: cmd/infra/aws/create_iam.go, cmd/infra/aws/destroy_iam.go, cmd/cluster/aws/create.go, cmd/cluster/aws/destroy.go, cmd/cluster/core/destroy.go.
  • Plumb SharedRole through to IAM create/destroy options.
• Operator:
  • Update credential secret sync in hypershift-operator/.../platform/aws/aws.go to use a slice instead of a map, allowing multiple secrets to reference the same ARN when using a shared role.

^{Written by Cursor Bugbot for commit 28895f2. This will update automatically on new commits. Configure here.}

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

This change introduces a new SharedRole option across the AWS cluster creation and destruction flows. A new --shared-role CLI flag enables users to opt into using a single shared IAM OIDC role for all service accounts instead of separate per-component roles. The feature is propagated through platform options to IAM operations.

Changes

Cohort / File(s)	Summary
CLI flag addition cmd/cluster/aws/create.go, cmd/cluster/aws/destroy.go	Add --shared-role CLI flag to AWS cluster creation and destruction commands, wired to platform and IAM options
Core struct fields cmd/cluster/core/destroy.go, cmd/infra/aws/create_iam.go, cmd/infra/aws/destroy_iam.go	Add SharedRole bool field to AWSPlatformDestroyOptions, CreateIAMOptions, and DestroyIAMOptions structs for configuration propagation
IAM creation logic cmd/infra/aws/iam.go, cmd/infra/aws/create_iam.go	Introduce conditional branching in CreateOIDCResources on SharedRole flag; add new CreateSharedOIDCRole method that builds aggregated trust policy from all bindings and creates single shared role, or use per-component role path if false
IAM destruction logic cmd/infra/aws/destroy_iam.go	Update DestroyOIDCResources to branch on SharedRole: delete "shared-role" if true, otherwise delete individual component roles; generalize DestroyOIDCRole to dynamically list and delete all inline policies instead of fixed set
Secret synchronization hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go	Replace map-based loop with slice-based iteration over (ARN, secret) pairs to handle multiple secrets sharing same ARN

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• IAM destruction changes (cmd/infra/aws/destroy_iam.go): Generalization of DestroyOIDCRole to dynamically handle inline policies requires careful review of error handling and logging behavior changes
• New shared role creation (cmd/infra/aws/iam.go): New CreateSharedOIDCRole method and conditional branching in CreateOIDCResources need verification for trust policy aggregation correctness and error propagation
• Control flow refactor (hypershift-operator/.../aws.go): Transition from map to slice iteration affects ARN deduplication logic and error handling across secret synchronization

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sjenning

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [sjenning]

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

[csrwng]

/lgtm

[sjenning]

/verified by @sjenning

[openshift-ci-robot]

@sjenning: This PR has been marked as verified by @sjenning.

Details

In response to this:

/verified by @sjenning

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 openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

cmd/infra/aws/destroy_iam.go (1)

190-191: Unused parameter includeAssumePolicy after refactoring.

The includeAssumePolicy parameter is still passed to callers but is never used within the function body. The refactored code now dynamically lists and deletes all inline policies, making this parameter obsolete.

Consider removing the unused parameter:

-func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string, includeAssumePolicy bool) error {
+func (o *DestroyIAMOptions) DestroyOIDCRole(client iamiface.IAMAPI, name string) error {

This would require updating all callers (lines 153, 158, 161, 164, 167, 170, 173, 176, 179, 182, 360, 363) to remove the boolean argument.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between b198f85 and 28895f2.

📒 Files selected for processing (7)

• cmd/cluster/aws/create.go (3 hunks)
• cmd/cluster/aws/destroy.go (2 hunks)
• cmd/cluster/core/destroy.go (1 hunks)
• cmd/infra/aws/create_iam.go (2 hunks)
• cmd/infra/aws/destroy_iam.go (4 hunks)
• cmd/infra/aws/iam.go (2 hunks)
• hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• cmd/infra/aws/iam.go
• cmd/infra/aws/destroy_iam.go
• hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go
• cmd/cluster/core/destroy.go
• cmd/cluster/aws/destroy.go
• cmd/infra/aws/create_iam.go
• cmd/cluster/aws/create.go

🧬 Code graph analysis (1)

cmd/infra/aws/iam.go (2)

cmd/cluster/aws/create.go (1)

• CreateIAMOptions (570-588)

cmd/infra/aws/create_iam.go (1)

• CreateIAMOptions (28-51)

🪛 golangci-lint (2.5.0)

cmd/infra/aws/destroy_iam.go

[error] 228-228: : # github.com/openshift/hypershift/sync-global-pullsecret [github.com/openshift/hypershift/sync-global-pullsecret.test]
sync-global-pullsecret/sync-global-pullsecret_test.go:228:23: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:234:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:247:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:257:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:270:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:283:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:296:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:309:26: undefined: MockdbusConn
sync-global-pullsecret/sync-global-pullsecret_test.go:327:12: undefined: NewMockdbusConn

(typecheck)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Red Hat Konflux / hypershift-operator-main-on-pull-request
• GitHub Check: Red Hat Konflux / hypershift-release-mce-211-on-pull-request
• GitHub Check: Red Hat Konflux / hypershift-cli-mce-211-on-pull-request
• GitHub Check: Cursor Bugbot

🔇 Additional comments (15)

cmd/cluster/core/destroy.go (1)

59-59: LGTM!

The SharedRole field addition to AWSPlatformDestroyOptions is correctly placed and follows the existing struct conventions. This enables proper propagation of the shared-role setting through the destroy path.

cmd/cluster/aws/destroy.go (2)

37-37: LGTM!

The --shared-role flag is correctly bound and follows the existing flag patterns in this file.

---

108-108: LGTM!

The SharedRole value is correctly propagated from AWSPlatform options to DestroyIAMOptions.

cmd/infra/aws/create_iam.go (2)

50-50: LGTM!

The SharedRole field is correctly added to CreateIAMOptions struct, enabling the shared-role creation path.

---

94-94: LGTM!

The --shared-role flag is correctly bound with a clear description matching the feature intent.

cmd/cluster/aws/create.go (3)

63-63: LGTM!

The SharedRole field correctly extends RawCreateOptions to support the shared-role feature in cluster creation.

---

506-506: LGTM!

The --shared-role flag is correctly bound with consistent description across create commands.

---

586-586: LGTM!

The SharedRole value is correctly propagated from awsOpts to CreateIAMOptions, completing the option chain for IAM creation.

cmd/infra/aws/destroy_iam.go (4)

32-32: LGTM!

The SharedRole field is correctly added to DestroyIAMOptions.

---

53-53: LGTM!

The --shared-role flag is correctly bound with consistent description.

---

150-185: LGTM!

The branching logic correctly handles both shared-role and individual component roles destruction paths. The shared role uses the name "shared-role" which should match the creation side.

---

222-248: LGTM!

The refactored inline policy deletion correctly uses ListRolePolicies to dynamically discover and delete all policies, which properly supports both shared roles (with multiple policies) and individual component roles.

cmd/infra/aws/iam.go (2)

872-892: LGTM! Clean conditional branching for shared role feature.

The implementation correctly branches between shared and individual role creation paths. The shared role ARN is properly propagated to all binding targets, and error handling is appropriate.

---

993-1062: LGTM! Well-structured shared role creation.

The CreateSharedOIDCRole function correctly:

• Deduplicates service accounts across all bindings
• Creates a unified trust policy with all service accounts
• Attaches each component's policy as a separate inline policy
• Conditionally adds the assume role policy when needed
• Provides clear logging and error messages

hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go (1)

301-316: Good fix for the duplicate ARN issue—slice-based approach correctly preserves shared role secrets.

The change from a map to a slice properly addresses the limitation where a map keyed by ARN would silently drop entries when multiple secrets share the same ARN. This implementation maintains all secret/ARN pairs and preserves the existing error aggregation behavior. Errors from missing or invalid ARNs will be caught during the syncSecret calls and aggregated in the errs slice, so no additional validation is needed here.

[openshift-ci-robot]

@sjenning: This pull request explicitly references no jira issue.

Details

In response to this:

Add shared-role flag to create/destroy IAM commands to support creating a single shared IAM role with all component policies instead of individual roles for each component. This reduces the number of IAM roles from 7+ to 1, reducing AWS API load and Roles quota issues.

Changes:

• Add SharedRole field to CreateIAMOptions and DestroyIAMOptions
• Add --shared-role flag to "hypershift create iam aws" command
• Add --shared-role flag to "hypershift destroy iam aws" command
• Implement CreateSharedOIDCRole to create single role with multiple inline policies
• Refactor DestroyOIDCRole to dynamically list and delete all inline policies, supporting both shared and individual roles
• Fix credential secret creation bug: change from map to slice to support multiple secrets with same ARN when using shared roles

The shared role includes all service accounts in its trust policy and contains separate inline policies for each component (ingress, image-registry, ebs-csi, cloud-controller, node-pool, control-plane-operator, network, karpenter, kms-provider).

---

[!NOTE]
Adds a --shared-role option to create/destroy IAM paths to use a single aggregated IAM role, updates deletion logic accordingly, and fixes credential secret syncing for shared ARNs.

• IAM:
  • Shared role support: Implement CreateSharedOIDCRole in cmd/infra/aws/iam.go to create one role (<infra-id>-shared-role) with combined trust policy and multiple inline policies; sets all role ARNs to the shared ARN.
  • Destroy logic: In cmd/infra/aws/destroy_iam.go, add SharedRole handling to delete the shared role or individual roles; refactor DestroyOIDCRole to list/delete all inline policies dynamically and detach managed policies.
• CLI/Options:
  • Add SharedRole field and --shared-role flag across create/destroy commands and option structs: cmd/infra/aws/create_iam.go, cmd/infra/aws/destroy_iam.go, cmd/cluster/aws/create.go, cmd/cluster/aws/destroy.go, cmd/cluster/core/destroy.go.
  • Plumb SharedRole through to IAM create/destroy options.
• Operator:
  • Update credential secret sync in hypershift-operator/.../platform/aws/aws.go to use a slice instead of a map, allowing multiple secrets to reference the same ARN when using a shared role.

^{Written by Cursor Bugbot for commit 28895f2. This will update automatically on new commits. Configure here.}

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 openshift-eng/jira-lifecycle-plugin repository.

[sjenning]

/override ci/prow/e2e-aws-4-20

[openshift-ci]

@sjenning: Overrode contexts on behalf of sjenning: ci/prow/e2e-aws-4-20

Details

In response to this:

/override ci/prow/e2e-aws-4-20

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.

[openshift-ci]

@sjenning: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[devguyio]

/cherry-pick release-4.20

[openshift-cherrypick-robot]

@devguyio: #7331 failed to apply on top of branch "release-4.20":

Applying: feat(aws): add shared-role support for IAM operations
Using index info to reconstruct a base tree...
M	cmd/cluster/aws/create.go
M	cmd/cluster/aws/destroy.go
M	cmd/cluster/core/destroy.go
M	cmd/infra/aws/destroy_iam.go
Falling back to patching base and 3-way merge...
Auto-merging cmd/cluster/aws/create.go
Auto-merging cmd/cluster/aws/destroy.go
Auto-merging cmd/cluster/core/destroy.go
Auto-merging cmd/infra/aws/destroy_iam.go
CONFLICT (content): Merge conflict in cmd/infra/aws/destroy_iam.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 feat(aws): add shared-role support for IAM operations

Details

In response to this:

/cherry-pick release-4.20

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.