Directly check out SHAs (#5)

This removes the necessity to set a branch when referencing SHAs in Git repositories.

Signed-off-by: Jan Schlicht <[email protected]>

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"`
 }

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
 }

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
 		}
 	}

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)
+		}
+	}
 }