WIP: etcd backups - #2383
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bhperry 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 |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (57)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough
ChangesOpenShift module replacements
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@go.mod`:
- Around line 5-8: Remove the github.com/openshift/api and
github.com/openshift/client-go replace directives from go.mod. Update their
required versions to compatible upstream releases where available, ensuring the
module builds against upstream OpenShift APIs without relying on private fork
replacements.
- Around line 5-8: Before merging the replacements for github.com/openshift/api
and github.com/openshift/client-go, document the fork provenance and validate
both modules for license compatibility, CVE/OSV advisories, integrity,
SBOM/provenance attestations, and Sigstore/cosign signatures; only retain the
replacements for production use once these checks pass.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 54c9d946-bf7d-4378-8d9a-987169776548
⛔ Files ignored due to path filters (61)
go.sumis excluded by!**/*.sumvendor/github.com/openshift/api/config/v1/types_infrastructure.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/types_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/features.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/features/features.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/register.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/types_etcdbackup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/types_etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.model_name.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/vendor/**,!vendor/**,!**/zz_generated*vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/backupspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/etcdbackupspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retentionnumberconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retentionpolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/retentionsizeconfig.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/applyconfigurations/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/fake/fake_backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/fake/fake_config_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1alpha1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1alpha1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/informers/externalversions/generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/backup.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/config/listers/config/v1alpha1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/backupjobreference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupjob.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackuppolicyretentionrule.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackuppolicyspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackuppolicystatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupspec.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupstatus.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupstorage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupstoragelocal.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1alpha1/etcdbackupstoragepvc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/applyconfigurations/utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1alpha1/etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1alpha1/fake/fake_etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1alpha1/fake/fake_operator_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1alpha1/generated_expansion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1alpha1/operator_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/informers/externalversions/generic.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1/etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1/interface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/listers/operator/v1alpha1/etcdbackuppolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/openshift/client-go/operator/listers/operator/v1alpha1/expansion_generated.gois excluded by!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (1)
go.mod
| replace github.com/openshift/api => github.com/bhperry/openshift-api v0.0.0-20260727162810-8887fbaa588a | ||
|
|
||
| replace github.com/openshift/client-go => github.com/bhperry/openshift-client-go v0.0.0-20260727173133-6958b40cc63d | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cat >"$tmp/go.mod" <<EOF
module downstream.test
go 1.26
require github.com/openshift/library-go v0.0.0
replace github.com/openshift/library-go => $(pwd)
EOF
cat >"$tmp/main.go" <<'EOF'
package main
import _ "github.com/openshift/library-go/pkg/operator/status"
func main() {}
EOF
(cd "$tmp" && go list -deps ./...)Repository: openshift/library-go
Length of output: 215
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== go.mod relevant lines =="
sed -n '1,18p' go.mod
echo
echo "== imports of replaced dependencies in tracked Go files =="
rg -n 'github.com/(openshift/api|openshift/client-go)' -g '*.go' | head -80
echo
echo "== changed files stat =="
git diff --stat || true
echo
echo "== deterministic module graph probe =="
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cat >"$tmp/go.mod" <<'EOF'
module upstream.test
go 1.20
require github.com/openshift/api v0.0.0 testmodule
EOF
cat >"$tmp/main.go" <<'EOF'
package main
import (
_ "github.com/openshift/api/image"
)
func main() {
}
EOF
echo "local import with invalid upstream module:"
set +e
(cd "$tmp" && go list -deps ./...)
local_status=$?
set -e
echo "exit=$local_status"
cat >"$tmp/go.mod" <<EOF
module main.test
go 1.20
require github.com/openshift/library-go v0.0.0 testmodule
replace github.com/openshift/library-go => $(pwd)
replace github.com/openshift/api => github.com/openshift/api v0.0.0 localmodule
EOF
cat >"$tmp/main.go" <<'EOF'
package main
import _ "github.com/openshift/library-go/pkg/controller/controllercmd"
func main() {
}
EOF
echo
echo "local main.go import with local replace on main module dependency:"
(cd "$tmp" && go list -deps ./...)
main_status=$?
echo "exit=$main_status"Repository: openshift/library-go
Length of output: 1155
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== go.mod relevant lines =="
sed -n '1,28p' go.mod
echo
echo "== imports of replaced dependencies in tracked Go files =="
rg -n 'github.com/(openshift/api|openshift/client-go)' -g 'go.mod' -g '*.go' | sed 's#^`#/`#github/openshift/library-go/#' | awk -F: '{print substr($0, RSTART+1, RLENGTH-RSTART-1) FS $2 ":" $3}' | head -120
echo
echo "== package declarations at each import file =="
python3 - <<'PY'
import pathlib, re
for p in pathlib.Path('go.mod','').glob('**/*.go'):
txt=p.read_text(errors='ignore')
if 'github.com/openshift/api' in txt or 'github.com/openshift/client-go' in txt:
pkg=re.search(r'(?m)^\s*package\s+(\w+)', txt)
print(f"{p} package={pkg.group(1) if pkg else '<none>'}")
PY
echo
echo "== Go replace behavior probe =="
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
mkdir -p "$tmp/libpkg"
cat >"$tmp/libpkg/go.mod" <<'EOF'
module example.test/libpkg
go 1.20
require example.test/mainmodule v1.0.0
replace example.test/mainmodule => $PWD/../main
EOF
cat >"$tmp/libpkg/main.go" <<'EOF'
package libpkg
import _ "example.test/mainmodule/pkg"
EOF
mkdir -p "$tmp/main/pkg"
cat >"$tmp/main/go.mod" <<'EOF'
module example.test/mainmodule
go 1.20
replace example.test/mainmodule => example.test/mainmodule v9.9.9
EOF
cat >"$tmp/main/pkg/pkg.go" <<'EOF'
package pkg
EOF
echo "downstream requiring the module containing a replace directive on itself:"
set +e
(cd "$tmp/libpkg" && go list -deps ./... 2>&1)
status=$?
set -e
echo "exit=$status"Repository: openshift/library-go
Length of output: 1516
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== go.mod relevant lines =="
sed -n '1,35p' go.mod
echo
echo "== tracked imports of replaced dependencies =="
python3 - <<'PY'
import pathlib,re
for p in pathlib.Path('.').rglob('*.go'):
if any(part.startswith('.') or part == 'vendor' for part in p.parts):
continue
txt=p.read_text(errors='ignore')
if 'github.com/openshift/api' in txt or 'github.com/openshift/client-go' in txt:
pkg=re.search(r'(?m)^\s*package\s+(\w+)', txt)
print(f"{p} package={pkg.group(1) if pkg else '<none>'}")
deps=re.findall(r'github\.com/openshift/(api|client-go)[^"\n]+', txt)
print(" deps=", deps)
PY
echo
echo "== Go replace directive non-inheritance probe =="
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
mkdir -p "$tmp/downstream" "$tmp/libpkg" "$tmp/main/pkg"
cat >"$tmp/libpkg/go.mod" <<'EOF'
module example.dev/libpkg
go 1.20
require example.dev/mainmodule v1.0.0
EOF
cat >"$tmp/libpkg/main.go" <<'EOF'
package libpkg
import _ "example.dev/mainmodule/pkg"
EOF
cat >"$tmp/main/go.mod" <<'EOF'
module example.dev/mainmodule
go 1.20
replace example.dev/mainmodule => example.dev/mainmodule v9.9.9
EOF
mkdir -p "$tmp/main/pkg"
echo 'package pkg' >"$tmp/main/pkg/pkg.go"
echo "Building downstream whose dep's go.mod contains a replace:"
set +e
(cd "$tmp/downstream" && go list -deps ./... 2>&1)
status=$?
set -e
echo "exit=$status"Repository: openshift/library-go
Length of output: 27275
Remove forked library replacements from go.mod.
replace directives are private to the module where they’re declared, so downstream consumers of github.com/openshift/library-go resolve the upstream OpenShift modules directly. Keeping these replacements here can mask real API changes but won’t be inherited by downstream module graphs.
Use upstream module versions when they’re available, or move forked APIs to packageable code that downstream consumers can rely on explicitly.
🤖 Prompt for 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.
In `@go.mod` around lines 5 - 8, Remove the github.com/openshift/api and
github.com/openshift/client-go replace directives from go.mod. Update their
required versions to compatible upstream releases where available, ensuring the
module builds against upstream OpenShift APIs without relying on private fork
replacements.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
go mod verify
go list -m -json all |
jq -s '.[] | select(.Path == "github.com/openshift/api" or
.Path == "github.com/openshift/client-go") |
{Path, Version, Replace}'
for module in github.com/bhperry/openshift-api github.com/bhperry/openshift-client-go; do
curl -fsSL https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"ecosystem\":\"Go\",\"name\":\"$module\"}}" |
jq --arg module "$module" '{module: $module, vulnerabilities: [.vulns[]?.id]}'
doneRepository: openshift/library-go
Length of output: 274
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "go.mod replacement lines:"
sed -n '1,80p' go.mod | nl -ba | sed -n '1,40p'
echo
echo "Module metadata without vendor:"
GOWORK=off go env -json | jq '{GOENV: .GOENV}' || true
GOWORK=off go list -mod=readonly -m -json all |
jq -s '.[] | select(.Path == "github.com/openshift/api" or .Path == "github.com/openshift/client-go") |
{ModulePath: .Path, Version: .Version, Replace: .Replace}'
echo
echo "OSV queries for forked modules:"
for module in github.com/bhperry/openshift-api github.com/bhperry/openshift-client-go; do
echo "--- $module"
curl -fsSL https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"ecosystem\":\"Go\",\"name\":\"$module\"}}" |
jq --arg module "$module" '{module: $module, vuln_count: (.vulns|length), vuln_ids: [.vulns[]??.id]} ' || true
done
echo
echo "Repository references:"
rg -n "github\.com/(bhperry|openshift)/(api|client-go)|replace github\.com/openshift/(api|client-go)" -S --glob '!vendor/**' .Repository: openshift/library-go
Length of output: 224
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "go.mod replacement lines:"
sed -n '1,80p' go.mod | awk '{printf "%6d %s\n", NR, $0}' | sed -n '1,40p'
echo
echo "Module metadata without vendor:"
GOWORK=off go list -mod=readonly -m -json all |
awk '
/^\{/ { p=$0; mod=0; repl="" }
/"Path":/ { match($0,/[^"]*$/); gsub(/[^"]*"/,"",p); mod=substr(p,2) }
/"Replace":/ { repl=$0 }
/"Version":/ {
match($0,/"Version":"([^"]*)"/,a)
if (mod == "github.com/openshift/api" || mod == "github.com/openshift/client-go")
printf "%s --- version: %s replace: %s\n", mod, a[1], repl
}
'
echo
echo "OSV queries for forked modules:"
for module in github.com/bhperry/openshift-api github.com/bhperry/openshift-client-go; do
echo "--- $module"
curl -fsSL https://api.osv.dev/v1/query \
-H 'content-type: application/json' \
-d "{\"package\":{\"ecosystem\":\"Go\",\"name\":\"$module\"}}" |
jq --arg module "$module" '{module: $module, vuln_count: (.vulns|length), vuln_ids: [.vulns[]??.id]}' || true
done
echo
echo "Repository references:"
rg -n "github\.com/(bhperry|openshift)/(api|client-go)|replace github\.com/openshift/(api|client-go)" -S --glob '!vendor/**' .Repository: openshift/library-go
Length of output: 50376
Document and validate the forked module replacements before merging.
The forks satisfy the pseudo-version requirement, but supply-chain validation must still cover license compatibility, CVE/OSV advisories, module integrity, SBOM/provenance attestations, and Sigstore/cosign signing before these personal-fork replacements are used in a production build.
🤖 Prompt for 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.
In `@go.mod` around lines 5 - 8, Before merging the replacements for
github.com/openshift/api and github.com/openshift/client-go, document the fork
provenance and validate both modules for license compatibility, CVE/OSV
advisories, integrity, SBOM/provenance attestations, and Sigstore/cosign
signatures; only retain the replacements for production use once these checks
pass.
Source: Path instructions
831e7ba to
c7f9667
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Summary by CodeRabbit