Update Openshift version supported range#80
Open
shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
Open
Update Openshift version supported range#80shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
shreyabiradar07 wants to merge 4 commits intokruize:mvp_demofrom
Conversation
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAligns the operator’s declared platform support with newer Kubernetes and OpenShift versions by bumping the minimum Kubernetes version, updating OpenShift version metadata, and simplifying RBAC permissions. Flow diagram for updated permission granter RBAC behaviorflowchart TD
A["KruizeOperator"] --> B["OperatorServiceAccount"]
B --> C["PermissionGranterRole"]
C --> D["Access replicasets"]
C --> E["Access daemonsets"]
C --> F["Access /metrics nonResourceURL"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The README now states "OpenShift v4.13+" while the bundle annotation constrains support to "v4.13-v4.21"; consider aligning the phrasing so users don't assume support beyond 4.21.
- Given the description notes underlying Kubernetes versions 1.26–1.34 for the new OpenShift range, double-check whether
minKubeVersionshould remain at 1.25.0 or be raised to 1.26.0 to match the effective platform baseline.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The README now states "OpenShift v4.13+" while the bundle annotation constrains support to "v4.13-v4.21"; consider aligning the phrasing so users don't assume support beyond 4.21.
- Given the description notes underlying Kubernetes versions 1.26–1.34 for the new OpenShift range, double-check whether `minKubeVersion` should remain at 1.25.0 or be raised to 1.26.0 to match the effective platform baseline.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In both the RBAC role.yaml and the kubebuilder RBAC markers, the apiGroup for CronJobs should remain
batch(notbatch/v1); API version goes in the resource manifest’sapiVersion, while RBACapiGroupsshould be the group name only. - The PR description mentions raising the minimum Kubernetes version to v1.25.0, but the manifests and README now require v1.26.0+; consider aligning the stated minimum version to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both the RBAC role.yaml and the kubebuilder RBAC markers, the apiGroup for CronJobs should remain `batch` (not `batch/v1`); API version goes in the resource manifest’s `apiVersion`, while RBAC `apiGroups` should be the group name only.
- The PR description mentions raising the minimum Kubernetes version to v1.25.0, but the manifests and README now require v1.26.0+; consider aligning the stated minimum version to avoid confusion.
## Individual Comments
### Comment 1
<location path="config/rbac/role.yaml" line_range="187" />
<code_context>
- - watch
- apiGroups:
- - batch
+ - batch/v1
resources:
- - jobs
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `batch/v1` in `apiGroups` is likely invalid; `apiGroups` should be the group without the version (e.g. `batch`).
In RBAC, `apiGroups` must be just the group name (e.g. `batch`), not `group/version`. The version belongs in the resource’s `apiVersion`, not in the RBAC rule. Using `batch/v1` here will likely break authorization for CronJobs; keep `apiGroups: ["batch"]` while using `batch/v1` for the CronJob API itself.
</issue_to_address>
### Comment 2
<location path="internal/controller/kruize_controller.go" line_range="71" />
<code_context>
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create
//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;create
-//+kubebuilder:rbac:groups=batch,resources=cronjobs,verbs=get;list;watch;create
+//+kubebuilder:rbac:groups=batch/v1,resources=cronjobs,verbs=get;list;watch;create
//+kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list
//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles,verbs=get;list;watch;create
</code_context>
<issue_to_address>
**issue (bug_risk):** Kubebuilder RBAC `groups` value should normally be the API group without version (e.g. `batch`, not `batch/v1`).
Using `batch/v1` here is likely incorrect for kubebuilder RBAC markers. `groups` should match the API group only (e.g. `batch`), not the versioned group, otherwise the generated RBAC may not work. Please keep this as `groups=batch` even though CronJob is now served from `batch/v1`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Shreya Biradar <shbirada@ibm.com>
Contributor
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since the minimum supported Kubernetes/OpenShift versions have been raised, double-check any remaining version references in deployment scripts or Helm charts (if present) to keep user-facing compatibility signals consistent with these manifest changes.
- With the removal of
horizontalpodautoscalersfrom the permission_granter role, verify that no code paths in the operator still attempt to access HPAs; if they do, consider scoping that access into a dedicated role instead of reintroducing broad permissions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since the minimum supported Kubernetes/OpenShift versions have been raised, double-check any remaining version references in deployment scripts or Helm charts (if present) to keep user-facing compatibility signals consistent with these manifest changes.
- With the removal of `horizontalpodautoscalers` from the permission_granter role, verify that no code paths in the operator still attempt to access HPAs; if they do, consider scoping that access into a dedicated role instead of reintroducing broad permissions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR updates the Operator's supported OpenShift version range from
v4.13 to v4.21as Red Hat currently maintains 9 active indices for the community catalog.Changes
v4.13-v4.21inbundle/metadata/annotations.yaml.v1.26.0+because the new OpenShift support range (v4.13–v4.21) is built on Kubernetes versions 1.26 through 1.34horizontalpodautoscaler(HPA) permissionSummary by Sourcery
Update documented and declared platform compatibility for the operator to align with newer Kubernetes and OpenShift versions.
Enhancements:
Documentation:
Summary by Sourcery
Update platform compatibility to require Kubernetes v1.26+ and OpenShift v4.13–v4.21 while aligning RBAC and metadata with newer Kubernetes APIs.
Enhancements:
Documentation:
Summary by Sourcery
Update documented and declared platform compatibility to require newer Kubernetes and OpenShift versions and simplify RBAC permissions.
Enhancements:
Documentation: