Skip to content

OCPBUGS-65885: added 1.34 required ELBv2 perms to CCM role#7339

Merged
openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
mtulio:OCPBUGS-65885
Jan 21, 2026
Merged

OCPBUGS-65885: added 1.34 required ELBv2 perms to CCM role#7339
openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
mtulio:OCPBUGS-65885

Conversation

@mtulio
Copy link
Copy Markdown
Contributor

@mtulio mtulio commented Dec 4, 2025

What this PR does / why we need it:

Added permission to IAM managed policy for control plane controllers affecting a specific case where it is required to reconcile the NLB target group with non-default attributes..

The new permissions was added after upstream PR https://github.com/kubernetes/cloud-provider-aws/pull/1214.

The ROSA managed policy is asked to update in https://issues.redhat.com/browse/SREP-2895

Jira Bug card Jira https://issues.redhat.com/browse/OCPBUGS-65885

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-65885

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.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2025
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Dec 4, 2025

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
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 4, 2025

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds two ELBV2 delegation methods to the AWS client wrapper, excludes those APIs from ELB delegation configuration in the client generator, and adds corresponding IAM actions to the kubeControllerPolicy and docs.

Changes

Cohort / File(s) Summary
ELBV2 Delegation Methods
cmd/infra/aws/delegating_client.go
Adds DescribeTargetGroupAttributesWithContext and ModifyTargetGroupAttributesWithContext methods on elbv2Client that delegate to cloudController.elbv2Client.
Client Generator Configuration
cmd/infra/aws/delegatingclientgenerator/main.go
Removes DescribeTargetGroupAttributes and ModifyTargetGroupAttributes from the ELB API set in adjustAPIs, excluding them from generated ELB delegation mappings.
IAM Policy
cmd/infra/aws/iam.go
Adds elasticloadbalancing:DescribeTargetGroupAttributes and elasticloadbalancing:ModifyTargetGroupAttributes to kubeControllerPolicy Actions and inserts a ROSA inline role policy permitting the same for the CCM.
Documentation
docs/content/reference/infrastructure/aws.md
Updates the KubeCloudControllerARN IAM policy snippet to include the two new ELB actions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Verify method signatures and correct forwarding of aws.Context, input types, and request.Option in delegating_client.go.
  • Confirm adjustAPIs exclusion is consistent with other ELB API groups in delegatingclientgenerator/main.go.
  • Check IAM action strings in cmd/infra/aws/iam.go match those added to the docs and that the ROSA inline policy is attached to the correct role.

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

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/test ?

@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Dec 4, 2025
@openshift-ci-robot
Copy link
Copy Markdown

@mtulio: This pull request references Jira Issue OCPBUGS-65885, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (jiezhao@redhat.com), skipping review request.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

What this PR does / why we need it:

Added permission to IAM managed policy for control plane controllers affecting a specific case where it is required to reconcile the NLB target group.

The behavior is added after the fix on kubernetes/cloud-provider-aws#1214

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
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Dec 4, 2025

@mtulio: The following commands are available to trigger required jobs:

/test e2e-aks
/test e2e-aks-4-20
/test e2e-aks-override
/test e2e-aws
/test e2e-aws-4-20
/test e2e-aws-override
/test e2e-aws-upgrade-hypershift-operator
/test e2e-kubevirt-aws-ovn-reduced
/test images
/test okd-scos-images
/test security
/test unit
/test verify
/test verify-deps

The following commands are available to trigger optional jobs:

/test e2e-aws-autonode
/test e2e-aws-metrics
/test e2e-aws-minimal
/test e2e-aws-techpreview
/test e2e-azure-aks-ovn-conformance
/test e2e-conformance
/test e2e-kubevirt-aws-ovn
/test e2e-kubevirt-azure-ovn
/test e2e-kubevirt-metal-conformance
/test e2e-openstack-aws
/test e2e-openstack-aws-conformance
/test e2e-openstack-aws-csi-cinder
/test e2e-openstack-aws-csi-manila
/test e2e-openstack-aws-nfv
/test reqserving-e2e-aws

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-hypershift-main-e2e-aks
pull-ci-openshift-hypershift-main-e2e-aks-4-20
pull-ci-openshift-hypershift-main-e2e-aws
pull-ci-openshift-hypershift-main-e2e-aws-upgrade-hypershift-operator
pull-ci-openshift-hypershift-main-e2e-kubevirt-aws-ovn-reduced
pull-ci-openshift-hypershift-main-images
pull-ci-openshift-hypershift-main-okd-scos-images
pull-ci-openshift-hypershift-main-security
pull-ci-openshift-hypershift-main-unit
pull-ci-openshift-hypershift-main-verify
Details

In response to this:

/test ?

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 openshift-ci bot added area/cli Indicates the PR includes changes for CLI area/platform/aws PR/issue for AWS (AWSPlatform) platform and removed do-not-merge/needs-area labels Dec 4, 2025
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/origin/main/e2e-conformance openshift/origin/pull/30525

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Dec 4, 2025

@mtulio, testwith: Error processing request. ERROR:

could not determine job runs: invalid format for additional PR: openshift/origin/pull/30525

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/test e2e-aws unit verify image security verify-deps e2e-aws

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/origin/main/e2e-conformance openshift/origin#30525

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Dec 4, 2025

@mtulio, testwith: could not generate prow job. ERROR:

BUG: test 'e2e-conformance' not found in injected config

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/hypershift/main/e2e-conformance openshift/origin#30525

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/hypershift/main/e2e-conformance openshift/origin#30525

PR rebased, will retry in following comment

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/hypershift/main/e2e-conformance openshift/origin#30525

@mtulio mtulio force-pushed the OCPBUGS-65885 branch 2 times, most recently from 47242e5 to 63ee5d0 Compare December 4, 2025 21:37
@mtulio mtulio changed the title OCPBUGS-65885: added elbv2:DescribeTargetGroupAttributes to managed controller OCPBUGS-65885: added missing ELBv2 permissions to control plane managed role Dec 4, 2025
@mtulio mtulio changed the title OCPBUGS-65885: added missing ELBv2 permissions to control plane managed role OCPBUGS-65885: added 1.34 required ELBv2 perms to CCM role Dec 4, 2025
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/testwith openshift/hypershift/main/e2e-conformance openshift/origin#30525

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 4, 2025

/test e2e-aws unit verify image security verify-deps e2e-aws

@mtulio mtulio changed the title OCPBUGS-65885: added 1.34 required ELBv2 perms to CCM role DNM/OCPBUGS-65885: added 1.34 required ELBv2 perms to CCM role Dec 5, 2025
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 9, 2025

/test verify

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Dec 9, 2025

/test unit verify-deps

@mtulio mtulio marked this pull request as draft January 20, 2026 18:45
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 20, 2026
@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Jan 20, 2026
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2026
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 20, 2026

/test e2e-aws

@mtulio mtulio force-pushed the OCPBUGS-65885 branch 2 times, most recently from a92e9c2 to 668afd7 Compare January 20, 2026 21:45
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 20, 2026

I found an very weird condition when appending inline policies when the role is the same (shared) : there is kind of race condition skipping the first entry (ingress policy for route53:ChangeResourceRecordSets), leading to CIO to be degraded failing the related e2e.

I realized the put is running in 70ms of difference, but the AWS API is eventually consistent. The approach taken in the last commit was to check if roles are the same and merge the conditionals/permissions, preserving the original name, otherwise it will create two inline policies when using ROSA managed Policies (UseROSAManagedPolicies).

Re-testing the two tests that was failing caused by this condition:

/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator

@mtulio mtulio marked this pull request as ready for review January 20, 2026 23:49
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 20, 2026
@openshift-ci openshift-ci bot requested review from bryan-cox and csrwng January 20, 2026 23:49
This fix[1] include updates in the policy cloud-controller used by
KubeCloudControllerARN required by upstream CCM and exposed by OTE
when enabled in the OCP payload, exercised in hypershift through job
presubmit e2e-conformance (requires OTE binary[2]).

There is also a hotfix on ROSA managed policy used on CI while managed policies is
updated by AWS[3].

The policy ensure CCM AWS have enough permissions to deliver the feautre
to HCP - already added in upstream[2].

[1] https://issues.redhat.com/browse/OCPBUGS-65885
[2] https://issues.redhat.com/browse/OCPSTRAT-2743
openshift/origin#30525
`https://github.com/kubernetes/cloud-provider-aws/pull/1214`
[3] https://issues.redhat.com/browse/SREP-2895
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

Fixing verify as I missed to update the docs.

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

/test e2e-aks

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

Hi @bryan-cox and @csrwng , would you mind taking a look at this new approach to update the ROSA managed roles with inline policies? Thanks

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

tests are now passing

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2026
@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

verified by @mtulio #7339 (comment) #7339 (comment)

@mtulio
Copy link
Copy Markdown
Contributor Author

mtulio commented Jan 21, 2026

/verified by @mtulio #7339 (comment) #7339 (comment)

@openshift-ci-robot
Copy link
Copy Markdown

@mtulio: This PR has been marked as verified by @mtulio https://github.com/openshift/hypershift/pull/7339#issuecomment-3774375691 https://github.com/openshift/hypershift/pull/7339#issuecomment-3775118981.

Details

In response to this:

/verified by @mtulio #7339 (comment) #7339 (comment)

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.

@jcpowermac
Copy link
Copy Markdown

/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Jan 21, 2026

@mtulio: 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.

@openshift-ci-robot
Copy link
Copy Markdown

@mtulio: Jira Issue OCPBUGS-65885: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-65885 has been moved to the MODIFIED state.

Details

In response to this:

What this PR does / why we need it:

Added permission to IAM managed policy for control plane controllers affecting a specific case where it is required to reconcile the NLB target group with non-default attributes..

The new permissions was added after upstream PR https://github.com/kubernetes/cloud-provider-aws/pull/1214.

The ROSA managed policy is asked to update in https://issues.redhat.com/browse/SREP-2895

Jira Bug card Jira https://issues.redhat.com/browse/OCPBUGS-65885

Which issue(s) this PR fixes:

Fixes https://issues.redhat.com/browse/OCPBUGS-65885

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-cherrypick-robot
Copy link
Copy Markdown

@mtulio: #7339 failed to apply on top of branch "release-4.21":

Applying: fix(ccm): added required permissions to CCM policy
Applying: docs(aws): required permissions on CCM to NLB TargetGroup
Using index info to reconstruct a base tree...
A	docs/content/reference/aggregated-docs.md
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): docs/content/reference/aggregated-docs.md deleted in HEAD and modified in docs(aws): required permissions on CCM to NLB TargetGroup. Version docs(aws): required permissions on CCM to NLB TargetGroup of docs/content/reference/aggregated-docs.md left in tree.
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 0002 docs(aws): required permissions on CCM to NLB TargetGroup

Details

In response to this:

/cherry-pick release-4.21

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-merge-robot
Copy link
Copy Markdown
Contributor

Fix included in accepted release 4.22.0-0.nightly-2026-01-22-122134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cli Indicates the PR includes changes for CLI area/documentation Indicates the PR includes changes for documentation area/platform/aws PR/issue for AWS (AWSPlatform) platform jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.