Merge https://github.com/openshift/oadp-operator:oadp-dev (7e53a85) into oadp-dev#2114
Merge https://github.com/openshift/oadp-operator:oadp-dev (7e53a85) into oadp-dev#2114oadp-rebasebot-app[bot] wants to merge 3 commits intoopenshift:oadp-devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds two optional string fields ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @oadp-rebasebot-app[bot]. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/hold |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml (1)
742-752:⚠️ Potential issue | 🟡 MinorUpdate the
podResourcesdescription to include ephemeral storage.The description still says this block only covers CPU and memory, which is no longer true after adding the new fields.
Proposed fix
- description: PodResources is the config for the CPU and memory resources setting. + description: PodResources is the config for the CPU, memory, and ephemeral storage resource settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml` around lines 742 - 752, The podResources schema description is outdated (mentions only CPU and memory) — update the description for the podResources (PodResources) object to include ephemeral storage as well so it reflects new fields like ephemeralStorageLimit and ephemeralStorageRequest; locate the description key under podResources in the CRD and change its text to mention CPU, memory and ephemeral storage, keeping surrounding property names cpuLimit, cpuRequest, ephemeralStorageLimit and ephemeralStorageRequest intact.
🧹 Nitpick comments (2)
bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml (1)
742-756: Consider updating the description to include ephemeral storage.The
podResourcesdescription on line 743 states "PodResources is the config for the CPU and memory resources setting" but now also includes ephemeral storage fields. Consider updating the description to reflect the expanded scope.📝 Suggested description update
podResources: - description: PodResources is the config for the CPU and memory resources setting. + description: PodResources is the config for the CPU, memory, and ephemeral storage resources setting. properties:Note: This would need to be changed in the Go API types, then regenerated via
make generate manifests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml` around lines 742 - 756, Update the podResources description to mention ephemeral storage as well as CPU and memory: locate the podResources schema (property name "podResources") and change the description text to something like "PodResources is the config for CPU, memory, and ephemeral storage resource settings" and then update the corresponding Go API type (the struct/field that defines PodResources) to use the same text and run the manifest generation (make generate manifests) so the YAML is regenerated with the new description; ensure fields like ephemeralStorageLimit and ephemeralStorageRequest remain documented.config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml (1)
649-652: Document the new ephemeral storage fields.These are public CRD fields now, but without descriptions users get no guidance in
oc explainor generated docs about expected quantity format or how they map to pod requests/limits. Please add source-field comments so the generated schema includes descriptions here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml` around lines 649 - 652, The CRD is missing user-facing descriptions for ephemeralStorageLimit and ephemeralStorageRequest; add source-field comments on the corresponding API struct fields (EphemeralStorageLimit, EphemeralStorageRequest) so the generated OpenAPI schema includes descriptions that explain the expected quantity format (e.g., "100Mi", "1Gi", "500K") and how these values map to pod resources (they populate container resources.requests[ephemeral-storage] and resources.limits[ephemeral-storage]). Include a brief example and note accepted units and that values follow Kubernetes resource.Quantity syntax so oc explain and generated docs surface this guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml`:
- Around line 742-752: The podResources schema description is outdated (mentions
only CPU and memory) — update the description for the podResources
(PodResources) object to include ephemeral storage as well so it reflects new
fields like ephemeralStorageLimit and ephemeralStorageRequest; locate the
description key under podResources in the CRD and change its text to mention
CPU, memory and ephemeral storage, keeping surrounding property names cpuLimit,
cpuRequest, ephemeralStorageLimit and ephemeralStorageRequest intact.
---
Nitpick comments:
In `@bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml`:
- Around line 742-756: Update the podResources description to mention ephemeral
storage as well as CPU and memory: locate the podResources schema (property name
"podResources") and change the description text to something like "PodResources
is the config for CPU, memory, and ephemeral storage resource settings" and then
update the corresponding Go API type (the struct/field that defines
PodResources) to use the same text and run the manifest generation (make
generate manifests) so the YAML is regenerated with the new description; ensure
fields like ephemeralStorageLimit and ephemeralStorageRequest remain documented.
In `@config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml`:
- Around line 649-652: The CRD is missing user-facing descriptions for
ephemeralStorageLimit and ephemeralStorageRequest; add source-field comments on
the corresponding API struct fields (EphemeralStorageLimit,
EphemeralStorageRequest) so the generated OpenAPI schema includes descriptions
that explain the expected quantity format (e.g., "100Mi", "1Gi", "500K") and how
these values map to pod resources (they populate container
resources.requests[ephemeral-storage] and resources.limits[ephemeral-storage]).
Include a brief example and note accepted units and that values follow
Kubernetes resource.Quantity syntax so oc explain and generated docs surface
this guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43a8c898-971d-4f2a-88e6-be0edaeeb3d6
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
bundle/manifests/oadp.openshift.io_dataprotectionapplications.yamlconfig/crd/bases/oadp.openshift.io_dataprotectionapplications.yamlgo.mod
288a893 to
4073616
Compare
4073616 to
379c903
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oadp-rebasebot-app[bot] 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oadp-rebasebot-app[bot] 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 |
379c903 to
87d940a
Compare
87d940a to
1b3d013
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 173: The go.mod entry pins the vulnerable module
go.opentelemetry.io/otel/sdk at v1.42.0; update that module requirement to
v1.43.0 or later (replace the version string for go.opentelemetry.io/otel/sdk)
and then run the Go tooling (e.g., go get go.opentelemetry.io/otel/sdk@v1.43.0
&& go mod tidy) to refresh go.sum; finally run the test/build to ensure no
breakages.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0141758f-304a-4747-b6f5-0c6e00b01299
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
bundle/manifests/oadp.openshift.io_dataprotectionapplications.yamlbundle/manifests/velero.io_backuprepositories.yamlconfig/crd/bases/oadp.openshift.io_dataprotectionapplications.yamlconfig/crd/bases/velero.io_backuprepositories.yamlgo.mod
✅ Files skipped from review due to trivial changes (2)
- bundle/manifests/velero.io_backuprepositories.yaml
- config/crd/bases/velero.io_backuprepositories.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
| go.opentelemetry.io/otel/trace v1.38.0 // indirect | ||
| go.opentelemetry.io/otel v1.42.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.42.0 // indirect | ||
| go.opentelemetry.io/otel/sdk v1.42.0 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Querying OSV for go.opentelemetry.io/otel/sdk@v1.42.0 ..."
curl -s https://api.osv.dev/v1/query \
-H 'Content-Type: application/json' \
-d '{"package":{"ecosystem":"Go","name":"go.opentelemetry.io/otel/sdk"},"version":"v1.42.0"}' \
| jq -r '
.vulns[]? as $v
| "id=\($v.id) aliases=\(($v.aliases // [])|join(",")) summary=\($v.summary // "n/a")",
($v.affected[]?.ranges[]?.events[]? | select(has("fixed")) | "fixed=\(.fixed)")
'
echo "Confirming pinned version in go.mod ..."
rg -n 'go\.opentelemetry\.io/otel/sdk v1\.42\.0' go.modRepository: openshift/oadp-operator
Length of output: 367
Fix HIGH security advisory in otel/sdk v1.42.0.
Line 173 pins go.opentelemetry.io/otel/sdk v1.42.0, which contains GHSA-hfvc-g4fc-pqhx (CVE-2026-39883)—a PATH hijacking vulnerability via unqualified BSD kenv command. Upgrade to v1.43.0 or later before merge.
🧰 Tools
🪛 OSV Scanner (2.3.5)
[HIGH] 173-173: go.opentelemetry.io/otel/sdk 1.42.0: opentelemetry-go: BSD kenv command not using absolute path enables PATH hijacking
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 173, The go.mod entry pins the vulnerable module
go.opentelemetry.io/otel/sdk at v1.42.0; update that module requirement to
v1.43.0 or later (replace the version string for go.opentelemetry.io/otel/sdk)
and then run the Go tooling (e.g., go get go.opentelemetry.io/otel/sdk@v1.43.0
&& go mod tidy) to refresh go.sum; finally run the test/build to ensure no
breakages.
Summary by CodeRabbit
New Features