ROSAENG-57757 | feat: add trust_policy_external_id to account-iam-resources#149
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe ChangesIAM trust policy external ID support
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold |
|
Hi @michaelryanmcneill. Thanks for your PR. I'm waiting for a terraform-redhat member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Working on additional manual testing before submitting for review. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/account-iam-resources/README.md (1)
1-85:⚠️ Potential issue | 🟠 MajorAdd/update
*.tftest.hclcoverage fortrust_policy_external_idbehavior inmodules/account-iam-resources— the module contains no*.tftest.hcl(and notests/directory), so the new trust-policy condition logic (external ID present vs null/empty, and ensuring Worker/ControlPlane roles never get the condition) is currently untested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/account-iam-resources/README.md` around lines 1 - 85, The repo is missing tftest coverage for the new trust policy external ID behavior in modules/account-iam-resources; add a *.tftest.hcl (or tests/ directory) that exercises the input trust_policy_external_id with three scenarios: (1) null/empty external ID -> ensure the aws_iam_policy_document/custom_trust_policy does not include the ExternalId condition, (2) non-empty external ID -> ensure the ExternalId condition is present in the custom_trust_policy for installer/support roles, and (3) verify Worker and ControlPlane role policy documents (the IAM roles produced by module account_iam_role / aws_iam_role resources) never include the ExternalId condition regardless of trust_policy_external_id; reference the module input trust_policy_external_id, data.aws_iam_policy_document.custom_trust_policy, and the account_iam_role/role resources to assert the expected policy document contents.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@modules/account-iam-resources/README.md`:
- Around line 1-85: The repo is missing tftest coverage for the new trust policy
external ID behavior in modules/account-iam-resources; add a *.tftest.hcl (or
tests/ directory) that exercises the input trust_policy_external_id with three
scenarios: (1) null/empty external ID -> ensure the
aws_iam_policy_document/custom_trust_policy does not include the ExternalId
condition, (2) non-empty external ID -> ensure the ExternalId condition is
present in the custom_trust_policy for installer/support roles, and (3) verify
Worker and ControlPlane role policy documents (the IAM roles produced by module
account_iam_role / aws_iam_role resources) never include the ExternalId
condition regardless of trust_policy_external_id; reference the module input
trust_policy_external_id, data.aws_iam_policy_document.custom_trust_policy, and
the account_iam_role/role resources to assert the expected policy document
contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b015f861-fc47-42e7-ad04-d7480f2d76c0
📒 Files selected for processing (4)
modules/account-iam-resources/README.mdmodules/account-iam-resources/main.tfmodules/account-iam-resources/outputs.tfmodules/account-iam-resources/variables.tf
|
/ok-to-test |
michaelryanmcneill
force-pushed
the
ROSA-786
branch
from
036f669 to
63ac8dc
Compare
|
/ok-to-test |
michaelryanmcneill
force-pushed
the
ROSA-786
branch
2 times, most recently
from
39c90e6 to
772571d
Compare
|
Test results: |
|
/unhold |
|
/ok-to-test |
|
Ready for final review @olucasfreitas, thanks! |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl (1)
105-244: ⚡ Quick winAdd explicit assertions for
output.trust_policy_external_idacross scenarios.The new feature contract includes output normalization (
null/""-> unset, non-empty -> preserved), but this test file currently validates onlyoutput.custom_trust_policy_json. Please assert the output value in each run to prevent regressions intime_sleeptrigger wiring/output exposure.As per coding guidelines, “if behavior changes and modules//tests/.tftest.hcl exists (or needs updates), run terraform test” and keep module behavior coverage updated, including changed interfaces/outputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl` around lines 105 - 244, Update each test run to explicitly assert the normalized output.trust_policy_external_id value: in runs "null_external_id_omits_condition" assert output.trust_policy_external_id is null (or equals null-equivalent); in "empty_external_id_omits_condition" assert it is an empty string or null-equivalent per normalization; in "non_empty_external_id_adds_condition_to_installer_and_support" assert output.trust_policy_external_id equals "test-external-id-12345"; and in "worker_and_control_plane_never_include_external_id" assert output.trust_policy_external_id equals "test-external-id-12345" as well. Use the existing run blocks and the output name trust_policy_external_id to add these assertions so the tests cover the output normalization contract and prevent regressions in time_sleep/output wiring.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/account-iam-resources/variables.tf`:
- Around line 33-37: The variable trust_policy_external_id currently accepts
whitespace-only strings as valid; add a validation block to normalize/reject
such inputs by ensuring the value is either null or contains non-whitespace
characters. Inside the trust_policy_external_id variable block add: validation {
condition = var == null || length(trim(var)) > 0 message =
"trust_policy_external_id must be null or a non-empty, non-whitespace string" }
so whitespace-only values are rejected at the variable boundary.
- Around line 33-37: The variable trust_policy_external_id is not marked
sensitive; update the variable block for trust_policy_external_id to include
sensitive = true so Terraform redacts it in CLI output and logs; locate the
variable "trust_policy_external_id" in
modules/account-iam-resources/variables.tf and add the sensitive = true
attribute inside that variable declaration.
---
Nitpick comments:
In `@modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl`:
- Around line 105-244: Update each test run to explicitly assert the normalized
output.trust_policy_external_id value: in runs
"null_external_id_omits_condition" assert output.trust_policy_external_id is
null (or equals null-equivalent); in "empty_external_id_omits_condition" assert
it is an empty string or null-equivalent per normalization; in
"non_empty_external_id_adds_condition_to_installer_and_support" assert
output.trust_policy_external_id equals "test-external-id-12345"; and in
"worker_and_control_plane_never_include_external_id" assert
output.trust_policy_external_id equals "test-external-id-12345" as well. Use the
existing run blocks and the output name trust_policy_external_id to add these
assertions so the tests cover the output normalization contract and prevent
regressions in time_sleep/output wiring.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e548fa5b-b63b-46b2-b286-a551cdee22ff
📒 Files selected for processing (5)
modules/account-iam-resources/README.mdmodules/account-iam-resources/main.tfmodules/account-iam-resources/outputs.tfmodules/account-iam-resources/tests/trust_policy_external_id.tftest.hclmodules/account-iam-resources/variables.tf
🚧 Files skipped from review as they are similar to previous changes (3)
- modules/account-iam-resources/outputs.tf
- modules/account-iam-resources/main.tf
- modules/account-iam-resources/README.md
michaelryanmcneill
force-pushed
the
ROSA-786
branch
from
645a60b to
e2dffdc
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
modules/ocm-role/main.tf (1)
5-5: 💤 Low valueRedundant
coalesceon variable with non-null default.
var.pathhas a default value of"/"(variables.tf line 33), socoalesce(var.path, "/")will always returnvar.pathand never the fallback. Remove thecoalescewrapper.♻️ Simplify to direct assignment
- path = coalesce(var.path, "/") + path = var.path🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/ocm-role/main.tf` at line 5, The assignment uses coalesce(var.path, "/") which is redundant because var.path has a non-null default of "/" in variables.tf; replace the coalesce expression with a direct use of var.path (update the assignment that sets path = coalesce(var.path, "/") to path = var.path) to simplify the code and remove the unnecessary coalesce call.modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl (2)
105-258: 💤 Low valueConsider documenting the array index mapping for
custom_trust_policy_json.The tests assume
custom_trust_policy_json[0]is Installer,[1]is Support,[2]is Worker, and[3]is ControlPlane. While this ordering is likely stable (based on thefor_eachiteration in main.tf), adding a brief comment above the test runs explaining this mapping would improve test maintainability.📝 Suggested comment
variables { account_role_prefix = "tf-test-acc" openshift_version = "4.14.24" } + +# custom_trust_policy_json output array indices: +# [0] = Installer, [1] = Support, [2] = Worker, [3] = ControlPlane run "null_external_id_omits_condition" {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl` around lines 105 - 258, Add a short comment above the test runs documenting the index-to-role mapping for output.custom_trust_policy_json (0=Installer, 1=Support, 2=Worker, 3=ControlPlane) so future readers understand the ordering assumed by the assertions; modify the tests file to include that single-line comment near the top of the run blocks (before the first run "null_external_id_omits_condition") referencing custom_trust_policy_json to make the mapping explicit and maintainable.
105-258: ⚡ Quick winConsider adding test coverage for the
trust_policy_external_idoutput.The test suite thoroughly validates
custom_trust_policy_jsonbehavior but does not verify that thetrust_policy_external_idoutput correctly reflects the input value (or normalized null for empty strings). Adding assertions on this output would complete the test coverage.💡 Example assertion to add
In the
null_external_id_omits_conditionrun:assert { condition = !strcontains(output.custom_trust_policy_json[1], "sts:ExternalId") error_message = "Support trust policy must not include sts:ExternalId when trust_policy_external_id is null." } + + assert { + condition = output.trust_policy_external_id == null + error_message = "trust_policy_external_id output must be null when input is null." + }In the
non_empty_external_id_adds_condition_to_installer_and_supportrun:assert { condition = strcontains( output.custom_trust_policy_json[1], "sts:ExternalId", ) error_message = "Support trust policy must include sts:ExternalId condition key." } + + assert { + condition = output.trust_policy_external_id == "test-external-id-12345" + error_message = "trust_policy_external_id output must match the configured value." + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl` around lines 105 - 258, Add assertions that verify the trust_policy_external_id output is correctly populated/normalized: in the "null_external_id_omits_condition" run add an assert that output.trust_policy_external_id == null (or not set) to confirm null input yields null output; in the "non_empty_external_id_adds_condition_to_installer_and_support" run add an assert that output.trust_policy_external_id == "test-external-id-12345" to confirm the output mirrors the provided value; reference the variable var.trust_policy_external_id and the output output.trust_policy_external_id when adding these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/ocm-role/versions.tf`:
- Line 5: Update the Terraform constraint in this example by changing the
versions.tf required_version setting: replace the current required_version value
with ">= 1.5.7" so the example matches the repo root terraform.required_version
minimum; ensure the single required_version attribute in the versions.tf file is
updated accordingly.
In `@hack/install-release-tool.sh`:
- Around line 148-155: The checkov case in hack/install-release-tool.sh
currently only maps darwin_amd64 and windows_amd64 (see the case patterns
darwin_amd64/windows_amd64 and the asset variable assignment) so darwin_arm64
falls to the default error path; either add a darwin_arm64) branch that sets
asset="checkov_darwin_ARM64.zip" (or the correct published filename) and add the
corresponding SHA256 line to hack/checksums/checkov-3.2.529.sha256sums, or
explicitly gate/document the lack of darwin_arm64 support at the caller level so
the script doesn't hard-fail (choose the mapping+checksum fix if you intend to
support Apple Silicon).
In `@modules/ocm-role/main.tf`:
- Around line 40-113: This module currently depends on the HCP-only data source
data.rhcs_hcp_policies.all_policies (used by
aws_iam_role.ocm_role.assume_role_policy and aws_iam_policy.*.policy), which
violates the Classic-only repo boundary; change the module to accept policies
via input variables (e.g., var.assume_role_policy,
var.standard_permission_policy, var.admin_permission_policy,
var.no_console_permission_policy) and replace all references to
data.rhcs_hcp_policies.all_policies in aws_iam_role.ocm_role and
aws_iam_policy.standard_permission_policy / ocm_admin_permission_policy /
ocm_no_console_permission_policy with those variables (or alternatively move
this module into the terraform-rhcs-rosa-hcp repo if HCP-specific logic is
required). Ensure variable defaults/validation are added and remove the
rhcs_hcp_policies data source usage.
---
Nitpick comments:
In `@modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl`:
- Around line 105-258: Add a short comment above the test runs documenting the
index-to-role mapping for output.custom_trust_policy_json (0=Installer,
1=Support, 2=Worker, 3=ControlPlane) so future readers understand the ordering
assumed by the assertions; modify the tests file to include that single-line
comment near the top of the run blocks (before the first run
"null_external_id_omits_condition") referencing custom_trust_policy_json to make
the mapping explicit and maintainable.
- Around line 105-258: Add assertions that verify the trust_policy_external_id
output is correctly populated/normalized: in the
"null_external_id_omits_condition" run add an assert that
output.trust_policy_external_id == null (or not set) to confirm null input
yields null output; in the
"non_empty_external_id_adds_condition_to_installer_and_support" run add an
assert that output.trust_policy_external_id == "test-external-id-12345" to
confirm the output mirrors the provided value; reference the variable
var.trust_policy_external_id and the output output.trust_policy_external_id when
adding these checks.
In `@modules/ocm-role/main.tf`:
- Line 5: The assignment uses coalesce(var.path, "/") which is redundant because
var.path has a non-null default of "/" in variables.tf; replace the coalesce
expression with a direct use of var.path (update the assignment that sets path =
coalesce(var.path, "/") to path = var.path) to simplify the code and remove the
unnecessary coalesce call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2f18a2c6-7003-45dd-b363-3f7d064a4a5a
📒 Files selected for processing (20)
DockerfileREADME.mdexamples/ocm-role/README.mdexamples/ocm-role/main.tfexamples/ocm-role/outputs.tfexamples/ocm-role/variables.tfexamples/ocm-role/versions.tfhack/install-release-tool.shmodules/account-iam-resources/README.mdmodules/account-iam-resources/main.tfmodules/account-iam-resources/outputs.tfmodules/account-iam-resources/tests/trust_policy_external_id.tftest.hclmodules/account-iam-resources/variables.tfmodules/ocm-role/README.mdmodules/ocm-role/main.tfmodules/ocm-role/outputs.tfmodules/ocm-role/tests/ocm_role.tftest.hclmodules/ocm-role/variables.tfmodules/ocm-role/versions.tfversions.tf
✅ Files skipped from review due to trivial changes (5)
- modules/ocm-role/versions.tf
- Dockerfile
- modules/ocm-role/README.md
- examples/ocm-role/README.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- modules/account-iam-resources/variables.tf
- modules/account-iam-resources/outputs.tf
- modules/account-iam-resources/README.md
- modules/account-iam-resources/main.tf
michaelryanmcneill
force-pushed
the
ROSA-786
branch
from
e2dffdc to
e4e659b
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/account-iam-resources/README.md (1)
31-31:⚠️ Potential issue | 🟠 MajorUpdate the module's provider constraint to match the root versions.tf.
The
modules/account-iam-resources/versions.tfspecifiesrhcs >= 1.6.2, but the rootversions.tfrequires>= 1.7.7. The README correctly reflects the module constraint, but the module itself must be updated to the root floor. Updatemodules/account-iam-resources/versions.tftorhcs >= 1.7.7.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/account-iam-resources/README.md` at line 31, Update the rhcs provider version constraint in modules/account-iam-resources/versions.tf from >= 1.6.2 to >= 1.7.7 to match the root versions.tf constraint. Locate the rhcs requirement block in the module's versions.tf file and change the required_version value to >= 1.7.7 so the module aligns with the root floor version requirement.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@modules/account-iam-resources/README.md`:
- Line 31: Update the rhcs provider version constraint in
modules/account-iam-resources/versions.tf from >= 1.6.2 to >= 1.7.7 to match the
root versions.tf constraint. Locate the rhcs requirement block in the module's
versions.tf file and change the required_version value to >= 1.7.7 so the module
aligns with the root floor version requirement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 12ab0430-4afe-496f-ae3e-b75e547d981a
📒 Files selected for processing (20)
DockerfileREADME.mdexamples/ocm-role/README.mdexamples/ocm-role/main.tfexamples/ocm-role/outputs.tfexamples/ocm-role/variables.tfexamples/ocm-role/versions.tfhack/install-release-tool.shmodules/account-iam-resources/README.mdmodules/account-iam-resources/main.tfmodules/account-iam-resources/outputs.tfmodules/account-iam-resources/tests/trust_policy_external_id.tftest.hclmodules/account-iam-resources/variables.tfmodules/ocm-role/README.mdmodules/ocm-role/main.tfmodules/ocm-role/outputs.tfmodules/ocm-role/tests/ocm_role.tftest.hclmodules/ocm-role/variables.tfmodules/ocm-role/versions.tfversions.tf
✅ Files skipped from review due to trivial changes (8)
- examples/ocm-role/outputs.tf
- versions.tf
- README.md
- modules/ocm-role/outputs.tf
- Dockerfile
- examples/ocm-role/versions.tf
- examples/ocm-role/README.md
- modules/ocm-role/README.md
🚧 Files skipped from review as they are similar to previous changes (10)
- examples/ocm-role/main.tf
- examples/ocm-role/variables.tf
- modules/ocm-role/versions.tf
- modules/ocm-role/variables.tf
- modules/account-iam-resources/outputs.tf
- modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl
- modules/ocm-role/main.tf
- modules/ocm-role/tests/ocm_role.tftest.hcl
- hack/install-release-tool.sh
- modules/account-iam-resources/main.tf
michaelryanmcneill
force-pushed
the
ROSA-786
branch
from
e4e659b to
c37c41e
Compare
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/account-iam-resources/outputs.tf`:
- Around line 24-32: The outputs trust_policy_external_id and
custom_trust_policy_json contain sensitive security material including
sts:ExternalId and trust policy details that should not appear in plain text
during terraform show, logs, or debug operations. Mark both output blocks as
sensitive by adding the sensitive = true attribute to each one to comply with
security guardrails and prevent exposure of this sensitive trust-policy
information.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2fa5ff84-faa0-4787-b396-f42ef983df54
📒 Files selected for processing (5)
modules/account-iam-resources/README.mdmodules/account-iam-resources/main.tfmodules/account-iam-resources/outputs.tfmodules/account-iam-resources/tests/trust_policy_external_id.tftest.hclmodules/account-iam-resources/variables.tf
🚧 Files skipped from review as they are similar to previous changes (3)
- modules/account-iam-resources/variables.tf
- modules/account-iam-resources/tests/trust_policy_external_id.tftest.hcl
- modules/account-iam-resources/main.tf
…ources Inject optional sts:ExternalId into installer and support account role trust policies, expose module input/output, and regenerate module documentation. Part of ROSA-786. Signed-off-by: michaelryanmcneill <michael@michaelryanmcneill.com>
michaelryanmcneill
force-pushed
the
ROSA-786
branch
from
c37c41e to
fa134cb
Compare
|
/lgtm |
|
@olucasfreitas: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. DetailsIn response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: michaelryanmcneill, olucasfreitas 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 |
|
/ok-to-test |
openshift-merge-bot
Bot
merged commit 98dbbe4
into
terraform-redhat:main
2 participants
PR Summary
Adds optional
trust_policy_external_idtomodules/account-iam-resourcesso installer and support ROSA Classic account roles can require an STS external ID in their IAM trust policies.Detailed Description of the Issue
ROSA customers using STS external IDs in account role trust policies need Terraform module support to inject
sts:ExternalIdwhen creating account roles. The HCP module already had partial support; Classicaccount-iam-resourcesdid not expose this capability.The RHCS provider (
terraform-provider-rhcs) validatessts.trust_policy_external_idat cluster create against these IAM policies. Module and cluster configuration must accept the same optional value.Related Issues and PRs
sts.trust_policy_external_idon cluster resources and create-time validationType of Change
Previous Behavior
modules/account-iam-resourceshad notrust_policy_external_idvariable.sts:ExternalIdcondition.Behavior After This Change
trust_policy_external_id(defaultnull).StringEqualsonsts:ExternalIdapplied to Installer and Support roles only.trust_policy_external_id(included intime_sleeptriggers).make terraform-docs.Example wiring
How to Test (Step-by-Step)
Preconditions
Test Steps
cd modules/account-iam-resources terraform fmt terraform validatetrust_policy_external_id = "test-external-id-123".sts:ExternalIdcondition is present with the expected value.trust_policy_external_id = null; confirm no external ID condition is present.Expected Results
Proof of the Fix
sts:ExternalIdon installer/supportterraform applyoutputBreaking Changes
Breaking Change Details / Migration Plan
N/A — new optional variable with default
null. Existing module consumers are unaffected until they opt in.Developer Verification Checklist
[JIRA-TICKET] | [TYPE]: <MESSAGE>.make pre-push-checkspasses (or each step:verify,verify-gen,lint,unit-tests,license-check,docs-lint).make terraform-docs).Summary by CodeRabbit
Release Notes
New Features
trust_policy_external_idinput to configure thests:ExternalIdcondition for installer and support IAM trust policies.trust_policy_external_idvia module output.Tests
sts:ExternalIdacross role trust policies.Documentation