From 3e9a5bfd03d8598e87281c6a1cfe083b7b462332 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 31 Mar 2023 18:21:29 +0200 Subject: [PATCH] Store digest of latest image in ImagePolicy 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. This commit changes the format of the data stored in the caching badger database from a list of strings to a list of `database.Tag` objects where each tag carries a tag name and a digest value. `ImageRepositoryReconciler` now fetched the digest of each image+tag when it scans the registry for new tags. To accomplish this it issues a HEAD request against the registry for each tag with the response carrying the digest in the headers. Since this is a potentially expensive operation involving network roundtrips for each tag, a goroutine is spawned for each HEAD request to parallelize the fetching process. Migration from the old database format to the new one is taken care of by the `badger.unmarshal` function which falls back to trying to unmarshal the data into a string slice in case the attempt to unmarshal it into a `database.Tag` slice fails. Subsequent `SetTags` calls then store the data in the new format. Because of its potential to significantly increase the amount of network requests, the feature is disabled by default and can be enabled using a feature flag for now. closes #214 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 ++ .../controllers/controllers_fuzzer_test.go | 6 +- .../controllers/imagepolicy_controller.go | 28 ++-- .../imagepolicy_controller_test.go | 38 +++-- .../controllers/imagerepository_controller.go | 45 +++++- .../imagerepository_controller_test.go | 88 ++++++----- internal/controllers/policy_test.go | 4 +- internal/controllers/scan_test.go | 4 +- internal/controllers/suite_test.go | 6 +- internal/database/{ => badger}/badger.go | 35 +++-- internal/database/{ => badger}/badger_test.go | 36 ++++- internal/database/badger/testdata/old.db | Bin 0 -> 72 bytes .../{controllers => database}/database.go | 14 +- internal/features/features.go | 9 ++ internal/policy/alphabetical.go | 21 ++- internal/policy/alphabetical_test.go | 133 ++++++++++++----- internal/policy/filter.go | 30 ++-- internal/policy/filter_test.go | 69 ++++++--- internal/policy/numerical.go | 16 +- internal/policy/numerical_test.go | 139 +++++++++++++----- internal/policy/policer.go | 4 +- internal/policy/semver.go | 16 +- internal/policy/semver_test.go | 63 +++++--- main.go | 11 +- 26 files changed, 583 insertions(+), 253 deletions(-) rename internal/database/{ => badger}/badger.go (66%) rename internal/database/{ => badger}/badger_test.go (67%) create mode 100644 internal/database/badger/testdata/old.db rename internal/{controllers => database}/database.go (71%) 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/internal/controllers/controllers_fuzzer_test.go b/internal/controllers/controllers_fuzzer_test.go index be586b03..b40d92da 100644 --- a/internal/controllers/controllers_fuzzer_test.go +++ b/internal/controllers/controllers_fuzzer_test.go @@ -50,7 +50,7 @@ import ( fuzz "github.com/AdaLogics/go-fuzz-headers" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/database" + ircbadger "github.com/fluxcd/image-reflector-controller/internal/database/badger" "github.com/fluxcd/image-reflector-controller/internal/test" ) @@ -252,7 +252,7 @@ func initFunc() { imageRepoReconciler = &ImageRepositoryReconciler{ Client: k8sMgr.GetClient(), - Database: database.NewBadgerDatabase(badgerDB), + Database: ircbadger.NewBadgerDatabase(badgerDB), EventRecorder: record.NewFakeRecorder(256), patchOptions: getPatchOptions(imageRepositoryOwnedConditions, "irc"), } @@ -263,7 +263,7 @@ func initFunc() { imagePolicyReconciler = &ImagePolicyReconciler{ Client: k8sMgr.GetClient(), - Database: database.NewBadgerDatabase(badgerDB), + Database: ircbadger.NewBadgerDatabase(badgerDB), EventRecorder: record.NewFakeRecorder(256), patchOptions: getPatchOptions(imagePolicyOwnedConditions, "irc"), } diff --git a/internal/controllers/imagepolicy_controller.go b/internal/controllers/imagepolicy_controller.go index 4e4b7f77..588c60bc 100644 --- a/internal/controllers/imagepolicy_controller.go +++ b/internal/controllers/imagepolicy_controller.go @@ -48,6 +48,7 @@ import ( pkgreconcile "github.com/fluxcd/pkg/runtime/reconcile" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/image-reflector-controller/internal/policy" ) @@ -108,7 +109,7 @@ type ImagePolicyReconciler struct { helper.Metrics ControllerName string - Database DatabaseReader + Database database.DatabaseReader ACLOptions acl.Options patchOptions []patch.Option @@ -259,6 +260,7 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP // Cleanup the last result. obj.Status.LatestImage = "" + obj.Status.LatestDigest = "" // Get ImageRepository from reference. repo, err := r.getImageRepository(ctx, obj) @@ -317,7 +319,8 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP } // Write the observations on status. - obj.Status.LatestImage = repo.Spec.Image + ":" + latest + obj.Status.LatestImage = repo.Spec.Image + ":" + latest.Name + obj.Status.LatestDigest = latest.Digest // 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 @@ -340,7 +343,7 @@ func (r *ImagePolicyReconciler) reconcile(ctx context.Context, sp *patch.SerialP } resultImage = repo.Spec.Image - resultTag = latest + resultTag = latest.Name conditions.Delete(obj, meta.ReadyCondition) @@ -386,36 +389,37 @@ func (r *ImagePolicyReconciler) getImageRepository(ctx context.Context, obj *ima // applyPolicy reads the tags of the given repository from the internal database // and applies the tag filters and constraints to return the latest image. -func (r *ImagePolicyReconciler) applyPolicy(ctx context.Context, obj *imagev1.ImagePolicy, repo *imagev1.ImageRepository) (string, error) { +func (r *ImagePolicyReconciler) applyPolicy(ctx context.Context, obj *imagev1.ImagePolicy, repo *imagev1.ImageRepository) (*database.Tag, error) { policer, err := policy.PolicerFromSpec(obj.Spec.Policy) if err != nil { - return "", errInvalidPolicy{err: fmt.Errorf("invalid policy: %w", err)} + return nil, errInvalidPolicy{err: fmt.Errorf("invalid policy: %w", err)} } // Read tags from database, apply and filter is configured and compute the // result. tags, err := r.Database.Tags(repo.Status.CanonicalImageName) if err != nil { - return "", fmt.Errorf("failed to read tags from database: %w", err) + return nil, fmt.Errorf("failed to read tags from database: %w", err) } if len(tags) == 0 { - return "", errNoTagsInDatabase + return nil, errNoTagsInDatabase } // Apply tag filter. if obj.Spec.FilterTags != nil { filter, err := policy.NewRegexFilter(obj.Spec.FilterTags.Pattern, obj.Spec.FilterTags.Extract) if err != nil { - return "", errInvalidPolicy{err: fmt.Errorf("failed to filter tags: %w", err)} + return nil, errInvalidPolicy{err: fmt.Errorf("failed to filter tags: %w", err)} } filter.Apply(tags) - tags = filter.Items() - latest, err := policer.Latest(tags) + tagNames := filter.Items() + latest, err := policer.Latest(tagNames) if err != nil { - return "", err + return nil, err } - return filter.GetOriginalTag(latest), nil + origTag := filter.GetOriginalTag(latest.Name) + return &origTag, nil } // Compute and return result. return policer.Latest(tags) diff --git a/internal/controllers/imagepolicy_controller_test.go b/internal/controllers/imagepolicy_controller_test.go index f12aebae..f754e77c 100644 --- a/internal/controllers/imagepolicy_controller_test.go +++ b/internal/controllers/imagepolicy_controller_test.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/image-reflector-controller/internal/policy" ) @@ -231,7 +232,7 @@ func TestImagePolicyReconciler_applyPolicy(t *testing.T) { filter *imagev1.TagFilter db *mockDatabase wantErr bool - wantResult string + wantResult *database.Tag }{ { name: "invalid policy", @@ -251,16 +252,21 @@ func TestImagePolicyReconciler_applyPolicy(t *testing.T) { wantErr: true, }, { - name: "semver, no tag filter", - policy: imagev1.ImagePolicyChoice{SemVer: &imagev1.SemVerPolicy{Range: "1.0.x"}}, - db: &mockDatabase{TagData: []string{"1.0.0", "2.0.0", "1.0.1", "1.2.0"}}, - wantResult: "1.0.1", + name: "semver, no tag filter", + policy: imagev1.ImagePolicyChoice{SemVer: &imagev1.SemVerPolicy{Range: "1.0.x"}}, + db: &mockDatabase{TagData: []database.Tag{ + {Name: "1.0.0"}, + {Name: "2.0.0"}, + {Name: "1.0.1"}, + {Name: "1.2.0"}, + }}, + wantResult: &database.Tag{Name: "1.0.1"}, }, { name: "invalid tag filter", policy: imagev1.ImagePolicyChoice{SemVer: &imagev1.SemVerPolicy{Range: "1.0.x"}}, filter: &imagev1.TagFilter{Pattern: "[="}, - db: &mockDatabase{TagData: []string{"1.0.0", "1.0.1"}}, + db: &mockDatabase{TagData: []database.Tag{{Name: "1.0.0"}, {Name: "1.0.1"}}}, wantErr: true, }, { @@ -270,10 +276,14 @@ func TestImagePolicyReconciler_applyPolicy(t *testing.T) { Pattern: "1.0.0-rc\\.(?P[0-9]+)", Extract: "$num", }, - db: &mockDatabase{TagData: []string{ - "1.0.0", "1.0.0-rc.1", "1.0.0-rc.2", "1.0.0-rc.3", "1.0.1-rc.2", + db: &mockDatabase{TagData: []database.Tag{ + {Name: "1.0.0"}, + {Name: "1.0.0-rc.1"}, + {Name: "1.0.0-rc.2"}, + {Name: "1.0.0-rc.3"}, + {Name: "1.0.1-rc.2"}, }}, - wantResult: "1.0.0-rc.3", + wantResult: &database.Tag{Name: "1.0.0-rc.3"}, }, { name: "valid tag filter with alphabetical policy", @@ -282,10 +292,14 @@ func TestImagePolicyReconciler_applyPolicy(t *testing.T) { Pattern: "foo-(?P[a-z]+)", Extract: "$word", }, - db: &mockDatabase{TagData: []string{ - "foo-aaa", "bar-bbb", "foo-zzz", "baz-nnn", "foo-ooo", + db: &mockDatabase{TagData: []database.Tag{ + {Name: "foo-aaa"}, + {Name: "bar-bbb"}, + {Name: "foo-zzz"}, + {Name: "baz-nnn"}, + {Name: "foo-ooo"}, }}, - wantResult: "foo-zzz", + wantResult: &database.Tag{Name: "foo-zzz"}, }, } diff --git a/internal/controllers/imagerepository_controller.go b/internal/controllers/imagerepository_controller.go index b19ed92a..9caff1a1 100644 --- a/internal/controllers/imagerepository_controller.go +++ b/internal/controllers/imagerepository_controller.go @@ -23,6 +23,7 @@ import ( "regexp" "sort" "strings" + "sync" "time" "github.com/google/go-containerregistry/pkg/authn" @@ -54,6 +55,7 @@ import ( "github.com/fluxcd/pkg/runtime/reconcile" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/image-reflector-controller/internal/secret" ) @@ -109,10 +111,11 @@ type ImageRepositoryReconciler struct { ControllerName string Database interface { - DatabaseWriter - DatabaseReader + database.DatabaseWriter + database.DatabaseReader } DeprecatedLoginOpts login.ProviderOptions + FetchDigests bool patchOptions []patch.Option } @@ -516,8 +519,44 @@ func (r *ImageRepositoryReconciler) scan(ctx context.Context, obj *imagev1.Image return 0, err } + storedTags := make([]database.Tag, 0, len(filteredTags)) + + resCh := make(chan database.Tag, len(filteredTags)) + var wg sync.WaitGroup + for _, tag := range filteredTags { + wg.Add(1) + go func(tag string, ch chan database.Tag) { + defer wg.Done() + res := database.Tag{ + Name: tag, + } + + if r.FetchDigests { + tagRef, err := name.ParseReference(strings.Join([]string{ref.Context().Name(), tag}, ":")) + if err != nil { + return + } + desc, err := remote.Head(tagRef, remote.WithContext(ctx)) + if err != nil { + return + } + res.Digest = desc.Digest.String() + } + + resCh <- res + + }(tag, resCh) + } + + wg.Wait() + close(resCh) + + for t := range resCh { + storedTags = append(storedTags, t) + } + canonicalName := ref.Context().String() - if err := r.Database.SetTags(canonicalName, filteredTags); err != nil { + if err := r.Database.SetTags(canonicalName, storedTags); err != nil { return 0, fmt.Errorf("failed to set tags for %q: %w", canonicalName, err) } diff --git a/internal/controllers/imagerepository_controller_test.go b/internal/controllers/imagerepository_controller_test.go index 4c3087a5..4f5d8769 100644 --- a/internal/controllers/imagerepository_controller_test.go +++ b/internal/controllers/imagerepository_controller_test.go @@ -35,19 +35,23 @@ import ( "github.com/fluxcd/pkg/runtime/conditions" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" + "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/image-reflector-controller/internal/secret" "github.com/fluxcd/image-reflector-controller/internal/test" ) // mockDatabase mocks the image repository database. type mockDatabase struct { - TagData []string + TagData []database.Tag ReadError error WriteError error } +var _ database.DatabaseReader = mockDatabase{} +var _ database.DatabaseWriter = &mockDatabase{} + // SetTags implements the DatabaseWriter interface of the Database. -func (db *mockDatabase) SetTags(repo string, tags []string) error { +func (db *mockDatabase) SetTags(repo string, tags []database.Tag) error { if db.WriteError != nil { return db.WriteError } @@ -56,7 +60,7 @@ func (db *mockDatabase) SetTags(repo string, tags []string) error { } // Tags implements the DatabaseReader interface of the Database. -func (db mockDatabase) Tags(repo string) ([]string, error) { +func (db mockDatabase) Tags(repo string) ([]database.Tag, error) { if db.ReadError != nil { return nil, db.ReadError } @@ -324,7 +328,7 @@ func TestImageRepositoryReconciler_shouldScan(t *testing.T) { ScanTime: metav1.NewTime(reconcileTime.Add(-time.Second * 30)), } }, - db: &mockDatabase{TagData: []string{"foo"}}, + db: &mockDatabase{TagData: []database.Tag{{Name: "foo"}}}, wantScan: false, wantNextScan: time.Second * 30, }, @@ -339,7 +343,7 @@ func TestImageRepositoryReconciler_shouldScan(t *testing.T) { ScanTime: metav1.NewTime(reconcileTime.Add(-time.Second * 30)), } }, - db: &mockDatabase{TagData: []string{"foo"}}, + db: &mockDatabase{TagData: []database.Tag{{Name: "foo"}}}, wantScan: true, wantNextScan: time.Minute, wantReason: scanReasonNewImageName, @@ -355,7 +359,7 @@ func TestImageRepositoryReconciler_shouldScan(t *testing.T) { ScanTime: metav1.NewTime(reconcileTime.Add(-time.Second * 30)), } }, - db: &mockDatabase{TagData: []string{"foo"}}, + db: &mockDatabase{TagData: []database.Tag{{Name: "foo"}}}, wantScan: true, wantNextScan: time.Minute, wantReason: scanReasonUpdatedExclusionList, @@ -384,7 +388,7 @@ func TestImageRepositoryReconciler_shouldScan(t *testing.T) { ScanTime: metav1.NewTime(reconcileTime.Add(-time.Second * 30)), } }, - db: &mockDatabase{TagData: []string{"foo"}, ReadError: errors.New("fail")}, + db: &mockDatabase{TagData: []database.Tag{{Name: "foo"}}, ReadError: errors.New("fail")}, wantErr: true, wantScan: false, wantNextScan: time.Minute, @@ -398,7 +402,7 @@ func TestImageRepositoryReconciler_shouldScan(t *testing.T) { ScanTime: metav1.NewTime(reconcileTime.Add(-time.Minute * 2)), } }, - db: &mockDatabase{TagData: []string{"foo"}}, + db: &mockDatabase{TagData: []database.Tag{{Name: "foo"}}}, wantScan: true, wantNextScan: time.Minute, wantReason: scanReasonInterval, @@ -439,48 +443,43 @@ func TestImageRepositoryReconciler_scan(t *testing.T) { defer registryServer.Close() tests := []struct { - name string - tags []string - exclusionList []string - annotation string - db *mockDatabase - wantErr bool - wantTags []string - wantLatestTags []string + name string + tags []string + exclusionList []string + annotation string + db *mockDatabase + wantErr bool + wantTags []database.Tag }{ { name: "no tags", wantErr: true, }, { - name: "simple tags", - tags: []string{"a", "b", "c", "d"}, - db: &mockDatabase{}, - wantTags: []string{"a", "b", "c", "d"}, - wantLatestTags: []string{"d", "c", "b", "a"}, + name: "simple tags", + tags: []string{"a", "b", "c", "d"}, + db: &mockDatabase{}, + wantTags: []database.Tag{{Name: "a"}, {Name: "b"}, {Name: "c"}, {Name: "d"}}, }, { - name: "simple tags, 10+", - tags: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"}, - db: &mockDatabase{}, - wantTags: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"}, - wantLatestTags: []string{"k", "j", "i", "h, g, f, e, d, c, b"}, + name: "simple tags, 10+", + tags: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"}, + db: &mockDatabase{}, + wantTags: []database.Tag{{Name: "a"}, {Name: "b"}, {Name: "c"}, {Name: "d"}, {Name: "e"}, {Name: "f"}, {Name: "g"}, {Name: "h"}, {Name: "i"}, {Name: "j"}, {Name: "k"}}, }, { - name: "with single exclusion pattern", - tags: []string{"a", "b", "c", "d"}, - exclusionList: []string{"c"}, - db: &mockDatabase{}, - wantTags: []string{"a", "b", "d"}, - wantLatestTags: []string{"d", "b", "a"}, + name: "with single exclusion pattern", + tags: []string{"a", "b", "c", "d"}, + exclusionList: []string{"c"}, + db: &mockDatabase{}, + wantTags: []database.Tag{{Name: "a"}, {Name: "b"}, {Name: "d"}}, }, { - name: "with multiple exclusion pattern", - tags: []string{"a", "b", "c", "d"}, - exclusionList: []string{"c", "a"}, - db: &mockDatabase{}, - wantTags: []string{"b", "d"}, - wantLatestTags: []string{"d", "b"}, + name: "with multiple exclusion pattern", + tags: []string{"a", "b", "c", "d"}, + exclusionList: []string{"c", "a"}, + db: &mockDatabase{}, + wantTags: []database.Tag{{Name: "b"}, {Name: "d"}}, }, { name: "bad exclusion pattern", @@ -495,12 +494,11 @@ func TestImageRepositoryReconciler_scan(t *testing.T) { wantErr: true, }, { - name: "with reconcile annotation", - tags: []string{"a", "b"}, - annotation: "foo", - db: &mockDatabase{}, - wantTags: []string{"a", "b"}, - wantLatestTags: []string{"b", "a"}, + name: "with reconcile annotation", + tags: []string{"a", "b"}, + annotation: "foo", + db: &mockDatabase{}, + wantTags: []database.Tag{{Name: "a"}, {Name: "b"}}, }, } @@ -536,7 +534,7 @@ func TestImageRepositoryReconciler_scan(t *testing.T) { g.Expect(err != nil).To(Equal(tt.wantErr)) if err == nil { g.Expect(tagCount).To(Equal(len(tt.wantTags))) - g.Expect(r.Database.Tags(imgRepo)).To(Equal(tt.wantTags)) + g.Expect(r.Database.Tags(imgRepo)).To(ConsistOf(tt.wantTags)) g.Expect(repo.Status.LastScanResult.TagCount).To(Equal(len(tt.wantTags))) g.Expect(repo.Status.LastScanResult.ScanTime).ToNot(BeZero()) if tt.annotation != "" { diff --git a/internal/controllers/policy_test.go b/internal/controllers/policy_test.go index 2f3cb9ac..7d4b3bc8 100644 --- a/internal/controllers/policy_test.go +++ b/internal/controllers/policy_test.go @@ -36,7 +36,7 @@ import ( "github.com/fluxcd/pkg/runtime/patch" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/database" + "github.com/fluxcd/image-reflector-controller/internal/database/badger" "github.com/fluxcd/image-reflector-controller/internal/test" // +kubebuilder:scaffold:imports ) @@ -110,7 +110,7 @@ func TestImagePolicyReconciler_crossNamespaceRefsDisallowed(t *testing.T) { r := &ImagePolicyReconciler{ Client: builder.Build(), - Database: database.NewBadgerDatabase(testBadgerDB), + Database: badger.NewBadgerDatabase(testBadgerDB), EventRecorder: record.NewFakeRecorder(32), ACLOptions: acl.Options{ NoCrossNamespaceRefs: true, diff --git a/internal/controllers/scan_test.go b/internal/controllers/scan_test.go index af30ef4e..f0d1a0fe 100644 --- a/internal/controllers/scan_test.go +++ b/internal/controllers/scan_test.go @@ -38,7 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/database" + "github.com/fluxcd/image-reflector-controller/internal/database/badger" "github.com/fluxcd/image-reflector-controller/internal/test" // +kubebuilder:scaffold:imports ) @@ -186,7 +186,7 @@ func TestImageRepositoryReconciler_repositorySuspended(t *testing.T) { r := &ImageRepositoryReconciler{ Client: builder.Build(), - Database: database.NewBadgerDatabase(testBadgerDB), + Database: badger.NewBadgerDatabase(testBadgerDB), patchOptions: getPatchOptions(imageRepositoryOwnedConditions, "irc"), } diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 171fa1d5..ff7aabb9 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -34,7 +34,7 @@ import ( "github.com/fluxcd/pkg/runtime/testenv" imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" - "github.com/fluxcd/image-reflector-controller/internal/database" + ircbadger "github.com/fluxcd/image-reflector-controller/internal/database/badger" // +kubebuilder:scaffold:imports ) @@ -83,7 +83,7 @@ func TestMain(m *testing.M) { if err = (&ImageRepositoryReconciler{ Client: testEnv, - Database: database.NewBadgerDatabase(testBadgerDB), + Database: ircbadger.NewBadgerDatabase(testBadgerDB), EventRecorder: record.NewFakeRecorder(256), }).SetupWithManager(testEnv, ImageRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), @@ -93,7 +93,7 @@ func TestMain(m *testing.M) { if err = (&ImagePolicyReconciler{ Client: testEnv, - Database: database.NewBadgerDatabase(testBadgerDB), + Database: ircbadger.NewBadgerDatabase(testBadgerDB), EventRecorder: record.NewFakeRecorder(256), }).SetupWithManager(testEnv, ImagePolicyReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), diff --git a/internal/database/badger.go b/internal/database/badger/badger.go similarity index 66% rename from internal/database/badger.go rename to internal/database/badger/badger.go index 5efb4fbe..22c5bcd8 100644 --- a/internal/database/badger.go +++ b/internal/database/badger/badger.go @@ -13,13 +13,15 @@ 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 database +package badger import ( "encoding/json" "fmt" "github.com/dgraph-io/badger/v3" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) const tagsPrefix = "tags" @@ -29,6 +31,9 @@ type BadgerDatabase struct { db *badger.DB } +var _ database.DatabaseWriter = &BadgerDatabase{} +var _ database.DatabaseReader = &BadgerDatabase{} + // NewBadgerDatabase creates and returns a new database implementation using // Badger for storing the image tags. func NewBadgerDatabase(db *badger.DB) *BadgerDatabase { @@ -40,8 +45,8 @@ func NewBadgerDatabase(db *badger.DB) *BadgerDatabase { // Tags implements the DatabaseReader interface, fetching the tags for the repo. // // If the repo does not exist, an empty set of tags is returned. -func (a *BadgerDatabase) Tags(repo string) ([]string, error) { - var tags []string +func (a *BadgerDatabase) Tags(repo string) ([]database.Tag, error) { + var tags []database.Tag err := a.db.View(func(txn *badger.Txn) error { var err error tags, err = getOrEmpty(txn, repo) @@ -54,7 +59,7 @@ func (a *BadgerDatabase) Tags(repo string) ([]string, error) { // the repo. // // It overwrites existing tag sets for the provided repo. -func (a *BadgerDatabase) SetTags(repo string, tags []string) error { +func (a *BadgerDatabase) SetTags(repo string, tags []database.Tag) error { b, err := marshal(tags) if err != nil { return err @@ -69,15 +74,15 @@ func keyForRepo(prefix, repo string) []byte { return []byte(fmt.Sprintf("%s:%s", prefix, repo)) } -func getOrEmpty(txn *badger.Txn, repo string) ([]string, error) { +func getOrEmpty(txn *badger.Txn, repo string) ([]database.Tag, error) { item, err := txn.Get(keyForRepo(tagsPrefix, repo)) if err == badger.ErrKeyNotFound { - return []string{}, nil + return []database.Tag{}, nil } if err != nil { return nil, err } - var tags []string + var tags []database.Tag err = item.Value(func(val []byte) error { tags, err = unmarshal(val) return err @@ -85,14 +90,22 @@ func getOrEmpty(txn *badger.Txn, repo string) ([]string, error) { return tags, err } -func marshal(t []string) ([]byte, error) { +func marshal(t []database.Tag) ([]byte, error) { return json.Marshal(t) } -func unmarshal(b []byte) ([]string, error) { - var tags []string +func unmarshal(b []byte) ([]database.Tag, error) { + var tags []database.Tag if err := json.Unmarshal(b, &tags); err != nil { - return nil, err + // If unmarshalling fails we may be operating on an old database so try to read the old format before eventually bailing out. + var tagsOld []string + if err2 := json.Unmarshal(b, &tagsOld); err2 != nil { + return nil, fmt.Errorf("failed unmarshaling values. First error: %s. Second error: %w", err, err2) + } + tags = make([]database.Tag, len(tagsOld)) + for idx, tag := range tagsOld { + tags[idx] = database.Tag{Name: tag} + } } return tags, nil } diff --git a/internal/database/badger_test.go b/internal/database/badger/badger_test.go similarity index 67% rename from internal/database/badger_test.go rename to internal/database/badger/badger_test.go index 36730fdd..92756335 100644 --- a/internal/database/badger_test.go +++ b/internal/database/badger/badger_test.go @@ -13,7 +13,7 @@ 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 database +package badger import ( "os" @@ -21,6 +21,7 @@ import ( "testing" "github.com/dgraph-io/badger/v3" + "github.com/fluxcd/image-reflector-controller/internal/database" ) const testRepo = "testing/testing" @@ -31,14 +32,18 @@ func TestGetWithUnknownRepo(t *testing.T) { tags, err := db.Tags(testRepo) fatalIfError(t, err) - if !reflect.DeepEqual([]string{}, tags) { - t.Fatalf("Tags() for unknown repo got %#v, want %#v", tags, []string{}) + if !reflect.DeepEqual([]database.Tag{}, tags) { + t.Fatalf("Tags() for unknown repo got %#v, want %#v", tags, []database.Tag{}) } } func TestSetTags(t *testing.T) { db := createBadgerDatabase(t) - tags := []string{"latest", "v0.0.1", "v0.0.2"} + tags := []database.Tag{ + {Name: "latest", Digest: "latest-digest"}, + {Name: "v0.0.1", Digest: "v0.0.1-digest"}, + {Name: "v0.0.2", Digest: "v0.0.2-digest"}, + } fatalIfError(t, db.SetTags(testRepo, tags)) @@ -51,8 +56,8 @@ func TestSetTags(t *testing.T) { func TestSetTagsOverwrites(t *testing.T) { db := createBadgerDatabase(t) - tags1 := []string{"latest", "v0.0.1", "v0.0.2"} - tags2 := []string{"latest", "v0.0.1", "v0.0.2", "v0.0.3"} + tags1 := []database.Tag{{Name: "latest", Digest: "latest-digest"}, {Name: "v0.0.1"}, {Name: "v0.0.2"}} + tags2 := []database.Tag{{Name: "latest", Digest: "new-digest"}, {Name: "v0.0.1"}, {Name: "v0.0.2"}, {Name: "v0.0.3"}} fatalIfError(t, db.SetTags(testRepo, tags1)) fatalIfError(t, db.SetTags(testRepo, tags2)) @@ -66,10 +71,10 @@ func TestSetTagsOverwrites(t *testing.T) { func TestGetOnlyFetchesForRepo(t *testing.T) { db := createBadgerDatabase(t) - tags1 := []string{"latest", "v0.0.1", "v0.0.2"} + tags1 := []database.Tag{{Name: "latest"}, {Name: "v0.0.1"}, {Name: "v0.0.2"}} fatalIfError(t, db.SetTags(testRepo, tags1)) testRepo2 := "another/repo" - tags2 := []string{"v0.0.3", "v0.0.4"} + tags2 := []database.Tag{{Name: "v0.0.3"}, {Name: "v0.0.4"}} fatalIfError(t, db.SetTags(testRepo2, tags2)) loaded, err := db.Tags(testRepo) @@ -79,6 +84,21 @@ func TestGetOnlyFetchesForRepo(t *testing.T) { } } +func TestReadOldData(t *testing.T) { + db := createBadgerDatabase(t) + f, err := os.Open("testdata/old.db") + fatalIfError(t, err) + fatalIfError(t, db.db.Load(f, 1)) + + loaded, err := db.Tags(testRepo) + fatalIfError(t, err) + + expected := []database.Tag{{Name: "latest"}, {Name: "v0.0.1"}, {Name: "v0.0.2"}} + if !reflect.DeepEqual(expected, loaded) { + t.Fatalf("Tags() failed, got %#v, want %#v", loaded, expected) + } +} + func createBadgerDatabase(t *testing.T) *BadgerDatabase { t.Helper() dir, err := os.MkdirTemp(os.TempDir(), "badger") diff --git a/internal/database/badger/testdata/old.db b/internal/database/badger/testdata/old.db new file mode 100644 index 0000000000000000000000000000000000000000..4d744518a380fa8aa45e11b10bdd5a7f9dd7e125 GIT binary patch literal 72 zcmZ=@fB-H#E|HSN^kS=$)Z&uNymWmiEhH1Il#>YJDCsDb8R!}48NwJwO0iOm3<``! Ij0^!x0Erq9+5i9m literal 0 HcmV?d00001 diff --git a/internal/controllers/database.go b/internal/database/database.go similarity index 71% rename from internal/controllers/database.go rename to internal/database/database.go index 129b4e16..26135607 100644 --- a/internal/controllers/database.go +++ b/internal/database/database.go @@ -14,11 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controllers +package database + +// Tag defines the data structure used to store tags in the database. +type Tag struct { + // Name represents the actual tag's value. + Name string `json:"name"` + // Digest represents the digest of the image referred to by the tag. + Digest string `json:"digest"` +} // DatabaseWriter implementations record the tags for an image repository. type DatabaseWriter interface { - SetTags(repo string, tags []string) error + SetTags(repo string, tags []Tag) error } // DatabaseReader implementations get the stored set of tags for an image @@ -27,5 +35,5 @@ type DatabaseWriter interface { // If no tags are availble for the repo, then implementations should return an // empty set of tags. type DatabaseReader interface { - Tags(repo string) ([]string, error) + Tags(repo string) ([]Tag, error) } diff --git a/internal/features/features.go b/internal/features/features.go index e369074c..f96edb03 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -27,12 +27,21 @@ const ( // When enabled, it will cache both object types, resulting in increased // memory usage and cluster-wide RBAC permissions (list and watch). CacheSecretsAndConfigMaps = "CacheSecretsAndConfigMaps" + // StoreImageDigests toggles fetching and storing the digests for all + // image tags discovered for an ImageRepository. + // + // When enabled, the digest of the latest image tag will be put into the + // `.status.latestImageDigest` field of an ImagePolicy. + StoreImageDigests = "StoreImageDigests" ) var features = map[string]bool{ // CacheSecretsAndConfigMaps // opt-in from v0.24 CacheSecretsAndConfigMaps: false, + // StoreImageDigests + // opt-in + StoreImageDigests: false, } // FeatureGates contains a list of all supported feature gates and their default diff --git a/internal/policy/alphabetical.go b/internal/policy/alphabetical.go index f588f66c..ba03e624 100644 --- a/internal/policy/alphabetical.go +++ b/internal/policy/alphabetical.go @@ -19,6 +19,8 @@ package policy import ( "fmt" "sort" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) const ( @@ -33,6 +35,8 @@ type Alphabetical struct { Order string } +var _ Policer = &Alphabetical{} + // NewAlphabetical constructs a Alphabetical object validating the provided // order argument func NewAlphabetical(order string) (*Alphabetical, error) { @@ -51,16 +55,25 @@ func NewAlphabetical(order string) (*Alphabetical, error) { } // Latest returns latest version from a provided list of strings -func (p *Alphabetical) Latest(versions []string) (string, error) { +func (p *Alphabetical) Latest(versions []database.Tag) (*database.Tag, error) { if len(versions) == 0 { - return "", fmt.Errorf("version list argument cannot be empty") + return nil, fmt.Errorf("version list argument cannot be empty") } - var sorted sort.StringSlice = versions + tagNames := make([]string, len(versions)) + tagsByName := make(map[string]database.Tag, len(versions)) + for idx, name := range versions { + tagNames[idx] = name.Name + tagsByName[name.Name] = name + } + + var sorted sort.StringSlice = tagNames if p.Order == AlphabeticalOrderDesc { sort.Sort(sorted) } else { sort.Sort(sort.Reverse(sorted)) } - return sorted[0], nil + selected := tagsByName[sorted[0]] + + return &selected, nil } diff --git a/internal/policy/alphabetical_test.go b/internal/policy/alphabetical_test.go index 525dec74..40905631 100644 --- a/internal/policy/alphabetical_test.go +++ b/internal/policy/alphabetical_test.go @@ -18,6 +18,10 @@ package policy import ( "testing" + + . "github.com/onsi/gomega" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) func TestNewAlphabetical(t *testing.T) { @@ -62,83 +66,136 @@ func TestAlphabetical_Latest(t *testing.T) { cases := []struct { label string order string - versions []string - expectedVersion string + versions []database.Tag + expectedVersion database.Tag expectErr bool }{ { - label: "With Ubuntu CalVer", - versions: []string{"16.04", "16.04.1", "16.10", "20.04", "20.10"}, - expectedVersion: "20.10", + label: "With Ubuntu CalVer", + versions: []database.Tag{ + {Name: "16.04"}, + {Name: "16.04.1"}, + {Name: "16.10"}, + {Name: "20.04"}, + {Name: "20.10", Digest: "20.10-dig"}, + }, + expectedVersion: database.Tag{Name: "20.10", Digest: "20.10-dig"}, }, { - label: "With Ubuntu CalVer descending", - versions: []string{"16.04", "16.04.1", "16.10", "20.04", "20.10"}, + label: "With Ubuntu CalVer descending", + versions: []database.Tag{ + {Name: "16.04", Digest: "16.04-dig"}, + {Name: "16.04.1"}, + {Name: "16.10"}, + {Name: "20.04"}, + {Name: "20.10"}, + }, order: AlphabeticalOrderDesc, - expectedVersion: "16.04", + expectedVersion: database.Tag{Name: "16.04", Digest: "16.04-dig"}, }, { - label: "With Ubuntu code names", - versions: []string{"xenial", "yakkety", "zesty", "artful", "bionic"}, - expectedVersion: "zesty", + label: "With Ubuntu code names", + versions: []database.Tag{ + {Name: "xenial"}, + {Name: "yakkety"}, + {Name: "zesty", Digest: "dig"}, + {Name: "artful"}, + {Name: "bionic"}, + }, + expectedVersion: database.Tag{Name: "zesty", Digest: "dig"}, }, { - label: "With Ubuntu code names descending", - versions: []string{"xenial", "yakkety", "zesty", "artful", "bionic"}, + label: "With Ubuntu code names descending", + versions: []database.Tag{ + {Name: "xenial"}, + {Name: "yakkety"}, + {Name: "zesty"}, + {Name: "artful", Digest: "aec070645fe53ee3b3763059376134f058cc337"}, + {Name: "bionic"}, + }, order: AlphabeticalOrderDesc, - expectedVersion: "artful", + expectedVersion: database.Tag{Name: "artful", Digest: "aec070645fe53ee3b3763059376134f058cc337"}, }, { - label: "With Timestamps", - versions: []string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}, - expectedVersion: "1606364286", + label: "With Timestamps", + versions: []database.Tag{ + {Name: "1606234201"}, + {Name: "1606364286", Digest: "1606364286-33383"}, + {Name: "1606334092"}, + {Name: "1606334284"}, + {Name: "1606334201"}, + }, + expectedVersion: database.Tag{Name: "1606364286", Digest: "1606364286-33383"}, }, { - label: "With Unix Timestamps desc", - versions: []string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}, + label: "With Unix Timestamps desc", + versions: []database.Tag{ + {Name: "1606234201", Digest: "1606234201@494781"}, + {Name: "1606364286"}, + {Name: "1606334092"}, + {Name: "1606334284"}, + {Name: "1606334201"}, + }, order: AlphabeticalOrderDesc, - expectedVersion: "1606234201", + expectedVersion: database.Tag{Name: "1606234201", Digest: "1606234201@494781"}, }, { - label: "With Unix Timestamps prefix", - versions: []string{"rel-1606234201", "rel-1606364286", "rel-1606334092", "rel-1606334284", "rel-1606334201"}, - expectedVersion: "rel-1606364286", + label: "With Unix Timestamps prefix", + versions: []database.Tag{ + {Name: "rel-1606234201"}, + {Name: "rel-1606364286", Digest: "80f95d07201fd766c027279813220d6fc6826038e45cdd4f2b78e00297beb337"}, + {Name: "rel-1606334092"}, + {Name: "rel-1606334284"}, + {Name: "rel-1606334201"}, + }, + expectedVersion: database.Tag{Name: "rel-1606364286", Digest: "80f95d07201fd766c027279813220d6fc6826038e45cdd4f2b78e00297beb337"}, }, { - label: "With RFC3339", - versions: []string{"2021-01-08T21-18-21Z", "2020-05-08T21-18-21Z", "2021-01-08T19-20-00Z", "1990-01-08T00-20-00Z", "2023-05-08T00-20-00Z"}, - expectedVersion: "2023-05-08T00-20-00Z", + label: "With RFC3339", + versions: []database.Tag{ + {Name: "2021-01-08T21-18-21Z"}, + {Name: "2020-05-08T21-18-21Z"}, + {Name: "2021-01-08T19-20-00Z"}, + {Name: "1990-01-08T00-20-00Z"}, + {Name: "2023-05-08T00-20-00Z", Digest: "MjAyMy0wNS0wOFQwMC0yMC0wMFo="}, + }, + expectedVersion: database.Tag{Name: "2023-05-08T00-20-00Z", Digest: "MjAyMy0wNS0wOFQwMC0yMC0wMFo="}, }, { - label: "With RFC3339 desc", - versions: []string{"2021-01-08T21-18-21Z", "2020-05-08T21-18-21Z", "2021-01-08T19-20-00Z", "1990-01-08T00-20-00Z", "2023-05-08T00-20-00Z"}, + label: "With RFC3339 desc", + versions: []database.Tag{ + {Name: "2021-01-08T21-18-21Z", Digest: "0"}, + {Name: "2020-05-08T21-18-21Z", Digest: "1"}, + {Name: "2021-01-08T19-20-00Z", Digest: "2"}, + {Name: "1990-01-08T00-20-00Z", Digest: "3"}, + {Name: "2023-05-08T00-20-00Z", Digest: "4"}, + }, order: AlphabeticalOrderDesc, - expectedVersion: "1990-01-08T00-20-00Z", + expectedVersion: database.Tag{Name: "1990-01-08T00-20-00Z", Digest: "3"}, }, { label: "Empty version list", - versions: []string{}, + versions: []database.Tag{}, expectErr: true, }, } for _, tt := range cases { t.Run(tt.label, func(t *testing.T) { + g := NewWithT(t) + policy, err := NewAlphabetical(tt.order) if err != nil { t.Fatalf("returned unexpected error: %s", err) } latest, err := policy.Latest(tt.versions) - if tt.expectErr && err == nil { - t.Fatalf("expecting error, got nil") - } - if !tt.expectErr && err != nil { - t.Fatalf("returned unexpected error: %s", err) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return } + g.Expect(err).NotTo(HaveOccurred()) - if latest != tt.expectedVersion { - t.Errorf("incorrect computed version returned, got '%s', expected '%s'", latest, tt.expectedVersion) - } + g.Expect(latest).To(Equal(&tt.expectedVersion)) }) } } diff --git a/internal/policy/filter.go b/internal/policy/filter.go index 0ff7f60b..4f234ad1 100644 --- a/internal/policy/filter.go +++ b/internal/policy/filter.go @@ -19,11 +19,13 @@ package policy import ( "fmt" "regexp" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) // RegexFilter represents a regular expression filter type RegexFilter struct { - filtered map[string]string + filtered map[string]database.Tag Regexp *regexp.Regexp Replace string @@ -42,31 +44,35 @@ func NewRegexFilter(pattern string, replace string) (*RegexFilter, error) { } // Apply will construct the filtered list of tags based on the provided list of tags -func (f *RegexFilter) Apply(list []string) { - f.filtered = map[string]string{} +func (f *RegexFilter) Apply(list []database.Tag) { + f.filtered = map[string]database.Tag{} for _, item := range list { - if submatches := f.Regexp.FindStringSubmatchIndex(item); len(submatches) > 0 { - tag := item + if submatches := f.Regexp.FindStringSubmatchIndex(item.Name); len(submatches) > 0 { + tag := item.Name if f.Replace != "" { result := []byte{} - result = f.Regexp.ExpandString(result, f.Replace, item, submatches) + result = f.Regexp.ExpandString(result, f.Replace, item.Name, submatches) tag = string(result) } - f.filtered[tag] = item + f.filtered[tag] = database.Tag{ + Name: item.Name, + Digest: item.Digest, + } } } } // Items returns the list of filtered tags -func (f *RegexFilter) Items() []string { - var filtered []string - for k := range f.filtered { - filtered = append(filtered, k) +func (f *RegexFilter) Items() []database.Tag { + var filtered []database.Tag + for filteredTag, v := range f.filtered { + v.Name = filteredTag + filtered = append(filtered, v) } return filtered } // GetOriginalTag returns the original tag before replace extraction -func (f *RegexFilter) GetOriginalTag(tag string) string { +func (f *RegexFilter) GetOriginalTag(tag string) database.Tag { return f.filtered[tag] } diff --git a/internal/policy/filter_test.go b/internal/policy/filter_test.go index d5f9b1e7..1a5603f3 100644 --- a/internal/policy/filter_test.go +++ b/internal/policy/filter_test.go @@ -17,45 +17,78 @@ limitations under the License. package policy import ( - "reflect" - "sort" "testing" + + "github.com/fluxcd/image-reflector-controller/internal/database" + . "github.com/onsi/gomega" ) func TestRegexFilter(t *testing.T) { cases := []struct { label string - tags []string + tags []database.Tag pattern string extract string - expected []string + expected []database.Tag + origTags map[string]int }{ { label: "none", - tags: []string{"a"}, - expected: []string{"a"}, + tags: []database.Tag{{Name: "a", Digest: "aa"}}, + expected: []database.Tag{{Name: "a", Digest: "aa"}}, + origTags: map[string]int{ + "a": 0, + }, }, { - label: "valid pattern", - tags: []string{"ver1", "ver2", "ver3", "rel1"}, - pattern: "^ver", - expected: []string{"ver1", "ver2", "ver3"}, + label: "valid pattern", + tags: []database.Tag{ + {Name: "ver1", Digest: "1rev"}, + {Name: "ver2", Digest: "2rev"}, + {Name: "ver3", Digest: "3rev"}, + {Name: "rel1", Digest: "1ler"}, + }, + pattern: "^ver", + expected: []database.Tag{ + {Name: "ver1", Digest: "1rev"}, + {Name: "ver2", Digest: "2rev"}, + {Name: "ver3", Digest: "3rev"}, + }, + origTags: map[string]int{ + "ver1": 0, + }, }, { - label: "valid pattern with capture group", - tags: []string{"ver1", "ver2", "ver3", "rel1"}, - pattern: `ver(\d+)`, - extract: `$1`, - expected: []string{"1", "2", "3"}, + label: "valid pattern with capture group", + tags: []database.Tag{ + {Name: "ver1", Digest: "foo"}, + {Name: "ver2", Digest: "bar"}, + {Name: "rel1", Digest: "qux"}, + {Name: "ver3", Digest: "baz"}, + }, + pattern: `ver(\d+)`, + extract: `$1`, + expected: []database.Tag{ + {Name: "1", Digest: "foo"}, + {Name: "2", Digest: "bar"}, + {Name: "3", Digest: "baz"}, + }, + origTags: map[string]int{ + "1": 0, + "2": 1, + "3": 3, + }, }, } for _, tt := range cases { t.Run(tt.label, func(t *testing.T) { + g := NewWithT(t) filter := newRegexFilter(tt.pattern, tt.extract) filter.Apply(tt.tags) - r := sort.StringSlice(filter.Items()) - if reflect.DeepEqual(r, tt.expected) { - t.Errorf("incorrect value returned, got '%s', expected '%s'", r, tt.expected) + g.Expect(filter.Items()).To(HaveLen(len(tt.expected))) + g.Expect(filter.Items()).To(ContainElements(tt.expected)) + for tagKey, idx := range tt.origTags { + g.Expect(filter.GetOriginalTag(tagKey)).To(Equal(tt.tags[idx])) } }) } diff --git a/internal/policy/numerical.go b/internal/policy/numerical.go index b7f32a08..bfac88d2 100644 --- a/internal/policy/numerical.go +++ b/internal/policy/numerical.go @@ -19,6 +19,8 @@ package policy import ( "fmt" "strconv" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) const ( @@ -33,6 +35,8 @@ type Numerical struct { Order string } +var _ Policer = &Numerical{} + // NewNumerical constructs a Numerical object validating the provided // order argument func NewNumerical(order string) (*Numerical, error) { @@ -51,17 +55,17 @@ func NewNumerical(order string) (*Numerical, error) { } // Latest returns latest version from a provided list of strings -func (p *Numerical) Latest(versions []string) (string, error) { +func (p *Numerical) Latest(versions []database.Tag) (*database.Tag, error) { if len(versions) == 0 { - return "", fmt.Errorf("version list argument cannot be empty") + return nil, fmt.Errorf("version list argument cannot be empty") } - var latest string + var latest database.Tag var pv float64 for i, version := range versions { - cv, err := strconv.ParseFloat(version, 64) + cv, err := strconv.ParseFloat(version.Name, 64) if err != nil { - return "", fmt.Errorf("failed to parse invalid numeric value '%s'", version) + return nil, fmt.Errorf("failed to parse invalid numeric value '%s'", version) } switch { @@ -75,5 +79,5 @@ func (p *Numerical) Latest(versions []string) (string, error) { pv = cv } - return latest, nil + return &latest, nil } diff --git a/internal/policy/numerical_test.go b/internal/policy/numerical_test.go index 497c7900..d8d26b08 100644 --- a/internal/policy/numerical_test.go +++ b/internal/policy/numerical_test.go @@ -20,6 +20,10 @@ import ( "math/rand" "testing" "time" + + . "github.com/onsi/gomega" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) func TestNewNumerical(t *testing.T) { @@ -64,88 +68,145 @@ func TestNumerical_Latest(t *testing.T) { cases := []struct { label string order string - versions []string - expectedVersion string + versions []database.Tag + expectedVersion database.Tag expectErr bool }{ { - label: "With unordered list of integers ascending", - versions: shuffle([]string{"-62", "-88", "73", "72", "15", "16", "15", "29", "-33", "-91"}), - expectedVersion: "73", + label: "With unordered list of integers ascending", + versions: shuffle([]database.Tag{ + {Name: "-62"}, + {Name: "-88"}, + {Name: "73", Digest: "foodigest"}, + {Name: "72"}, + {Name: "15"}, + {Name: "16"}, + {Name: "15"}, + {Name: "29"}, + {Name: "-33"}, + {Name: "-91"}, + }), + expectedVersion: database.Tag{Name: "73", Digest: "foodigest"}, }, { - label: "With unordered list of integers descending", - versions: shuffle([]string{"5", "-8", "-78", "25", "70", "-4", "80", "92", "-20", "-24"}), + label: "With unordered list of integers descending", + versions: shuffle([]database.Tag{ + {Name: "5"}, + {Name: "-8"}, + {Name: "-78", Digest: "somedig"}, + {Name: "25"}, + {Name: "70"}, + {Name: "-4"}, + {Name: "80"}, + {Name: "92"}, + {Name: "-20"}, + {Name: "-24"}, + }), order: NumericalOrderDesc, - expectedVersion: "-78", + expectedVersion: database.Tag{Name: "-78", Digest: "somedig"}, }, { - label: "With unordered list of floats ascending", - versions: shuffle([]string{"47.40896403322944", "-27.8520927455902", "-27.930666514224427", "-31.352485948094568", "-50.41072694704882", "-21.962849842263736", "24.71884721436865", "-39.99177354004344", "53.47333823144817", "3.2008658570411086"}), - expectedVersion: "53.47333823144817", + label: "With unordered list of floats ascending", + versions: shuffle([]database.Tag{ + {Name: "47.40896403322944"}, + {Name: "-27.8520927455902"}, + {Name: "-27.930666514224427"}, + {Name: "-31.352485948094568"}, + {Name: "-50.41072694704882"}, + {Name: "-21.962849842263736"}, + {Name: "24.71884721436865"}, + {Name: "-39.99177354004344"}, + {Name: "53.47333823144817", Digest: "47333823144817"}, + {Name: "3.2008658570411086"}, + }), + expectedVersion: database.Tag{Name: "53.47333823144817", Digest: "47333823144817"}, }, { - label: "With unordered list of floats descending", - versions: shuffle([]string{"-65.27202780220686", "57.82948329142309", "22.40184684363291", "-86.36934305697784", "-90.29082099756083", "-12.041712603564264", "77.70488240399305", "-38.98425003883552", "16.06867070412028", "53.735674335181216"}), + label: "With unordered list of floats descending", + versions: shuffle([]database.Tag{ + {Name: "-65.27202780220686"}, + {Name: "57.82948329142309"}, + {Name: "22.40184684363291"}, + {Name: "-86.36934305697784"}, + {Name: "-90.29082099756083", Digest: "-90"}, + {Name: "-12.041712603564264"}, + {Name: "77.70488240399305"}, + {Name: "-38.98425003883552"}, + {Name: "16.06867070412028"}, + {Name: "53.735674335181216"}, + }), order: NumericalOrderDesc, - expectedVersion: "-90.29082099756083", + expectedVersion: database.Tag{Name: "-90.29082099756083", Digest: "-90"}, }, { - label: "With Unix Timestamps ascending", - versions: shuffle([]string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}), - expectedVersion: "1606364286", + label: "With Unix Timestamps ascending", + versions: shuffle([]database.Tag{ + {Name: "1606234201"}, + {Name: "1606364286", Digest: "find-me"}, + {Name: "1606334092"}, + {Name: "1606334284"}, + {Name: "1606334201"}, + }), + expectedVersion: database.Tag{Name: "1606364286", Digest: "find-me"}, }, { - label: "With Unix Timestamps descending", - versions: shuffle([]string{"1606234201", "1606364286", "1606334092", "1606334284", "1606334201"}), + label: "With Unix Timestamps descending", + versions: shuffle([]database.Tag{ + {Name: "1606234201", Digest: "foobar"}, + {Name: "1606364286"}, + {Name: "1606334092"}, + {Name: "1606334284"}, + {Name: "1606334201"}, + }), order: NumericalOrderDesc, - expectedVersion: "1606234201", + expectedVersion: database.Tag{Name: "1606234201", Digest: "foobar"}, }, { label: "With single value ascending", - versions: []string{"1"}, - expectedVersion: "1", + versions: []database.Tag{{Name: "1"}}, + expectedVersion: database.Tag{Name: "1"}, }, { label: "With single value descending", - versions: []string{"1"}, + versions: []database.Tag{{Name: "1"}}, order: NumericalOrderDesc, - expectedVersion: "1", + expectedVersion: database.Tag{Name: "1"}, }, { - label: "With invalid numerical value", - versions: []string{"0", "1a", "b"}, + label: "With invalid numerical value", + versions: []database.Tag{{Name: "0"}, + {Name: "1a"}, + {Name: "b"}, + }, expectErr: true, }, { label: "Empty version list", - versions: []string{}, + versions: []database.Tag{}, expectErr: true, }, } for _, tt := range cases { t.Run(tt.label, func(t *testing.T) { + g := NewWithT(t) + policy, err := NewNumerical(tt.order) - if err != nil { - t.Fatalf("returned unexpected error: %s", err) - } + g.Expect(err).NotTo(HaveOccurred()) + latest, err := policy.Latest(tt.versions) - if tt.expectErr && err == nil { - t.Fatalf("expecting error, got nil") - } - if !tt.expectErr && err != nil { - t.Fatalf("returned unexpected error: %s", err) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return } + g.Expect(err).NotTo(HaveOccurred()) - if latest != tt.expectedVersion { - t.Errorf("incorrect computed version returned, got '%s', expected '%s'", latest, tt.expectedVersion) - } + g.Expect(latest).To(Equal(&tt.expectedVersion), "incorrect computed version returned") }) } } -func shuffle(list []string) []string { +func shuffle(list []database.Tag) []database.Tag { rand.Seed(time.Now().UnixNano()) rand.Shuffle(len(list), func(i, j int) { list[i], list[j] = list[j], list[i] }) return list diff --git a/internal/policy/policer.go b/internal/policy/policer.go index c70ca922..67730c13 100644 --- a/internal/policy/policer.go +++ b/internal/policy/policer.go @@ -16,7 +16,9 @@ limitations under the License. package policy +import "github.com/fluxcd/image-reflector-controller/internal/database" + // Policer is an interface representing a policy implementation type type Policer interface { - Latest([]string) (string, error) + Latest([]database.Tag) (*database.Tag, error) } diff --git a/internal/policy/semver.go b/internal/policy/semver.go index 64988731..7620c44a 100644 --- a/internal/policy/semver.go +++ b/internal/policy/semver.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/Masterminds/semver/v3" + "github.com/fluxcd/image-reflector-controller/internal/database" "github.com/fluxcd/pkg/version" ) @@ -44,22 +45,25 @@ func NewSemVer(r string) (*SemVer, error) { } // Latest returns latest version from a provided list of strings -func (p *SemVer) Latest(versions []string) (string, error) { +func (p *SemVer) Latest(versions []database.Tag) (*database.Tag, error) { if len(versions) == 0 { - return "", fmt.Errorf("version list argument cannot be empty") + return nil, fmt.Errorf("version list argument cannot be empty") } var latestVersion *semver.Version + var latestTag *database.Tag for _, tag := range versions { - if v, err := version.ParseVersion(tag); err == nil { + tag := tag + if v, err := version.ParseVersion(tag.Name); err == nil { if p.constraint.Check(v) && (latestVersion == nil || v.GreaterThan(latestVersion)) { latestVersion = v + latestTag = &tag } } } - if latestVersion != nil { - return latestVersion.Original(), nil + if latestTag != nil { + return latestTag, nil } - return "", fmt.Errorf("unable to determine latest version from provided list") + return nil, fmt.Errorf("unable to determine latest version from provided list") } diff --git a/internal/policy/semver_test.go b/internal/policy/semver_test.go index 12754440..96aaaddc 100644 --- a/internal/policy/semver_test.go +++ b/internal/policy/semver_test.go @@ -18,6 +18,10 @@ package policy import ( "testing" + + . "github.com/onsi/gomega" + + "github.com/fluxcd/image-reflector-controller/internal/database" ) func TestNewSemVer(t *testing.T) { @@ -56,37 +60,52 @@ func TestSemVer_Latest(t *testing.T) { cases := []struct { label string semverRange string - versions []string - expectedVersion string + versions []database.Tag + expectedVersion database.Tag expectErr bool }{ { - label: "With valid format", - versions: []string{"1.0.0", "1.0.0.1", "1.0.0p", "1.0.1", "1.2.0", "0.1.0"}, + label: "With valid format", + versions: []database.Tag{ + {Name: "1.0.0", Digest: "foo"}, + {Name: "1.0.0.1", Digest: "bar"}, + {Name: "1.0.0p", Digest: "baz"}, + {Name: "1.0.1", Digest: "qux"}, + {Name: "1.2.0", Digest: "faa"}, + {Name: "0.1.0", Digest: "quux"}, + }, semverRange: "1.0.x", - expectedVersion: "1.0.1", + expectedVersion: database.Tag{Name: "1.0.1", Digest: "qux"}, }, { - label: "With valid format prefix", - versions: []string{"v1.2.3", "v1.0.0", "v0.1.0"}, + label: "With valid format prefix", + versions: []database.Tag{ + {Name: "v1.2.3", Digest: "v1.2.3-digest"}, + {Name: "v1.0.0", Digest: "v1.0.0-digest"}, + {Name: "v0.1.0", Digest: "v0.1.0-dig"}, + }, semverRange: "1.0.x", - expectedVersion: "v1.0.0", + expectedVersion: database.Tag{Name: "v1.0.0", Digest: "v1.0.0-digest"}, }, { - label: "With invalid format prefix", - versions: []string{"b1.2.3", "b1.0.0", "b0.1.0"}, + label: "With invalid format prefix", + versions: []database.Tag{ + {Name: "b1.2.3"}, + {Name: "b1.0.0"}, + {Name: "b0.1.0"}, + }, semverRange: "1.0.x", expectErr: true, }, { label: "With empty list", - versions: []string{}, + versions: []database.Tag{}, semverRange: "1.0.x", expectErr: true, }, { label: "With non-matching version list", - versions: []string{"1.2.0"}, + versions: []database.Tag{{Name: "1.2.0"}}, semverRange: "1.0.x", expectErr: true, }, @@ -94,22 +113,20 @@ func TestSemVer_Latest(t *testing.T) { for _, tt := range cases { t.Run(tt.label, func(t *testing.T) { + g := NewWithT(t) + policy, err := NewSemVer(tt.semverRange) - if err != nil { - t.Fatalf("returned unexpected error: %s", err) - } + g.Expect(err).NotTo(HaveOccurred()) latest, err := policy.Latest(tt.versions) - if tt.expectErr && err == nil { - t.Fatalf("expecting error, got nil") - } - if !tt.expectErr && err != nil { - t.Fatalf("returned unexpected error: %s", err) + if tt.expectErr { + g.Expect(err).To(HaveOccurred()) + return } - if latest != tt.expectedVersion { - t.Errorf("incorrect computed version returned, got '%s', expected '%s'", latest, tt.expectedVersion) - } + g.Expect(err).NotTo(HaveOccurred()) + + g.Expect(latest).To(Equal(&tt.expectedVersion), "incorrect computed version returned") }) } } diff --git a/main.go b/main.go index aae1c762..52adcbf1 100644 --- a/main.go +++ b/main.go @@ -47,7 +47,7 @@ import ( imagev1 "github.com/fluxcd/image-reflector-controller/api/v1beta2" "github.com/fluxcd/image-reflector-controller/internal/controllers" - "github.com/fluxcd/image-reflector-controller/internal/database" + ircbadger "github.com/fluxcd/image-reflector-controller/internal/database/badger" "github.com/fluxcd/image-reflector-controller/internal/features" ) @@ -128,7 +128,7 @@ func main() { os.Exit(1) } defer badgerDB.Close() - db := database.NewBadgerDatabase(badgerDB) + db := ircbadger.NewBadgerDatabase(badgerDB) watchNamespace := "" if !watchOptions.AllNamespaces { @@ -145,6 +145,12 @@ func main() { disableCacheFor = append(disableCacheFor, &corev1.Secret{}, &corev1.ConfigMap{}) } + fetchDigests, err := features.Enabled(features.StoreImageDigests) + if err != nil { + setupLog.Error(err, "unable to check feature gate "+features.StoreImageDigests) + os.Exit(1) + } + restConfig := client.GetConfigOrDie(clientOptions) watchSelector, err := helper.GetWatchSelector(watchOptions) @@ -207,6 +213,7 @@ func main() { AzureAutoLogin: azureAutoLogin, GcpAutoLogin: gcpAutoLogin, }, + FetchDigests: fetchDigests, }).SetupWithManager(mgr, controllers.ImageRepositoryReconcilerOptions{ MaxConcurrentReconciles: concurrent, RateLimiter: helper.GetRateLimiter(rateLimiterOptions),