Skip to content

Commit 48da00d

Browse files
authored
Merge pull request #1870 from cappyzawa/remove-tlsconfig-servername-pinning
Remove ServerName pinning from TLS config
2 parents cd5eebf + 683719d commit 48da00d

File tree

8 files changed

+11
-59
lines changed

8 files changed

+11
-59
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ require (
3838
github.com/fluxcd/pkg/lockedfile v0.6.0
3939
github.com/fluxcd/pkg/masktoken v0.7.0
4040
github.com/fluxcd/pkg/oci v0.52.0
41-
github.com/fluxcd/pkg/runtime v0.79.0
41+
github.com/fluxcd/pkg/runtime v0.80.0
4242
github.com/fluxcd/pkg/sourceignore v0.13.0
4343
github.com/fluxcd/pkg/ssh v0.20.0
4444
github.com/fluxcd/pkg/tar v0.13.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,8 @@ github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3
398398
github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU=
399399
github.com/fluxcd/pkg/oci v0.52.0 h1:rkHMtXYm21MtDrjNcR5KScqOe6C1JHPExoShuVdNm8M=
400400
github.com/fluxcd/pkg/oci v0.52.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4=
401-
github.com/fluxcd/pkg/runtime v0.79.0 h1:9tv79EiQDx/QJH9mYDd9kZ9WybCVWBUGoiBHij+eKkc=
402-
github.com/fluxcd/pkg/runtime v0.79.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
401+
github.com/fluxcd/pkg/runtime v0.80.0 h1:vknT2vdQSGTFnAhz4xGk2ZXUWCrXh3whsISStgA57Go=
402+
github.com/fluxcd/pkg/runtime v0.80.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw=
403403
github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU=
404404
github.com/fluxcd/pkg/sourceignore v0.13.0/go.mod h1:Z9H1GoBx0ljOhptnzoV0PL6Nd/UzwKcSphP27lqb4xI=
405405
github.com/fluxcd/pkg/ssh v0.20.0 h1:Ak0laIYIc/L8lEfqls/LDWRW8wYPESGaravQsCRGLb8=

internal/controller/bucket_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ func (r *BucketReconciler) setupCredentials(ctx context.Context, obj *sourcev1.B
831831
Namespace: obj.GetNamespace(),
832832
Name: obj.Spec.CertSecretRef.Name,
833833
}
834-
tlsConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, obj.Spec.Endpoint, secrets.WithSystemCertPool())
834+
tlsConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, secrets.WithSystemCertPool())
835835
if err != nil {
836836
return nil, fmt.Errorf("failed to get TLS config: %w", err)
837837
}
@@ -842,7 +842,7 @@ func (r *BucketReconciler) setupCredentials(ctx context.Context, obj *sourcev1.B
842842
Namespace: obj.GetNamespace(),
843843
Name: obj.Spec.STS.CertSecretRef.Name,
844844
}
845-
stsTLSConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, obj.Spec.STS.Endpoint, secrets.WithSystemCertPool())
845+
stsTLSConfig, err = secrets.TLSConfigFromSecretRef(ctx, r.Client, secretRef, secrets.WithSystemCertPool())
846846
if err != nil {
847847
return nil, fmt.Errorf("failed to get STS TLS config: %w", err)
848848
}

internal/controller/gitrepository_controller.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -689,8 +689,7 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1
689689
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e)
690690
return nil, e
691691
}
692-
targetURL := fmt.Sprintf("%s://%s", u.Scheme, u.Host)
693-
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTargetURL(targetURL), secrets.WithTLSSystemCertPool())
692+
authMethods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLSSystemCertPool())
694693
if err != nil {
695694
return nil, err
696695
}

internal/controller/helmrepository_controller_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
474474
repoURL, err := repository.NormalizeURL(serverURL)
475475
t.Expect(err).ToNot(HaveOccurred())
476476

477-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
477+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
478478
t.Expect(err).ToNot(HaveOccurred())
479479

480480
getterOpts := []helmgetter.Option{
@@ -526,7 +526,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
526526
repoURL, err := repository.NormalizeURL(serverURL)
527527
t.Expect(err).ToNot(HaveOccurred())
528528

529-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
529+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
530530
t.Expect(err).ToNot(HaveOccurred())
531531

532532
getterOpts := []helmgetter.Option{
@@ -580,7 +580,7 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
580580
repoURL, err := repository.NormalizeURL(serverURL)
581581
t.Expect(err).ToNot(HaveOccurred())
582582

583-
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL)
583+
tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret)
584584
t.Expect(err).ToNot(HaveOccurred())
585585

586586
getterOpts := []helmgetter.Option{

internal/controller/ocirepository_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev
10071007
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
10081008
// This ensures source-controller continues to work with both system and user-provided CA certificates.
10091009
var tlsOpts = []secrets.TLSConfigOption{secrets.WithSystemCertPool()}
1010-
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, tlsOpts...)
1010+
return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, tlsOpts...)
10111011
}
10121012

10131013
// reconcileStorage ensures the current state of the storage matches the

internal/helm/getter/client_opts.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
127127
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
128128
// This ensures HelmRepository continues to work with both system and user-provided CA certificates.
129129
var tlsOpts = []secrets.TLSConfigOption{secrets.WithSystemCertPool()}
130-
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, tlsOpts...)
130+
tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, tlsOpts...)
131131
if err != nil {
132132
return false, nil, nil, fmt.Errorf("failed to construct Helm client's TLS config: %w", err)
133133
}
@@ -148,7 +148,6 @@ func configureAuthentication(ctx context.Context, c client.Client, obj *sourcev1
148148
// extend approach (system CAs + user CA) rather than the default replace approach (user CA only).
149149
// This ensures HelmRepository auth methods work with both system and user-provided CA certificates.
150150
var authOpts = []secrets.AuthMethodsOption{
151-
secrets.WithTargetURL(obj.Spec.URL),
152151
secrets.WithTLSSystemCertPool(),
153152
}
154153
methods, err := secrets.AuthMethodsFromSecret(ctx, secret, authOpts...)

internal/helm/getter/client_opts_test.go

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -298,49 +298,3 @@ func TestGetClientOpts_registryTLSLoginOption(t *testing.T) {
298298
})
299299
}
300300
}
301-
302-
func TestConfigureAuthentication_WithTargetURL(t *testing.T) {
303-
g := NewWithT(t)
304-
305-
tlsCA, err := os.ReadFile("../../controller/testdata/certs/ca.pem")
306-
if err != nil {
307-
t.Errorf("could not read CA file: %s", err)
308-
return
309-
}
310-
311-
helmRepo := &helmv1.HelmRepository{
312-
ObjectMeta: metav1.ObjectMeta{
313-
Name: "test-repo",
314-
Namespace: "default",
315-
},
316-
Spec: helmv1.HelmRepositorySpec{
317-
URL: "https://example.com/charts",
318-
},
319-
}
320-
321-
secret := &corev1.Secret{
322-
ObjectMeta: metav1.ObjectMeta{
323-
Name: "auth-secret",
324-
Namespace: "default",
325-
},
326-
Data: map[string][]byte{
327-
"username": []byte("testuser"),
328-
"password": []byte("testpass"),
329-
"ca.crt": tlsCA,
330-
},
331-
}
332-
333-
client := fakeclient.NewClientBuilder().WithObjects(secret).Build()
334-
helmRepo.Spec.SecretRef = &meta.LocalObjectReference{Name: secret.Name}
335-
336-
opts := &ClientOpts{}
337-
deprecatedTLS, certSecret, authSecret, err := configureAuthentication(context.TODO(), client, helmRepo, opts, helmRepo.Spec.URL)
338-
g.Expect(err).ToNot(HaveOccurred())
339-
g.Expect(deprecatedTLS).To(BeTrue()) // TLS from SecretRef is deprecated
340-
g.Expect(certSecret).To(BeNil())
341-
g.Expect(authSecret).To(Equal(secret))
342-
343-
// Regression test: verify ServerName is set from target URL when WithTargetURL is used
344-
g.Expect(opts.TlsConfig).ToNot(BeNil())
345-
g.Expect(opts.TlsConfig.ServerName).To(Equal("example.com"))
346-
}

0 commit comments

Comments
 (0)