Repository navigation
oadp-1.5: OADP-6765: feat(bsl): concatenate all CA certificates from BSLs and include system defaults - #1969
Merged
Conversation
…em defaults Instead of using "first one wins" approach, now collects and concatenates all unique CA certificates from BackupStorageLocations. Also includes system default CA certificates when custom certificates are present. Changes: - Modified processCACertForBSLs() to collect all unique CA certificates - Added deduplication logic to avoid including same certificate multiple times - Added getSystemCACertificates() helper to retrieve system CA bundles - System defaults are only included when custom CAs are present - Updated tests to verify concatenation and deduplication behavior This allows for more flexible multi-cloud/multi-endpoint configurations where different BSLs may require different CA certificates. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… conflicts with Velero management Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
Skipping CI for Draft Pull Request. |
|
@kaovilai: This pull request references OADP-6765 which is a valid jira issue. 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. |
kaovilai
marked this pull request as ready for review
September 29, 2025 12:30
openshift-ci
Bot
added
approved
Member
Author
|
feedback from officehours:
|
Contributor
|
…d tests for validation Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
kaovilai
force-pushed
the
OADP-6765-oadp-1.5
branch
from
September 29, 2025 16:04
9acd4cd to
dfacfd4
Compare
kaovilai
commented
Sep 29, 2025
Comment on lines
+1043
to
+1049
| caPaths := []string{ | ||
| "/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu | ||
| "/etc/pki/tls/certs/ca-bundle.crt", // RHEL/CentOS/Fedora | ||
| "/etc/ssl/ca-bundle.pem", // OpenSSL | ||
| "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // RHEL 7+ | ||
| "/etc/ssl/cert.pem", // Alpine/OpenSSL | ||
| } |
Member
Author
There was a problem hiding this comment.
"/etc/pki/tls/certs/ca-bundle.crt"
works for wes oadp controller manager. softlinked to "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem"
We can consider just this single path if it has all we need for normal aws operations.
Contributor
|
shubham-pampattiwar
previously approved these changes
Sep 29, 2025
sseago
previously approved these changes
Sep 29, 2025
kaovilai
dismissed stale reviews from sseago and shubham-pampattiwar
via
September 29, 2025 18:24
964b4ca
shubham-pampattiwar
previously approved these changes
Sep 29, 2025
sseago
previously approved these changes
Sep 29, 2025
Member
Author
|
/retest |
Contributor
|
tested w/ multiple apps, mysql-persistent and parks app w/ image streams etc. |
Member
Author
one test case failed. checking if flake |
kaovilai
force-pushed
the
OADP-6765-oadp-1.5
branch
from
September 29, 2025 21:44
964b4ca to
831815c
Compare
|
New changes are detected. LGTM label has been removed. |
Contributor
|
@kaovilai hrm.. |
…e with e2e Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
…e test files Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Member
Author
|
The added e2e cases is fixed. |
shubham-pampattiwar
approved these changes
Sep 30, 2025
|
@kaovilai: 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. |
Contributor
|
testing this again.. @kaovilai thanks for having it lined up and passing. |
Member
Author
|
I didn't change the single thing about the controller code since your last test so I expect same results |
Contributor
|
ran through various tests again w/ multiple bsl and certs |
kaovilai
commented
Sep 30, 2025
Member
Author
There was a problem hiding this comment.
Consider cert rotation, configmap rollout/updates to velero deployment.
kubernetes/kubernetes#22368
says to relink deployment to new configmap to propagate cert change
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, shubham-pampattiwar, weshayutin 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 |
Member
Author
|
/cherry-pick oadp-dev |
Contributor
|
@kaovilai: new pull request created: #1972 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 kubernetes-sigs/prow repository. |
Contributor
|
/cherry-pick oadp-1.4 |
Contributor
|
@sseago: #1969 failed to apply on top of branch "oadp-1.4": 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 kubernetes-sigs/prow repository. |
redhat-chai-bot
added a commit
to redhat-chai-bot/oadp-operator
that referenced
this pull request
Sep 23, 2026
Restore gofmt import ordering in controllers/bsl_test.go. The cherry-pick of openshift#1969 left github.com/stretchr/testify/assert out of gofmt import order, which makes `make test` fail at the fmt-isupdated gate (CI runs `make test submit-coverage`). Combined with the DPA/CloudStorage namespace alignment already on this branch, TestProcessCACertForBSLs now passes and `make test` is green (unit tests, fmt, api, and bundle checks all pass). Signed-off-by: Chai Bot <chai-bot@redhat.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
9 tasks
openshift-merge-bot Bot
pushed a commit
that referenced
this pull request
Oct 5, 2026
…BSLs and include system defaults (#1969) * feat(bsl): concatenate all CA certificates from BSLs and include system defaults Instead of using "first one wins" approach, now collects and concatenates all unique CA certificates from BackupStorageLocations. Also includes system default CA certificates when custom certificates are present. Changes: - Modified processCACertForBSLs() to collect all unique CA certificates - Added deduplication logic to avoid including same certificate multiple times - Added getSystemCACertificates() helper to retrieve system CA bundles - System defaults are only included when custom CAs are present - Updated tests to verify concatenation and deduplication behavior This allows for more flexible multi-cloud/multi-endpoint configurations where different BSLs may require different CA certificates. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * feat(bsl): ensure BSL reconciliation preserves default field to avoid conflicts with Velero management Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> * feat(bsl): enhance CA certificate processing for multiple BSLs and add tests for validation Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> * PEM verify + `podman run -v `pwd`:`pwd` -w `pwd` quay.io/konveyor/builder:ubi9-v1.23 sh -c "make lint-fix"` Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> * refactor(nginx): reorganize deployment YAML structure to be compatible with e2e Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> * feat(e2e): add CA certificate handling for default e2e BSL in multiple test files Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> --------- Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com> Co-authored-by: Claude <noreply@anthropic.com> (cherry picked from commit dcead30)
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
6 participants
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.
Why the changes were made
How to test the changes made