From cc3e9019161b4fc6a2723bd2ca41529ad26a74e2 Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Wed, 15 Jul 2020 11:56:10 +0200 Subject: [PATCH] Directly check out SHAs (#5) This removes the necessity to set a branch when referencing SHAs in Git repositories. Signed-off-by: Jan Schlicht --- pkg/apis/operator/v1alpha1/types.go | 7 +-- pkg/internal/apis/operator/types.go | 7 +-- pkg/internal/resolvers/git/git.go | 25 ++++++--- pkg/internal/resolvers/git/git_test.go | 78 ++++++++++++++++++++------ 4 files changed, 85 insertions(+), 32 deletions(-) diff --git a/pkg/apis/operator/v1alpha1/types.go b/pkg/apis/operator/v1alpha1/types.go index a376f65..09254ac 100644 --- a/pkg/apis/operator/v1alpha1/types.go +++ b/pkg/apis/operator/v1alpha1/types.go @@ -51,11 +51,10 @@ type Git struct { // Directory where the KUDO operator is defined in the Git repository. Directory string `yaml:"directory"` - // Tag (or branch) of the KUDO operator version. + // Tag of the KUDO operator version. Either this or 'SHA' has to be set. Tag string `yaml:"tag,omitempty"` - // Optional SHA of the KUDO operator version if a branch is used instead of - // a tag. If this isn't set, the latest commit of the referenced branch will - // be used. + // SHA of the KUDO operator version if a branch is used instead of + // a tag. Either this or 'Tag' has to be set. SHA string `yaml:"sha,omitempty"` } diff --git a/pkg/internal/apis/operator/types.go b/pkg/internal/apis/operator/types.go index 21f2743..582fab3 100644 --- a/pkg/internal/apis/operator/types.go +++ b/pkg/internal/apis/operator/types.go @@ -42,11 +42,10 @@ type Git struct { // Directory where the KUDO operator is defined in the Git repository. Directory string - // Tag (or branch) of the KUDO operator version. + // Tag of the KUDO operator version. Either this or 'SHA' has to be set. Tag string - // Optional SHA of the KUDO operator version if a branch is used instead of - // a tag. If this isn't set, the latest commit of the referenced branch will - // be used. + // SHA of the KUDO operator version if a branch is used instead of + // a tag. Either this or 'Tag' has to be set. SHA string } diff --git a/pkg/internal/resolvers/git/git.go b/pkg/internal/resolvers/git/git.go index bab524b..2e44198 100644 --- a/pkg/internal/resolvers/git/git.go +++ b/pkg/internal/resolvers/git/git.go @@ -3,6 +3,7 @@ package git import ( "bufio" "context" + "errors" "os/exec" "path" @@ -42,6 +43,10 @@ func NewResolver(url, branch, sha string, operatorDirectory string) Resolver { func (r Resolver) Resolve(ctx context.Context) (afero.Fs, resolvers.Remover, error) { fs := afero.NewOsFs() + if r.Branch == "" && r.SHA == "" { + return nil, nil, errors.New("neither branch nor SHA provided") + } + tempDir, err := afero.TempDir(fs, "", "") if err != nil { return nil, nil, err @@ -69,16 +74,20 @@ func (r Resolver) Resolve(ctx context.Context) (afero.Fs, resolvers.Remover, err } func gitClone(ctx context.Context, tempDir, url, branch, sha string) error { - logger := log.WithField("url", url). - WithField("branch", branch). - WithField("sha", sha) + if branch != "" { + logger := log.WithField("url", url).WithField("branch", branch) - if err := runAndLog(ctx, logger, "git", "clone", "--branch", branch, "--single-branch", url, tempDir); err != nil { - return err - } + if err := runAndLog(ctx, logger, "git", "clone", "--branch", branch, "--single-branch", url, tempDir); err != nil { + return err + } + } else { + logger := log.WithField("url", url).WithField("sha", sha) + + if err := runAndLog(ctx, logger, "git", "clone", url, tempDir); err != nil { + return err + } - if sha != "" { - if err := runAndLog(ctx, logger, "git", "-C", tempDir, "reset", "--hard", sha); err != nil { + if err := runAndLog(ctx, logger, "git", "-C", tempDir, "checkout", sha); err != nil { return err } } diff --git a/pkg/internal/resolvers/git/git_test.go b/pkg/internal/resolvers/git/git_test.go index 3c75cdd..8c71b37 100644 --- a/pkg/internal/resolvers/git/git_test.go +++ b/pkg/internal/resolvers/git/git_test.go @@ -9,25 +9,71 @@ import ( ) func TestResolve(t *testing.T) { - testClone := func(ctx context.Context, tempDir, url, branch, sha string) error { - if url == "example.org" && branch == "test" && sha == "" { - return nil - } + tests := []struct { + name string + cloneFake func(context.Context, string, string, string, string) error + branch string + sha string + expectErr bool + }{ + { + name: "resolve branch", + cloneFake: func(ctx context.Context, tempDir, url, branch, sha string) error { + if url == "example.org" && branch == "test" && sha == "" { + return nil + } - return errors.New("wrong URL and branch") - } + return errors.New("wrong URL and branch") + }, + branch: "test", + sha: "", + expectErr: false, + }, + { + name: "resolve SHA", + cloneFake: func(ctx context.Context, tempDir, url, branch, sha string) error { + if url == "example.org" && branch == "" && sha == "abcdefg" { + return nil + } - resolver := &Resolver{ - URL: "example.org", - Branch: "test", - OperatorDirectory: "operator", - gitClone: testClone, + return errors.New("wrong URL and SHA") + }, + branch: "", + sha: "abcdefg", + expectErr: false, + }, + { + name: "neither branch nor SHA set", + cloneFake: nil, + branch: "", + sha: "", + expectErr: true, + }, } - _, remover, err := resolver.Resolve(context.Background()) - assert.NoError(t, err) + for _, test := range tests { + test := test + + resolver := &Resolver{ + URL: "example.org", + Branch: test.branch, + SHA: test.sha, + OperatorDirectory: "operator", + gitClone: test.cloneFake, + } + + _, remover, err := resolver.Resolve(context.Background()) - defer func() { - assert.NoError(t, remover()) - }() + if remover != nil { + defer func() { + assert.NoError(t, remover()) + }() + } + + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + } }