fix(RELEASE-2158): use trusted-ca mounts to support self hosted Quay#360
fix(RELEASE-2158): use trusted-ca mounts to support self hosted Quay#360querti wants to merge 1 commit intokonflux-ci:mainfrom
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
querti
force-pushed
the
add-trusted-ca-support
branch
from
2583420 to
6829795
Compare
Review Summary by QodoAdd trusted-ca mounts for self-hosted Quay support
WalkthroughsDescription• Add trusted-ca ConfigMap support for custom CA certificates • Mount CA bundle to standard system certificate paths • Enable self-hosted Quay instances with custom CA support • Make CA configuration optional with sensible defaults Diagramflowchart LR
params["Task Parameters"]
params -- "caTrustConfigMapName" --> configmap["ConfigMap Volume"]
params -- "caTrustConfigMapKey" --> configmap
configmap -- "ca-bundle.crt" --> mount1["/etc/pki/tls/certs/"]
configmap -- "ca-bundle.crt" --> mount2["/etc/ssl/certs/"]
mount1 --> gotools["Go-based Tools<br/>cosign, mobster"]
mount2 --> gotools
File Changes1. tasks/augment-component-sboms-ta/0.3/augment-component-sboms-ta.yaml
|
Code Review by Qodo
1. Optional subPath mount fails
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| - name: trusted-ca | ||
| configMap: | ||
| name: $(params.caTrustConfigMapName) | ||
| items: | ||
| - key: $(params.caTrustConfigMapKey) | ||
| path: ca-bundle.crt | ||
| optional: true | ||
|
|
||
| stepTemplate: | ||
| volumeMounts: | ||
| - mountPath: /var/workdir | ||
| name: workdir | ||
| - name: trusted-ca | ||
| mountPath: /etc/pki/tls/certs/ca-bundle.crt | ||
| subPath: ca-bundle.crt | ||
| readOnly: true | ||
| - name: trusted-ca | ||
| mountPath: /etc/ssl/certs/ca-custom-bundle.crt | ||
| subPath: ca-bundle.crt | ||
| readOnly: true |
There was a problem hiding this comment.
1. Optional subpath mount fails 🐞 Bug ☼ Reliability
The trusted-ca ConfigMap volume is marked optional: true, but it is always mounted via subPath into /etc/...; if the ConfigMap or key is missing, the subPath file won’t exist and the Pod can fail to start before any step executes.
Agent Prompt
### Issue description
The Task marks the `trusted-ca` ConfigMap volume as `optional: true`, but mounts a specific file from it using `subPath`. If the ConfigMap (or key) is absent, the `subPath` file won’t exist, and the Pod may fail to start before executing any steps.
### Issue Context
The intent appears to be: “use trusted-ca when present, otherwise proceed normally.” The current approach does not reliably allow that.
### Fix Focus Areas
- tasks/augment-component-sboms-ta/0.3/augment-component-sboms-ta.yaml[185-204]
### Suggested fix approaches (pick one)
1) **If the ConfigMap is required in practice:** remove `optional: true` so failures are explicit and consistent.
2) **If it must be optional:** avoid `subPath` mounts for the optional file. For example:
- Mount the ConfigMap volume to a neutral directory (e.g. `/var/run/trusted-ca/`) without `subPath`.
- In each step (or via a shared script wrapper), conditionally detect `/var/run/trusted-ca/ca-bundle.crt` and either:
- set `SSL_CERT_FILE` / tool-specific CA env var to point to it, or
- append/copy it into the system trust store (only if you have permissions and won’t clobber system CAs).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
querti
force-pushed
the
add-trusted-ca-support
branch
from
6829795 to
0ee0191
Compare
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #360 +/- ##
=======================================
Coverage 95.54% 95.54%
=======================================
Files 57 57
Lines 3863 3863
=======================================
Hits 3691 3691
Misses 172 172
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
querti
force-pushed
the
add-trusted-ca-support
branch
from
0ee0191 to
aa18caa
Compare
Add trusted-ca volume mounts to system certificate paths to be consistent with other tasks in the push-to-external-registry pipeline. Go-based tools (cosign, mobster) require the CA bundle at /etc/ssl/certs/ to include it in the system cert pool. This is needed for push-to-external-registry to work with self-hosted Quay instances that use custom CA certificates. Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
querti
force-pushed
the
add-trusted-ca-support
branch
from
aa18caa to
78ad47e
Compare
|
/retest |
|
Is the jira RELEASE-2158 correctly assigned to this? I see the Jira talks about Quay |
I see the relation now.. please ignore |
| - mountPath: /var/workdir | ||
| name: workdir | ||
| - name: trusted-ca | ||
| mountPath: /etc/pki/tls/certs/ca-bundle.crt |
There was a problem hiding this comment.
I encountered some problems with this setup on fresh clusters without the configmap. See our Slack messages.
There was a problem hiding this comment.
This issue has been filed as RELEASE-2383. This PR is blocked until the right approach for mounting is established in RELEASE-2383.
There was a problem hiding this comment.
I don't understand why this PR is blocked on anything. In the build-pipelines and tasks we use custom CA bundles for long time and it works perfectly fine. We can just replicate the way it's done in build definition.
Note that depend on the task, the built in trust store should be updated, of the tools that need the custom CA bundle, need to get a reference to the path it was mounted in.
You can this how it was implemented in the git-clone task https://github.com/konflux-ci/build-definitions/blob/main/task/git-clone/0.1/git-clone.yaml
There was a problem hiding this comment.
You are suggesting that we explicitly pass the cert to every tool that needs it, right? That is definitely a viable solution, but if we commit to that path, we would need to do this for every task in push-to-external-registry pipeline. Perhaps there is also an alternative solution where we can mount the cert without risk and we wouldn't need to make such extensive changes to all of our tasks. I'm not saying what the right solution should be, but I think it should be established and implemented as a part of RELEASE-2383, not in this PR. I will then apply the chosen approach here, whatever it may be.
There was a problem hiding this comment.
I'm saying that it depends on the task.
-
You can mount it to any location, and the update the global ca certs of the systemlike done in the build tasks - https://github.com/konflux-ci/build-definitions/blob/b5248cc849ac74735b36b4ba45ab950fce28854c/task/buildah-oci-ta/0.9/buildah-oci-ta.yaml#L510
-
You can mount it to a known location used by a particular programming language (for example
/etc/ssl/certs/when the tool is written in go) -
You can mount it to any location you want, and expose the path using an environment variable (like some of the Python libraries support).
The important thing to keep in mind:
-
The configmap with the trusted ca should be optional, that is the task pod can start and work properly if it doesn't exist.
-
We should be careful with overriding existing certificates builtinto the image if some tools need them.
There was a problem hiding this comment.
Based on your comment I'm not sure if we are in agreement or not. There seem to be multiple approaches to solve this and we should establish the best one by weighing pros and cons. The pattern of usage in this task is repeated across many tasks in push-to-external-registry, so once we establish the right approach , we can use it here as well. My point is that this PR shouldn't be the place where this is established.
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
|
Closing in favor of #363 |
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
Fix remaining incompatibilities in the push-to-external-registry pipeline when used with self-hosted Quay instances: - make-repo-public: detect Quay registries via /api/v1/discovery API instead of hardcoding quay.io. Enforce single Quay registry limit per release due to how the API token is passed. Add custom CA bundle support for self-signed certificates. - collect-signing-params, collect-tpa-params: mount trusted CA to system cert paths, consistent with other tasks in the pipeline. The mobster augment-component-sboms task is updated with trusted-ca mounts in a companion PR [1]. Self-hosted Quay e2e test improvements: - Enable SBOM tasks (TPA and signing) by adding required secrets to the vault and configuring the Conforma public key. - Add verification that released repository is made public. Other changes: - Remove duplicated diagnoseFailedPLR function in wait-for-release.sh, use shared diagnose_failed_pipelinerun from test-functions.sh. [1] konflux-ci/mobster#360 Assisted-by: Cursor Signed-off-by: Lubomir Gallovic <lgallovi@redhat.com>
5 participants
Add trusted-ca volume mounts to system certificate paths to be consistent with other tasks in the push-to-external-registry pipeline. Go-based tools (cosign, mobster) require the CA bundle at /etc/ssl/certs/ to include it in the system cert pool. This is needed for push-to-external-registry to work with self-hosted Quay instances that use custom CA certificates.
Assisted-by: Cursor