diff --git a/go.mod b/go.mod index 12a42d0b9..a713b6b8b 100644 --- a/go.mod +++ b/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.73.0 + github.com/fluxcd/pkg/runtime v0.75.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 diff --git a/go.sum b/go.sum index 848fb49fa..e5204a362 100644 --- a/go.sum +++ b/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.73.0 h1:BV3qEwMT3lfHA2lterT3Es62z6EkJr2ST/jkyBmmskQ= -github.com/fluxcd/pkg/runtime v0.73.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw= +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/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= diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index 8beb0850f..c0a5b3357 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/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) + tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false) 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) + tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false) 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) + tlsConfig, err := secrets.TLSConfigFromSecret(context.TODO(), secret, serverURL, false) t.Expect(err).ToNot(HaveOccurred()) getterOpts := []helmgetter.Option{ diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index ed407c201..9d9ca18d6 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -43,7 +43,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/sets" kuberecorder "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" "k8s.io/utils/ptr" @@ -60,6 +59,7 @@ import ( "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/runtime/predicates" rreconcile "github.com/fluxcd/pkg/runtime/reconcile" + "github.com/fluxcd/pkg/runtime/secrets" "github.com/fluxcd/pkg/sourceignore" "github.com/fluxcd/pkg/tar" "github.com/fluxcd/pkg/version" @@ -77,7 +77,6 @@ import ( "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" - "github.com/fluxcd/source-controller/internal/tls" "github.com/fluxcd/source-controller/internal/util" ) @@ -355,14 +354,21 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch return sreconcile.ResultEmpty, e } - proxyURL, err := r.getProxyURL(ctx, obj) - if err != nil { - e := serror.NewGeneric( - fmt.Errorf("failed to get proxy address: %w", err), - sourcev1.AuthenticationFailedReason, - ) - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) - return sreconcile.ResultEmpty, e + var proxyURL *url.URL + if obj.Spec.ProxySecretRef != nil { + var err error + proxyURL, err = secrets.ProxyURLFromSecretRef(ctx, r.Client, types.NamespacedName{ + Name: obj.Spec.ProxySecretRef.Name, + Namespace: obj.GetNamespace(), + }) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to get proxy address: %w", err), + sourcev1.AuthenticationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } } if _, ok := keychain.(soci.Anonymous); obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.GenericOCIProvider && ok { @@ -920,44 +926,36 @@ func (r *OCIRepositoryReconciler) getTagBySemver(repo name.Repository, exp strin // configuration. If no auth is specified a default keychain with // anonymous access is returned func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *sourcev1.OCIRepository) (authn.Keychain, error) { - pullSecretNames := sets.NewString() + var imagePullSecrets []corev1.Secret // lookup auth secret if obj.Spec.SecretRef != nil { - pullSecretNames.Insert(obj.Spec.SecretRef.Name) + var imagePullSecret corev1.Secret + secretRef := types.NamespacedName{Namespace: obj.Namespace, Name: obj.Spec.SecretRef.Name} + err := r.Get(ctx, secretRef, &imagePullSecret) + if err != nil { + r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.AuthenticationFailedReason, + "auth secret '%s' not found", obj.Spec.SecretRef.Name) + return nil, err + } + imagePullSecrets = append(imagePullSecrets, imagePullSecret) } // lookup service account if obj.Spec.ServiceAccountName != "" { - serviceAccountName := obj.Spec.ServiceAccountName - serviceAccount := corev1.ServiceAccount{} - err := r.Get(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: serviceAccountName}, &serviceAccount) + saRef := types.NamespacedName{Namespace: obj.Namespace, Name: obj.Spec.ServiceAccountName} + saSecrets, err := secrets.PullSecretsFromServiceAccountRef(ctx, r.Client, saRef) if err != nil { return nil, err } - for _, ips := range serviceAccount.ImagePullSecrets { - pullSecretNames.Insert(ips.Name) - } + imagePullSecrets = append(imagePullSecrets, saSecrets...) } // if no pullsecrets available return an AnonymousKeychain - if len(pullSecretNames) == 0 { + if len(imagePullSecrets) == 0 { return soci.Anonymous{}, nil } - // lookup image pull secrets - imagePullSecrets := make([]corev1.Secret, len(pullSecretNames)) - for i, imagePullSecretName := range pullSecretNames.List() { - imagePullSecret := corev1.Secret{} - err := r.Get(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: imagePullSecretName}, &imagePullSecret) - if err != nil { - r.eventLogf(ctx, obj, eventv1.EventTypeTrace, sourcev1.AuthenticationFailedReason, - "auth secret '%s' not found", imagePullSecretName) - return nil, err - } - imagePullSecrets[i] = imagePullSecret - } - return k8schain.NewFromPullSecrets(ctx, imagePullSecrets) } @@ -995,65 +993,11 @@ func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *sourcev return nil, nil } - certSecretName := types.NamespacedName{ + secretName := types.NamespacedName{ Namespace: obj.Namespace, Name: obj.Spec.CertSecretRef.Name, } - var certSecret corev1.Secret - if err := r.Get(ctx, certSecretName, &certSecret); err != nil { - return nil, err - } - - tlsConfig, _, err := tls.KubeTLSClientConfigFromSecret(certSecret, "") - if err != nil { - return nil, err - } - if tlsConfig == nil { - tlsConfig, _, err = tls.TLSClientConfigFromSecret(certSecret, "") - if err != nil { - return nil, err - } - if tlsConfig != nil { - ctrl.LoggerFrom(ctx). - Info("warning: specifying TLS auth data via `certFile`/`keyFile`/`caFile` is deprecated, please use `tls.crt`/`tls.key`/`ca.crt` instead") - } - } - - return tlsConfig, nil -} - -// getProxyURL gets the proxy configuration for the transport based on the -// specified proxy secret reference in the OCIRepository object. -func (r *OCIRepositoryReconciler) getProxyURL(ctx context.Context, obj *sourcev1.OCIRepository) (*url.URL, error) { - if obj.Spec.ProxySecretRef == nil || obj.Spec.ProxySecretRef.Name == "" { - return nil, nil - } - - proxySecretName := types.NamespacedName{ - Namespace: obj.Namespace, - Name: obj.Spec.ProxySecretRef.Name, - } - var proxySecret corev1.Secret - if err := r.Get(ctx, proxySecretName, &proxySecret); err != nil { - return nil, err - } - - proxyData := proxySecret.Data - address, ok := proxyData["address"] - if !ok { - return nil, fmt.Errorf("invalid proxy secret '%s/%s': key 'address' is missing", - obj.Namespace, obj.Spec.ProxySecretRef.Name) - } - proxyURL, err := url.Parse(string(address)) - if err != nil { - return nil, fmt.Errorf("failed to parse proxy address '%s': %w", address, err) - } - user, hasUser := proxyData["username"] - password, hasPassword := proxyData["password"] - if hasUser || hasPassword { - proxyURL.User = url.UserPassword(string(user), string(password)) - } - return proxyURL, nil + return secrets.TLSConfigFromSecretRef(ctx, r.Client, secretName, obj.Spec.URL, obj.Spec.Insecure) } // reconcileStorage ensures the current state of the storage matches the diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index aa024082f..fe026cad9 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -644,7 +644,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) { }, }, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "%s", "cannot append certificate into certificate pool: invalid CA certificate"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "%s", "failed to parse CA certificate"), }, }, { @@ -913,7 +913,7 @@ func TestOCIRepository_CertSecret(t *testing.T) { }, }, expectreadyconition: false, - expectedstatusmessage: "failed to generate transport for '': tls: failed to find any PEM data in key input", + expectedstatusmessage: "failed to generate transport for '': failed to parse TLS certificate and key: tls: failed to find any PEM data in key input", }, } @@ -3705,188 +3705,3 @@ func TestOCIContentConfigChanged(t *testing.T) { }) } } - -func TestOCIRepositoryReconciler_getProxyURL(t *testing.T) { - tests := []struct { - name string - ociRepo *sourcev1.OCIRepository - objects []client.Object - expectedURL string - expectedErr string - }{ - { - name: "empty proxySecretRef", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: nil, - }, - }, - }, - { - name: "non-existing proxySecretRef", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "non-existing", - }, - }, - }, - expectedErr: "secrets \"non-existing\" not found", - }, - { - name: "missing address in proxySecretRef", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{}, - }, - }, - expectedErr: "invalid proxy secret '/dummy': key 'address' is missing", - }, - { - name: "invalid address in proxySecretRef", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{ - "address": {0x7f}, - }, - }, - }, - expectedErr: "failed to parse proxy address '\x7f': parse \"\\x7f\": net/url: invalid control character in URL", - }, - { - name: "no user, no password", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{ - "address": []byte("http://proxy.example.com"), - }, - }, - }, - expectedURL: "http://proxy.example.com", - }, - { - name: "user, no password", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{ - "address": []byte("http://proxy.example.com"), - "username": []byte("user"), - }, - }, - }, - expectedURL: "http://user:@proxy.example.com", - }, - { - name: "no user, password", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{ - "address": []byte("http://proxy.example.com"), - "password": []byte("password"), - }, - }, - }, - expectedURL: "http://:password@proxy.example.com", - }, - { - name: "user, password", - ociRepo: &sourcev1.OCIRepository{ - Spec: sourcev1.OCIRepositorySpec{ - ProxySecretRef: &meta.LocalObjectReference{ - Name: "dummy", - }, - }, - }, - objects: []client.Object{ - &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "dummy", - }, - Data: map[string][]byte{ - "address": []byte("http://proxy.example.com"), - "username": []byte("user"), - "password": []byte("password"), - }, - }, - }, - expectedURL: "http://user:password@proxy.example.com", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - c := fakeclient.NewClientBuilder(). - WithScheme(testEnv.Scheme()). - WithObjects(tt.objects...). - Build() - - r := &OCIRepositoryReconciler{ - Client: c, - } - - u, err := r.getProxyURL(ctx, tt.ociRepo) - if tt.expectedErr == "" { - g.Expect(err).To(BeNil()) - } else { - g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) - } - if tt.expectedURL == "" { - g.Expect(u).To(BeNil()) - } else { - g.Expect(u.String()).To(Equal(tt.expectedURL)) - } - }) - } -} diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 0c5eaf0cb..cbcd09d9d 100644 --- a/internal/helm/getter/client_opts.go +++ b/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) + tlsConfig, err := secrets.TLSConfigFromSecret(ctx, secret, obj.Spec.URL, obj.Spec.Insecure) 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) + methods, err := secrets.AuthMethodsFromSecret(ctx, secret, secrets.WithTLS(obj.Spec.URL, obj.Spec.Insecure)) if err != nil { return false, nil, nil, fmt.Errorf("failed to detect authentication methods: %w", err) }