OCPBUGS-98329: Add aggregated cluster role for extended per-provider RBAC#623
OCPBUGS-98329: Add aggregated cluster role for extended per-provider RBAC#623mdbooth wants to merge 5 commits into
Conversation
Boxcutter expects the rest mapper to be available, and will panic if it is not.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@mdbooth: This pull request references Jira Issue OCPBUGS-98329, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe installer now receives provider permissions through an aggregated ClusterRole and binding. Its tracking cache uses the controller-manager REST mapper, while controller tests increase logging verbosity and refine VAP synchronization setup. ChangesInstaller permissions and tracking
Controller test setup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/pipeline auto |
|
Pipeline controller notification The |
|
Scheduling tests matching the |
|
/testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
|
Failed due to a typo in the featuregate name /testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
|
/testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
1 similar comment
|
/testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
|
For anybody keeping count: these are all infra capacity issues. Prow has not yet managed to execute this job at all. It has not failed. /testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
|
/testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
|
/retest-required |
|
/test e2e-aws-capi-disconnected-techpreview e2e-metal3-capi-techpreview |
|
/jira refresh |
|
@mdbooth: This pull request references Jira Issue OCPBUGS-98329, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@mdbooth: This pull request references Jira Issue OCPBUGS-98329, which is valid. 3 validation(s) were run on this bug
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 openshift-eng/jira-lifecycle-plugin repository. |
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 `@docs/provider-contract.md`:
- Around line 17-27: Update the “Namespaced to openshift-capi-installer” heading
and its namespace reference in the provider contract documentation to use
openshift-cluster-api, matching the RBAC contract and manifests-gen behavior.
Leave the listed resources and aggregated ClusterRole guidance unchanged.
🪄 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: 1975620e-7073-4210-aaa8-6b5b61e59af2
📒 Files selected for processing (4)
docs/provider-contract.mdmanifests/0000_30_cluster-api-operator_03_capi-installer-clusterrole.yamlmanifests/0000_30_cluster-api-operator_04_capi-installer-clusterrolebinding.yamlpkg/controllers/installer/installer_controller.go
|
Looks like another flake. I'll investigate it separately. /test unit |
|
/testwith openshift/cluster-capi-operator/main/e2e-metal3-capi-techpreview openshift/cluster-api-provider-metal3#86 |
At level 0 we get almost no logs.
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 `@pkg/controllers/machinesync/machine_sync_controller_test.go`:
- Around line 1487-1488: Insert a blank line between the test machine
construction statements and the following By(...) calls in the affected test
cases, including the sentinel CAPI machine and MAPI machine setup blocks, to
satisfy wsl_v5 whitespace requirements.
🪄 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: 04b8f212-fe73-4766-a130-c6c89b0ca663
📒 Files selected for processing (7)
pkg/controllers/installer/revision_reconciler_test.gopkg/controllers/installer/suite_test.gopkg/controllers/machinemigration/suite_test.gopkg/controllers/machinesetmigration/suite_test.gopkg/controllers/machinesetsync/suite_test.gopkg/controllers/machinesync/machine_sync_controller_test.gopkg/controllers/machinesync/suite_test.go
|
Scheduling tests matching the |
Adds
openshift-capi-installer:provider-permissionsClusterRole and binds it to capi-installer. This is an aggregated cluster rule which uses the labelcapi-operator.openshift.io/aggregate-to-installer. If a provider's manifests include any object which the default CAPIO RBAC does not cover they can specify their additional requirements in their own repo as a separate ClusterRole which aggregates to this one.Also fixes a bug initialising boxcutter's tracking cache which turns missing RBAC into a panic.
Summary by CodeRabbit