-
Notifications
You must be signed in to change notification settings - Fork 39
CM-826: Rebase istio-csr with upstream v0.15.0 #363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CM-826: Rebase istio-csr with upstream v0.15.0 #363
Conversation
|
@arun717: This pull request references CM-826 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 story to target the "4.22.0" version, but no target version was set. 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. |
WalkthroughThis pull request bumps cert-manager-istio-csr from v0.14.2 to v0.15.0 across build files, Kubernetes manifests, operator CSV, manager config, and embedded asset manifests. No control-flow or API declaration changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/manager/manager.yaml (1)
86-90: Update ISTIOCSR_OPERAND_IMAGE_VERSION to match the image tag.The RELATED_IMAGE_CERT_MANAGER_ISTIOCSR was updated to v0.15.0, but ISTIOCSR_OPERAND_IMAGE_VERSION is still 0.14.2. This variable is used to set the app.kubernetes.io/version label on deployed resources, creating an inconsistency between the actual deployed image version and what the operator reports. Update ISTIOCSR_OPERAND_IMAGE_VERSION to 0.15.0.
🤖 Fix all issues with AI Agents
In @bundle/manifests/cert-manager-operator.clusterserviceversion.yaml:
- Line 827: The CSV's replaces field currently points to
"cert-manager-operator.v1.18.0" which matches the current CSV version (v1.18.0)
and thus incorrectly indicates self-replacement; update the replaces value to
the previous release (e.g., "cert-manager-operator.v1.17.0") so the replaces
field refers to the prior CSV instead of the current one, ensuring the CSV's
replaces metadata is correct.
- Line 253: The olm.skipRange annotation currently reads '>=1.18.0 <1.18.0',
which is impossible to satisfy; update the value for the olm.skipRange
annotation (the key "olm.skipRange" in the manifest) to a valid range such as
'>=1.17.0 <1.18.0' or, if you intended to skip 1.18.x, '>=1.18.0 <1.19.0' so OLM
upgrade logic works correctly.
In @config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml:
- Line 21: The olm.skipRange value is logically empty (">=1.18.0 <1.18.0");
update the skipRange on the ClusterServiceVersion metadata to a valid semver
range that allows upgrades from the previous minor (e.g., change the
olm.skipRange expression to ">=1.17.0 <1.18.0") so OLM upgrade semantics work
correctly; locate the olm.skipRange entry in the cert-manager-operator CSV and
replace the existing string accordingly.
- Line 152: The replaces field currently points to the same version causing a
circular OLM reference; update the replaces value in the ClusterServiceVersion
manifest (the replaces: entry) from "cert-manager-operator.v1.18.0" to the
previous release "cert-manager-operator.v1.17.0" so the CSV correctly declares
it replaces the prior version instead of itself.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (16)
Makefilebindata/istio-csr/cert-manager-istio-csr-clusterrole.yamlbindata/istio-csr/cert-manager-istio-csr-clusterrolebinding.yamlbindata/istio-csr/cert-manager-istio-csr-deployment.yamlbindata/istio-csr/cert-manager-istio-csr-leases-role.yamlbindata/istio-csr/cert-manager-istio-csr-leases-rolebinding.yamlbindata/istio-csr/cert-manager-istio-csr-metrics-service.yamlbindata/istio-csr/cert-manager-istio-csr-role.yamlbindata/istio-csr/cert-manager-istio-csr-rolebinding.yamlbindata/istio-csr/cert-manager-istio-csr-service.yamlbindata/istio-csr/cert-manager-istio-csr-serviceaccount.yamlbindata/istio-csr/istiod-certificate.yamlbundle/manifests/cert-manager-operator.clusterserviceversion.yamlconfig/manager/manager.yamlconfig/manifests/bases/cert-manager-operator.clusterserviceversion.yamlpkg/operator/assets/bindata.go
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
Makefilebindata/istio-csr/cert-manager-istio-csr-service.yamlbindata/istio-csr/cert-manager-istio-csr-deployment.yamlbindata/istio-csr/istiod-certificate.yamlbindata/istio-csr/cert-manager-istio-csr-serviceaccount.yamlbindata/istio-csr/cert-manager-istio-csr-metrics-service.yamlbindata/istio-csr/cert-manager-istio-csr-clusterrole.yamlconfig/manifests/bases/cert-manager-operator.clusterserviceversion.yamlbindata/istio-csr/cert-manager-istio-csr-clusterrolebinding.yamlbundle/manifests/cert-manager-operator.clusterserviceversion.yamlconfig/manager/manager.yamlbindata/istio-csr/cert-manager-istio-csr-leases-rolebinding.yamlpkg/operator/assets/bindata.gobindata/istio-csr/cert-manager-istio-csr-leases-role.yamlbindata/istio-csr/cert-manager-istio-csr-role.yamlbindata/istio-csr/cert-manager-istio-csr-rolebinding.yaml
🔇 Additional comments (15)
Makefile (1)
8-8: LGTM!The version bump from v0.14.2 to v0.15.0 is correctly applied and aligns with the PR objective to rebase istio-csr to upstream v0.15.0.
bundle/manifests/cert-manager-operator.clusterserviceversion.yaml (2)
709-709: LGTM!The istio-csr image reference is correctly updated to v0.15.0, consistent with the version bump throughout the PR.
825-826: LGTM!The relatedImages entry is correctly updated to v0.15.0, maintaining consistency with the environment variable reference.
bindata/istio-csr/cert-manager-istio-csr-service.yaml (1)
10-10: LGTM!The version label is correctly updated to v0.15.0, maintaining consistency across istio-csr manifests.
bindata/istio-csr/istiod-certificate.yaml (1)
9-9: LGTM!The version label is correctly updated to v0.15.0, consistent with the istio-csr version bump across all manifests.
bindata/istio-csr/cert-manager-istio-csr-metrics-service.yaml (1)
10-10: LGTM!The version label is correctly updated to v0.15.0, maintaining consistency with other istio-csr Service manifests.
bindata/istio-csr/cert-manager-istio-csr-role.yaml (1)
7-7: LGTM! Version label correctly updated.The version label update to v0.15.0 is consistent with the upstream rebase objective.
bindata/istio-csr/cert-manager-istio-csr-clusterrolebinding.yaml (1)
7-7: LGTM! Version label correctly updated.The version label update to v0.15.0 aligns with the rebase to upstream v0.15.0.
bindata/istio-csr/cert-manager-istio-csr-leases-role.yaml (1)
7-7: LGTM! Version label correctly updated.The version label update to v0.15.0 is consistent with the upstream rebase.
bindata/istio-csr/cert-manager-istio-csr-serviceaccount.yaml (1)
7-7: LGTM! Version label correctly updated.The version label update to v0.15.0 aligns with the rebase objective.
bindata/istio-csr/cert-manager-istio-csr-leases-rolebinding.yaml (1)
9-9: LGTM! Version label correctly updated.The version label update to v0.15.0 is consistent with the upstream rebase to v0.15.0.
bindata/istio-csr/cert-manager-istio-csr-clusterrole.yaml (1)
7-7: LGTM! Version label updated correctly.The version label update from v0.14.2 to v0.15.0 aligns with the PR's objective to rebase istio-csr to upstream v0.15.0.
bindata/istio-csr/cert-manager-istio-csr-rolebinding.yaml (1)
9-9: LGTM! Version label correctly updated.The version label update is consistent with the istio-csr v0.15.0 rebase.
bindata/istio-csr/cert-manager-istio-csr-deployment.yaml (1)
9-9: LGTM! Deployment version updated consistently.The version label and container image references are all consistently updated to v0.15.0 across the Deployment metadata, pod template labels, and container image tag.
Also applies to: 22-22, 29-29
pkg/operator/assets/bindata.go (1)
2277-2277: LGTM! Consistent version updates to v0.15.0.All istio-csr resource version labels and the container image tag have been consistently updated to v0.15.0. The changes are correctly applied across all related resources (ClusterRole, ClusterRoleBinding, Deployment, Roles, RoleBindings, Services, ServiceAccount, and Certificate).
Also applies to: 2328-2328, 2364-2364, 2377-2377, 2384-2384, 2463-2463, 2509-2509, 2545-2545, 2579-2579, 2626-2626, 2662-2662, 2696-2696, 2725-2725
bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
chiragkyal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Steps followed:
Made use of the update script in this PR: #309
Modified the script to handle istio-csr updates.
The new script changes will be pushed as part of the original PR: #309
I think Istio-CSR manifest update is fairly easier task since we have make targets for them as compared to the actual rebase, and can be done independently of the proposed rebase script. Do we have a good reason to couple them?
bundle/manifests/cert-manager-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
bharath-b-rh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except for couple of nits.
config/manifests/bases/cert-manager-operator.clusterserviceversion.yaml
Outdated
Show resolved
Hide resolved
bharath-b-rh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, could you check and fix the verify CI job. Once done, this is good to be merged.
chiragkyal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash/modify the commits a bit. We can have two commits: first with the changes made manually, and next with the changes made through make targets.
|
@arun717: 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. |
|
/label tide/merge-method-squash |
lunarwhite
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/label qe-approved
|
@arun717: This pull request references CM-826 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 story to target the "4.22.0" version, but no target version was set. 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arun717, bharath-b-rh, lunarwhite 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 |
Rebase istio-csr with upstream v0.15.0
Steps followed:
Made use of the update script in this PR: #309
Modified the script to handle istio-csr updates.
The new script changes will be pushed as part of the original PR: #309