SPLAT-2743: aws/ccm - remove inline iam policy patch

[mtulio]

What this PR does / why we need it:

This PR removes inline policies added when the managed policies were not updated, now AWS has published the policy we don't need those permissions anymore. References:

• Managed policy update request: https://redhat.atlassian.net/browse/ROSAENG-21795
• AWS Managed Policies: https://docs.aws.amazon.com/aws-managed-policy/latest/reference/ROSAKubeControllerPolicy.html
• This PR is blocked by CI issues reported in PR SPLAT-2741: Add SetSecurityGroups perms for AWS CCM for BYO SG on AWS NLB #8401 (nice to have there merged first)

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

Summary by CodeRabbit

• Bug Fixes
  • Updated AWS IAM permissions used during cluster setup for Cloud Controller Manager to better match current requirements, removing now-unneeded permissions related to target group attribute access and modification.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

[openshift-ci-robot]

@mtulio: This pull request references SPLAT-2743 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Special notes for your reviewer:

Checklist:

• Subject and description added to both, commit and PR.
• Relevant issues have been referenced.
• This change includes docs.
• This change includes unit tests.

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.

[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

[mtulio]

/test ?

[coderabbitai]

📝 Walkthrough

Walkthrough

CreateOIDCResources in cmd/infra/aws/iam.go now defines the CCM inline IAM statement with an empty Action list instead of the previous ELB TargetGroupAttributes permissions, and the adjacent comment text was updated.

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PR only changes an IAM policy comment/document in cmd/infra/aws/iam.go; no Ginkgo test titles were added or modified.
Test Structure And Quality	✅ Passed	Only cmd/infra/aws/iam.go changed; no Ginkgo test files or test code were modified, so the test-structure check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Diff only changes AWS IAM and unrelated controller/refactor code; no new anti-affinity, topology spread, nodeSelector, or PDB constraints were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the PR only edits cmd/infra/aws/iam.go IAM policy text, so IPv4/disconnected-network concerns don’t apply.
No-Weak-Crypto	✅ Passed	Changed code is IAM policy JSON only; grep found no weak-crypto APIs/literals or secret comparisons in cmd/infra/aws/iam.go.
Container-Privileges	✅ Passed	PR only changes IAM policy logic in cmd/infra/aws/iam.go; no container/K8s manifest privilege flags (privileged, hostNetwork, allowPrivilegeEscalation, etc.) are present.
No-Sensitive-Data-In-Logs	✅ Passed	No new log statements or sensitive fields were added; the change only empties an IAM action list and removes issue/Slack refs from a comment.
Title check	✅ Passed	The title clearly matches the main change: removing the inline IAM policy patch for aws/ccm.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[mtulio]

/test e2e-conformance e2e-aws-ovn-conformance-techpreview

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mtulio
Once this PR has been reviewed and has the lgtm label, please assign csrwng 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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

938-987: 🎯 Functional Correctness | 🟠 Major

Remove the CCM inline policy entirely. Action: [] is not a valid IAM statement, so PutRolePolicy will reject this document; the shared-role path still writes the same invalid policy, so this patch needs to drop the CCM branch instead of emitting an empty statement.

🤖 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 `@cmd/infra/aws/iam.go` around lines 938 - 987, The CCM inline policy in the
role policy setup is invalid because it uses an empty Action list, so the
current CCM branch in iam.go will be rejected by IAM. Remove the CCM policy
creation path entirely from the shared-role and separate-role handling around
the ccmPolicyStatement / ccmRoleName logic, and keep only the ingress role
policy updates via PutRolePolicy. Make sure any references to the CCM policy
document are deleted so no empty-statement policy is emitted.

🤖 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 `@cmd/infra/aws/iam.go`:
- Around line 938-987: The CCM inline policy in the role policy setup is invalid
because it uses an empty Action list, so the current CCM branch in iam.go will
be rejected by IAM. Remove the CCM policy creation path entirely from the
shared-role and separate-role handling around the ccmPolicyStatement /
ccmRoleName logic, and keep only the ingress role policy updates via
PutRolePolicy. Make sure any references to the CCM policy document are deleted
so no empty-statement policy is emitted.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 4a06ce93-7fc9-48bf-877c-eec094f9d472

📥 Commits

Reviewing files that changed from the base of the PR and between 02675db and 3f8f85d.

📒 Files selected for processing (1)

• cmd/infra/aws/iam.go

[codecov]

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 43.01%. Comparing base (02675db) to head (518ffaf).
⚠️ Report is 34 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/infra/aws/iam.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8835      +/-   ##
==========================================
- Coverage   43.01%   43.01%   -0.01%     
==========================================
  Files         766      766              
  Lines       94769    94746      -23     
==========================================
- Hits        40765    40751      -14     
+ Misses      51185    51176       -9     
  Partials     2819     2819              

Files with missing lines	Coverage Δ
cmd/infra/aws/iam.go	29.25% <0.00%> (+0.33%)	⬆️

... and 1 file with indirect coverage changes

Flag	Coverage Δ
cmd-support	36.43% <0.00%> (+0.01%)	⬆️
cpo-hostedcontrolplane	45.01% <ø> (ø)
cpo-other	45.00% <ø> (ø)
hypershift-operator	53.11% <ø> (-0.03%)	⬇️
other	31.69% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mtulio]

this PR is waiting for updates on #8401

/hold

[hypershift-jira-solve-ci]

I now have the complete picture. Here is the analysis:

Test Failure Analysis Complete

Job Information

• Prow Job: codecov/patch and codecov/project (GitHub Check Runs, not Prow CI)
• Build ID: Check Run IDs 84089179611 (patch) and 84089176059 (project)
• PR: SPLAT-2743: aws/ccm - remove inline iam policy patch #8835 — SPLAT-2743: aws/ccm - remove inline iam policy patch
• Changed File: cmd/infra/aws/iam.go (+1 / -10 lines)

Test Failure Analysis

Error

codecov/patch: 0.00% of diff hit (target 43.01%) — 1 line in changes missing coverage
codecov/project: 43.01% (-0.01%) compared to base 02675db

Summary

Both Codecov check failures are caused by a single uncovered line in the PR diff. The PR modifies cmd/infra/aws/iam.go to empty the IAM Action array in the CCM inline policy statement (removing elasticloadbalancing:DescribeTargetGroupAttributes and elasticloadbalancing:ModifyTargetGroupAttributes). The change modifies a string literal assignment (ccmPolicyStatement := ...) that is the only new/changed executable line. Since the CreateOIDCResources function containing this line has no unit test exercising it, the single changed line has 0% patch coverage. This drops overall project coverage by 0.01% (from 43.02% to 43.01%), triggering the project check failure as well. The file cmd/infra/aws/iam.go has only 29.25% coverage overall.

Root Cause

The root cause is lack of unit test coverage for the modified code path, not a product bug or test regression.

The PR changes one executable line — the ccmPolicyStatement string literal assignment at line ~944 of cmd/infra/aws/iam.go inside the CreateOIDCResources method. This method interacts with real AWS IAM APIs and has no unit tests covering it (the file is only 29.25% covered overall). Since the diff produces exactly 1 new/modified executable line and 0 lines are covered by tests, Codecov reports 0% patch coverage.

codecov/patch failure: The patch coverage target is 43.01% (matching project baseline). The diff achieves 0%, failing the threshold.

codecov/project failure: The PR removes 10 lines (some covered) and adds 1 uncovered line. Net effect: 23 fewer lines, 14 fewer hits, 9 fewer misses → overall coverage drops by 0.01% from 43.02% to 43.01%. The project check fails because coverage decreased relative to the base commit 02675db.

This is a cosmetic/configuration-level failure. The change is a trivial policy string modification (emptying an Action array), and the function it lives in requires AWS API mocking to test — it has never been covered by unit tests.

Recommendations

1. These failures are safe to bypass for this PR. The change is a 1-line string literal modification in an already-uncovered function. It does not introduce new logic, branches, or error handling that would benefit from testing.

2. To resolve codecov/patch: Either (a) add a unit test for CreateOIDCResources that mocks the AWS IAM client (the function accepts an awsapi.IAMClient interface, so mocking is feasible), or (b) have a maintainer with merge permissions override/dismiss the Codecov checks if they are not required status checks.

3. To resolve codecov/project: The -0.01% drop is within noise. If the project check is a required status check, the same unit test from recommendation 2 would resolve it, or an admin can dismiss it.

4. Long-term: Consider adding cmd/infra/aws/iam.go to the ignore list in codecov.yml if the team does not intend to unit-test AWS infrastructure provisioning code, or increase the coverage threshold tolerance to avoid blocking trivial changes.

Evidence

Evidence	Detail
Failing checks	codecov/patch (0.00% patch coverage, target 43.01%) and codecov/project (43.01%, -0.01% vs base)
Changed file	cmd/infra/aws/iam.go — 1 addition, 10 deletions
Uncovered line	ccmPolicyStatement := \{ ... "Action": [], ... }`` (string literal assignment, line ~944)
File coverage	cmd/infra/aws/iam.go at 29.25% overall (+0.33% from this PR due to line removal)
Coverage diff	Files: 766→766, Lines: 94769→94746 (-23), Hits: 40765→40751 (-14), Misses: 51185→51176 (-9)
Base commit	02675db (main branch, report is 34 commits behind head)
Nature of change	Removes two ELB IAM actions from CCM inline policy, empties the Action array — purely a policy string change
No test exists	CreateOIDCResources has no unit test — it calls real AWS IAM APIs via interface but no mock tests exist

---