From d19d5eaff6105eacd8e8f2176dd0eb11d508878d Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 5 May 2023 18:40:21 +0200 Subject: [PATCH] Store digest of latest image in ImagePolicy's status The new API field `.status.latestDigest` in the `ImagePolicy` kind stores the digest of the image referred to by the the `.status.latestImage` field. This new field can be used to pin an image to an immutable descriptor rather than to a potentially moving tag, increasing the security of workloads deployed on a cluster. The goal is to make use of the digest in IAC so that manifests can be updated with the actual image digest. Signed-off-by: Max Jonas Werner --- api/v1beta2/imagepolicy_types.go | 4 + ...image.toolkit.fluxcd.io_imagepolicies.yaml | 4 + docs/api/image-reflector.md | 13 + docs/spec/v1beta2/imagepolicies.md | 38 +- .../controllers/imagepolicy_controller.go | 32 +- .../controllers/imagerepository_controller.go | 138 +------ .../imagerepository_controller_test.go | 258 +------------ internal/controllers/policy_test.go | 5 +- internal/controllers/suite_test.go | 18 +- internal/registry/helper.go | 51 +++ internal/registry/helper_test.go | 342 ++++++++++++++++++ internal/registry/options.go | 124 +++++++ main.go | 15 +- 13 files changed, 639 insertions(+), 403 deletions(-) create mode 100644 internal/registry/helper.go create mode 100644 internal/registry/helper_test.go create mode 100644 internal/registry/options.go diff --git a/api/v1beta2/imagepolicy_types.go b/api/v1beta2/imagepolicy_types.go index a7369a77..672d3665 100644 --- a/api/v1beta2/imagepolicy_types.go +++ b/api/v1beta2/imagepolicy_types.go @@ -105,6 +105,10 @@ type ImagePolicyStatus struct { // the image repository, when filtered and ordered according to // the policy. LatestImage string `json:"latestImage,omitempty"` + // LatestDigest is the digest of the latest image stored in the + // accompanying LatestImage field. + // +optional + LatestDigest string `json:"latestDigest,omitempty"` // ObservedPreviousImage is the observed previous LatestImage. It is used // to keep track of the previous and current images. // +optional diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml index 4e6ec8af..4c09094a 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml @@ -383,6 +383,10 @@ spec: - type type: object type: array + latestDigest: + description: LatestDigest is the digest of the latest image stored + in the accompanying LatestImage field. + type: string latestImage: description: LatestImage gives the first in the list of images scanned by the image repository, when filtered and ordered according to diff --git a/docs/api/image-reflector.md b/docs/api/image-reflector.md index f4eeceeb..b0b81091 100644 --- a/docs/api/image-reflector.md +++ b/docs/api/image-reflector.md @@ -313,6 +313,19 @@ the policy.

+latestDigest
+ +string + + + +(Optional) +

LatestDigest is the digest of the latest image stored in the +accompanying LatestImage field.

+ + + + observedPreviousImage
string diff --git a/docs/spec/v1beta2/imagepolicies.md b/docs/spec/v1beta2/imagepolicies.md index e0685a59..b804525c 100644 --- a/docs/spec/v1beta2/imagepolicies.md +++ b/docs/spec/v1beta2/imagepolicies.md @@ -36,7 +36,8 @@ In the above example: are then used to select the latest tag based on the policy defined in `.spec.policy`. - The latest image is constructed with the ImageRepository image and the - selected tag, and reported in the `.status.latestImage`. + selected tag, and reported in the `.status.latestImage` field. +- The selected tag's digest is reported in the `.status.latestDigest` field. This example can be run by saving the manifest into `imagepolicy.yaml`. @@ -65,6 +66,7 @@ Status: Reason: Succeeded Status: True Type: Ready + Latest Digest: sha256:2d9a00b3981628a533ff43352193b1838b0a4bf6b0033444286f563205e51a2c Latest Image: ghcr.io/stefanprodan/podinfo:5.1.4 Observed Generation: 1 Events: @@ -331,7 +333,7 @@ specific ImagePolicy, e.g. ### Latest Image -The ImagePolicy reports the latest select image from the ImageRepository tags in +The ImagePolicy reports the latest selected image from the ImageRepository tags in `.status.latestImage` for the resource. Example: @@ -344,8 +346,40 @@ metadata: name: status: latestImage: ghcr.io/stefanprodan/podinfo:5.1.4 +[...] ``` +### Latest Digest + +The ImagePolicy reports the digest value of the latest selected image from the ImageRepoistory tags +in `.status.latestDigest` for the resource. Image digests are an immutable reference to a certain image +and allow for a stricter policy to be applied in comparison to tags which are mutable. + +Example: + +```yaml +--- +apiVersion: image.toolkit.fluxcd.io/v1beta2 +kind: ImagePolicy +metadata: + name: +status: + latestDigest: sha256:2d9a00b3981628a533ff43352193b1838b0a4bf6b0033444286f563205e51a2c +[...] +``` + +{{% alert color="warning" %}} +:warning: Note that image-reflector-controller will not update the digest in case the tag is mutated to point to a +different image version. If you really need to update the latest digest, set the field to an empty value and trigger +another reconciliation: + +```sh +$ k patch imagepolicy podinfo --subresource=status --type=merge -p '{"status":{"latestDigest": null}}' +imagepolicy.image.toolkit.fluxcd.io/podinfo patched +$ flux reconcile image repository -n default podinfo +``` +{{% /alert %}} + ### Observed Previous Image The ImagePolicy reports the previously observed latest image in diff --git a/internal/controllers/imagepolicy_controller.go b/internal/controllers/imagepolicy_controller.go index 4e4b7f77..29bdef43 100644 --- a/internal/controllers/imagepolicy_controller.go +++ b/internal/controllers/imagepolicy_controller.go @@ -20,9 +20,11 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -49,6 +51,7 @@ import ( imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" "github.com/fluxcd/image-reflector-controller/internal/policy" + "github.com/fluxcd/image-reflector-controller/internal/registry" ) // errAccessDenied is returned when an ImageRepository reference in ImagePolicy @@ -110,6 +113,7 @@ type ImagePolicyReconciler struct { ControllerName string Database DatabaseReader ACLOptions acl.Options + RegistryHelper registry.Helper patchOptions []patch.Option } @@ -258,7 +262,7 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP } // Cleanup the last result. - obj.Status.LatestImage = "" + obj.Status.LatestImage, obj.Status.LatestDigest = "", "" // Get ImageRepository from reference. repo, err := r.getImageRepository(ctx, obj) @@ -327,6 +331,15 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP if oldObj.Status.LatestImage != obj.Status.LatestImage { obj.Status.ObservedPreviousImage = oldObj.Status.LatestImage } + + // update the digest only when the tag changed or when the digest hasn't been set, yet (or has been reset manually)2 + if oldObj.Status.LatestImage != obj.Status.LatestImage || obj.Status.LatestDigest == "" { + obj.Status.LatestDigest, err = r.fetchDigest(ctx, repo, latest, obj) + if err != nil { + result, retErr = ctrl.Result{}, fmt.Errorf("failed fetching digest of %s: %w", obj.Status.LatestImage, err) + return + } + } // Parse the observed previous image if any and extract previous tag. This // is used to determine image tag update path. if obj.Status.ObservedPreviousImage != "" { @@ -348,6 +361,23 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP return } +func (r *ImagePolicyReconciler) fetchDigest(ctx context.Context, repo *imagev1.ImageRepository, latest string, obj *imagev1.ImagePolicy) (string, error) { + ref := strings.Join([]string{repo.Spec.Image, latest}, ":") + tagRef, err := name.ParseReference(ref) + if err != nil { + return "", fmt.Errorf("failed parsing reference %q: %w", ref, err) + } + opts, err := r.RegistryHelper.GetAuthOptions(ctx, *repo) + if err != nil { + return "", fmt.Errorf("failed to configure authentication options: %w", err) + } + desc, err := remote.Head(tagRef, opts...) + if err != nil { + return "", fmt.Errorf("failed fetching descriptor for %q: %w", tagRef.String(), err) + } + return desc.Digest.String(), nil +} + // getImageRepository tries to fetch an ImageRepository referenced by the given // ImagePolicy if it's accessible. func (r *ImagePolicyReconciler) getImageRepository(ctx context.Context, obj *imagev1.ImagePolicy) (*imagev1.ImageRepository, error) { diff --git a/internal/controllers/imagerepository_controller.go b/internal/controllers/imagerepository_controller.go index b19ed92a..39a79b61 100644 --- a/internal/controllers/imagerepository_controller.go +++ b/internal/controllers/imagerepository_controller.go @@ -22,18 +22,14 @@ import ( "fmt" "regexp" "sort" - "strings" "time" - "github.com/google/go-containerregistry/pkg/authn" - "github.com/google/go-containerregistry/pkg/authn/k8schain" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" kuberecorder "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -45,8 +41,6 @@ import ( eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" "github.com/fluxcd/pkg/apis/meta" - "github.com/fluxcd/pkg/oci" - "github.com/fluxcd/pkg/oci/auth/login" "github.com/fluxcd/pkg/runtime/conditions" helper "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/patch" @@ -54,7 +48,7 @@ import ( "github.com/fluxcd/pkg/runtime/reconcile" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/secret" + "github.com/fluxcd/image-reflector-controller/internal/registry" ) // latestTagsCount is the number of tags to use as latest tags. @@ -112,7 +106,8 @@ type ImageRepositoryReconciler struct { DatabaseWriter DatabaseReader } - DeprecatedLoginOpts login.ProviderOptions + + RegistryHelper registry.Helper patchOptions []patch.Option } @@ -253,7 +248,7 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser } // Parse image reference. - ref, err := parseImageReference(obj.Spec.Image) + ref, err := registry.ParseImageReference(obj.Spec.Image) if err != nil { conditions.MarkStalled(obj, imagev1.ImageURLInvalidReason, err.Error()) result, retErr = ctrl.Result{}, nil @@ -261,7 +256,7 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser } conditions.Delete(obj, meta.StalledCondition) - opts, err := r.setAuthOptions(ctx, obj, ref) + opts, err := r.RegistryHelper.GetAuthOptions(ctx, *obj) if err != nil { e := fmt.Errorf("failed to configure authentication options: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, imagev1.AuthenticationFailedReason, e.Error()) @@ -327,107 +322,6 @@ func (r *ImageRepositoryReconciler) reconcile(ctx context.Context, sp *patch.Ser return } -// setAuthOptions returns authentication options required to scan a repository. -func (r *ImageRepositoryReconciler) setAuthOptions(ctx context.Context, obj *imagev1.ImageRepository, ref name.Reference) ([]remote.Option, error) { - timeout := obj.GetTimeout() - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - - // Configure authentication strategy to access the registry. - var options []remote.Option - var authSecret corev1.Secret - var auth authn.Authenticator - var authErr error - - if obj.Spec.SecretRef != nil { - if err := r.Get(ctx, types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.SecretRef.Name, - }, &authSecret); err != nil { - return nil, err - } - auth, authErr = secret.AuthFromSecret(authSecret, ref) - } else { - // Build login provider options and use it to attempt registry login. - opts := login.ProviderOptions{} - switch obj.GetProvider() { - case "aws": - opts.AwsAutoLogin = true - case "azure": - opts.AzureAutoLogin = true - case "gcp": - opts.GcpAutoLogin = true - default: - opts = r.DeprecatedLoginOpts - } - auth, authErr = login.NewManager().Login(ctx, obj.Spec.Image, ref, opts) - } - if authErr != nil { - // If it's not unconfigured provider error, abort reconciliation. - // Continue reconciliation if it's unconfigured providers for scanning - // public repositories. - if !errors.Is(authErr, oci.ErrUnconfiguredProvider) { - return nil, authErr - } - } - if auth != nil { - options = append(options, remote.WithAuth(auth)) - } - - // Load any provided certificate. - if obj.Spec.CertSecretRef != nil { - var certSecret corev1.Secret - if obj.Spec.SecretRef != nil && obj.Spec.SecretRef.Name == obj.Spec.CertSecretRef.Name { - certSecret = authSecret - } else { - if err := r.Get(ctx, types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.CertSecretRef.Name, - }, &certSecret); err != nil { - return nil, err - } - } - - tr, err := secret.TransportFromSecret(&certSecret) - if err != nil { - return nil, err - } - options = append(options, remote.WithTransport(tr)) - } - - if obj.Spec.ServiceAccountName != "" { - serviceAccount := corev1.ServiceAccount{} - // Lookup service account - if err := r.Get(ctx, types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.ServiceAccountName, - }, &serviceAccount); err != nil { - return nil, err - } - - 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: obj.GetNamespace(), - Name: ips.Name, - }, &saAuthSecret); err != nil { - return nil, err - } - imagePullSecrets[i] = saAuthSecret - } - keychain, err := k8schain.NewFromPullSecrets(ctx, imagePullSecrets) - if err != nil { - return nil, err - } - options = append(options, remote.WithAuthFromKeychain(keychain)) - } - } - - return options, nil -} - // shouldScan takes an image repo and the time now, and returns whether // the repository should be scanned now, and how long to wait for the // next scan. It also returns the reason for the scan. @@ -462,7 +356,7 @@ func (r *ImageRepositoryReconciler) shouldScan(obj imagev1.ImageRepository, now // If the canonical image name of the image is different from the last // observed name, scan now. - ref, err := parseImageReference(obj.Spec.Image) + ref, err := registry.ParseImageReference(obj.Spec.Image) if err != nil { return false, scanInterval, "", err } @@ -563,26 +457,6 @@ func eventLogf(ctx context.Context, r kuberecorder.EventRecorder, obj runtime.Ob r.Eventf(obj, eventType, reason, msg) } -// parseImageReference parses the given URL into a container registry repository -// reference. -func parseImageReference(url string) (name.Reference, error) { - if s := strings.Split(url, "://"); len(s) > 1 { - return nil, fmt.Errorf(".spec.image value should not start with URL scheme; remove '%s://'", s[0]) - } - - ref, err := name.ParseReference(url) - if err != nil { - return nil, err - } - - imageName := strings.TrimPrefix(url, ref.Context().RegistryStr()) - if s := strings.Split(imageName, ":"); len(s) > 1 { - return nil, fmt.Errorf(".spec.image value should not contain a tag; remove ':%s'", s[1]) - } - - return ref, nil -} - // filterOutTags filters the given tags through the given regular expression // patterns and returns a list of tags that don't match with the pattern. func filterOutTags(tags []string, patterns []string) ([]string, error) { diff --git a/internal/controllers/imagerepository_controller_test.go b/internal/controllers/imagerepository_controller_test.go index 4c3087a5..9841be13 100644 --- a/internal/controllers/imagerepository_controller_test.go +++ b/internal/controllers/imagerepository_controller_test.go @@ -22,20 +22,16 @@ import ( "testing" "time" - "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/secret" + "github.com/fluxcd/image-reflector-controller/internal/registry" "github.com/fluxcd/image-reflector-controller/internal/test" ) @@ -63,213 +59,6 @@ func (db mockDatabase) Tags(repo string) ([]string, error) { return db.TagData, nil } -func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) { - testImg := "example.com/foo/bar" - testSecretName := "test-secret" - testTLSSecretName := "test-tls-secret" - testServiceAccountName := "test-service-account" - testNamespace := "test-ns" - - dockerconfigjson := []byte(` -{ - "auths": { - "example.com": { - "username": "user", - "password": "pass" - } - } -}`) - - testSecret := &corev1.Secret{} - testSecret.Name = testSecretName - testSecret.Namespace = testNamespace - testSecret.Type = corev1.SecretTypeDockerConfigJson - testSecret.Data = map[string][]byte{".dockerconfigjson": dockerconfigjson} - g := NewWithT(t) - - // Create a test TLS server to get valid cert data. The server is never - // started or used below. - _, rootCertPEM, clientCertPEM, clientKeyPEM, _, err := test.CreateTLSServer() - g.Expect(err).To(Not(HaveOccurred())) - - testTLSSecret := &corev1.Secret{} - testTLSSecret.Name = testTLSSecretName - testTLSSecret.Namespace = testNamespace - testTLSSecret.Type = corev1.SecretTypeTLS - testTLSSecret.Data = map[string][]byte{ - secret.CACert: rootCertPEM, - secret.ClientCert: clientCertPEM, - secret.ClientKey: clientKeyPEM, - } - - // Secret with docker config and TLS secrets. - testSecretWithTLS := testSecret.DeepCopy() - testSecretWithTLS.Data = map[string][]byte{ - ".dockerconfigjson": dockerconfigjson, - secret.CACert: rootCertPEM, - secret.ClientCert: clientCertPEM, - secret.ClientKey: clientKeyPEM, - } - - // ServiceAccount without image pull secret. - testServiceAccount := &corev1.ServiceAccount{} - testServiceAccount.Name = testServiceAccountName - testServiceAccount.Namespace = testNamespace - - // ServiceAccount with image pull secret. - testServiceAccountWithSecret := testServiceAccount.DeepCopy() - testServiceAccountWithSecret.ImagePullSecrets = []corev1.LocalObjectReference{{Name: testSecretName}} - - tests := []struct { - name string - mockObjs []client.Object - imageRepoSpec imagev1.ImageRepositorySpec - wantErr bool - }{ - { - name: "no auth options", - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - }, - }, - { - name: "secret ref with existing secret", - mockObjs: []client.Object{testSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - SecretRef: &meta.LocalObjectReference{ - Name: testSecretName, - }, - }, - }, - { - name: "secret ref with non-existing secret", - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - SecretRef: &meta.LocalObjectReference{ - Name: "non-existing-secret", - }, - }, - wantErr: true, - }, - { - name: "contextual login", - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: "123456789000.dkr.ecr.us-east-2.amazonaws.com/test", - Provider: "aws", - }, - wantErr: true, - }, - { - name: "cloud provider repo without login", - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: "123456789000.dkr.ecr.us-east-2.amazonaws.com/test", - }, - }, - { - name: "cert secret ref with existing secret", - mockObjs: []client.Object{testTLSSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - CertSecretRef: &meta.LocalObjectReference{ - Name: testTLSSecretName, - }, - }, - }, - { - name: "cert secret ref with non-existing secret", - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - CertSecretRef: &meta.LocalObjectReference{ - Name: "non-existing-secret", - }, - }, - wantErr: true, - }, - { - name: "secret ref and cert secret ref", - mockObjs: []client.Object{testSecret, testTLSSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - SecretRef: &meta.LocalObjectReference{ - Name: testSecretName, - }, - CertSecretRef: &meta.LocalObjectReference{ - Name: testTLSSecretName, - }, - }, - }, - { - name: "same secret ref and cert secret ref", - mockObjs: []client.Object{testSecretWithTLS}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - SecretRef: &meta.LocalObjectReference{ - Name: testSecretName, - }, - CertSecretRef: &meta.LocalObjectReference{ - Name: testSecretName, - }, - }, - }, - { - name: "service account without pull secret", - mockObjs: []client.Object{testServiceAccount}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - ServiceAccountName: testServiceAccountName, - }, - }, - { - name: "service account with pull secret", - mockObjs: []client.Object{testServiceAccountWithSecret, testSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - ServiceAccountName: testServiceAccountName, - }, - }, - { - name: "service account with non-existing pull secret", - mockObjs: []client.Object{testServiceAccountWithSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - ServiceAccountName: testServiceAccountName, - }, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - clientBuilder := fake.NewClientBuilder() - clientBuilder.WithObjects(tt.mockObjs...) - - r := &ImageRepositoryReconciler{ - EventRecorder: record.NewFakeRecorder(32), - Client: clientBuilder.Build(), - patchOptions: getPatchOptions(imageRepositoryOwnedConditions, "irc"), - } - - obj := &imagev1.ImageRepository{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "reconcile-repo-", - Generation: 1, - Namespace: testNamespace, - }, - } - obj.Spec = tt.imageRepoSpec - - ref, err := name.ParseReference(obj.Spec.Image) - g.Expect(err).ToNot(HaveOccurred()) - - _, err = r.setAuthOptions(ctx, obj, ref) - g.Expect(err != nil).To(Equal(tt.wantErr)) - }) - } -} - func TestImageRepositoryReconciler_shouldScan(t *testing.T) { testImage := "example.com/foo/bar" tests := []struct { @@ -527,7 +316,7 @@ func TestImageRepositoryReconciler_scan(t *testing.T) { repo.SetAnnotations(map[string]string{meta.ReconcileRequestAnnotation: tt.annotation}) } - ref, err := parseImageReference(imgRepo) + ref, err := registry.ParseImageReference(imgRepo) g.Expect(err).ToNot(HaveOccurred()) opts := []remote.Option{} @@ -603,49 +392,6 @@ func TestGetLatestTags(t *testing.T) { } } -func TestParseImageReference(t *testing.T) { - tests := []struct { - name string - url string - wantErr bool - wantRef string - }{ - { - name: "simple valid url", - url: "example.com/foo/bar", - wantRef: "example.com/foo/bar", - }, - { - name: "with scheme prefix", - url: "https://example.com/foo/bar", - wantErr: true, - }, - { - name: "with tag", - url: "example.com/foo/bar:baz", - wantErr: true, - }, - { - name: "with host port", - url: "example.com:9999/foo/bar", - wantErr: false, - wantRef: "example.com:9999/foo/bar", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - - ref, err := parseImageReference(tt.url) - g.Expect(err != nil).To(Equal(tt.wantErr)) - if err == nil { - g.Expect(ref.String()).To(Equal(tt.wantRef)) - } - }) - } -} - func TestFilterOutTags(t *testing.T) { tests := []struct { name string diff --git a/internal/controllers/policy_test.go b/internal/controllers/policy_test.go index 2f3cb9ac..78c899aa 100644 --- a/internal/controllers/policy_test.go +++ b/internal/controllers/policy_test.go @@ -219,9 +219,12 @@ func TestImagePolicyReconciler_calculateImageFromRepoTags(t *testing.T) { if !tt.wantFailure { g.Eventually(func() bool { err := testEnv.Get(ctx, polName, &pol) - return err == nil && pol.Status.LatestImage != "" + return err == nil && + pol.Status.LatestImage != "" && + pol.Status.LatestDigest != "" }, timeout, interval).Should(BeTrue()) g.Expect(pol.Status.LatestImage).To(Equal(imgRepo + tt.wantImageTag)) + g.Expect(pol.Status.LatestDigest).To(MatchRegexp("^sha256:[a-z0-9]{64}")) } else { g.Eventually(func() bool { err := testEnv.Get(ctx, polName, &pol) diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 171fa1d5..2caff0be 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -30,11 +30,13 @@ import ( "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" + "github.com/fluxcd/pkg/oci/auth/login" "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/testenv" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" "github.com/fluxcd/image-reflector-controller/internal/database" + "github.com/fluxcd/image-reflector-controller/internal/registry" // +kubebuilder:scaffold:imports ) @@ -81,10 +83,13 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to create new Badger database: %v", err)) } + regHelper := registry.NewDefaultHelper(testEnv, login.ProviderOptions{}) + if err = (&ImageRepositoryReconciler{ - Client: testEnv, - Database: database.NewBadgerDatabase(testBadgerDB), - EventRecorder: record.NewFakeRecorder(256), + Client: testEnv, + Database: database.NewBadgerDatabase(testBadgerDB), + EventRecorder: record.NewFakeRecorder(256), + RegistryHelper: regHelper, }).SetupWithManager(testEnv, ImageRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { @@ -92,9 +97,10 @@ func TestMain(m *testing.M) { } if err = (&ImagePolicyReconciler{ - Client: testEnv, - Database: database.NewBadgerDatabase(testBadgerDB), - EventRecorder: record.NewFakeRecorder(256), + Client: testEnv, + Database: database.NewBadgerDatabase(testBadgerDB), + EventRecorder: record.NewFakeRecorder(256), + RegistryHelper: regHelper, }).SetupWithManager(testEnv, ImagePolicyReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { diff --git a/internal/registry/helper.go b/internal/registry/helper.go new file mode 100644 index 00000000..af3b736e --- /dev/null +++ b/internal/registry/helper.go @@ -0,0 +1,51 @@ +package registry + +import ( + "context" + "fmt" + "strings" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/pkg/oci/auth/login" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1/remote" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type Helper interface { + GetAuthOptions(ctx context.Context, obj imagev1.ImageRepository) ([]remote.Option, error) +} + +type DefaultHelper struct { + k8sClient client.Client + DeprecatedLoginOpts login.ProviderOptions +} + +var _ Helper = DefaultHelper{} + +func NewDefaultHelper(c client.Client, deprecatedLoginOpts login.ProviderOptions) DefaultHelper { + return DefaultHelper{ + k8sClient: c, + DeprecatedLoginOpts: deprecatedLoginOpts, + } +} + +// ParseImageReference parses the given URL into a container registry repository +// reference. +func ParseImageReference(url string) (name.Reference, error) { + if s := strings.Split(url, "://"); len(s) > 1 { + return nil, fmt.Errorf(".spec.image value should not start with URL scheme; remove '%s://'", s[0]) + } + + ref, err := name.ParseReference(url) + if err != nil { + return nil, err + } + + imageName := strings.TrimPrefix(url, ref.Context().RegistryStr()) + if s := strings.Split(imageName, ":"); len(s) > 1 { + return nil, fmt.Errorf(".spec.image value should not contain a tag; remove ':%s'", s[1]) + } + + return ref, nil +} diff --git a/internal/registry/helper_test.go b/internal/registry/helper_test.go new file mode 100644 index 00000000..e45178b3 --- /dev/null +++ b/internal/registry/helper_test.go @@ -0,0 +1,342 @@ +package registry_test + +import ( + "context" + "testing" + + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/oci/auth/login" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + 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" +) + +func TestDefaultHelperAuthOptions(t *testing.T) { + testImg := "example.com/foo/bar" + + testSecretName := "test-secret" + testTLSSecretName := "test-tls-secret" + testServiceAccountName := "test-service-account" + testNamespace := "test-ns" + + dockerconfigjson := []byte(` +{ + "auths": { + "example.com": { + "username": "user", + "password": "pass" + } + } +}`) + + testSecret := &corev1.Secret{} + testSecret.Name = testSecretName + testSecret.Namespace = testNamespace + testSecret.Type = corev1.SecretTypeDockerConfigJson + testSecret.Data = map[string][]byte{".dockerconfigjson": dockerconfigjson} + g := NewWithT(t) + + // Create a test TLS server to get valid cert data. The server is never + // started or used below. + _, rootCertPEM, clientCertPEM, clientKeyPEM, _, err := test.CreateTLSServer() + g.Expect(err).To(Not(HaveOccurred())) + + testTLSSecret := &corev1.Secret{} + testTLSSecret.Name = testTLSSecretName + testTLSSecret.Namespace = testNamespace + testTLSSecret.Type = corev1.SecretTypeTLS + testTLSSecret.Data = map[string][]byte{ + secret.CACert: rootCertPEM, + secret.ClientCert: clientCertPEM, + secret.ClientKey: clientKeyPEM, + } + + // Secret with docker config and TLS secrets. + testSecretWithTLS := testSecret.DeepCopy() + testSecretWithTLS.Data = map[string][]byte{ + ".dockerconfigjson": dockerconfigjson, + secret.CACert: rootCertPEM, + secret.ClientCert: clientCertPEM, + secret.ClientKey: clientKeyPEM, + } + + // ServiceAccount without image pull secret. + testServiceAccount := &corev1.ServiceAccount{} + testServiceAccount.Name = testServiceAccountName + testServiceAccount.Namespace = testNamespace + + // ServiceAccount with image pull secret. + testServiceAccountWithSecret := testServiceAccount.DeepCopy() + testServiceAccountWithSecret.ImagePullSecrets = []corev1.LocalObjectReference{{Name: testSecretName}} + + tests := []struct { + name string + repo imagev1.ImageRepository + k8sObjs []client.Object + expectErr bool + expectOpts int + }{ + { + name: "fails with empty image reference", + repo: imagev1.ImageRepository{}, + expectErr: true, + expectOpts: 0, + }, + { + name: "succeeds with no auth options", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + }, + }, + expectErr: false, + expectOpts: 0, + }, + { + name: "succeeds with ECR image ref", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + Image: "123456789000.dkr.ecr.us-east-2.amazonaws.com/test", + }, + }, + expectErr: false, + expectOpts: 0, + }, + { + name: "fails with contextual login but no auth credentials", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + Image: "123456789000.dkr.ecr.us-east-2.amazonaws.com/test", + Provider: "aws", + }, + }, + expectErr: true, + expectOpts: 0, + }, + { + name: "fails with missing secret", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + SecretRef: &meta.LocalObjectReference{ + Name: "does-not-exist", + }, + }, + }, + expectErr: true, + expectOpts: 0, + }, + { + name: "fails with wrong secret type", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + SecretRef: &meta.LocalObjectReference{ + Name: "registry-auth", + }, + }, + }, + k8sObjs: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry-auth", + }, + Type: corev1.SecretTypeOpaque, + }, + }, + expectErr: true, + expectOpts: 0, + }, + { + name: "fails with empty secret data", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + SecretRef: &meta.LocalObjectReference{ + Name: "registry-auth", + }, + }, + }, + k8sObjs: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "registry-auth", + }, + Type: corev1.SecretTypeDockerConfigJson, + }, + }, + expectErr: true, + expectOpts: 0, + }, + { + name: "adds authenticator from secret", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + SecretRef: &meta.LocalObjectReference{ + Name: testSecretName, + }, + }, + }, + k8sObjs: []client.Object{testSecret}, + expectErr: false, + expectOpts: 1, + }, + { + name: "fails with non-existing cert secret ref", + repo: imagev1.ImageRepository{ + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + CertSecretRef: &meta.LocalObjectReference{ + Name: "non-existing-secret", + }, + }, + }, + expectErr: true, + expectOpts: 0, + }, + { + name: "sets transport from cert secret ref", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + CertSecretRef: &meta.LocalObjectReference{ + Name: testTLSSecretName, + }, + }, + }, + k8sObjs: []client.Object{testTLSSecret}, + expectErr: false, + expectOpts: 1, + }, + { + name: "sets transport and auth from secret ref and cert secret ref", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + SecretRef: &meta.LocalObjectReference{ + Name: testSecretName, + }, + CertSecretRef: &meta.LocalObjectReference{ + Name: testTLSSecretName, + }, + }, + }, + k8sObjs: []client.Object{testSecret, testTLSSecret}, + expectErr: false, + expectOpts: 2, + }, + { + name: "sets transport and auth from same secret and cert ref", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + SecretRef: &meta.LocalObjectReference{ + Name: testSecretName, + }, + CertSecretRef: &meta.LocalObjectReference{ + Name: testSecretName, + }, + }, + }, + k8sObjs: []client.Object{testSecretWithTLS}, + expectErr: false, + expectOpts: 2, + }, + { + name: "sets auth option from SA with pull secret", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + ServiceAccountName: testServiceAccountName, + }, + }, + k8sObjs: []client.Object{testSecret, testServiceAccountWithSecret}, + expectErr: false, + expectOpts: 1, + }, + { + name: "fails with SA an non-existing pull secret", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + ServiceAccountName: testServiceAccountName, + }, + }, + k8sObjs: []client.Object{testServiceAccountWithSecret}, + expectErr: true, + expectOpts: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + k8sClient := fake.NewClientBuilder(). + WithObjects(tt.k8sObjs...). + Build() + h := registry.NewDefaultHelper(k8sClient, login.ProviderOptions{}) + + opts, err := h.GetAuthOptions(context.Background(), tt.repo) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(opts).To(HaveLen(tt.expectOpts)) + }) + } +} + +func TestParseImageReference(t *testing.T) { + tests := []struct { + name string + url string + wantErr bool + wantRef string + }{ + { + name: "simple valid url", + url: "example.com/foo/bar", + wantRef: "example.com/foo/bar", + }, + { + name: "with scheme prefix", + url: "https://example.com/foo/bar", + wantErr: true, + }, + { + name: "with tag", + url: "example.com/foo/bar:baz", + wantErr: true, + }, + { + name: "with host port", + url: "example.com:9999/foo/bar", + wantErr: false, + wantRef: "example.com:9999/foo/bar", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ref, err := registry.ParseImageReference(tt.url) + g.Expect(err != nil).To(Equal(tt.wantErr)) + if err == nil { + g.Expect(ref.String()).To(Equal(tt.wantRef)) + } + }) + } +} diff --git a/internal/registry/options.go b/internal/registry/options.go new file mode 100644 index 00000000..b91ed031 --- /dev/null +++ b/internal/registry/options.go @@ -0,0 +1,124 @@ +package registry + +import ( + "context" + "errors" + "fmt" + + "github.com/fluxcd/pkg/oci" + "github.com/fluxcd/pkg/oci/auth/login" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" + "github.com/google/go-containerregistry/pkg/v1/remote" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/secret" +) + +// GetAuthOptions returns authentication options required to scan a repository. +func (h DefaultHelper) GetAuthOptions(ctx context.Context, obj imagev1.ImageRepository) ([]remote.Option, error) { + timeout := obj.GetTimeout() + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + // Configure authentication strategy to access the registry. + var options []remote.Option + var authSecret corev1.Secret + var auth authn.Authenticator + var authErr error + + ref, err := ParseImageReference(obj.Spec.Image) + if err != nil { + return nil, fmt.Errorf("failed parsing image reference: %w", err) + } + + if obj.Spec.SecretRef != nil { + if err := h.k8sClient.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.SecretRef.Name, + }, &authSecret); err != nil { + return nil, err + } + auth, authErr = secret.AuthFromSecret(authSecret, ref) + } else { + // Build login provider options and use it to attempt registry login. + opts := login.ProviderOptions{} + switch obj.GetProvider() { + case "aws": + opts.AwsAutoLogin = true + case "azure": + opts.AzureAutoLogin = true + case "gcp": + opts.GcpAutoLogin = true + default: + opts = h.DeprecatedLoginOpts + } + auth, authErr = login.NewManager().Login(ctx, obj.Spec.Image, ref, opts) + } + if authErr != nil { + // If it's not unconfigured provider error, abort reconciliation. + // Continue reconciliation if it's unconfigured providers for scanning + // public repositories. + if !errors.Is(authErr, oci.ErrUnconfiguredProvider) { + return nil, authErr + } + } + if auth != nil { + options = append(options, remote.WithAuth(auth)) + } + + // Load any provided certificate. + if obj.Spec.CertSecretRef != nil { + var certSecret corev1.Secret + if obj.Spec.SecretRef != nil && obj.Spec.SecretRef.Name == obj.Spec.CertSecretRef.Name { + certSecret = authSecret + } else { + if err := h.k8sClient.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.CertSecretRef.Name, + }, &certSecret); err != nil { + return nil, err + } + } + + tr, err := secret.TransportFromSecret(&certSecret) + if err != nil { + return nil, err + } + options = append(options, remote.WithTransport(tr)) + } + + if obj.Spec.ServiceAccountName != "" { + serviceAccount := corev1.ServiceAccount{} + // Lookup service account + if err := h.k8sClient.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.ServiceAccountName, + }, &serviceAccount); err != nil { + return nil, err + } + + if len(serviceAccount.ImagePullSecrets) > 0 { + imagePullSecrets := make([]corev1.Secret, len(serviceAccount.ImagePullSecrets)) + for i, ips := range serviceAccount.ImagePullSecrets { + var saAuthSecret corev1.Secret + if err := h.k8sClient.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: ips.Name, + }, &saAuthSecret); err != nil { + return nil, err + } + imagePullSecrets[i] = saAuthSecret + } + keychain, err := k8schain.NewFromPullSecrets(ctx, imagePullSecrets) + if err != nil { + return nil, err + } + options = append(options, remote.WithAuthFromKeychain(keychain)) + } + } + + return options, nil +} diff --git a/main.go b/main.go index aae1c762..12239a03 100644 --- a/main.go +++ b/main.go @@ -49,6 +49,7 @@ import ( "github.com/fluxcd/image-reflector-controller/internal/controllers" "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/image-reflector-controller/internal/features" + "github.com/fluxcd/image-reflector-controller/internal/registry" ) const controllerName = "image-reflector-controller" @@ -196,17 +197,20 @@ func main() { metricsH := helper.MustMakeMetrics(mgr) + deprecatedLoginOptions := login.ProviderOptions{ + AwsAutoLogin: awsAutoLogin, + AzureAutoLogin: azureAutoLogin, + GcpAutoLogin: gcpAutoLogin, + } + registryHelper := registry.NewDefaultHelper(mgr.GetClient(), deprecatedLoginOptions) + if err := (&controllers.ImageRepositoryReconciler{ Client: mgr.GetClient(), EventRecorder: eventRecorder, Metrics: metricsH, Database: db, ControllerName: controllerName, - DeprecatedLoginOpts: login.ProviderOptions{ - AwsAutoLogin: awsAutoLogin, - AzureAutoLogin: azureAutoLogin, - GcpAutoLogin: gcpAutoLogin, - }, + RegistryHelper: registryHelper, }).SetupWithManager(mgr, controllers.ImageRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -221,6 +225,7 @@ func main() { Database: db, ACLOptions: aclOptions, ControllerName: controllerName, + RegistryHelper: registryHelper, }).SetupWithManager(mgr, controllers.ImagePolicyReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions),