Fall back to Rancher-configured CA bundles #3233
Open
Conversation
This file contains 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
weyfonk
force-pushed
the
2750-custom-ca-fallback
branch
from
13c1226 to
90bbe29
Compare
This commit provides a new `cert` package, containing logic for extraction of CA bundle information from known Rancher secrets. That new package is expected to be used in Fleet clients which will need to fall back to Rancher CA bundles when no custom bundle is provided.
This makes use of the new `cert` package to use a CA bundle from Rancher secrets, if configured and if the `GitRepo` does not specify any such CA bundle.
This makes use of the new `cert` package to use a CA bundle from Rancher secrets, if configured and if the `GitRepo` does not already specify any such CA bundle. This does not alter the git cloner itself, but allows the CA bundle secret built by the gitOps controller (and later mounted for use by the git cloner) to use CA bundle information extracted from Rancher secrets as an alternative source.
This makes use of the new `cert` package to use a CA bundle from Rancher secrets, if configured and if the `GitRepo` does not already specify any Helm secret. If those conditions are met, the gitOps controller creates a new secret with CA bundle information from Rancher. This does not alter the Helm client itself, and could be optimised to avoid duplicating the CA bundle secret in case it already exists, with identical contents, due to no CA bundle being configured on the `GitRepo`.
Those expectations must take the `Eventually` function's `Gomega` argument into account to properly work inside that block.
This had been omitted in a previous commit, leading to the git cloner being unable to clone a git repository hosted on a server using HTTPS with certificates signed by a private CA.
This covers fallback in git clients. It uses an updated version of the test git server, which serves HTTPS, beside HTTP, with a custom CA bundle.
This caters for an edge case whereby a `GitRepo` may specify a Helm secret, for instance through `HelmSecretName`, which does not contain a CA bundle. When that happens, Fleet will fall back to Rancher-configured CA bundles, if any, while any existing CA bundle in the `GitRepo`'s Helm secret would, as before, be used with higher precedence.
This makes use of new Rancher-like secret [1] creation logic when setting up Fleet for end-to-end testing purposes. This allows tests validating fallback to Rancher-configured CA bundles to pass. [1]: https://ranchermanager.docs.rancher.com/getting-started/installation-and-upgrade/installation-references/helm-chart-options#additional-trusted-cas
That logic was already close to covered in unit tests. This moves it there, to remove clutter in integration tests, at the risk of adding some to unit tests.
weyfonk
force-pushed
the
2750-custom-ca-fallback
branch
from
90bbe29 to
d9582ea
Compare
0xavi0
approved these changes
Feb 11, 2025
There was a problem hiding this comment.
LGTM.
I wonder if we could get the Rancher secrets (names and values) from a configMap or something. (not sure if such configMap exists in Rancher or we could have that in fleet to centralise things)
And... in the same way....I would maybe set all the hardcoded strings for certs paths, secret values, keys, secret names, etc... as constants or functions. (So it is easier and faster to change them if needed)
| @@ -50,5 +50,6 @@ k3d cluster create "$name" \ | |||
| -p "$(( 8081 + offs )):8081@server:0" \ | |||
| -p "$(( 8082 + offs )):8082@server:0" \ | |||
| -p "$unique_tls_port:443@server:0" \ | |||
| -p "8043:4343@server:0" \ | |||
There was a problem hiding this comment.
Question: Why is not using the offset like the rest?
|
|
||
| addr, err := githelper.GetExternalRepoAddr(env, HTTPSPort, repoName) | ||
| Expect(err).ToNot(HaveOccurred()) | ||
| addr = strings.Replace(addr, "http://", "https://", 1) |
There was a problem hiding this comment.
Question:
Are these tests the same than the HTTP ones except for this line?
(I haven't checked 100% but both blocks look very similar)
If so... maybe better to just add the http/https as a variable and reuse the code?
| @@ -62,9 +64,38 @@ var _ = Describe("Single Cluster Examples", Label("infra-setup"), func() { | |||
| }).Should(ContainSubstring("sleeper-")) | |||
| }) | |||
| }) | |||
| Context("containing a private HTTP-based helm chart with repo path and no configured Helm secret name", Label("helm-registry"), func() { | |||
There was a problem hiding this comment.
It says "and no configured Helm secret" but it is passing a secret? is it a typo?
| // Fall back to Rancher-configured secrets | ||
| // We need to copy secret data from Rancher, because Rancher secrets live in a different namespace and | ||
| // can therefore not be used as sources for a volume. | ||
| secretName := fmt.Sprintf("%s-rancher-cabundle", gitrepo.Name) |
There was a problem hiding this comment.
Nit: Maybe better to have these names in constants or a function? Similar to the caBundleName function.
(see the main comment for other hardcoded strings)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This enables Fleet to rely on Rancher-configured CA bundle secrets in the following clients:
GitRepo's spec. Fleet will extract CA bundle data from Rancher secrets if (and only if) no CA bundle is provided for aGitRepo.HelmSecretNamenorHelmSecretNameForPathssecret is provided on aGitRepoThis is tested through a combination of:
Limitations
This does not cover imagescan (pending further discussion on the future of that feature), nor the HelmOps controller.
Falling back to Rancher CA bundles in HelmOps would require reading the HelmApp's
HelmSecretNamefield, and populating a CA bundle if that field is empty or if the referenced secret does not contain any CA bundle.How this should be done to support both version handling in the controller and chart downloads from the agent, is unclear for now: should an empty
HelmSecretNamebe overwritten by the HelmOps controller, or an incomplete secret edited to add a Rancher CA bundle? This could be handled in a follow-up patch.Refers to #2750
Related docs PR: rancher/fleet-docs#212