refactor: Consolidate CA trust setup into import-additional-cas#2991
refactor: Consolidate CA trust setup into import-additional-cas#2991davdhacs wants to merge 2 commits intokonflux-ubi9-rhel9-migrationfrom
Conversation
|
@davdhacs: The following test failed, say
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. |
davdhacs
force-pushed
the
refactor-ca-trust-scripts
branch
from
0a688a4 to
17e5bdb
Compare
|
|
||
| update-ca-trust extract | ||
| # Copy the StackRox root CA if available (mounted by the operator). | ||
| copy_existing /run/secrets/stackrox.io/certs |
There was a problem hiding this comment.
@mclasmeier what do you think about moving scanner's "root ca" copy into here to more-closely match the main/sensor setup?
There was a problem hiding this comment.
You mean, as it is currently being done in this PR here, right?
Seems reasonable to me. Of course we shouldn't be over-engineering this, but I have been wondering if it would be a a good idea to generalize copy_existing() slightly, as in:
if $1 is a directory {
current behaviour of this function
} else if $1 is a file {
// Expect $1 to be a directly referenced CA pem file
// case for the stackrox CA pem
} else {
warn
}
This way we would consolidate the copying in a single function and could just invoke it with different arguments.
WDYT? Just an idea, definitely not a must from my side.
There was a problem hiding this comment.
That would be better for re-using the function, but I'm thinking to keep the old file-missing behavior makes it simpler with only the cp line.
Merge trust-root-ca into import-additional-cas to align with how stackrox/stackrox handles CA trust (single script, single update-ca-trust call). This eliminates a redundant update-ca-trust invocation and simplifies the entrypoint. Changes: - Add copy_single() to import-additional-cas for the StackRox root CA at /run/secrets/stackrox.io/certs/ca.pem - Remove trust-root-ca script and its references in entrypoint.sh and create-bundle.sh - update-ca-trust extract --output is now called once instead of twice Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
davdhacs
force-pushed
the
refactor-ca-trust-scripts
branch
from
17e5bdb to
6700798
Compare
| echo "Copying StackRox root CA from '$CA_PATH'" | ||
| cp -v -L "$CA_PATH" /etc/pki/ca-trust/source/anchors/ | ||
| else | ||
| echo "No StackRox root CA found at $CA_PATH" |
There was a problem hiding this comment.
Is this an expected situation? Wondering if we should do something like
echo >&2 "WARNING: No StackRox root CA found at '$CA_PATH'"
There was a problem hiding this comment.
I don't know. I'll change it to match the existing behavior (always copy). Then the error will stay the same as what it would be now when it isn't mounted.
There was a problem hiding this comment.
I moved the lines from the trust-root-ca script so that the behavior stays the same and we reduce to one update-ca-trust call instead of the two that it had before.
Are you okay with this? I think matching prior behavior for this removes any concern of whether the failure case is expected or not.
2 participants
Description
Consolidate the CA trust setup to align with how stackrox/stackrox handles it (see stackrox/stackrox#19454).
The scanner had a separate
trust-root-cascript that copied the StackRox root CA and calledupdate-ca-trusta second time. The stackrox main/sensor images don't have this — they handle everything in a singleimport-additional-casscript with oneupdate-ca-trustcall.Changes
trust-root-cascript into theimport-additional-casscript before update-ca-trust is calledupdate-ca-trust extract --outputis now called once instead of twiceBenefits
update-ca-trustinvocationChecklist
Testing Performed
CI (especially slim-e2e-tests and e2e-tests) will validate that the scanner starts correctly with the consolidated CA trust setup.