Merge pull request #1855 from cappyzawa/feat/helm-oci-controllers-runtime-secrets-v076

Enforce TLS certificate verification in Helm/OCI Repository controllers

Author: matheuscscp

go.mod

@@ -38,7 +38,7 @@ require (
 	github.com/fluxcd/pkg/lockedfile v0.6.0
 	github.com/fluxcd/pkg/masktoken v0.7.0
 	github.com/fluxcd/pkg/oci v0.51.0
-	github.com/fluxcd/pkg/runtime v0.75.0
+	github.com/fluxcd/pkg/runtime v0.76.0
 	github.com/fluxcd/pkg/sourceignore v0.13.0
 	github.com/fluxcd/pkg/ssh v0.20.0
 	github.com/fluxcd/pkg/tar v0.13.0

go.sum

@@ -398,8 +398,8 @@ github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3
 github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
 github.com/fluxcd/pkg/oci v0.51.0 h1:9oYnm+T4SCVSBif9gn80ALJkMGSERabVMDJiaMIdr7Y=
 github.com/fluxcd/pkg/oci v0.51.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
-github.com/fluxcd/pkg/runtime v0.75.0 h1:wIaODmU5D54nyrehTqA9oQDFoi6BbBj/24adLStXc0I=
-github.com/fluxcd/pkg/runtime v0.75.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
+github.com/fluxcd/pkg/runtime v0.76.0 h1:VoN508i65E/zK0iNXk1Ubvb2VcA8uADqckF+7nuof20=
+github.com/fluxcd/pkg/runtime v0.76.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
 github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU=
 github.com/fluxcd/pkg/sourceignore v0.13.0/go.mod h1:Z9H1GoBx0ljOhptnzoV0PL6Nd/UzwKcSphP27lqb4xI=
 github.com/fluxcd/pkg/ssh v0.20.0 h1:Ak0laIYIc/L8lEfqls/LDWRW8wYPESGaravQsCRGLb8=

internal/controller/helmrepository_controller_test.go

@@ -482,7 +482,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				repoURL, err := repository.NormalizeURL(serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
-				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
+				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
 				getterOpts := []helmgetter.Option{
@@ -534,7 +534,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				repoURL, err := repository.NormalizeURL(serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
-				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
+				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
 				getterOpts := []helmgetter.Option{
@@ -588,7 +588,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
 				repoURL, err := repository.NormalizeURL(serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
-				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false)
+				tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
 				t.Expect(err).ToNot(HaveOccurred())
 
 				getterOpts := []helmgetter.Option{

internal/controller/ocirepository_controller.go

@@ -986,6 +986,11 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *sourcev1.O
 func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev1.OCIRepository) (*cryptotls.Config, error) {
 	if obj.Spec.CertSecretRef == nil || obj.Spec.CertSecretRef.Name == "" {
 		if obj.Spec.Insecure {
+			// NOTE: This is the only place in Flux where InsecureSkipVerify is allowed.
+			// This exception is made for OCIRepository to maintain backward compatibility
+			// with tools like crane that require insecure connections without certificates.
+			// This only applies when no CertSecretRef is provided AND insecure is explicitly set.
+			// All other controllers must NOT allow InsecureSkipVerify per our security policy.
 			return &cryptotls.Config{
 				InsecureSkipVerify: true,
 			}, nil
@@ -997,7 +1002,7 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev
 		Namespace: obj.Namespace,
 		Name:      obj.Spec.CertSecretRef.Name,
 	}
-	return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, obj.Spec.Insecure)
+	return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL)
 }
 
 // reconcileStorage ensures the current state of the storage matches the

internal/helm/getter/client_opts.go

@@ -122,7 +122,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
 		}
 		certSecret = secret
 
-		tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, obj.Spec.Insecure)
+		tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL)
 		if err != nil {
 			return false, nil, nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
 		}
@@ -138,7 +138,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
 		}
 		authSecret = secret
 
-		methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLS(obj.Spec.URL, obj.Spec.Insecure))
+		methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(obj.Spec.URL))
 		if err != nil {
 			return false, nil, nil, fmt.Errorf("failed to detect authentication methods: %w", err)
 		}

internal/helm/getter/client_opts_test.go

@@ -298,3 +298,49 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
 		})
 	}
 }
+
+func TestConfigureAuthentication_WithTargetURL(t *testing.T) {
+	g := NewWithT(t)
+
+	tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem")
+	if err != nil {
+		t.Errorf("could not read CA file: %s", err)
+		return
+	}
+
+	helmRepo := &helmv1.HelmRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-repo",
+			Namespace: "default",
+		},
+		Spec: helmv1.HelmRepositorySpec{
+			URL: "https://example.com/charts",
+		},
+	}
+
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "auth-secret",
+			Namespace: "default",
+		},
+		Data: map[string][]byte{
+			"username": []byte("testuser"),
+			"password": []byte("testpass"),
+			"ca.crt":   tlsCA,
+		},
+	}
+
+	client := fakeclient.NewClientBuilder().WithObjects(secret).Build()
+	helmRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secret.Name}
+
+	opts := &ClientOpts{}
+	deprecatedTLS, certSecret, authSecret, err := configureAuthentication(context.TODO(), client, helmRepo, opts, helmRepo.Spec.URL)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(deprecatedTLS).To(BeTrue()) // TLS from SecretRef is deprecated
+	g.Expect(certSecret).To(BeNil())
+	g.Expect(authSecret).To(Equal(secret))
+
+	// Regression test: verify ServerName is set from target URL when WithTargetURL is used
+	g.Expect(opts.TlsConfig).ToNot(BeNil())
+	g.Expect(opts.TlsConfig.ServerName).To(Equal("example.com"))
+}