diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index e41e3304..47566a84 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2022 The Flux authors +Copyright 2023 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/api/v1beta2/imagepolicy_types.go b/api/v1beta2/imagepolicy_types.go index ae14cd69..59ff96c1 100644 --- a/api/v1beta2/imagepolicy_types.go +++ b/api/v1beta2/imagepolicy_types.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Flux authors +Copyright 2023 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -42,8 +42,26 @@ type ImagePolicySpec struct { // ordered and compared. // +optional FilterTags *TagFilter `json:"filterTags,omitempty"` + // DigestReflectionPolicy governs the setting of the `.status.latestRef.digest` field. + // +kubebuilder:default:=Never + DigestReflectionPolicy ReflectionPolicy `json:"digestReflectionPolicy,omitempty"` } +// ReflectionPolicy describes a policy for if/when to reflect a value from the registry in a certain resource field. +// +kubebuilder:validation:Enum=Always;IfNotPresent;Never +type ReflectionPolicy string + +const ( + // ReflectAlways means that a value is always reflected with the latest value from the registry even if this would + // overwrite an existing value in the object. + ReflectAlways ReflectionPolicy = "Always" + // ReflectIfNotPresent means that the target value is only reflected from the registry if it is empty. It will + // never be overwritten afterwards, even if it changes in the registry. + ReflectIfNotPresent ReflectionPolicy = "IfNotPresent" + // ReflectNever means that no reflection will happen at all. + ReflectNever ReflectionPolicy = "Never" +) + // ImagePolicyChoice is a union of all the types of policy that can be // supplied. type ImagePolicyChoice struct { @@ -101,16 +119,45 @@ type TagFilter struct { Extract string `json:"extract"` } +// ImageRef represents an image reference. +type ImageRef struct { + // Name is the bare image's name. + Name string `json:"image,omitempty"` + // Tag is the image's tag. + Tag string `json:"tag,omitempty"` + // Digest is the image's digest. + // +optional + Digest string `json:"digest,omitempty"` +} + +func (r ImageRef) String() string { + res := r.Name + ":" + r.Tag + if r.Digest != "" { + res += "@" + r.Digest + } + return res +} + // ImagePolicyStatus defines the observed state of ImagePolicy type ImagePolicyStatus struct { // LatestImage gives the first in the list of images scanned by // the image repository, when filtered and ordered according to // the policy. + // Deprecated: Replaced by the composite "latestRef" field. LatestImage string `json:"latestImage,omitempty"` // ObservedPreviousImage is the observed previous LatestImage. It is used // to keep track of the previous and current images. + // Deprecated: Replaced by the composite "observedPreviousRef" field. // +optional ObservedPreviousImage string `json:"observedPreviousImage,omitempty"` + // LatestRef gives the first in the list of images scanned by + // the image repository, when filtered and ordered according + // to the policy. + LatestRef *ImageRef `json:"latestRef,omitempty"` + // ObservedPreviousRef is the observed previous LatestRef. It is used + // to keep track of the previous and current images. + // +optional + ObservedPreviousRef *ImageRef `json:"observedPreviousRef,omitempty"` // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` // +optional @@ -142,6 +189,13 @@ type ImagePolicy struct { Status ImagePolicyStatus `json:"status,omitempty"` } +func (p ImagePolicy) GetDigestReflectionPolicy() ReflectionPolicy { + if p.Spec.DigestReflectionPolicy != "" { + return p.Spec.DigestReflectionPolicy + } + return ReflectNever +} + //+kubebuilder:object:root=true // ImagePolicyList contains a list of ImagePolicy diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 29f2b40d..5ff87a85 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -2,7 +2,7 @@ // +build !ignore_autogenerated /* -Copyright 2022 The Flux authors +Copyright 2023 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -157,6 +157,16 @@ func (in *ImagePolicySpec) DeepCopy() *ImagePolicySpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImagePolicyStatus) DeepCopyInto(out *ImagePolicyStatus) { *out = *in + if in.LatestRef != nil { + in, out := &in.LatestRef, &out.LatestRef + *out = new(ImageRef) + **out = **in + } + if in.ObservedPreviousRef != nil { + in, out := &in.ObservedPreviousRef, &out.ObservedPreviousRef + *out = new(ImageRef) + **out = **in + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) @@ -176,6 +186,21 @@ func (in *ImagePolicyStatus) DeepCopy() *ImagePolicyStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ImageRef) DeepCopyInto(out *ImageRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ImageRef. +func (in *ImageRef) DeepCopy() *ImageRef { + if in == nil { + return nil + } + out := new(ImageRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ImageRepository) DeepCopyInto(out *ImageRepository) { *out = *in diff --git a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml index 74af4e42..d7981507 100644 --- a/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml +++ b/config/crd/bases/image.toolkit.fluxcd.io_imagepolicies.yaml @@ -230,6 +230,15 @@ spec: description: ImagePolicySpec defines the parameters for calculating the ImagePolicy. properties: + digestReflectionPolicy: + default: Never + description: DigestReflectionPolicy governs the setting of the `.status.latestRef.digest` + field. + enum: + - Always + - IfNotPresent + - Never + type: string filterTags: description: FilterTags enables filtering for only a subset of tags based on a set of rules. If no rules are provided, all the tags @@ -383,17 +392,47 @@ spec: type: object type: array latestImage: - description: LatestImage gives the first in the list of images scanned + description: 'LatestImage gives the first in the list of images scanned by the image repository, when filtered and ordered according to - the policy. + the policy. Deprecated: Replaced by the composite "latestRef" field.' type: string + latestRef: + description: LatestRef gives the first in the list of images scanned + by the image repository, when filtered and ordered according to + the policy. + properties: + digest: + description: Digest is the image's digest. + type: string + image: + description: Name is the bare image's name. + type: string + tag: + description: Tag is the image's tag. + type: string + type: object observedGeneration: format: int64 type: integer observedPreviousImage: - description: ObservedPreviousImage is the observed previous LatestImage. - It is used to keep track of the previous and current images. + description: 'ObservedPreviousImage is the observed previous LatestImage. + It is used to keep track of the previous and current images. Deprecated: + Replaced by the composite "observedPreviousRef" field.' type: string + observedPreviousRef: + description: ObservedPreviousRef is the observed previous LatestRef. + It is used to keep track of the previous and current images. + properties: + digest: + description: Digest is the image's digest. + type: string + image: + description: Name is the bare image's name. + type: string + tag: + description: Tag is the image's tag. + type: string + type: object type: object type: object served: true diff --git a/docs/api/v1beta2/image-reflector.md b/docs/api/v1beta2/image-reflector.md index 576479fa..e83d27da 100644 --- a/docs/api/v1beta2/image-reflector.md +++ b/docs/api/v1beta2/image-reflector.md @@ -131,6 +131,19 @@ rules. If no rules are provided, all the tags from the repository will be ordered and compared.

+ + +digestReflectionPolicy
+ + +ReflectionPolicy + + + + +

DigestReflectionPolicy governs the setting of the .status.latestRef.digest field.

+ + @@ -277,6 +290,19 @@ rules. If no rules are provided, all the tags from the repository will be ordered and compared.

+ + +digestReflectionPolicy
+ + +ReflectionPolicy + + + + +

DigestReflectionPolicy governs the setting of the .status.latestRef.digest field.

+ + @@ -308,7 +334,8 @@ string

LatestImage gives the first in the list of images scanned by the image repository, when filtered and ordered according to -the policy.

+the policy. +Deprecated: Replaced by the composite “latestRef” field.

@@ -321,6 +348,37 @@ string (Optional)

ObservedPreviousImage is the observed previous LatestImage. It is used +to keep track of the previous and current images. +Deprecated: Replaced by the composite “observedPreviousRef” field.

+ + + + +latestRef
+ + +ImageRef + + + + +

LatestRef gives the first in the list of images scanned by +the image repository, when filtered and ordered according +to the policy.

+ + + + +observedPreviousRef
+ + +ImageRef + + + + +(Optional) +

ObservedPreviousRef is the observed previous LatestRef. It is used to keep track of the previous and current images.

@@ -352,6 +410,61 @@ int64 +

ImageRef +

+

+(Appears on: +ImagePolicyStatus) +

+

ImageRef represents an image reference.

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+image
+ +string + +
+

Name is the bare image’s name.

+
+tag
+ +string + +
+

Tag is the image’s tag.

+
+digest
+ +string + +
+(Optional) +

Digest is the image’s digest.

+
+
+

ImageRepository

ImageRepository is the Schema for the imagerepositories API

@@ -872,6 +985,13 @@ would select 0.

+

ReflectionPolicy +(string alias)

+

+(Appears on: +ImagePolicySpec) +

+

ReflectionPolicy describes a policy for if/when to reflect a value from the registry in a certain resource field.

ScanResult

diff --git a/docs/spec/v1beta2/imagepolicies.md b/docs/spec/v1beta2/imagepolicies.md index 8b73621f..f51167f0 100644 --- a/docs/spec/v1beta2/imagepolicies.md +++ b/docs/spec/v1beta2/imagepolicies.md @@ -21,6 +21,7 @@ metadata: spec: imageRepositoryRef: name: podinfo + digestReflectionPolicy: IfNotPresent policy: semver: range: 5.1.x @@ -37,8 +38,8 @@ In the above example: the scanned tags from the internal database for the image name. The read tags 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`. +- The latest image's name is derived from the ImageRepository image and reported + together with the selected tag and digest in the `.status.latestRef` object. This example can be run by saving the manifest into `imagepolicy.yaml`. @@ -68,6 +69,10 @@ Status: Status: True Type: Ready Latest Image: ghcr.io/stefanprodan/podinfo:5.1.4 + Latest Ref: + Digest: sha256:2d9a00b3981628a533ff43352193b1838b0a4bf6b0033444286f563205e51a2c + Image: ghcr.io/stefanprodan/podinfo + Tag: 5.1.4 Observed Generation: 1 Events: Type Reason Age From Message @@ -250,6 +255,19 @@ spec: In the above example, the timestamp value from the tag pattern is extracted and used in the policy rule to determine the latest tag. +### Digest Reflection + +`.spec.digestReflectionPolicy` is a field that governs the reflection of the selected image's +digest in the ImagePolicy's `.status.latestRef.digest` field. The field has three possible values: + +- `Never`: If the field is set to `Never` (the default) the digest will not be reflected at all. +- `Always`: This value leads to the digest of the latest tag to always be reflected in + `.status.latestRef.digest`. An existing, potentially different digest will be overwritten with the + most recent value retrieved from the image registry even if the tag didn't change. This may be useful + to track mutable tags like `latest`. +- `IfNotPresent`: This value will only store the digest of the latest tag once and never overwrite an + existing value unless the tag has changed as well. This is the safest option to track immutable tags. + ## Working with ImagePolicy ### Triggering a reconcile @@ -333,6 +351,9 @@ specific ImagePolicy, e.g. ### Latest Image +**Warning:** This field is deprecated in favor of `.status.latestRef.image` and will be +removed in a future release. + The ImagePolicy reports the latest select image from the ImageRepository tags in `.status.latestImage` for the resource. @@ -350,6 +371,9 @@ status: ### Observed Previous Image +**Warning:** This field is deprecated in favor of `.status.observedPreviousRef.image` +and will be removed in a future release. + The ImagePolicy reports the previously observed latest image in `.status.observedPreviousImage` for the resource. This is used by the ImagePolicy to determine an upgrade path of an ImagePolicy update. This field @@ -368,6 +392,53 @@ status: observedPreviousImage: ghcr.io/stefanprodan/podinfo:5.1.4 ``` +### Latest Ref + +The ImagePolicy reports the latest selected image from the ImageRepository tags in +`.status.latestRef` for the resource. The field `.status.latestRef.digest` is dependent +on the [chosen digest reflection policy](#digest-reflection) and is only set for the +`Always` or `IfNotPresent` policies. + +Example: + +```yaml +--- +apiVersion: image.toolkit.fluxcd.io/v1beta2 +kind: ImagePolicy +metadata: + name: +status: + latestRef: + image: ghcr.io/stefanprodan/podinfo + tag: 5.1.4 + digest: sha256:2d9a00b3981628a533ff43352193b1838b0a4bf6b0033444286f563205e51a2c +[...] +``` + +### Observed Previous Ref + +The ImagePolicy reports the previously observed latest image in +`.status.observedPreviousRef` for the resource. This is used by the +ImagePolicy to determine an upgrade path of an ImagePolicy update. This field +is reset when the ImagePolicy fails due to some reason to be able to distinguish +between a failure recovery and a genuine latest image upgrade. + +Example: + +```yaml +apiVersion: image.toolkit.fluxcd.io/v1beta2 +kind: ImagePolicy +metadata: + name: +status: + latestRef: + image: ghcr.io/stefanprodan/podinfo + tag: 6.2.1 + observedPreviousRef: + image: ghcr.io/stefanprodan/podinfo + tag: 5.1.4 +``` + ### Conditions An ImagePolicy enters various states during its lifecycle, reflected as diff --git a/hack/boilerplate.go.txt b/hack/boilerplate.go.txt index 44d2aa16..eb512145 100644 --- a/hack/boilerplate.go.txt +++ b/hack/boilerplate.go.txt @@ -1,5 +1,5 @@ /* -Copyright 2022 The Flux authors +Copyright 2023 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/internal/controller/imagepolicy_controller.go b/internal/controller/imagepolicy_controller.go index b5fc9103..1c000bdf 100644 --- a/internal/controller/imagepolicy_controller.go +++ b/internal/controller/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" @@ -48,6 +50,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 @@ -106,9 +109,10 @@ type ImagePolicyReconciler struct { kuberecorder.EventRecorder helper.Metrics - ControllerName string - Database DatabaseReader - ACLOptions acl.Options + ControllerName string + Database DatabaseReader + ACLOptions acl.Options + AuthOptionsGetter registry.AuthOptionsGetter patchOptions []patch.Option } @@ -213,9 +217,7 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP var resultImage, resultTag, previousTag string - // If there's no error and no requeue is requested, it's a success. Unlike - // other reconcilers, this reconciler doesn't requeue on its own with a - // RequeueAfter value. + // If there's no error and no requeue is requested, it's a success. isSuccess := func(res ctrl.Result, err error) bool { if err != nil || res.Requeue { return false @@ -256,6 +258,7 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP // Cleanup the last result. obj.Status.LatestImage = "" + obj.Status.LatestRef = nil // Get ImageRepository from reference. repo, err := r.getImageRepository(ctx, obj) @@ -315,21 +318,33 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP // Write the observations on status. obj.Status.LatestImage = repo.Spec.Image + ":" + latest + lr := imagev1.ImageRef{Name: repo.Spec.Image, Tag: latest} + obj.Status.LatestRef = &lr + + if err := r.updateDigest(ctx, repo, obj, oldObj, latest); err != nil { + result, retErr = ctrl.Result{}, err + return + } + // If the old latest image and new latest image don't match, set the old // image as the observed previous image. // NOTE: The following allows the previous image to be set empty when // there's a failure and a subsequent recovery from it. This behavior helps // avoid creating an update event as there's no previous image to infer // from. Recovery from a failure shouldn't result in an update event. - if oldObj.Status.LatestImage != obj.Status.LatestImage { + if oldObj.Status.LatestRef != nil && + *oldObj.Status.LatestRef != *obj.Status.LatestRef { obj.Status.ObservedPreviousImage = oldObj.Status.LatestImage + obj.Status.ObservedPreviousRef = oldObj.Status.LatestRef.DeepCopy() } + // Parse the observed previous image if any and extract previous tag. This // is used to determine image tag update path. - if obj.Status.ObservedPreviousImage != "" { - prevRef, err := name.NewTag(obj.Status.ObservedPreviousImage) + if obj.Status.ObservedPreviousRef != nil { + img := obj.Status.ObservedPreviousRef.Name + ":" + obj.Status.ObservedPreviousRef.Tag + prevRef, err := name.NewTag(img) if err != nil { - e := fmt.Errorf("failed to parse previous image '%s': %w", obj.Status.ObservedPreviousImage, err) + e := fmt.Errorf("failed to parse previous image '%s': %w", img, err) conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error()) result, retErr = ctrl.Result{}, e } @@ -345,6 +360,47 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP return } +func (r *ImagePolicyReconciler) updateDigest(ctx context.Context, repo *imagev1.ImageRepository, obj, oldObj *imagev1.ImagePolicy, tag string) error { + if obj.GetDigestReflectionPolicy() == imagev1.ReflectNever { + obj.Status.LatestRef.Digest = "" + return nil + } + + if obj.GetDigestReflectionPolicy() == imagev1.ReflectIfNotPresent && + oldObj.Status.LatestRef != nil && + oldObj.Status.LatestRef.Digest != "" && + obj.Status.LatestRef.Name == oldObj.Status.LatestRef.Name && + obj.Status.LatestRef.Tag == oldObj.Status.LatestRef.Tag { + obj.Status.LatestRef.Digest = oldObj.Status.LatestRef.Digest + return nil + } + + var err error + obj.Status.LatestRef.Digest, err = r.fetchDigest(ctx, repo, tag, obj) + if err != nil { + return fmt.Errorf("failed fetching digest of %s: %w", obj.Status.LatestRef.String(), err) + } + + return nil +} + +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.AuthOptionsGetter(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/controller/imagepolicy_controller_test.go b/internal/controller/imagepolicy_controller_test.go index c6ec6c40..1afad417 100644 --- a/internal/controller/imagepolicy_controller_test.go +++ b/internal/controller/imagepolicy_controller_test.go @@ -23,10 +23,15 @@ import ( aclapis "github.com/fluxcd/pkg/apis/acl" "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/oci/auth/login" "github.com/fluxcd/pkg/runtime/acl" + v1 "github.com/google/go-containerregistry/pkg/v1" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,6 +39,8 @@ 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" + "github.com/fluxcd/image-reflector-controller/internal/test" ) func TestImagePolicyReconciler_deleteBeforeFinalizer(t *testing.T) { @@ -72,6 +79,95 @@ func TestImagePolicyReconciler_deleteBeforeFinalizer(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) } +func TestStatusMigrationToImageRef(t *testing.T) { + g := NewWithT(t) + + s := runtime.NewScheme() + utilruntime.Must(imagev1.AddToScheme(s)) + utilruntime.Must(corev1.AddToScheme(s)) + + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "imagepolicy-" + randStringRunes(5), + }, + } + + imageRepo := &imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "status-migration-test", + }, + Spec: imagev1.ImageRepositorySpec{ + Image: "ghcr.io/stefanprodan/podinfo", + }, + Status: imagev1.ImageRepositoryStatus{ + LastScanResult: &imagev1.ScanResult{ + TagCount: 3, + LatestTags: []string{"1.0.0", "1.1.0", "2.0.0"}, + }, + }, + } + imagePol := &imagev1.ImagePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "status-migration-test", + Generation: 1, + Finalizers: []string{imagev1.ImageFinalizer}, + }, + Spec: imagev1.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: imageRepo.Name, + }, + Policy: imagev1.ImagePolicyChoice{ + SemVer: &imagev1.SemVerPolicy{ + Range: "1.0", + }, + }, + }, + Status: imagev1.ImagePolicyStatus{ + LatestImage: "ghcr.io/stefanprodan/podinfo:1.0.0", + }, + } + + c := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(ns, imageRepo, imagePol). + WithStatusSubresource(imagePol). + Build() + + r := &ImagePolicyReconciler{ + EventRecorder: record.NewFakeRecorder(32), + Client: c, + Database: &mockDatabase{TagData: imageRepo.Status.LastScanResult.LatestTags}, + AuthOptionsGetter: registry.NewAuthOptionsGetter(c, login.ProviderOptions{ + AwsAutoLogin: false, + AzureAutoLogin: false, + GcpAutoLogin: false, + }), + } + res, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: ns.Name, + Name: imagePol.Name, + }, + }) + + g.Expect(err).NotTo(HaveOccurred(), "reconciliation failed") + g.Expect(res).To(Equal(ctrl.Result{})) + + g.Expect(c.Get(context.Background(), client.ObjectKeyFromObject(imagePol), imagePol)). + To(Succeed(), "failed getting image policy") + + g.Expect(imagePol.Status.LatestImage).To(Equal("ghcr.io/stefanprodan/podinfo:1.0.0"), "unexpected latest image") + g.Expect(imagePol.Status.LatestRef).To(Equal(&imagev1.ImageRef{ + Name: "ghcr.io/stefanprodan/podinfo", + Tag: "1.0.0", + Digest: "", + }), "unexpected latest ref") + g.Expect(imagePol.Status.ObservedPreviousImage).To(BeEmpty(), "unexpected observed previous image") + g.Expect(imagePol.Status.ObservedPreviousRef).To(BeNil(), "unexpected observed previous ref") +} + func TestImagePolicyReconciler_getImageRepository(t *testing.T) { testImageRepoName := "test-repo" testNamespace1 := "test-ns1" // Default namespace of ImagePolicy. @@ -262,6 +358,339 @@ func TestImagePolicyReconciler_getImageRepository(t *testing.T) { } } +func TestImagePolicyReconciler_digestReflection(t *testing.T) { + registryServer := test.NewRegistryServer() + defer registryServer.Close() + + versions := []string{"v1.0.0", "v1.1.0", "v1.1.1", "v2.0.0"} + imgRepo, images1stPass, err := test.LoadImages(registryServer, "foo/bar", versions) + if err != nil { + t.Fatalf("could not load images into test registry: %s", err) + } + + var images2ndPass map[string]v1.Hash + + tests := []struct { + name string + semVerPolicy2ndPass string + refPolicy1stPass imagev1.ReflectionPolicy + refPolicy2ndPass imagev1.ReflectionPolicy + digest1stPass func() string + digest2ndPass func() string + previousRef2ndPass func() *imagev1.ImageRef + }{ + { + name: "missing policy leaves digest empty", + refPolicy1stPass: "", + digest1stPass: func() string { + return "" + }, + digest2ndPass: func() string { + return "" + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return nil + }, + }, + { + name: "'Never' policy leaves digest empty", + refPolicy1stPass: imagev1.ReflectNever, + digest1stPass: func() string { + return "" + }, + digest2ndPass: func() string { + return "" + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return nil + }, + }, + { + name: "'Always' policy always updates digest", + refPolicy1stPass: imagev1.ReflectAlways, + refPolicy2ndPass: imagev1.ReflectAlways, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images2ndPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: images1stPass["v1.1.1"].String(), + } + }, + }, + { + name: "'IfNotPresent' policy updates digest when new tag is selected", + semVerPolicy2ndPass: "v2.x", + refPolicy1stPass: imagev1.ReflectIfNotPresent, + refPolicy2ndPass: imagev1.ReflectIfNotPresent, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images2ndPass["v2.0.0"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: images1stPass["v1.1.1"].String(), + } + }, + }, + { + name: "'IfNotPresent' policy only sets digest once", + refPolicy1stPass: imagev1.ReflectIfNotPresent, + refPolicy2ndPass: imagev1.ReflectIfNotPresent, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images1stPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return nil + }, + }, + { + name: "changing 'Never' to 'IfNotPresent' sets observedPreviousRef correctly", + refPolicy1stPass: imagev1.ReflectNever, + refPolicy2ndPass: imagev1.ReflectIfNotPresent, + digest1stPass: func() string { + return "" + }, + digest2ndPass: func() string { + return images2ndPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: "", + } + }, + }, + { + name: "unsetting 'Always' policy removes digest", + refPolicy1stPass: imagev1.ReflectAlways, + refPolicy2ndPass: imagev1.ReflectNever, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return "" + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: images1stPass["v1.1.1"].String(), + } + }, + }, + { + name: "unsetting 'IfNotPresent' policy removes digest", + refPolicy1stPass: imagev1.ReflectIfNotPresent, + refPolicy2ndPass: imagev1.ReflectNever, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return "" + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: images1stPass["v1.1.1"].String(), + } + }, + }, + { + name: "changing 'IfNotPresent' to 'Always' sets new digest", + refPolicy1stPass: imagev1.ReflectIfNotPresent, + refPolicy2ndPass: imagev1.ReflectAlways, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images2ndPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return &imagev1.ImageRef{ + Name: imgRepo, + Tag: "v1.1.1", + Digest: images1stPass["v1.1.1"].String(), + } + }, + }, + { + name: "changing 'Always' to 'IfNotPresent' leaves digest untouched", + refPolicy1stPass: imagev1.ReflectAlways, + refPolicy2ndPass: imagev1.ReflectIfNotPresent, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images1stPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return nil + }, + }, + { + name: "selecting same tag with different policy leaves observedPreviousRef empty", + refPolicy1stPass: imagev1.ReflectIfNotPresent, + semVerPolicy2ndPass: "=v1.1.1", + refPolicy2ndPass: imagev1.ReflectIfNotPresent, + digest1stPass: func() string { + return images1stPass["v1.1.1"].String() + }, + digest2ndPass: func() string { + return images1stPass["v1.1.1"].String() + }, + previousRef2ndPass: func() *imagev1.ImageRef { + return nil + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + g := NewWithT(t) + + // Create namespace where ImagePolicy exists. + ns := &corev1.Namespace{} + ns.Name = "digref-test" + + // Create ImageRepository. + imageRepo := &imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "digref-test", + }, + Spec: imagev1.ImageRepositorySpec{ + Image: imgRepo, + }, + Status: imagev1.ImageRepositoryStatus{ + LastScanResult: &imagev1.ScanResult{ + TagCount: len(versions), + LatestTags: versions, + }, + }, + } + + imagePol := &imagev1.ImagePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "digref-test", + Finalizers: []string{imagev1.ImageFinalizer}, + }, + Spec: imagev1.ImagePolicySpec{ + ImageRepositoryRef: meta.NamespacedObjectReference{ + Name: imageRepo.Name, + }, + DigestReflectionPolicy: tt.refPolicy1stPass, + Policy: imagev1.ImagePolicyChoice{ + SemVer: &imagev1.SemVerPolicy{ + Range: "v1.x", + }, + }, + }, + } + + s := runtime.NewScheme() + utilruntime.Must(imagev1.AddToScheme(s)) + utilruntime.Must(corev1.AddToScheme(s)) + + c := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(ns, imageRepo, imagePol). + WithStatusSubresource(imagePol). + Build() + + g.Expect( + c.Get(context.Background(), client.ObjectKeyFromObject(imageRepo), imageRepo), + ).To(Succeed(), "failed getting image repo") + + r := &ImagePolicyReconciler{ + EventRecorder: record.NewFakeRecorder(32), + Client: c, + Database: &mockDatabase{TagData: imageRepo.Status.LastScanResult.LatestTags}, + AuthOptionsGetter: registry.NewAuthOptionsGetter(c, login.ProviderOptions{ + AwsAutoLogin: false, + AzureAutoLogin: false, + GcpAutoLogin: false, + }), + } + + res, err := r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: ns.Name, + Name: imagePol.Name, + }, + }) + + g.Expect(err).NotTo(HaveOccurred(), "reconciliation failed") + g.Expect(res).To(Equal(ctrl.Result{})) + + g.Expect( + c.Get(context.Background(), client.ObjectKeyFromObject(imagePol), imagePol), + ).To(Succeed(), "failed getting image policy") + + g.Expect(imagePol.Status.LatestRef.Digest). + To(Equal(tt.digest1stPass()), "unexpected 1st pass digest in status") + g.Expect(imagePol.Status.ObservedPreviousRef).To(BeNil(), + "observedPreviousRef should always be nil after a single pass") + + // Now, change the policy (if the test desires it) and overwrite the existing latest tag with a new image + + if tt.refPolicy1stPass != tt.refPolicy2ndPass { + imagePol.Spec.DigestReflectionPolicy = tt.refPolicy2ndPass + } + if tt.semVerPolicy2ndPass != "" { + imagePol.Spec.Policy.SemVer.Range = tt.semVerPolicy2ndPass + } + + g.Expect( + c.Update(context.Background(), imagePol), + ).To(Succeed(), "failed updating image policy for 2nd pass") + + if _, images2ndPass, err = test.LoadImages(registryServer, "foo/bar", versions); err != nil { + t.Fatalf("could not overwrite tag: %s", err) + } + + defer func() { + // the new 1st pass is the old 2nd pass in the next sub-test + images1stPass = images2ndPass + }() + + res, err = r.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: ns.Name, + Name: imagePol.Name, + }, + }) + + g.Expect(err).NotTo(HaveOccurred(), "reconciliation failed") + g.Expect(res).To(Equal(ctrl.Result{})) + + g.Expect( + c.Get(context.Background(), client.ObjectKeyFromObject(imagePol), imagePol), + ).To(Succeed(), "failed getting image policy") + g.Expect(imagePol.Status.LatestRef.Digest). + To(Equal(tt.digest2ndPass()), "unexpected 2nd pass digest in status") + g.Expect(imagePol.Status.ObservedPreviousRef).To(Equal(tt.previousRef2ndPass()), + "unexpected content in .status.observedPreviousRef") + }) + } +} + func TestImagePolicyReconciler_applyPolicy(t *testing.T) { tests := []struct { name string diff --git a/internal/controller/imagerepository_controller.go b/internal/controller/imagerepository_controller.go index 0ae58872..9f36868b 100644 --- a/internal/controller/imagerepository_controller.go +++ b/internal/controller/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 + + AuthOptionsGetter registry.AuthOptionsGetter patchOptions []patch.Option } @@ -249,7 +244,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 @@ -257,7 +252,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.AuthOptionsGetter(ctx, *obj) if err != nil { e := fmt.Errorf("failed to configure authentication options: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, imagev1.AuthenticationFailedReason, e.Error()) @@ -323,117 +318,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.TransportFromKubeTLSSecret(&certSecret) - if err != nil { - return nil, err - } - if tr.TLSClientConfig == nil { - tr, err = secret.TransportFromSecret(&certSecret) - if err != nil { - return nil, err - } - if tr.TLSClientConfig != 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") - } - } - 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. @@ -468,7 +352,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 } @@ -569,26 +453,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/controller/imagerepository_controller_test.go b/internal/controller/imagerepository_controller_test.go index df25ac43..b14efe15 100644 --- a/internal/controller/imagerepository_controller_test.go +++ b/internal/controller/imagerepository_controller_test.go @@ -22,7 +22,6 @@ 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" @@ -30,13 +29,12 @@ import ( "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "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" ) @@ -98,231 +96,6 @@ func TestImageRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { g.Expect(err).NotTo(HaveOccurred()) } -func TestImageRepositoryReconciler_setAuthOptions(t *testing.T) { - testImg := "example.com/foo/bar" - testSecretName := "test-secret" - testTLSSecretName := "test-tls-secret" - testDeprecatedTLSSecretName := "test-deprecated-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.CACrtKey: rootCertPEM, - corev1.TLSCertKey: clientCertPEM, - corev1.TLSPrivateKeyKey: clientKeyPEM, - } - - testDeprecatedTLSSecret := &corev1.Secret{} - testDeprecatedTLSSecret.Name = testDeprecatedTLSSecretName - testDeprecatedTLSSecret.Namespace = testNamespace - testDeprecatedTLSSecret.Type = corev1.SecretTypeTLS - testDeprecatedTLSSecret.Data = map[string][]byte{ - secret.CACert: rootCertPEM, - secret.ClientCert: clientCertPEM, - secret.ClientKey: clientKeyPEM, - } - - // Docker config secret with TLS data. - testDockerCfgSecretWithTLS := testSecret.DeepCopy() - testDockerCfgSecretWithTLS.Data = map[string][]byte{ - secret.CACrtKey: rootCertPEM, - corev1.TLSCertKey: clientCertPEM, - corev1.TLSPrivateKeyKey: 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 existing secret using deprecated keys", - mockObjs: []client.Object{testDeprecatedTLSSecret}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - CertSecretRef: &meta.LocalObjectReference{ - Name: testDeprecatedTLSSecretName, - }, - }, - }, - { - 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: "cert secret ref of type docker config", - mockObjs: []client.Object{testDockerCfgSecretWithTLS}, - imageRepoSpec: imagev1.ImageRepositorySpec{ - Image: testImg, - CertSecretRef: &meta.LocalObjectReference{ - Name: testSecretName, - }, - }, - wantErr: true, - }, - { - 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 { @@ -561,7 +334,7 @@ func TestImageRepositoryReconciler_scan(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - imgRepo, err := test.LoadImages(registryServer, "test-fetch-"+randStringRunes(5), tt.tags) + imgRepo, _, err := test.LoadImages(registryServer, "test-fetch-"+randStringRunes(5), tt.tags) g.Expect(err).ToNot(HaveOccurred()) r := ImageRepositoryReconciler{ @@ -580,7 +353,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{} @@ -656,49 +429,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/controller/policy_test.go b/internal/controller/policy_test.go index 47cc9611..b8c9fcc6 100644 --- a/internal/controller/policy_test.go +++ b/internal/controller/policy_test.go @@ -51,7 +51,7 @@ func TestImagePolicyReconciler_crossNamespaceRefsDisallowed(t *testing.T) { defer registryServer.Close() versions := []string{"1.0.1", "1.0.2", "1.1.0-alpha"} - imgRepo, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), versions) + imgRepo, _, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), versions) g.Expect(err).ToNot(HaveOccurred()) namespaceLabels := map[string]string{ @@ -171,7 +171,7 @@ func TestImagePolicyReconciler_calculateImageFromRepoTags(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - imgRepo, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), tt.versions) + imgRepo, _, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), tt.versions) g.Expect(err).ToNot(HaveOccurred()) repo := imagev1.ImageRepository{ @@ -219,9 +219,14 @@ 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.LatestRef != nil }, timeout, interval).Should(BeTrue()) - g.Expect(pol.Status.LatestImage).To(Equal(imgRepo + tt.wantImageTag)) + g.Expect(pol.Status.LatestRef.String()).To(Equal(imgRepo + tt.wantImageTag)) + g.Expect(pol.Status.ObservedPreviousImage).To(Equal(""), + "single reconciliation should leave status.observedPreviousImage empty") + g.Expect(pol.Status.ObservedPreviousRef).To(BeNil(), + "single reconciliation should leave status.observedPreviousRef nil") } else { g.Eventually(func() bool { err := testEnv.Get(ctx, polName, &pol) @@ -237,6 +242,7 @@ func TestImagePolicyReconciler_calculateImageFromRepoTags(t *testing.T) { checker.WithT(g).CheckErr(ctx, &pol) g.Expect(testEnv.Delete(ctx, &pol)).To(Succeed()) + g.Expect(testEnv.Delete(ctx, &repo)).To(Succeed()) }) } } @@ -276,7 +282,7 @@ func TestImagePolicyReconciler_filterTags(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - imgRepo, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), tt.versions) + imgRepo, _, err := test.LoadImages(registryServer, "test-semver-policy-"+randStringRunes(5), tt.versions) g.Expect(err).ToNot(HaveOccurred()) repo := imagev1.ImageRepository{ @@ -329,9 +335,9 @@ func TestImagePolicyReconciler_filterTags(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.LatestRef != nil }, timeout, interval).Should(BeTrue()) - g.Expect(pol.Status.LatestImage).To(Equal(imgRepo + tt.wantImageTag)) + g.Expect(pol.Status.LatestRef.String()).To(Equal(imgRepo + tt.wantImageTag)) } else { g.Eventually(func() bool { err := testEnv.Get(ctx, polName, &pol) @@ -440,7 +446,7 @@ func TestImagePolicyReconciler_accessImageRepo(t *testing.T) { g := NewWithT(t) versions := []string{"1.0.0", "1.0.1"} - imgRepo, err := test.LoadImages(registryServer, "acl-image-"+randStringRunes(5), versions) + imgRepo, _, err := test.LoadImages(registryServer, "acl-image-"+randStringRunes(5), versions) g.Expect(err).ToNot(HaveOccurred()) ctx, cancel := context.WithTimeout(context.Background(), contextTimeout) @@ -505,9 +511,9 @@ func TestImagePolicyReconciler_accessImageRepo(t *testing.T) { if tt.wantAccessible { g.Eventually(func() bool { err := testEnv.Get(ctx, polName, &pol) - return err == nil && pol.Status.LatestImage != "" + return err == nil && pol.Status.LatestRef != nil }, timeout, interval).Should(BeTrue()) - g.Expect(pol.Status.LatestImage).To(Equal(imgRepo + ":1.0.1")) + g.Expect(pol.Status.LatestRef.String()).To(Equal(imgRepo + ":1.0.1")) } else { g.Eventually(func() bool { _ = testEnv.Get(ctx, polName, &pol) diff --git a/internal/controller/scan_test.go b/internal/controller/scan_test.go index 0ef2f5d4..43f95df7 100644 --- a/internal/controller/scan_test.go +++ b/internal/controller/scan_test.go @@ -116,7 +116,7 @@ func TestImageRepositoryReconciler_fetchImageTags(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - imgRepo, err := test.LoadImages(registryServer, "test-fetch-"+randStringRunes(5), tt.versions) + imgRepo, _, err := test.LoadImages(registryServer, "test-fetch-"+randStringRunes(5), tt.versions) g.Expect(err).ToNot(HaveOccurred()) repo := imagev1.ImageRepository{ @@ -209,7 +209,7 @@ func TestImageRepositoryReconciler_reconcileAtAnnotation(t *testing.T) { registryServer := test.NewRegistryServer() defer registryServer.Close() - imgRepo, err := test.LoadImages(registryServer, "test-annot-"+randStringRunes(5), []string{"1.0.0"}) + imgRepo, _, err := test.LoadImages(registryServer, "test-annot-"+randStringRunes(5), []string{"1.0.0"}) g.Expect(err).ToNot(HaveOccurred()) repo := imagev1.ImageRepository{ @@ -289,7 +289,7 @@ func TestImageRepositoryReconciler_authRegistry(t *testing.T) { }() versions := []string{"0.1.0", "0.1.1", "0.2.0", "1.0.0", "1.0.1", "1.0.2", "1.1.0-alpha"} - imgRepo, err := test.LoadImages(registryServer, "test-authn-"+randStringRunes(5), + imgRepo, _, err := test.LoadImages(registryServer, "test-authn-"+randStringRunes(5), versions, remote.WithAuth(&authn.Basic{ Username: username, Password: password, @@ -339,7 +339,7 @@ func TestImageRepositoryReconciler_imageAttribute_schemePrefix(t *testing.T) { registryServer := test.NewRegistryServer() defer registryServer.Close() - imgRepo, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) + imgRepo, _, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) g.Expect(err).ToNot(HaveOccurred()) imgRepo = "https://" + imgRepo @@ -367,7 +367,7 @@ func TestImageRepositoryReconciler_imageAttribute_schemePrefix(t *testing.T) { ready = apimeta.FindStatusCondition(repo.GetConditions(), meta.ReadyCondition) return ready != nil && ready.Reason == imagev1.ImageURLInvalidReason }, timeout, interval).Should(BeTrue()) - g.Expect(ready.Message).To(ContainSubstring("should not start with URL scheme")) + g.Expect(ready.Message).To(ContainSubstring("should not include URL scheme")) // Check if the object status is valid. condns := &conditionscheck.Conditions{NegativePolarity: imageRepositoryNegativeConditions} @@ -384,7 +384,7 @@ func TestImageRepositoryReconciler_imageAttribute_withTag(t *testing.T) { registryServer := test.NewRegistryServer() defer registryServer.Close() - imgRepo, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) + imgRepo, _, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) g.Expect(err).ToNot(HaveOccurred()) imgRepo = imgRepo + ":1.0.0" @@ -429,7 +429,7 @@ func TestImageRepositoryReconciler_imageAttribute_hostPort(t *testing.T) { registryServer := test.NewRegistryServer() defer registryServer.Close() - imgRepo, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) + imgRepo, _, err := test.LoadImages(registryServer, "test-fetch", []string{"1.0.0"}) g.Expect(err).ToNot(HaveOccurred()) imgRepo = strings.ReplaceAll(imgRepo, "127.0.0.1", "localhost") @@ -505,7 +505,7 @@ func TestImageRepositoryReconciler_authRegistryWithServiceAccount(t *testing.T) }() versions := []string{"0.1.0", "0.1.1", "0.2.0", "1.0.0", "1.0.1", "1.0.2", "1.1.0-alpha"} - imgRepo, err := test.LoadImages(registryServer, "test-authn-"+randStringRunes(5), + imgRepo, _, err := test.LoadImages(registryServer, "test-authn-"+randStringRunes(5), versions, remote.WithAuth(&authn.Basic{ Username: username, Password: password, diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index cbb4b5a7..ed62b34e 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -31,11 +31,13 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "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 ) @@ -60,10 +62,6 @@ var ( ctx = ctrl.SetupSignalHandler() ) -func init() { - rand.Seed(time.Now().UnixNano()) -} - func TestMain(m *testing.M) { utilruntime.Must(imagev1.AddToScheme(scheme.Scheme)) @@ -89,10 +87,13 @@ func TestMain(m *testing.M) { panic(fmt.Sprintf("Failed to create new Badger database: %v", err)) } + optGetter := registry.NewAuthOptionsGetter(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), + AuthOptionsGetter: optGetter, }).SetupWithManager(testEnv, ImageRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { @@ -100,9 +101,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), + AuthOptionsGetter: optGetter, }).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..0a163ceb --- /dev/null +++ b/internal/registry/helper.go @@ -0,0 +1,44 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package registry + +import ( + "fmt" + "strings" + + "github.com/google/go-containerregistry/pkg/name" +) + +// ParseImageReference parses the given reference string into a container +// registry repository reference. +func ParseImageReference(refs string) (name.Reference, error) { + if s := strings.Split(refs, "://"); len(s) > 1 { + return nil, fmt.Errorf("image reference value should not include URL scheme; remove '%s://'", s[0]) + } + + ref, err := name.ParseReference(refs) + if err != nil { + return nil, err + } + + imageName := strings.TrimPrefix(refs, 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/options.go b/internal/registry/options.go new file mode 100644 index 00000000..4657e09d --- /dev/null +++ b/internal/registry/options.go @@ -0,0 +1,168 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +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" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/secret" +) + +// AuthOptionsGetter is a function to extract information out of an ImageRepository and create +// options from it that can be used to interact with an OCI registry. +type AuthOptionsGetter func(ctx context.Context, obj imagev1.ImageRepository) ([]remote.Option, error) + +// NewAuthOptionsGetter returns an AuthOptionsGetter function that builds a slice of options from an +// ImageRepository by looking up references to Secrets etc. on the Kubernetes cluster using the provided +// client interface. If no external authentication provider is configured on the ImageRepository, the given +// ProviderOptions are used for authentication. Options are extracted from the following ImageRepository spec +// fields: +// +// - spec.image +// - spec.secretRef +// - spec.provider +// - spec.certSecretRef +// - spec.serviceAccountName +func NewAuthOptionsGetter(c client.Client, deprecatedLoginOpts login.ProviderOptions) AuthOptionsGetter { + return func(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 %q: %w", obj.Spec.Image, err) + } + + if obj.Spec.SecretRef != nil { + if err := c.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 = 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 := c.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.CertSecretRef.Name, + }, &certSecret); err != nil { + return nil, err + } + } + + tr, err := secret.TransportFromKubeTLSSecret(&certSecret) + if err != nil { + return nil, err + } + if tr.TLSClientConfig == nil { + tr, err = secret.TransportFromSecret(&certSecret) + if err != nil { + return nil, err + } + if tr.TLSClientConfig != 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") + } + } + options = append(options, remote.WithTransport(tr)) + } + + if obj.Spec.ServiceAccountName != "" { + serviceAccount := corev1.ServiceAccount{} + // Lookup service account + if err := c.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 := c.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/internal/registry/options_test.go b/internal/registry/options_test.go new file mode 100644 index 00000000..18f1e3e1 --- /dev/null +++ b/internal/registry/options_test.go @@ -0,0 +1,289 @@ +/* +Copyright 2023 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +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 TestNewAuthOptionsGetter(t *testing.T) { + testImg := "example.com/foo/bar" + testSecretName := "test-secret" + testTLSSecretName := "test-tls-secret" + testDeprecatedTLSSecretName := "test-deprecated-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, + } + + testDeprecatedTLSSecret := &corev1.Secret{} + testDeprecatedTLSSecret.Name = testDeprecatedTLSSecretName + testDeprecatedTLSSecret.Namespace = testNamespace + testDeprecatedTLSSecret.Type = corev1.SecretTypeTLS + testDeprecatedTLSSecret.Data = map[string][]byte{ + secret.CACert: rootCertPEM, + secret.ClientCert: clientCertPEM, + secret.ClientKey: clientKeyPEM, + } + + // Secret with docker config and TLS secrets. + testDockerCfgSecretWithTLS := testSecret.DeepCopy() + testDockerCfgSecretWithTLS.Data = map[string][]byte{ + secret.CACrtKey: rootCertPEM, + corev1.TLSCertKey: clientCertPEM, + corev1.TLSPrivateKeyKey: 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 + k8sObjs []client.Object + repo imagev1.ImageRepository + expectErr bool + expectOpts int + }{ + { + 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 auth options cert secret ref with existing secret using deprecated keys", + k8sObjs: []client.Object{testDeprecatedTLSSecret}, + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + CertSecretRef: &meta.LocalObjectReference{ + Name: testDeprecatedTLSSecretName, + }, + }, + }, + expectErr: false, + expectOpts: 1, + }, + { + name: "fails with cert secret ref of type docker config", + repo: imagev1.ImageRepository{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace}, + Spec: imagev1.ImageRepositorySpec{ + Image: testImg, + CertSecretRef: &meta.LocalObjectReference{ + Name: testSecretName, + }, + }, + }, + k8sObjs: []client.Object{testDockerCfgSecretWithTLS}, + expectErr: true, + }, + { + 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() + getter := registry.NewAuthOptionsGetter(k8sClient, login.ProviderOptions{}) + + opts, err := getter(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/test/registry.go b/internal/test/registry.go index c26e7462..c7414f0f 100644 --- a/internal/test/registry.go +++ b/internal/test/registry.go @@ -27,6 +27,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/registry" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/random" "github.com/google/go-containerregistry/pkg/v1/remote" ) @@ -78,22 +79,29 @@ func RegistryName(srv *httptest.Server) string { // image repo // name. https://github.com/google/go-containerregistry/blob/v0.1.1/pkg/registry/compatibility_test.go // has an example of loading a test registry with a random image. -func LoadImages(srv *httptest.Server, imageName string, versions []string, options ...remote.Option) (string, error) { +func LoadImages(srv *httptest.Server, imageName string, versions []string, options ...remote.Option) (string, map[string]v1.Hash, error) { imgRepo := RegistryName(srv) + "/" + imageName + imgRes := make(map[string]v1.Hash, 0) + for _, tag := range versions { imgRef, err := name.NewTag(imgRepo + ":" + tag) if err != nil { - return imgRepo, err + return imgRepo, nil, err } img, err := random.Image(512, 1) if err != nil { - return imgRepo, err + return imgRepo, nil, err } if err := remote.Write(imgRef, img, options...); err != nil { - return imgRepo, err + return imgRepo, nil, err + } + dig, err := img.Digest() + if err != nil { + return imgRepo, nil, err } + imgRes[tag] = dig } - return imgRepo, nil + return imgRepo, imgRes, nil } // the go-containerregistry test registry implementation does not diff --git a/internal/test/registry_test.go b/internal/test/registry_test.go index 7d078e04..7a9fb5df 100644 --- a/internal/test/registry_test.go +++ b/internal/test/registry_test.go @@ -32,7 +32,7 @@ func TestRegistryHandler(t *testing.T) { defer srv.Close() uploadedTags := []string{"tag1", "tag2"} - repoString, err := LoadImages(srv, "testimage", uploadedTags) + repoString, _, err := LoadImages(srv, "testimage", uploadedTags) g.Expect(err).ToNot(HaveOccurred()) repo, _ := name.NewRepository(repoString) diff --git a/main.go b/main.go index a94fadde..3f2272d3 100644 --- a/main.go +++ b/main.go @@ -52,6 +52,7 @@ import ( "github.com/fluxcd/image-reflector-controller/internal/controller" "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" @@ -205,17 +206,20 @@ func main() { metricsH := helper.NewMetrics(mgr, metrics.MustMakeRecorder(), imagev1.ImageFinalizer) + deprecatedLoginOptions := login.ProviderOptions{ + AwsAutoLogin: awsAutoLogin, + AzureAutoLogin: azureAutoLogin, + GcpAutoLogin: gcpAutoLogin, + } + authOptionsGetter := registry.NewAuthOptionsGetter(mgr.GetClient(), deprecatedLoginOptions) + if err := (&controller.ImageRepositoryReconciler{ - Client: mgr.GetClient(), - EventRecorder: eventRecorder, - Metrics: metricsH, - Database: db, - ControllerName: controllerName, - DeprecatedLoginOpts: login.ProviderOptions{ - AwsAutoLogin: awsAutoLogin, - AzureAutoLogin: azureAutoLogin, - GcpAutoLogin: gcpAutoLogin, - }, + Client: mgr.GetClient(), + EventRecorder: eventRecorder, + Metrics: metricsH, + Database: db, + ControllerName: controllerName, + AuthOptionsGetter: authOptionsGetter, }).SetupWithManager(mgr, controller.ImageRepositoryReconcilerOptions{ RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil { @@ -223,12 +227,13 @@ func main() { os.Exit(1) } if err := (&controller.ImagePolicyReconciler{ - Client: mgr.GetClient(), - EventRecorder: eventRecorder, - Metrics: metricsH, - Database: db, - ACLOptions: aclOptions, - ControllerName: controllerName, + Client: mgr.GetClient(), + EventRecorder: eventRecorder, + Metrics: metricsH, + Database: db, + ACLOptions: aclOptions, + ControllerName: controllerName, + AuthOptionsGetter: authOptionsGetter, }).SetupWithManager(mgr, controller.ImagePolicyReconcilerOptions{ RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil {