From fc736ed0e17042b5d89398d2da38566ff3773a8b Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Fri, 27 Feb 2026 11:09:50 +0100 Subject: [PATCH 1/5] Fall back to Rancher CA bundles for HelmOps Resolve the CA bundle in the HelmOps controller and store it in BundleHelmOptions.CABundle so the agent can use it without needing access to cattle-system secrets. The agent service account only has access to its own namespace. Also restart helmops pods in dev/update-controller-k3d so that redeployments pick up the new controller binary. --- charts/fleet-crd/templates/crds.yaml | 42 +++++++++ dev/update-controller-k3d | 1 + e2e/single-cluster/helmop_test.go | 85 ++++++++++++++++++ .../helmops/controller/controller_test.go | 89 +++++++++++++++++++ .../helmops/controller/suite_test.go | 18 ++++ internal/bundlereader/helm.go | 7 ++ internal/bundlereader/helm_test.go | 67 +++++++++++++- .../helmops/reconciler/helmop_controller.go | 38 +++++++- .../reconciler/helmop_controller_test.go | 33 +++++++ .../helmops/reconciler/polling_job.go | 18 ++-- .../fleet.cattle.io/v1alpha1/bundle_types.go | 7 ++ .../v1alpha1/zz_generated.deepcopy.go | 9 +- pkg/cert/cabundle.go | 2 +- 13 files changed, 399 insertions(+), 17 deletions(-) diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml index ce3b4e2e88..adf1854665 100644 --- a/charts/fleet-crd/templates/crds.yaml +++ b/charts/fleet-crd/templates/crds.yaml @@ -194,6 +194,20 @@ spec: should be downloaded from a helm chart' properties: + helmOpCABundle: + description: 'CABundle is a PEM encoded CA bundle used to validate + TLS connections to + + the Helm registry. It is resolved by the controller (which + has access to + + Rancher''s cattle-system CA secrets) and stored here so the + agent can use + + it without requiring access to those secrets.' + format: byte + nullable: true + type: string helmOpInsecureSkipTLSVerify: description: InsecureSkipTLSverify will use insecure HTTPS to clone the helm app resource. @@ -2067,6 +2081,20 @@ spec: not a git repository.' nullable: true properties: + helmOpCABundle: + description: 'CABundle is a PEM encoded CA bundle used to validate + TLS connections to + + the Helm registry. It is resolved by the controller (which + has access to + + Rancher''s cattle-system CA secrets) and stored here so the + agent can use + + it without requiring access to those secrets.' + format: byte + nullable: true + type: string helmOpInsecureSkipTLSVerify: description: InsecureSkipTLSverify will use insecure HTTPS to clone the helm app resource. @@ -8269,6 +8297,20 @@ spec: not a git repository.' nullable: true properties: + helmOpCABundle: + description: 'CABundle is a PEM encoded CA bundle used to validate + TLS connections to + + the Helm registry. It is resolved by the controller (which + has access to + + Rancher''s cattle-system CA secrets) and stored here so the + agent can use + + it without requiring access to those secrets.' + format: byte + nullable: true + type: string helmOpInsecureSkipTLSVerify: description: InsecureSkipTLSverify will use insecure HTTPS to clone the helm app resource. diff --git a/dev/update-controller-k3d b/dev/update-controller-k3d index daf725b8f9..476372146f 100755 --- a/dev/update-controller-k3d +++ b/dev/update-controller-k3d @@ -20,3 +20,4 @@ fleet_ctx=$(kubectl config current-context) k3d image import rancher/fleet:dev -m direct -c "${fleet_ctx#k3d-}" kubectl delete pod -l app=fleet-controller -n cattle-fleet-system kubectl delete pod -l app=gitjob -n cattle-fleet-system +kubectl delete pod -l app=helmops -n cattle-fleet-system diff --git a/e2e/single-cluster/helmop_test.go b/e2e/single-cluster/helmop_test.go index b70bdc63a2..3809cc8b78 100644 --- a/e2e/single-cluster/helmop_test.go +++ b/e2e/single-cluster/helmop_test.go @@ -520,6 +520,91 @@ var _ = Describe("HelmOp resource tests with tarball source", Label("infra-setup }) }) +var _ = Describe("HelmOp resource falls back to Rancher CA bundle", Label("infra-setup", "helm-registry"), Ordered, func() { + // This test mirrors the GitOps E2E test "should succeed when not configuring any CA" + // in go_getter_custom_ca_test.go. The dev/create-secrets script places the root CA + // into cattle-system/tls-ca-additional. ChartMuseum is served with a cert signed by + // that root CA. A HelmOp with a credentials-only secret (no cacerts) and + // InsecureSkipTLSVerify=false must therefore succeed via the Rancher CA fallback. + const ( + name = "rancher-ca-fallback" + secretName = "helmop-rancher-ca-creds" + ) + + var ( + namespace string + k kubectl.Command + ) + + BeforeAll(func() { + k = env.Kubectl.Namespace(env.Namespace) + out, err := k.Create( + "secret", "generic", secretName, + "--from-literal=username="+os.Getenv("CI_OCI_USERNAME"), + "--from-literal=password="+os.Getenv("CI_OCI_PASSWORD"), + // no cacerts — TLS trust must come from cattle-system/tls-ca-additional + ) + if strings.Contains(out, "already exists") { + err = nil + } + Expect(err).ToNot(HaveOccurred(), out) + }) + + JustBeforeEach(func() { + namespace = testenv.NewNamespaceName( + name, + rand.New(rand.NewSource(time.Now().UnixNano())), + ) + + // URL without embedded credentials so the secret is the only auth source. + repo := fmt.Sprintf("https://chartmuseum-service.%s.svc.cluster.local:8081", cmd.InfraNamespace) + err := testenv.ApplyTemplate(k, testenv.AssetPath("helmop/helmop.yaml"), struct { + Name string + Namespace string + Repo string + Chart string + PollingInterval time.Duration + HelmSecretName string + InsecureSkipTLSVerify bool + Version string + }{ + name, + namespace, + repo, + "sleeper-chart", + 5 * time.Second, + secretName, + false, // strict TLS — relies on Rancher CA bundle fallback + "0.1.0", + }) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterAll(func() { + out, err := k.Delete("helmop", name) + Expect(err).ToNot(HaveOccurred(), out) + out, err = k.Delete("secret", secretName) + Expect(err).ToNot(HaveOccurred(), out) + }) + + It("deploys the chart using the Rancher CA bundle from cattle-system", func() { + Eventually(func(g Gomega) { + outPods, _ := k.Namespace(namespace).Get("pods") + g.Expect(outPods).To(ContainSubstring("sleeper-")) + }).Should(Succeed()) + Eventually(func(g Gomega) { + outDeployments, _ := k.Namespace(namespace).Get("deployments") + g.Expect(outDeployments).To(ContainSubstring("sleeper")) + }).Should(Succeed()) + + By("setting the expected version in the helmop Status") + Eventually(func() string { + out, _ := k.Get("helmop", name, "-o=jsonpath={.status.version}") + return out + }).Should(Equal("0.1.0")) + }) +}) + // getExternalHelmAddr retrieves the external URL where our local Helm registry can be reached. func getExternalHelmAddr(k kubectl.Command) (string, error) { if v := os.Getenv("external_ip"); v != "" { diff --git a/integrationtests/helmops/controller/controller_test.go b/integrationtests/helmops/controller/controller_test.go index a3de0a8320..15e761b490 100644 --- a/integrationtests/helmops/controller/controller_test.go +++ b/integrationtests/helmops/controller/controller_test.go @@ -4,6 +4,7 @@ import ( "crypto/sha256" "crypto/subtle" "crypto/tls" + "encoding/pem" "fmt" "io" "log" @@ -230,6 +231,26 @@ func checkBundleIsAsExpected(g Gomega, bundle fleet.Bundle, helmop fleet.HelmOp, g.Expect(controllerutil.ContainsFinalizer(&bundle, finalize.BundleFinalizer)).To(BeTrue()) } +// createRancherCASecret creates a secret in cattle-system using the +// certificate from svr and registers a DeferCleanup to delete it. +func createRancherCASecret(svr *httptest.Server, secretName, dataKey string) { + certPEM := pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: svr.TLS.Certificates[0].Certificate[0], + }) + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: "cattle-system", + }, + Data: map[string][]byte{dataKey: certPEM}, + } + Expect(k8sClient.Create(ctx, secret)).ToNot(HaveOccurred()) + DeferCleanup(func() { + _ = k8sClient.Delete(ctx, secret) + }) +} + func updateHelmOp(helmop fleet.HelmOp) error { backoff := retry.DefaultBackoff backoff.Steps = 10 @@ -1267,5 +1288,73 @@ var _ = Describe("HelmOps controller", func() { }).Should(Succeed()) }) }) + + When("connecting to a https server with a CA bundle from Rancher tls-ca secret", func() { + BeforeEach(func() { + targets = []fleet.BundleTarget{} + helmop = getRandomHelmOpWithTargets("test-rancher-tlsca", targets) + helmop.Spec.Helm.Version = "" + helmop.Spec.HelmSecretName = "" + + svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, helmRepoIndex) + })) + DeferCleanup(svr.Close) + + helmop.Spec.Helm.Repo = svr.URL + helmop.Spec.Helm.Chart = "alpine" + helmop.Spec.InsecureSkipTLSverify = false + doAfterNamespaceCreated = func() { + createRancherCASecret(svr, "tls-ca", "cacerts.pem") + } + }) + + It("creates a bundle with the latest version it got from the index", func() { + Eventually(func(g Gomega) { + bundle := &fleet.Bundle{} + ns := types.NamespacedName{Name: helmop.Name, Namespace: helmop.Namespace} + err := k8sClient.Get(ctx, ns, bundle) + g.Expect(err).ToNot(HaveOccurred()) + t := []fleet.BundleTarget{{Name: "default", ClusterGroup: "default"}} + helmop.Spec.Helm.Version = "0.2.0" + checkBundleIsAsExpected(g, *bundle, helmop, t) + }).Should(Succeed()) + }) + }) + + When("connecting to a https server with a CA bundle from Rancher tls-ca-additional secret", func() { + BeforeEach(func() { + targets = []fleet.BundleTarget{} + helmop = getRandomHelmOpWithTargets("test-rancher-tlsca-additional", targets) + helmop.Spec.Helm.Version = "" + helmop.Spec.HelmSecretName = "" + + svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + fmt.Fprint(w, helmRepoIndex) + })) + DeferCleanup(svr.Close) + + helmop.Spec.Helm.Repo = svr.URL + helmop.Spec.Helm.Chart = "alpine" + helmop.Spec.InsecureSkipTLSverify = false + doAfterNamespaceCreated = func() { + createRancherCASecret(svr, "tls-ca-additional", "ca-additional.pem") + } + }) + + It("creates a bundle with the latest version it got from the index", func() { + Eventually(func(g Gomega) { + bundle := &fleet.Bundle{} + ns := types.NamespacedName{Name: helmop.Name, Namespace: helmop.Namespace} + err := k8sClient.Get(ctx, ns, bundle) + g.Expect(err).ToNot(HaveOccurred()) + t := []fleet.BundleTarget{{Name: "default", ClusterGroup: "default"}} + helmop.Spec.Helm.Version = "0.2.0" + checkBundleIsAsExpected(g, *bundle, helmop, t) + }).Should(Succeed()) + }) + }) }) }) diff --git a/integrationtests/helmops/controller/suite_test.go b/integrationtests/helmops/controller/suite_test.go index 9a81fde6fb..4e6994bbc9 100644 --- a/integrationtests/helmops/controller/suite_test.go +++ b/integrationtests/helmops/controller/suite_test.go @@ -19,6 +19,8 @@ import ( "github.com/rancher/fleet/internal/manifest" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -128,6 +130,22 @@ var _ = BeforeSuite(func() { err = mgr.Start(ctx) Expect(err).ToNot(HaveOccurred(), "failed to run manager") }() + + // Create Rancher-like namespace for CA bundle secrets + err = k8sClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cattle-system", + }, + }) + Expect(err).ToNot(HaveOccurred()) + + DeferCleanup(func() { + _ = k8sClient.Delete(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cattle-system", + }, + }) + }) }) var _ = AfterSuite(func() { diff --git a/internal/bundlereader/helm.go b/internal/bundlereader/helm.go index f0faac759d..735e7338b3 100644 --- a/internal/bundlereader/helm.go +++ b/internal/bundlereader/helm.go @@ -32,6 +32,13 @@ func GetManifestFromHelmChart(ctx context.Context, c client.Reader, bd *fleet.Bu } auth.InsecureSkipVerify = bd.Spec.HelmChartOptions.InsecureSkipTLSverify + // Use the Rancher CA bundle that was pre-resolved by the controller and stored in + // HelmChartOptions.CABundle. The agent service account cannot read cattle-system + // secrets directly, so the controller must pass the bundle through. + if len(auth.CABundle) == 0 { + auth.CABundle = bd.Spec.HelmChartOptions.CABundle + } + chartURL, err := ChartURL(ctx, *helm, auth, true) if err != nil { return nil, err diff --git a/internal/bundlereader/helm_test.go b/internal/bundlereader/helm_test.go index 31e3a9ea6c..1038d8d3e4 100644 --- a/internal/bundlereader/helm_test.go +++ b/internal/bundlereader/helm_test.go @@ -7,6 +7,8 @@ import ( "context" "crypto/sha256" "crypto/subtle" + "crypto/x509" + "encoding/pem" "fmt" "io" "net/http" @@ -246,10 +248,14 @@ func newTLSServer(index string, withAuth bool) *httptest.Server { func TestGetManifestFromHelmChart(t *testing.T) { cases := []struct { - name string - bd fleet.BundleDeployment - readerCalls func(*mocks.MockReader) - requiresAuth bool + name string + bd fleet.BundleDeployment + readerCalls func(*mocks.MockReader) + requiresAuth bool + // injectSrvCert: when true, the test loop extracts the httptest server's + // TLS CA certificate and writes it into HelmChartOptions.CABundle before + // calling GetManifestFromHelmChart, simulating what the controller does. + injectSrvCert bool expectedNilManifest bool expectedResources []fleet.BundleResource expectedErrNotNil bool @@ -382,6 +388,43 @@ func TestGetManifestFromHelmChart(t *testing.T) { expectedErrNotNil: false, expectedError: "", }, + { + name: "load directory with CA bundle from HelmChartOptions", + bd: fleet.BundleDeployment{ + Spec: fleet.BundleDeploymentSpec{ + Options: fleet.BundleDeploymentOptions{ + Helm: &fleet.HelmOptions{ + Repo: "##URL##", // will be replaced by the mock server url + Chart: "sleeper", + }, + }, + HelmChartOptions: &fleet.BundleHelmOptions{ + // CABundle will be injected from the test server's cert. + InsecureSkipTLSverify: false, + }, + }, + }, + readerCalls: func(c *mocks.MockReader) {}, + requiresAuth: false, + injectSrvCert: true, + expectedNilManifest: false, + expectedResources: []fleet.BundleResource{ + { + Name: "sleeper-chart/templates/deployment.yaml", + Content: deployment, + }, + { + Name: "sleeper-chart/values.yaml", + Content: values, + }, + { + Name: "sleeper-chart/Chart.yaml", + Content: chartYAML, + }, + }, + expectedErrNotNil: false, + expectedError: "", + }, } mockCtrl := gomock.NewController(t) @@ -397,6 +440,22 @@ func TestGetManifestFromHelmChart(t *testing.T) { srv := newTLSServer(helmRepoIndex, c.requiresAuth) defer srv.Close() + if c.injectSrvCert { + // Extract the httptest server's TLS CA certificate and inject it as + // HelmChartOptions.CABundle, mirroring what the controller does at + // bundle creation time. + tlsCert := srv.TLS.Certificates[0] + leaf, err := x509.ParseCertificate(tlsCert.Certificate[0]) + if err != nil { + t.Fatalf("parsing server certificate: %v", err) + } + var buf bytes.Buffer + if err := pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: leaf.Raw}); err != nil { + t.Fatalf("encoding server certificate: %v", err) + } + c.bd.Spec.HelmChartOptions.CABundle = buf.Bytes() + } + resourcePrefix := "" if c.bd.Spec.Options.Helm != nil { c.bd.Spec.Options.Helm.Repo = strings.ReplaceAll(c.bd.Spec.Options.Helm.Repo, "##URL##", srv.URL) diff --git a/internal/cmd/controller/helmops/reconciler/helmop_controller.go b/internal/cmd/controller/helmops/reconciler/helmop_controller.go index 0138f67b29..d613ec076e 100644 --- a/internal/cmd/controller/helmops/reconciler/helmop_controller.go +++ b/internal/cmd/controller/helmops/reconciler/helmop_controller.go @@ -36,6 +36,7 @@ import ( ctrlquartz "github.com/rancher/fleet/internal/cmd/controller/quartz" "github.com/rancher/fleet/internal/metrics" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + "github.com/rancher/fleet/pkg/cert" "github.com/rancher/fleet/pkg/durations" "github.com/rancher/fleet/pkg/sharding" ) @@ -176,7 +177,17 @@ func (r *HelmOpReconciler) createUpdateBundle(ctx context.Context, helmop *fleet // calculate the new representation of the helmop resource bundle := r.calculateBundle(helmop) - if err := r.handleVersion(ctx, b, bundle, helmop); err != nil { + // Resolve the Rancher CA bundle in the controller and store it in HelmOpOptions so + // the agent (whose service account cannot read cattle-system secrets) can use it. + // The resolved bundle is also forwarded to handleVersion so getChartVersion can reuse + // it without a second cattle-system lookup. + cab, err := cert.GetRancherCABundle(ctx, r.Client) + if err != nil { + return nil, fmt.Errorf("could not get Rancher CA bundle: %w", err) + } + bundle.Spec.HelmOpOptions.CABundle = cab + + if err := r.handleVersion(ctx, b, bundle, helmop, cab); err != nil { return nil, err } @@ -251,7 +262,10 @@ func (r *HelmOpReconciler) calculateBundle(helmop *fleet.HelmOp) *fleet.Bundle { // // This is calculated in the upstream cluster so all downstream bundle deployments have the same // version. (Potentially we could be gathering the version at the very moment it is being updated, for example) -func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.Bundle, bundle *fleet.Bundle, helmop *fleet.HelmOp) error { +// +// caBundle is an optional pre-resolved CA bundle (may be nil). When non-nil it is forwarded to +// getChartVersion so a second cattle-system secret lookup is avoided. +func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.Bundle, bundle *fleet.Bundle, helmop *fleet.HelmOp, caBundle []byte) error { if helmop == nil { return fmt.Errorf("the provided HelmOp is nil; this should not happen") } @@ -262,7 +276,7 @@ func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.B return nil } - version, err := getChartVersion(ctx, r.Client, *helmop) + version, err := getChartVersion(ctx, r.Client, *helmop, caBundle) if err != nil { return err } @@ -518,7 +532,9 @@ func helmChartSpecChanged(o *fleet.HelmOptions, n *fleet.HelmOptions, statusVers // getChartVersion fetches the latest chart version from the Helm registry referenced by helmop, and returns it. // If this fails, it returns an empty version along with an error. -func getChartVersion(ctx context.Context, c client.Client, helmop fleet.HelmOp) (string, error) { +// caBundle is an optional pre-resolved Rancher CA bundle. When nil and no CA bundle is set in auth, +// getChartVersion resolves the bundle itself via GetRancherCABundle. +func getChartVersion(ctx context.Context, c client.Client, helmop fleet.HelmOp, caBundle []byte) (string, error) { auth := bundlereader.Auth{} if helmop.Spec.HelmSecretName != "" { req := types.NamespacedName{Namespace: helmop.Namespace, Name: helmop.Spec.HelmSecretName} @@ -530,6 +546,20 @@ func getChartVersion(ctx context.Context, c client.Client, helmop fleet.HelmOp) } auth.InsecureSkipVerify = helmop.Spec.InsecureSkipTLSverify + // Fall back to a Rancher-configured CA bundle if no CA bundle is set. + // Use a pre-resolved bundle when available to avoid a redundant cattle-system lookup. + if len(auth.CABundle) == 0 { + if len(caBundle) > 0 { + auth.CABundle = caBundle + } else { + cab, err := cert.GetRancherCABundle(ctx, c) + if err != nil { + return "", fmt.Errorf("could not get Rancher CA bundle: %w", err) + } + auth.CABundle = cab + } + } + version, err := bundlereader.ChartVersion(ctx, *helmop.Spec.Helm, auth) if err != nil { return "", fmt.Errorf("could not get a chart version: %w", err) diff --git a/internal/cmd/controller/helmops/reconciler/helmop_controller_test.go b/internal/cmd/controller/helmops/reconciler/helmop_controller_test.go index 227196a742..f8cbbd484a 100644 --- a/internal/cmd/controller/helmops/reconciler/helmop_controller_test.go +++ b/internal/cmd/controller/helmops/reconciler/helmop_controller_test.go @@ -24,6 +24,7 @@ import ( "github.com/reugn/go-quartz/quartz" batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -449,6 +450,9 @@ func TestReconcile_ErrorCreatingBundleIsShownInStatus(t *testing.T) { }, ) + // Fall-back CA bundle look-up (cattle-system/tls-ca and tls-ca-additional). + expectCABundleLookup(client) + statusClient := mocks.NewMockStatusWriter(mockCtrl) client.EXPECT().Status().Return(statusClient).Times(1) statusClient.EXPECT().Patch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do( @@ -626,6 +630,9 @@ func TestReconcile_CreatesBundleAndUpdatesStatus(t *testing.T) { }, ) + // Fall-back CA bundle look-up (cattle-system/tls-ca and tls-ca-additional). + expectCABundleLookup(client) + statusClient := mocks.NewMockStatusWriter(mockCtrl) client.EXPECT().Status().Return(statusClient).Times(1) statusClient.EXPECT().Patch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Do( @@ -1234,6 +1241,9 @@ func TestReconcile_ManagePollingJobs(t *testing.T) { // Only expected in happy cases. If errors happen, only status updates are expected. client.EXPECT().Update(gomock.Any(), matchesBundle(c.helmOp.Name, c.helmOp.Namespace), gomock.Any()).Return(nil).AnyTimes() + // Fall-back CA bundle look-up (cattle-system/tls-ca and tls-ca-additional). + expectCABundleLookup(client) + statusClient := mocks.NewMockStatusWriter(mockCtrl) statusClient.EXPECT().Patch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) @@ -1316,3 +1326,26 @@ func (tm *typeMatcher) Matches(x interface{}) bool { func (tm *typeMatcher) String() string { return "is of type " + reflect.TypeOf(tm.t).String() } + +// expectCABundleLookup sets up narrow mock expectations for the two cattle-system +// secrets read by GetRancherCABundle (tls-ca and tls-ca-additional), returning +// NotFound for both. This avoids an overly broad matcher that would silently +// absorb unrelated Secret Gets. +func expectCABundleLookup(client *mocks.MockK8sClient) { + client.EXPECT().Get( + gomock.Any(), + types.NamespacedName{Namespace: "cattle-system", Name: "tls-ca"}, + OfType(&corev1.Secret{}), + gomock.Any(), + ).AnyTimes().DoAndReturn(func(_ context.Context, _ types.NamespacedName, _ *corev1.Secret, _ ...interface{}) error { + return k8serrors.NewNotFound(schema.GroupResource{}, "tls-ca") + }) + client.EXPECT().Get( + gomock.Any(), + types.NamespacedName{Namespace: "cattle-system", Name: "tls-ca-additional"}, + OfType(&corev1.Secret{}), + gomock.Any(), + ).AnyTimes().DoAndReturn(func(_ context.Context, _ types.NamespacedName, _ *corev1.Secret, _ ...interface{}) error { + return k8serrors.NewNotFound(schema.GroupResource{}, "tls-ca-additional") + }) +} diff --git a/internal/cmd/controller/helmops/reconciler/polling_job.go b/internal/cmd/controller/helmops/reconciler/polling_job.go index 772027f9dd..44f3ee8f7c 100644 --- a/internal/cmd/controller/helmops/reconciler/polling_job.go +++ b/internal/cmd/controller/helmops/reconciler/polling_job.go @@ -124,15 +124,21 @@ func (j *helmPollingJob) pollHelm(ctx context.Context) error { return j.updateErrorStatus(ctx, h, pollingTimestamp, origErr) } - version, err := getChartVersion(ctx, j.client, *h) - if err != nil { - return fail(err, "FailedToGetNewChartVersion") + // Fetch the bundle first so we can pass its stored CA bundle to + // getChartVersion and avoid a redundant cattle-system secret lookup. + b := &fleet.Bundle{} + if err := j.client.Get(ctx, nsName, b); err != nil { + return fail(fmt.Errorf("could not get bundle before polling: %w", err), "FailedToGetBundle") } - b := &fleet.Bundle{} + var storedCABundle []byte + if b.Spec.HelmOpOptions != nil { + storedCABundle = b.Spec.HelmOpOptions.CABundle + } - if err := j.client.Get(ctx, nsName, b); err != nil { - return fail(fmt.Errorf("could not get bundle before patching its version: %w", err), "FailedToGetBundle") + version, err := getChartVersion(ctx, j.client, *h, storedCABundle) + if err != nil { + return fail(err, "FailedToGetNewChartVersion") } orig := b.DeepCopy() diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go index aef17f9c4e..4f47311a49 100644 --- a/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go +++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go @@ -457,4 +457,11 @@ type BundleHelmOptions struct { // InsecureSkipTLSverify will use insecure HTTPS to clone the helm app resource. InsecureSkipTLSverify bool `json:"helmOpInsecureSkipTLSVerify,omitempty"` + + // CABundle is a PEM encoded CA bundle used to validate TLS connections to + // the Helm registry. It is resolved by the controller (which has access to + // Rancher's cattle-system CA secrets) and stored here so the agent can use + // it without requiring access to those secrets. + // +nullable + CABundle []byte `json:"helmOpCABundle,omitempty"` } diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go index bbd4786e72..f53714b09b 100644 --- a/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go @@ -286,7 +286,7 @@ func (in *BundleDeploymentSpec) DeepCopyInto(out *BundleDeploymentSpec) { if in.HelmChartOptions != nil { in, out := &in.HelmChartOptions, &out.HelmChartOptions *out = new(BundleHelmOptions) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -364,6 +364,11 @@ func (in *BundleDisplay) DeepCopy() *BundleDisplay { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BundleHelmOptions) DeepCopyInto(out *BundleHelmOptions) { *out = *in + if in.CABundle != nil { + in, out := &in.CABundle, &out.CABundle + *out = make([]byte, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BundleHelmOptions. @@ -568,7 +573,7 @@ func (in *BundleSpec) DeepCopyInto(out *BundleSpec) { if in.HelmOpOptions != nil { in, out := &in.HelmOpOptions, &out.HelmOpOptions *out = new(BundleHelmOptions) - **out = **in + (*in).DeepCopyInto(*out) } } diff --git a/pkg/cert/cabundle.go b/pkg/cert/cabundle.go index 3b8c20db70..213654df1e 100644 --- a/pkg/cert/cabundle.go +++ b/pkg/cert/cabundle.go @@ -12,7 +12,7 @@ import ( const rancherNS = "cattle-system" -func GetRancherCABundle(ctx context.Context, c client.Client) ([]byte, error) { +func GetRancherCABundle(ctx context.Context, c client.Reader) ([]byte, error) { secret := &corev1.Secret{} err := c.Get(ctx, types.NamespacedName{Namespace: rancherNS, Name: "tls-ca"}, secret) From 81055342d828c8ceb3c199bd4ec69fda4e1ff584 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Fri, 27 Feb 2026 11:48:16 +0100 Subject: [PATCH 2/5] Remove obsolete OCI no-TLS negative test The test relied on the OCI registry being untrusted when InsecureSkipTLSVerify is false and no CA bundle is provided. Since the Rancher CA bundle fallback now supplies the fleet CI root CA (stored in cattle-system/tls-ca-additional), the Zot OCI registry is trusted automatically and the chart deploys successfully. --- e2e/single-cluster/helmop_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/e2e/single-cluster/helmop_test.go b/e2e/single-cluster/helmop_test.go index 3809cc8b78..a661d43a42 100644 --- a/e2e/single-cluster/helmop_test.go +++ b/e2e/single-cluster/helmop_test.go @@ -371,21 +371,6 @@ var _ = Describe("HelmOp resource with polling of OCI registry", Label("infra-se }) }) - Context("containing a valid helmop description pointing to an oci registry and not TLS", func() { - BeforeEach(func() { - namespace = "helmop-ns2" - name = "basic-oci-no-tls" - insecure = false - - repo = fmt.Sprintf("%s/sleeper-chart", ociRef) - }) - It("does not deploy the chart because of TLS", func() { - Consistently(func() string { - out, _ := k.Namespace(namespace).Get("pods") - return out - }, 5*time.Second, time.Second).ShouldNot(ContainSubstring("sleeper-")) - }) - }) }) When("applying a helmop resource which cannot be deployed", func() { From b9e8759eafb5f7cbd77a88fe89c63f07dbb8b838 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Wed, 4 Mar 2026 14:18:09 +0100 Subject: [PATCH 3/5] Clarify CA bundle comment wording Note that Rancher's cattle-system secrets may not exist in standalone Fleet installations. Also use "CA bundle" consistently in the bundlereader comment. --- charts/fleet-crd/templates/crds.yaml | 12 ++++++------ internal/bundlereader/helm.go | 2 +- pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml index adf1854665..1e7f625e9d 100644 --- a/charts/fleet-crd/templates/crds.yaml +++ b/charts/fleet-crd/templates/crds.yaml @@ -201,8 +201,8 @@ spec: the Helm registry. It is resolved by the controller (which has access to - Rancher''s cattle-system CA secrets) and stored here so the - agent can use + Rancher''s cattle-system CA secrets, if any) and stored here + so the agent can use it without requiring access to those secrets.' format: byte @@ -2088,8 +2088,8 @@ spec: the Helm registry. It is resolved by the controller (which has access to - Rancher''s cattle-system CA secrets) and stored here so the - agent can use + Rancher''s cattle-system CA secrets, if any) and stored here + so the agent can use it without requiring access to those secrets.' format: byte @@ -8304,8 +8304,8 @@ spec: the Helm registry. It is resolved by the controller (which has access to - Rancher''s cattle-system CA secrets) and stored here so the - agent can use + Rancher''s cattle-system CA secrets, if any) and stored here + so the agent can use it without requiring access to those secrets.' format: byte diff --git a/internal/bundlereader/helm.go b/internal/bundlereader/helm.go index 735e7338b3..30bb28f478 100644 --- a/internal/bundlereader/helm.go +++ b/internal/bundlereader/helm.go @@ -34,7 +34,7 @@ func GetManifestFromHelmChart(ctx context.Context, c client.Reader, bd *fleet.Bu // Use the Rancher CA bundle that was pre-resolved by the controller and stored in // HelmChartOptions.CABundle. The agent service account cannot read cattle-system - // secrets directly, so the controller must pass the bundle through. + // secrets directly, so the controller must pass the CA bundle through. if len(auth.CABundle) == 0 { auth.CABundle = bd.Spec.HelmChartOptions.CABundle } diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go index 4f47311a49..8d0edebfce 100644 --- a/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go +++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundle_types.go @@ -460,7 +460,7 @@ type BundleHelmOptions struct { // CABundle is a PEM encoded CA bundle used to validate TLS connections to // the Helm registry. It is resolved by the controller (which has access to - // Rancher's cattle-system CA secrets) and stored here so the agent can use + // Rancher's cattle-system CA secrets, if any) and stored here so the agent can use // it without requiring access to those secrets. // +nullable CABundle []byte `json:"helmOpCABundle,omitempty"` From 0c6522f3883c3cdad0180513d8426b08e8d92b57 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Wed, 4 Mar 2026 14:18:14 +0100 Subject: [PATCH 4/5] Read CABundle from bundle spec in handleVersion The CA bundle is stored in bundle.Spec.HelmOpOptions.CABundle before handleVersion is called, so passing it again as a separate parameter is redundant. Remove the parameter and read it from the bundle directly. --- .../helmops/reconciler/helmop_controller.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/cmd/controller/helmops/reconciler/helmop_controller.go b/internal/cmd/controller/helmops/reconciler/helmop_controller.go index d613ec076e..6ee28777b7 100644 --- a/internal/cmd/controller/helmops/reconciler/helmop_controller.go +++ b/internal/cmd/controller/helmops/reconciler/helmop_controller.go @@ -179,15 +179,13 @@ func (r *HelmOpReconciler) createUpdateBundle(ctx context.Context, helmop *fleet // Resolve the Rancher CA bundle in the controller and store it in HelmOpOptions so // the agent (whose service account cannot read cattle-system secrets) can use it. - // The resolved bundle is also forwarded to handleVersion so getChartVersion can reuse - // it without a second cattle-system lookup. cab, err := cert.GetRancherCABundle(ctx, r.Client) if err != nil { return nil, fmt.Errorf("could not get Rancher CA bundle: %w", err) } bundle.Spec.HelmOpOptions.CABundle = cab - if err := r.handleVersion(ctx, b, bundle, helmop, cab); err != nil { + if err := r.handleVersion(ctx, b, bundle, helmop); err != nil { return nil, err } @@ -262,10 +260,7 @@ func (r *HelmOpReconciler) calculateBundle(helmop *fleet.HelmOp) *fleet.Bundle { // // This is calculated in the upstream cluster so all downstream bundle deployments have the same // version. (Potentially we could be gathering the version at the very moment it is being updated, for example) -// -// caBundle is an optional pre-resolved CA bundle (may be nil). When non-nil it is forwarded to -// getChartVersion so a second cattle-system secret lookup is avoided. -func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.Bundle, bundle *fleet.Bundle, helmop *fleet.HelmOp, caBundle []byte) error { +func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.Bundle, bundle *fleet.Bundle, helmop *fleet.HelmOp) error { if helmop == nil { return fmt.Errorf("the provided HelmOp is nil; this should not happen") } @@ -276,7 +271,7 @@ func (r *HelmOpReconciler) handleVersion(ctx context.Context, oldBundle *fleet.B return nil } - version, err := getChartVersion(ctx, r.Client, *helmop, caBundle) + version, err := getChartVersion(ctx, r.Client, *helmop, bundle.Spec.HelmOpOptions.CABundle) if err != nil { return err } From 70b02e7d474c4d04230a0e89b51c87d62c81ebbf Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Wed, 4 Mar 2026 14:18:21 +0100 Subject: [PATCH 5/5] Drop static version assertion from CA bundle E2E test The version "0.1.0" is a literal in the HelmOp spec, not a resolved constraint, so asserting it appears in status adds no signal. --- e2e/single-cluster/helmop_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/e2e/single-cluster/helmop_test.go b/e2e/single-cluster/helmop_test.go index a661d43a42..c01f4f7cdc 100644 --- a/e2e/single-cluster/helmop_test.go +++ b/e2e/single-cluster/helmop_test.go @@ -582,11 +582,6 @@ var _ = Describe("HelmOp resource falls back to Rancher CA bundle", Label("infra g.Expect(outDeployments).To(ContainSubstring("sleeper")) }).Should(Succeed()) - By("setting the expected version in the helmop Status") - Eventually(func() string { - out, _ := k.Get("helmop", name, "-o=jsonpath={.status.version}") - return out - }).Should(Equal("0.1.0")) }) })