Merge pull request #791 from cappyzawa/feat-runtime-secrets-migration-pr1

Migrate secrets handling to pkg/runtime/secrets

Author: matheuscscp

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/fluxcd/pkg/apis/meta v1.12.0
 	github.com/fluxcd/pkg/auth v0.18.0
 	github.com/fluxcd/pkg/cache v0.9.0
-	github.com/fluxcd/pkg/runtime v0.60.0
+	github.com/fluxcd/pkg/runtime v0.63.0
 	github.com/fluxcd/pkg/version v0.7.0
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-containerregistry v0.20.5

go.sum

@@ -167,8 +167,8 @@ github.com/fluxcd/pkg/auth v0.18.0 h1:71pGdKe0PVKWQvM3hEuyd3FD9dEUHtMuKMbUeiMl4a
 github.com/fluxcd/pkg/auth v0.18.0/go.mod h1:4h6s8VBNuec3tWd4xIReLw8BYPOKaIegjNMEbA4ikTU=
 github.com/fluxcd/pkg/cache v0.9.0 h1:EGKfOLMG3fOwWnH/4Axl5xd425mxoQbZzlZoLfd8PDk=
 github.com/fluxcd/pkg/cache v0.9.0/go.mod h1:jMwabjWfsC5lW8hE7NM3wtGNwSJ38Javx6EKbEi7INU=
-github.com/fluxcd/pkg/runtime v0.60.0 h1:d++EkV3FlycB+bzakB5NumwY4J8xts8i7lbvD6jBLeU=
-github.com/fluxcd/pkg/runtime v0.60.0/go.mod h1:UeU0/eZLErYC/1bTmgzBfNXhiHy9fuQzjfLK0HxRgxY=
+github.com/fluxcd/pkg/runtime v0.63.0 h1:55J7ascGmXyTXWGwhD21N9fU7jC1l5rhdzjgNXs6aZg=
+github.com/fluxcd/pkg/runtime v0.63.0/go.mod h1:7pxGvaU0Yy1cDIUhiHAHhCx2yCLnkcVsplbYZG6j4JY=
 github.com/fluxcd/pkg/version v0.7.0 h1:jZT5I6WFy1KlM40nHCSqlHmjC1VT1/DfmbAdOkIVVJc=
 github.com/fluxcd/pkg/version v0.7.0/go.mod h1:3BjQDJXIZJmeJLXnfa2yG/sNAT1t5oeLAPfnSjOHNuA=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=

internal/controller/imagepolicy_controller_test.go

@@ -175,14 +175,14 @@ func TestImagePolicyReconciler_ignoresImageRepoNotReadyEvent(t *testing.T) {
 	g.Eventually(func() bool {
 		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
 		return err == nil && conditions.IsReady(imagePolicy)
-	}).Should(BeTrue())
+	}, timeout, interval).Should(BeTrue())
 
 	// Wait a bit and check that the ImagePolicy remains ready.
 	time.Sleep(time.Second)
 	g.Eventually(func() bool {
 		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(imagePolicy), imagePolicy)
 		return err == nil && conditions.IsReady(imagePolicy)
-	}).Should(BeTrue())
+	}, timeout, interval).Should(BeTrue())
 }
 
 func TestImagePolicyReconciler_imageRepoRevisionLifeCycle(t *testing.T) {

internal/controller/imagerepository_controller_test.go

@@ -43,10 +43,10 @@ import (
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/conditions"
 	"github.com/fluxcd/pkg/runtime/patch"
+	"github.com/fluxcd/pkg/runtime/secrets"
 
 	imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2"
 	"github.com/fluxcd/image-reflector-controller/internal/registry"
-	"github.com/fluxcd/image-reflector-controller/internal/secret"
 	"github.com/fluxcd/image-reflector-controller/internal/test"
 )
 
@@ -688,9 +688,9 @@ func TestImageRepositoryReconciler_TLS(t *testing.T) {
 	testTLSSecret.Namespace = testNamespace
 	testTLSSecret.Type = corev1.SecretTypeTLS
 	testTLSSecret.Data = map[string][]byte{
-		secret.CACrtKey:         rootCertPEM,
-		corev1.TLSCertKey:       clientCertPEM,
-		corev1.TLSPrivateKeyKey: clientKeyPEM,
+		secrets.CACertKey:  rootCertPEM,
+		secrets.TLSCertKey: clientCertPEM,
+		secrets.TLSKeyKey:  clientKeyPEM,
 	}
 
 	// Construct ImageRepository.

internal/registry/options.go

@@ -18,7 +18,6 @@ package registry
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"net/url"
 
@@ -27,15 +26,14 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/remote"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/fluxcd/pkg/auth"
 	authutils "github.com/fluxcd/pkg/auth/utils"
 	"github.com/fluxcd/pkg/cache"
+	"github.com/fluxcd/pkg/runtime/secrets"
 
 	imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2"
-	"github.com/fluxcd/image-reflector-controller/internal/secret"
 )
 
 // AuthOptionsGetter builds a slice of options from an ImageRepository by looking up references to Secrets etc.
@@ -65,7 +63,7 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 	var proxyURL *url.URL
 	var err error
 	if repo.Spec.ProxySecretRef != nil {
-		proxyURL, err = r.GetProxyURL(ctx, repo)
+		proxyURL, err = secrets.ProxyURLFromSecret(ctx, r.Client, repo.Spec.ProxySecretRef.Name, repo.Namespace)
 		if err != nil {
 			return nil, err
 		}
@@ -80,22 +78,8 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 	var options []remote.Option
 	var authSecret corev1.Secret
 	var authenticator authn.Authenticator
-	var authErr error
 
-	if repo.Spec.SecretRef != nil {
-		ref, err := ParseImageReference(repo.Spec.Image, repo.Spec.Insecure)
-		if err != nil {
-			return nil, fmt.Errorf("failed parsing image reference %q: %w", repo.Spec.Image, err)
-		}
-
-		if err := r.Get(ctx, types.NamespacedName{
-			Namespace: repo.GetNamespace(),
-			Name:      repo.Spec.SecretRef.Name,
-		}, &authSecret); err != nil {
-			return nil, err
-		}
-		authenticator, authErr = secret.AuthFromSecret(authSecret, ref)
-	} else if provider := repo.GetProvider(); provider != "generic" {
+	if provider := repo.GetProvider(); provider != "" && provider != "generic" {
 		// Build login provider options and use it to attempt registry login.
 		var opts []auth.Option
 		if proxyURL != nil {
@@ -111,12 +95,11 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 		if r.TokenCache != nil {
 			opts = append(opts, auth.WithCache(*r.TokenCache, *involvedObject))
 		}
-		authenticator, authErr = authutils.GetArtifactRegistryCredentials(ctx, provider, repo.Spec.Image, opts...)
-	}
-	if authErr != nil {
-		return nil, authErr
-	}
-	if authenticator != nil {
+		var err error
+		authenticator, err = authutils.GetArtifactRegistryCredentials(ctx, provider, repo.Spec.Image, opts...)
+		if err != nil {
+			return nil, err
+		}
 		options = append(options, remote.WithAuth(authenticator))
 	}
 
@@ -134,20 +117,10 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 			}
 		}
 
-		tlsConfig, err := secret.TLSConfigFromKubeTLSSecret(&certSecret)
+		tlsConfig, err := secrets.TLSConfigFromSecret(ctx, r.Client, certSecret.Name, certSecret.Namespace)
 		if err != nil {
 			return nil, err
 		}
-		if tlsConfig == nil {
-			tlsConfig, err = secret.TLSConfigFromSecret(&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")
-			}
-		}
 		if tlsConfig != nil {
 			transportOptions = append(transportOptions, func(t *http.Transport) {
 				t.TLSClientConfig = tlsConfig
@@ -164,29 +137,32 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 		options = append(options, remote.WithTransport(tr))
 	}
 
-	if authenticator == nil && repo.Spec.ServiceAccountName != "" {
-		serviceAccount := corev1.ServiceAccount{}
-		// Lookup service account
-		if err := r.Get(ctx, types.NamespacedName{
-			Namespace: repo.GetNamespace(),
-			Name:      repo.Spec.ServiceAccountName,
-		}, &serviceAccount); err != nil {
-			return nil, err
+	if authenticator == nil {
+		var pullSecrets []corev1.Secret
+
+		if repo.Spec.SecretRef != nil {
+			var s corev1.Secret
+			key := types.NamespacedName{
+				Name:      repo.Spec.SecretRef.Name,
+				Namespace: repo.GetNamespace(),
+			}
+			if err := r.Get(ctx, key, &s); err != nil {
+				return nil, err
+			}
+			pullSecrets = append(pullSecrets, s)
 		}
 
-		if len(serviceAccount.ImagePullSecrets) > 0 {
-			imagePullSecrets := make([]corev1.Secret, len(serviceAccount.ImagePullSecrets))
-			for i, ips := range serviceAccount.ImagePullSecrets {
-				var saAuthSecret corev1.Secret
-				if err := r.Get(ctx, types.NamespacedName{
-					Namespace: repo.GetNamespace(),
-					Name:      ips.Name,
-				}, &saAuthSecret); err != nil {
-					return nil, err
-				}
-				imagePullSecrets[i] = saAuthSecret
+		if repo.Spec.ServiceAccountName != "" {
+			s, err := secrets.PullSecretsFromServiceAccount(ctx, r.Client,
+				repo.Spec.ServiceAccountName, repo.GetNamespace())
+			if err != nil {
+				return nil, err
 			}
-			keychain, err := k8schain.NewFromPullSecrets(ctx, imagePullSecrets)
+			pullSecrets = append(pullSecrets, s...)
+		}
+
+		if len(pullSecrets) > 0 {
+			keychain, err := k8schain.NewFromPullSecrets(ctx, pullSecrets)
 			if err != nil {
 				return nil, err
 			}
@@ -196,37 +172,3 @@ func (r *AuthOptionsGetter) GetOptions(ctx context.Context, repo *imagev1.ImageR
 
 	return options, nil
 }
-
-// GetProxyURL gets the proxy configuration for the transport based on the
-// specified proxy secret reference in the ImageRepository object.
-func (r *AuthOptionsGetter) GetProxyURL(ctx context.Context, obj *imagev1.ImageRepository) (*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
-}