From a6887eaaa00d23e4c8d7213221aeee54cefdbd86 Mon Sep 17 00:00:00 2001 From: Kent Rancourt Date: Wed, 31 Jan 2024 14:29:22 -0500 Subject: [PATCH] corresponding controller changes Signed-off-by: Kent Rancourt --- internal/controller/git/git.go | 16 + internal/controller/warehouses/git.go | 217 ++++++++- internal/controller/warehouses/git_test.go | 426 +++++++++++++++++- internal/controller/warehouses/warehouses.go | 21 +- .../controller/warehouses/warehouses_test.go | 15 +- 5 files changed, 659 insertions(+), 36 deletions(-) diff --git a/internal/controller/git/git.go b/internal/controller/git/git.go index f039450913..e6c11e3356 100644 --- a/internal/controller/git/git.go +++ b/internal/controller/git/git.go @@ -70,6 +70,8 @@ type Repo interface { // LastCommitID returns the ID (sha) of the most recent commit to the current // branch. LastCommitID() (string, error) + // ListTags returns a slice of tags in the repository. + ListTags() ([]string, error) // CommitMessage returns the text of the most recent commit message associated // with the specified commit ID. CommitMessage(id string) (string, error) @@ -353,6 +355,20 @@ func (r *repo) LastCommitID() (string, error) { errors.Wrap(err, "error obtaining ID of last commit") } +func (r *repo) ListTags() ([]string, error) { + tagsBytes, err := libExec.Exec(r.buildCommand("tag", "--list", "--sort", "-creatordate")) + if err != nil { + return nil, errors.Wrapf(err, "error listing tags for repo %q", r.url) + } + tags := []string{} + scanner := bufio.NewScanner(bytes.NewReader(tagsBytes)) + scanner.Split(bufio.ScanLines) + for scanner.Scan() { + tags = append(tags, strings.TrimSpace(scanner.Text())) + } + return tags, nil +} + func (r *repo) CommitMessage(id string) (string, error) { msgBytes, err := libExec.Exec( r.buildCommand("log", "-n", "1", "--pretty=format:%s", id), diff --git a/internal/controller/warehouses/git.go b/internal/controller/warehouses/git.go index fffc2e6987..f4d3765630 100644 --- a/internal/controller/warehouses/git.go +++ b/internal/controller/warehouses/git.go @@ -2,8 +2,11 @@ package warehouses import ( "context" + "regexp" + "sort" "strings" + "github.com/Masterminds/semver" "github.com/pkg/errors" kargoapi "github.com/akuity/kargo/api/v1alpha1" @@ -14,11 +17,12 @@ import ( type gitMeta struct { Commit string + Tag string Message string Author string } -func (r *reconciler) getLatestCommits( +func (r *reconciler) selectCommits( ctx context.Context, namespace string, subs []kargoapi.RepoSubscription, @@ -51,7 +55,7 @@ func (r *reconciler) getLatestCommits( logger.Debug("found no credentials for git repo") } - gm, err := r.getLatestCommitMetaFn(ctx, *s.Git, repoCreds) + gm, err := r.selectCommitMetaFn(ctx, *s.Git, repoCreds) if err != nil { return nil, errors.Wrapf( err, @@ -67,6 +71,7 @@ func (r *reconciler) getLatestCommits( RepoURL: sub.RepoURL, ID: gm.Commit, Branch: sub.Branch, + Tag: gm.Tag, Message: gm.Message, }, ) @@ -74,7 +79,10 @@ func (r *reconciler) getLatestCommits( return latestCommits, nil } -func getLatestCommitMeta( +// selectCommitMeta uses criteria from the provided GitSubscription to select +// an appropriate revision of the repository also specified by the subscription +// and return metadata associated with that revision. +func (r *reconciler) selectCommitMeta( ctx context.Context, sub kargoapi.GitSubscription, creds *git.RepoCredentials, @@ -83,39 +91,212 @@ func getLatestCommitMeta( if creds == nil { creds = &git.RepoCredentials{} } + if sub.CommitSelectionStrategy == "" { + sub.CommitSelectionStrategy = kargoapi.CommitSelectionStrategyNewestFromBranch + } repo, err := git.Clone( sub.RepoURL, *creds, &git.CloneOptions{ - Branch: sub.Branch, - SingleBranch: true, - Shallow: true, + Branch: sub.Branch, + // We can optimize with shallow, single branch clones only when the + // commit selection strategy is newest from branch + SingleBranch: sub.CommitSelectionStrategy == kargoapi.CommitSelectionStrategyNewestFromBranch, + Shallow: sub.CommitSelectionStrategy == kargoapi.CommitSelectionStrategyNewestFromBranch, InsecureSkipTLSVerify: sub.InsecureSkipTLSVerify, }, ) if err != nil { return nil, errors.Wrapf(err, "error cloning git repo %q", sub.RepoURL) } - var gm gitMeta - gm.Commit, err = repo.LastCommitID() + selectedTag, selectedCommit, err := r.selectTagAndCommitID(repo, sub) if err != nil { return nil, errors.Wrapf( err, - "error determining last commit ID from git repo %q (branch: %q)", + "error selecting commit from git repo %q", sub.RepoURL, - sub.Branch, ) } - msg, err := repo.CommitMessage(gm.Commit) - // Since we currently store commit messages in Stage status, we only capture - // the first line of the commit message for brevity - gm.Message = strings.Split(strings.TrimSpace(msg), "\n")[0] + msg, err := repo.CommitMessage(selectedCommit) if err != nil { // This is best effort, so just log the error - logger.Warnf("failed to get message from commit %q: %v", gm.Commit, err) + logger.Warnf("failed to get message from commit %q: %v", selectedCommit, err) + } + return &gitMeta{ + Commit: selectedCommit, + Tag: selectedTag, + // Since we currently store commit messages in Stage status, we only capture + // the first line of the commit message for brevity + Message: strings.Split(strings.TrimSpace(msg), "\n")[0], + // TODO: support git author + }, nil +} + +// selectTagAndCommitID uses criteria from the provided GitSubscription to +// select and return an appropriate revision of the repository also specified by +// the subscription. +func (r *reconciler) selectTagAndCommitID( + repo git.Repo, + sub kargoapi.GitSubscription, +) (string, string, error) { + if sub.CommitSelectionStrategy == kargoapi.CommitSelectionStrategyNewestFromBranch { + // In this case, there is nothing to do except return the commit ID at the + // head of the branch. + commit, err := r.getLastCommitIDFn(repo) + return "", commit, errors.Wrapf( + err, + "error determining commit ID at head of branch %q in git repo %q", + sub.Branch, + sub.RepoURL, + ) + } + + tags, err := r.listTagsFn(repo) // These are ordered newest to oldest + if err != nil { + return "", "", + errors.Wrapf(err, "error listing tags from git repo %q", sub.RepoURL) + } + + // Narrow down the list of tags to those that are allowed and not ignored + allowRegex, err := regexp.Compile(sub.AllowTags) + if err != nil { + return "", "", + errors.Wrapf(err, "error compiling regular expression %q", sub.AllowTags) + } + filteredTags := make([]string, 0, len(tags)) + for _, tagName := range tags { + if allows(tagName, allowRegex) && !ignores(tagName, sub.IgnoreTags) { + filteredTags = append(filteredTags, tagName) + } + } + if len(filteredTags) == 0 { + return "", "", + errors.Errorf("found no applicable tags in repo %q", sub.RepoURL) + } + + var selectedTag string + switch sub.CommitSelectionStrategy { + case kargoapi.CommitSelectionStrategyLexical: + selectedTag = selectLexicallyLastTag(filteredTags) + case kargoapi.CommitSelectionStrategyNewestTag: + selectedTag = filteredTags[0] // These are already ordered newest to oldest + case kargoapi.CommitSelectionStrategySemVer: + if selectedTag, err = + selectSemverTag(filteredTags, sub.SemverConstraint); err != nil { + return "", "", err + } + default: + return "", "", errors.Errorf( + "unknown commit selection strategy %q", + sub.CommitSelectionStrategy, + ) + } + if selectedTag == "" { + return "", "", errors.Errorf("found no applicable tags in repo %q", sub.RepoURL) + } + + // Checkout the selected tag and return the commit ID + if err = r.checkoutTagFn(repo, selectedTag); err != nil { + return "", "", errors.Wrapf( + err, + "error checking out tag %q from git repo %q", + selectedTag, + sub.RepoURL, + ) } - // TODO: support git author - //gm.Author, err = repo.Author(gm.Commit) + commit, err := r.getLastCommitIDFn(repo) + return selectedTag, commit, errors.Wrapf( + err, + "error determining commit ID of tag %q in git repo %q", + selectedTag, + sub.RepoURL, + ) +} + +// allows returns true if the given tag name matches the given regular +// expression or if the regular expression is nil. It returns false otherwise. +func allows(tagName string, allowRegex *regexp.Regexp) bool { + if allowRegex == nil { + return true + } + return allowRegex.MatchString(tagName) +} + +// ignores returns true if the given tag name is in the given list of ignored +// tag names. It returns false otherwise. +func ignores(tagName string, ignore []string) bool { + for _, i := range ignore { + if i == tagName { + return true + } + } + return false +} + +// selectLexicallyLastTag sorts the provided tag name in reverse lexicographic +// order and returns the first tag name in the sorted list. If the list is +// empty, it returns an empty string. +func selectLexicallyLastTag(tagNames []string) string { + if len(tagNames) == 0 { + return "" + } + sort.Slice(tagNames, func(i, j int) bool { + return tagNames[i] > tagNames[j] + }) + return tagNames[0] +} + +// selectSemverTag narrows the provided list of tag names to those that are +// valid semantic versions. If constraintStr is non-empty, it further narrows +// the list to those that satisfy the constraint. If the narrowed list is +// non-empty, it sorts the list in reverse semver order and returns the first +// tag name in the sorted list. If the narrowed list is empty, it returns an +// empty string. +func selectSemverTag(tagNames []string, constraintStr string) (string, error) { + var constraint *semver.Constraints + if constraintStr != "" { + var err error + if constraint, err = semver.NewConstraint(constraintStr); err != nil { + return "", errors.Wrapf( + err, + "error parsing semver constraint %q", + constraintStr, + ) + } + } + semvers := make([]*semver.Version, 0, len(tagNames)) + for _, tagName := range tagNames { + sv, err := semver.NewVersion(tagName) + if err != nil { + continue // tagName wasn't a semantic version + } + if constraint == nil || constraint.Check(sv) { + semvers = append(semvers, sv) + } + } + if len(semvers) == 0 { + return "", nil + } + sort.Slice(semvers, func(i, j int) bool { + if comp := semvers[i].Compare(semvers[j]); comp != 0 { + return comp > 0 + } + // If the semvers tie, break the tie lexically using the original strings + // used to construct the semvers. This ensures a deterministic comparison + // of equivalent semvers, e.g., 1.0 and 1.0.0. + return semvers[i].Original() > semvers[j].Original() + }) + return semvers[0].Original(), nil +} + +func (r *reconciler) getLastCommitID(repo git.Repo) (string, error) { + return repo.LastCommitID() +} + +func (r *reconciler) listTags(repo git.Repo) ([]string, error) { + return repo.ListTags() +} - return &gm, nil +func (r *reconciler) checkoutTag(repo git.Repo, tag string) error { + return repo.Checkout(tag) } diff --git a/internal/controller/warehouses/git_test.go b/internal/controller/warehouses/git_test.go index 96c60493fe..0506ab8c47 100644 --- a/internal/controller/warehouses/git_test.go +++ b/internal/controller/warehouses/git_test.go @@ -2,18 +2,20 @@ package warehouses import ( "context" + "regexp" "strings" "testing" "github.com/pkg/errors" "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client/fake" kargoapi "github.com/akuity/kargo/api/v1alpha1" "github.com/akuity/kargo/internal/controller/git" "github.com/akuity/kargo/internal/credentials" ) -func TestGetLatestCommits(t *testing.T) { +func TestSelectCommits(t *testing.T) { testCases := []struct { name string reconciler *reconciler @@ -59,7 +61,7 @@ func TestGetLatestCommits(t *testing.T) { return credentials.Credentials{}, false, nil }, }, - getLatestCommitMetaFn: func( + selectCommitMetaFn: func( context.Context, kargoapi.GitSubscription, *git.RepoCredentials, @@ -92,7 +94,7 @@ func TestGetLatestCommits(t *testing.T) { return credentials.Credentials{}, false, nil }, }, - getLatestCommitMetaFn: func( + selectCommitMetaFn: func( context.Context, kargoapi.GitSubscription, *git.RepoCredentials, @@ -118,7 +120,7 @@ func TestGetLatestCommits(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { testCase.assertions( - testCase.reconciler.getLatestCommits( + testCase.reconciler.selectCommits( context.Background(), "fake-namespace", []kargoapi.RepoSubscription{ @@ -134,10 +136,11 @@ func TestGetLatestCommits(t *testing.T) { } } -func TestGetLatestCommitMeta(t *testing.T) { +func TestSelectCommitMeta(t *testing.T) { testCases := []struct { name string sub kargoapi.GitSubscription + reconciler *reconciler assertions func(*gitMeta, error) }{ { @@ -145,6 +148,7 @@ func TestGetLatestCommitMeta(t *testing.T) { sub: kargoapi.GitSubscription{ RepoURL: "fake-url", // This should force a failure }, + reconciler: &reconciler{}, assertions: func(_ *gitMeta, err error) { require.Error(t, err) require.Contains(t, err.Error(), "error cloning git repo") @@ -155,6 +159,7 @@ func TestGetLatestCommitMeta(t *testing.T) { sub: kargoapi.GitSubscription{ RepoURL: "https://github.com/akuity/kargo.git", }, + reconciler: newReconciler(fake.NewClientBuilder().Build(), nil), assertions: func(gm *gitMeta, err error) { require.NoError(t, err) require.NotEmpty(t, gm.Commit) @@ -166,7 +171,416 @@ func TestGetLatestCommitMeta(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { testCase.assertions( - getLatestCommitMeta(context.TODO(), testCase.sub, nil), + testCase.reconciler.selectCommitMeta( + context.Background(), + testCase.sub, + nil, + ), + ) + }) + } +} + +func TestSelectCommitID(t *testing.T) { + testCases := []struct { + name string + sub kargoapi.GitSubscription + reconciler *reconciler + assertions func(tag string, commit string, err error) + }{ + { + name: "newest from branch; error getting commit ID", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyNewestFromBranch, + }, + reconciler: &reconciler{ + getLastCommitIDFn: func(git.Repo) (string, error) { + return "", errors.New("something went wrong") + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains( + t, + err.Error(), + "error determining commit ID at head of branch", + ) + require.Contains(t, err.Error(), "something went wrong") + }, + }, + { + name: "newest from branch; success", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyNewestFromBranch, + }, + reconciler: &reconciler{ + getLastCommitIDFn: func(git.Repo) (string, error) { + return "fake-commit", nil + }, + }, + assertions: func(tag, commit string, err error) { + require.NoError(t, err) + require.Empty(t, tag) + require.Equal(t, "fake-commit", commit) + }, + }, + { + name: "error listing tags", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return nil, errors.New("something went wrong") + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error listing tags from git repo") + require.Contains(t, err.Error(), "something went wrong") + }, + }, + { + name: "error compiling allow regex", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + AllowTags: "[", // This should force a failure + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc"}, nil + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error compiling regular expression") + }, + }, + { + name: "all tags get filtered out", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + IgnoreTags: []string{"abc"}, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc"}, nil + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "found no applicable tags in repo") + }, + }, + { + name: "unknown selection strategy", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategy("invalid"), + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc"}, nil + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "unknown commit selection strategy") + }, + }, + { + name: "error checking out tag", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc"}, nil + }, + checkoutTagFn: func(git.Repo, string) error { + return errors.New("something went wrong") + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error checking out tag") + require.Contains(t, err.Error(), "something went wrong") + }, + }, + { + name: "error getting commit ID", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc"}, nil + }, + checkoutTagFn: func(git.Repo, string) error { + return nil + }, + getLastCommitIDFn: func(git.Repo) (string, error) { + return "", errors.New("something went wrong") + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error determining commit ID of tag") + require.Contains(t, err.Error(), "something went wrong") + }, + }, + { + name: "lexical success", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyLexical, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc", "xyz"}, nil + }, + checkoutTagFn: func(git.Repo, string) error { + return nil + }, + getLastCommitIDFn: func(git.Repo) (string, error) { + return "fake-commit", nil + }, + }, + assertions: func(tag, commit string, err error) { + require.NoError(t, err) + require.Equal(t, "xyz", tag) + require.Equal(t, "fake-commit", commit) + }, + }, + { + name: "newest tag success", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategyNewestTag, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"abc", "xyz"}, nil + }, + checkoutTagFn: func(git.Repo, string) error { + return nil + }, + getLastCommitIDFn: func(git.Repo) (string, error) { + return "fake-commit", nil + }, + }, + assertions: func(tag, commit string, err error) { + require.Equal(t, "abc", tag) + require.NoError(t, err) + require.Equal(t, "fake-commit", commit) + }, + }, + { + name: "semver error selecting tag", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategySemVer, + SemverConstraint: "invalid", // This should force a failure + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"1.0.0", "2.0.0"}, nil + }, + }, + assertions: func(_, _ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error parsing semver constraint") + }, + }, + { + name: "semver success", + sub: kargoapi.GitSubscription{ + CommitSelectionStrategy: kargoapi.CommitSelectionStrategySemVer, + }, + reconciler: &reconciler{ + listTagsFn: func(git.Repo) ([]string, error) { + return []string{"1.0.0", "2.0.0"}, nil + }, + checkoutTagFn: func(git.Repo, string) error { + return nil + }, + getLastCommitIDFn: func(git.Repo) (string, error) { + return "fake-commit", nil + }, + }, + assertions: func(tag, commit string, err error) { + require.NoError(t, err) + require.Equal(t, "2.0.0", tag) + require.Equal(t, "fake-commit", commit) + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testCase.assertions( + testCase.reconciler.selectTagAndCommitID( + nil, + testCase.sub, + ), + ) + }) + } +} + +func TestAllows(t *testing.T) { + testCases := []struct { + name string + regex *regexp.Regexp + tag string + allowed bool + }{ + { + name: "no regex specified", + tag: "abc", + allowed: true, + }, + { + name: "allowed", + regex: regexp.MustCompile("[a-z]+"), + tag: "abc", + allowed: true, + }, + { + name: "not allowed", + regex: regexp.MustCompile("[a-z]+"), + tag: "123", + allowed: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal( + t, + testCase.allowed, + allows(testCase.tag, testCase.regex), + ) + }) + } +} + +func TestIgnores(t *testing.T) { + testCases := []struct { + name string + ignore []string + tag string + ignored bool + }{ + { + name: "ignored", + ignore: []string{"abc"}, + tag: "abc", + ignored: true, + }, + { + name: "not ignored", + ignore: []string{"abc"}, + tag: "123", + ignored: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal( + t, + testCase.ignored, + ignores(testCase.tag, testCase.ignore), + ) + }) + } +} + +func TestSelectLexicallyLastTag(t *testing.T) { + testCases := []struct { + name string + tags []string + expected string + }{ + { + name: "empty/nil tag list", + tags: nil, + expected: "", + }, + { + name: "non-empty tag list", + tags: []string{"abc", "xyz", "foo", "bar"}, + expected: "xyz", + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal( + t, + testCase.expected, + selectLexicallyLastTag(testCase.tags), + ) + }) + } +} + +func TestSelectSemverTag(t *testing.T) { + testCases := []struct { + name string + constraint string + tags []string + assertions func(string, error) + }{ + { + name: "error parsing constraint", + constraint: "invalid", + tags: nil, + assertions: func(_ string, err error) { + require.Error(t, err) + require.Contains(t, err.Error(), "error parsing semver constraint") + }, + }, + { + name: "empty/nil tag list", + tags: nil, + assertions: func(tag string, err error) { + require.NoError(t, err) + require.Empty(t, tag) + }, + }, + { + name: "no semantic tags in tag list", + tags: []string{"abc", "xyz", "foo", "bar"}, + assertions: func(tag string, err error) { + require.NoError(t, err) + require.Empty(t, tag) + }, + }, + { + name: "no constraint matches", + constraint: ">=2.0.0", + tags: []string{"v1.0.0", "v1.2.3"}, + assertions: func(tag string, err error) { + require.NoError(t, err) + require.Empty(t, tag) + }, + }, + { + name: "success with no constraint", + tags: []string{"v1.0.0", "v1.2.3"}, + assertions: func(tag string, err error) { + require.NoError(t, err) + require.Equal(t, "v1.2.3", tag) + }, + }, + { + name: "success with constraint", + constraint: "<2.0.0", + tags: []string{"v1.0.0", "v2.2.3"}, + assertions: func(tag string, err error) { + require.NoError(t, err) + require.Equal(t, "v1.0.0", tag) + }, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + testCase.assertions( + selectSemverTag(testCase.tags, testCase.constraint), ) }) } diff --git a/internal/controller/warehouses/warehouses.go b/internal/controller/warehouses/warehouses.go index b86ce57600..2311dd82bf 100644 --- a/internal/controller/warehouses/warehouses.go +++ b/internal/controller/warehouses/warehouses.go @@ -40,12 +40,18 @@ type reconciler struct { *kargoapi.Warehouse, ) (*kargoapi.Freight, error) - getLatestCommitsFn func( + selectCommitsFn func( ctx context.Context, namespace string, subs []kargoapi.RepoSubscription, ) ([]kargoapi.GitCommit, error) + getLastCommitIDFn func(repo git.Repo) (string, error) + + listTagsFn func(repo git.Repo) ([]string, error) + + checkoutTagFn func(repo git.Repo, tag string) error + getLatestImagesFn func( ctx context.Context, namespace string, @@ -77,7 +83,7 @@ type reconciler struct { creds *helm.Credentials, ) (string, error) - getLatestCommitMetaFn func( + selectCommitMetaFn func( context.Context, kargoapi.GitSubscription, *git.RepoCredentials, @@ -143,12 +149,15 @@ func newReconciler( freightAliasGenerator: moniker.New(), } r.getLatestFreightFromReposFn = r.getLatestFreightFromRepos - r.getLatestCommitsFn = r.getLatestCommits + r.selectCommitsFn = r.selectCommits + r.getLastCommitIDFn = r.getLastCommitID + r.listTagsFn = r.listTags + r.checkoutTagFn = r.checkoutTag r.getLatestImagesFn = r.getLatestImages r.getImageRefsFn = getImageRefs r.getLatestChartsFn = r.getLatestCharts r.getLatestChartVersionFn = helm.GetLatestChartVersion - r.getLatestCommitMetaFn = getLatestCommitMeta + r.selectCommitMetaFn = r.selectCommitMeta r.getAvailableFreightAliasFn = r.getAvailableFreightAlias r.createFreightFn = kubeClient.Create return r @@ -281,7 +290,7 @@ func (r *reconciler) getLatestFreightFromRepos( ) (*kargoapi.Freight, error) { logger := logging.LoggerFromContext(ctx) - latestCommits, err := r.getLatestCommitsFn( + selectedCommits, err := r.selectCommitsFn( ctx, warehouse.Namespace, warehouse.Spec.Subscriptions, @@ -320,7 +329,7 @@ func (r *reconciler) getLatestFreightFromRepos( Namespace: warehouse.Namespace, OwnerReferences: []metav1.OwnerReference{*ownerRef}, }, - Commits: latestCommits, + Commits: selectedCommits, Images: latestImages, Charts: latestCharts, } diff --git a/internal/controller/warehouses/warehouses_test.go b/internal/controller/warehouses/warehouses_test.go index 541fb35216..0ebe696952 100644 --- a/internal/controller/warehouses/warehouses_test.go +++ b/internal/controller/warehouses/warehouses_test.go @@ -29,12 +29,15 @@ func TestNewReconciler(t *testing.T) { // Assert that all overridable behaviors were initialized to a default: require.NotNil(t, e.getLatestFreightFromReposFn) - require.NotNil(t, e.getLatestCommitsFn) + require.NotNil(t, e.selectCommitsFn) + require.NotNil(t, e.getLastCommitIDFn) + require.NotNil(t, e.listTagsFn) + require.NotNil(t, e.checkoutTagFn) require.NotNil(t, e.getLatestImagesFn) require.NotNil(t, e.getImageRefsFn) require.NotNil(t, e.getLatestChartsFn) require.NotNil(t, e.getLatestChartVersionFn) - require.NotNil(t, e.getLatestCommitMetaFn) + require.NotNil(t, e.selectCommitMetaFn) require.NotNil(t, e.getAvailableFreightAliasFn) require.NotNil(t, e.createFreightFn) } @@ -217,7 +220,7 @@ func TestGetLatestFreightFromRepos(t *testing.T) { { name: "error getting latest git commits", reconciler: &reconciler{ - getLatestCommitsFn: func( + selectCommitsFn: func( context.Context, string, []kargoapi.RepoSubscription, @@ -235,7 +238,7 @@ func TestGetLatestFreightFromRepos(t *testing.T) { { name: "error getting latest images", reconciler: &reconciler{ - getLatestCommitsFn: func( + selectCommitsFn: func( context.Context, string, []kargoapi.RepoSubscription, @@ -264,7 +267,7 @@ func TestGetLatestFreightFromRepos(t *testing.T) { { name: "error getting latest charts", reconciler: &reconciler{ - getLatestCommitsFn: func( + selectCommitsFn: func( context.Context, string, []kargoapi.RepoSubscription, @@ -300,7 +303,7 @@ func TestGetLatestFreightFromRepos(t *testing.T) { { name: "success", reconciler: &reconciler{ - getLatestCommitsFn: func( + selectCommitsFn: func( context.Context, string, []kargoapi.RepoSubscription,