Inject AWS IAM SetSecurityGroups perm on upgrades to 4.23 - #82862
Conversation
|
Skipping CI for Draft Pull Request. |
|
/test all |
WalkthroughThe upgrade detection script now extracts source and target minor versions. It permits patching from 4.x versions through 4.22 to 4.23 or 5.x. Other upgrade combinations remain skipped with updated messages. ChangesUpgrade eligibility detection
Estimated code review effort: 2 (Simple) | ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@ci-operator/step-registry/openshift/upgrade/aws/patch-master-iam/openshift-upgrade-aws-patch-master-iam-commands.sh`:
- Around line 20-27: Update the upgrade guard in the patch-master-IAM command
flow to prevent downgrades from 4.24+ to 4.23 from entering the 4.23 path. Use
the existing source and target version components to require a non-decreasing
minor version for same-major 4.x upgrades, while preserving the existing
4.x-to-5.x behavior and skip message.
- Around line 22-27: The IAM policy granting
elasticloadbalancing:SetSecurityGroups must stop using Resource "*" and be
restricted to the upgrade cluster’s managed load balancer ARNs, or an equivalent
condition targeting only that cluster’s load balancers. Update the policy
configuration in the patch-master-IAM flow while preserving the existing
upgrade-version gating and required action.
🪄 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: Pro Plus
Run ID: 76b1bf5c-523f-4e87-b573-bda063103416
📒 Files selected for processing (1)
ci-operator/step-registry/openshift/upgrade/aws/patch-master-iam/openshift-upgrade-aws-patch-master-iam-commands.sh
|
/pj-rehearse periodic-ci-openshift-release-main-ci-4.23-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade |
|
@mfbonfigli: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Fix to make sure to inject the missing elasticloadbalancing:SetSecurityGroups permission also on cluster upgrades from 4.x to 4.23 as the new feature requiring the permission is present on 5.0/4.23.
61ec5fc to
f0f0516
Compare
|
/test all |
|
[REHEARSALNOTIFIER]
A total of 1636 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-release-main-ci-4.23-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade |
|
@mfbonfigli: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-ci-5.0-upgrade-from-stable-4.22-e2e-aws-ovn-upgrade |
|
@mfbonfigli: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mfbonfigli, mtulio, vr4manta 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 |
|
@mfbonfigli: all tests passed! 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. |
|
/pj-rehearse ack |
|
@mfbonfigli: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
…82862) Fix to make sure to inject the missing elasticloadbalancing:SetSecurityGroups permission also on cluster upgrades from 4.x to 4.23 as the new feature requiring the permission is present on 5.0/4.23.
…82862) Fix to make sure to inject the missing elasticloadbalancing:SetSecurityGroups permission also on cluster upgrades from 4.x to 4.23 as the new feature requiring the permission is present on 5.0/4.23.
Context
PR #81974 added the openshift-upgrade-aws-patch-master-iam step to work around OCPBUGS-98763: the OpenShift installer didn't include
elasticloadbalancing:SetSecurityGroupsin the master IAM role policy until 5.0.Clusters installed before that and upgraded to a release whose CCM expects the permission hit AccessDenied when calling SetSecurityGroups on NLBs, since the master role is never updated during an in-place upgrade.
That fix only patched the role for 4.x → 5.x upgrades, but some jobs upgrade the cluster from 4.x → 4.23 which is effectively equivalent to 5.0, so they hit the same AccessDenied failure.
Change
This PR extends the version check in
openshift-upgrade-aws-patch-master-iam-commands.shso the inline policy is also applied when the target version is 4.23.*, in addition to the existing 4.x → 5.x case. No job/workflow wiring changes are needed since the step is already wired into the shared AWS upgrade workflows and self-gates based on source/target version.Summary by CodeRabbit
The AWS upgrade IAM patch in OpenShift CI now applies
elasticloadbalancing:SetSecurityGroupswhen upgrading from 4.x through 4.22 to 4.23.* or 5.x. This preventsAccessDeniedfailures for older clusters that lack the permission in the master IAM role policy.