Repository navigation
[oadp-1.6] OADP-8056: create Secret from inline CACert and use CACertRef for BSL cert rotation - #2464
Conversation
… rotation Velero upstream added CACertRef (Secret-based CA cert reference) to replace inline CACert, so a BSL's CA bundle can be rotated by updating the Secret instead of requiring the BSL object itself to change. OADP's CloudStorage-backed BSL paths only accepted inline CACert bytes on the DPA. When provided, create/own a Secret containing those bytes and set CACertRef on the resulting Velero BSL instead, giving users cert rotation support without needing to know about CACertRef. An explicit CACertRef in the DPA is passed through unchanged so users can manage their own Secret. Also updates processCACertForBSLs (the separate AWS_CA_BUNDLE ConfigMap aggregation for the Velero deployment) to resolve CACertRef in addition to inline CACert, since BSLs built through the paths above no longer carry inline CACert bytes. Fixes OADP-8056 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…ret test golangci-lint (revive) flagged TestDPAReconciler_reconcileCACertSecret's local `scheme` variable for shadowing the imported k8s.io/client-go kubernetes/scheme package, failing ci/prow/unit-test on PR openshift#2454. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…CertRef resolution errors Addresses CodeRabbit findings on this PR: - reconcileCACertSecret now refuses to create, update, or delete a Secret at the generated name unless it already carries OADP's own label (i.e. OADP created it), instead of blindly overwriting or deleting whatever Secret happens to occupy that name. - Switching a BSL from inline CACert to an explicit CACertRef now cleans up the Secret OADP previously generated for it, instead of leaving it orphaned. - resolveCACertBytes (used by processCACertForBSLs' AWS_CA_BUNDLE aggregation) now returns an error instead of silently returning nil bytes when a required (non-optional) CACertRef can't be resolved, so a broken reference surfaces as a reconcile error rather than a BSL silently missing its CA cert. Errors for DPA-spec'd BSLs propagate and fail reconciliation; errors for extra BSLs found in-cluster but not in the DPA spec are logged and skipped instead, since one unrelated BSL's broken CACertRef shouldn't block the DPA's own ConfigMap update. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…nsumers CodeRabbit flagged that when a DPA both sets inline CACert and CACertRef (the CRD doesn't reject the combination), reconcileCACertSecret prefers CACertRef for the Velero BSL's ObjectStorage, but resolveCACertBytes preferred inline CACert for the separate AWS_CA_BUNDLE ConfigMap aggregation -- the two consumers of the same DPA fields could end up trusting different CA material. resolveCACertBytes now prefers CACertRef when set, matching reconcileCACertSecret's own precedence, falling back to inline CACert only when CACertRef is nil. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Matches Velero upstream's own deprecation of BackupStorageLocation's inline CACert field in favor of CACertRef (velero-io/velero#9141), so the CRD schema description now steers users toward CACertRef for new configs, since it supports cert rotation via a Secret update instead of requiring the BSL/DPA to change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…BSL coverage - lib.DpaCustomResource gains BSLCacertRef, wired into BackupStorageLocationSpec() alongside the existing BSLCacert, so e2e tests can build a Velero-typed BSL with either. - cacert_suite_test.go: the minio-backed BSL scenario (Available + AWS_CA_BUNDLE + real backup/delete over the custom-CA TLS connection) now runs twice via a shared helper -- once with the legacy inline CACert (kept for backward-compat coverage) and once, as the new default, with CACertRef pointing at a user-managed Secret. The "no CACert" negative case is unchanged. - New cacert_cloudstorage_suite_test.go covers OADP-8056 itself: for a CloudStorage-backed BackupLocation with inline caCert, OADP must create an owned Secret and set caCertRef on the resulting Velero BSL instead of copying the CA bytes inline, and must delete that Secret again once the inline CACert is removed. Uses the suite's existing real cloud bucket/credentials; only asserts reconciliation and the resulting BSL/Secret state, since pointing an unrelated test CA at the real bucket's TLS endpoint isn't expected to succeed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
The CRD requires spec.configuration.nodeAgent.uploaderType (enum restic/kopia); the hand-built spec in buildCloudStorageDpaSpec omitted it and the API server rejected the DPA with a 422. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
DPA validation requires a non-empty prefix on CloudStorage-backed BackupLocations when backupImages is enabled (default); without it the DPA never reaches Reconciled=True and the suite times out. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Verified live against a real cluster (PR#2454 installed via install-oadp-from-pr): ReconcileBackupStorageLocations requires at least one default BSL, so the single CloudStorage-backed BackupLocation in this suite must set Default: true, or DPA reconcile fails with "no default backupstoragelocations configured" and the test times out waiting for IsReconciledTrue(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@openshift-cherrypick-robot: Ignoring requests to cherry-pick non-bug issues: OADP-8056 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. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
openshift-ci
Bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: openshift-cherrypick-robot, shubham-pampattiwar, sseago 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 |
|
level=error msg=failed to fetch Cluster: failed to generate asset "Cluster": failed to create cluster: infrastructure is not ready: client rate limiter Wait returned an error: rate: Wait(n=1) would exceed context deadline |
|
@openshift-cherrypick-robot: 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. |
5 participants
This is an automated cherry-pick of #2454
/assign kaovilai