Fix support for --ref when adding git profile registries (#864)

* Fix support for --ref when adding git profile registries

Author: chrnorm

pkg/git/clone.go

@@ -11,6 +11,11 @@ import (
 
 // Clone wraps the command 'git clone'.
 func Clone(repoURL string, repoDirPath string) error {
+	return CloneWithRef(repoURL, repoDirPath, "")
+}
+
+// CloneWithRef wraps the command 'git clone' and checks out a specific ref.
+func CloneWithRef(repoURL string, repoDirPath string, ref string) error {
 	clio.Debugf("git clone %s\n", repoURL)
 
 	cmd := exec.Command("git", "clone", repoURL, repoDirPath)
@@ -30,5 +35,25 @@ func Clone(repoURL string, repoDirPath string) error {
 	}
 	clio.Debugf("Successfully cloned %s", repoURL)
 
+	// If a ref is specified, checkout that ref
+	if ref != "" {
+		clio.Debugf("git -C %s checkout %s\n", repoDirPath, ref)
+		checkoutCmd := exec.Command("git", "-C", repoDirPath, "checkout", ref)
+
+		stderr, _ := checkoutCmd.StderrPipe()
+		if err := checkoutCmd.Start(); err != nil {
+			return err
+		}
+
+		scanner := bufio.NewScanner(stderr)
+		for scanner.Scan() {
+			if strings.Contains(strings.ToLower(scanner.Text()), "error") || strings.Contains(strings.ToLower(scanner.Text()), "fatal") {
+				return errors.New(scanner.Text())
+			}
+			clio.Info(scanner.Text())
+		}
+		clio.Debugf("Successfully checked out %s", ref)
+	}
+
 	return nil
 }

pkg/git/clone_test.go

@@ -0,0 +1,78 @@
+package git
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestCloneWithRef(t *testing.T) {
+	// Create a temporary directory for the test
+	tempDir, err := os.MkdirTemp("", "git-clone-test")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer func() {
+		_ = os.RemoveAll(tempDir)
+	}()
+
+	// Define test cases
+	tests := []struct {
+		name     string
+		repoURL  string
+		ref      string
+		wantErr  bool
+	}{
+		{
+			name:    "clone with main branch",
+			repoURL: "https://github.com/octocat/Hello-World.git",
+			ref:     "master",
+			wantErr: false,
+		},
+		{
+			name:    "clone with specific commit",
+			repoURL: "https://github.com/octocat/Hello-World.git",
+			ref:     "7fd1a60", // First 7 chars of a commit hash
+			wantErr: false,
+		},
+		{
+			name:    "clone with empty ref",
+			repoURL: "https://github.com/octocat/Hello-World.git",
+			ref:     "",
+			wantErr: false,
+		},
+		{
+			name:    "clone with non-existent ref",
+			repoURL: "https://github.com/octocat/Hello-World.git",
+			ref:     "non-existent-branch",
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a unique directory for each test
+			cloneDir := filepath.Join(tempDir, tt.name)
+			
+			err := CloneWithRef(tt.repoURL, cloneDir, tt.ref)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("CloneWithRef() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			// If we expected success and got it, verify the clone
+			if !tt.wantErr && err == nil {
+				// Check if the directory exists
+				if _, err := os.Stat(cloneDir); os.IsNotExist(err) {
+					t.Errorf("Clone directory does not exist: %s", cloneDir)
+				}
+
+				// Check if .git directory exists
+				gitDir := filepath.Join(cloneDir, ".git")
+				if _, err := os.Stat(gitDir); os.IsNotExist(err) {
+					t.Errorf(".git directory does not exist in clone: %s", gitDir)
+				}
+			}
+		})
+	}
+}

pkg/git/pull.go

@@ -11,9 +11,29 @@ import (
 
 // Pull wraps the command 'git pull'.
 func Pull(repoDirPath string, shouldSilentLogs bool) error {
+	return PullRef(repoDirPath, "", shouldSilentLogs)
+}
+
+// PullRef wraps the command 'git pull' for a specific ref.
+func PullRef(repoDirPath string, ref string, shouldSilentLogs bool) error {
+	// if ref is specified, ensure we're on the right branch first
+	if ref != "" {
+		clio.Debugf("git -C %s checkout %s\n", repoDirPath, ref)
+		checkoutCmd := exec.Command("git", "-C", repoDirPath, "checkout", ref)
+		if err := checkoutCmd.Run(); err != nil {
+			return err
+		}
+	}
+
+	// determine what to pull
+	pullRef := "HEAD"
+	if ref != "" {
+		pullRef = ref
+	}
+
 	// pull the repo here.
-	clio.Debugf("git -C %s pull %s %s\n", repoDirPath, "origin", "HEAD")
-	cmd := exec.Command("git", "-C", repoDirPath, "pull", "origin", "HEAD")
+	clio.Debugf("git -C %s pull %s %s\n", repoDirPath, "origin", pullRef)
+	cmd := exec.Command("git", "-C", repoDirPath, "pull", "origin", pullRef)
 
 	// StderrPipe returns a pipe that will be connected to the command's
 	// standard error when the command starts.

pkg/granted/registry/add.go

@@ -29,7 +29,7 @@ var AddCommand = cli.Command{
 		&cli.StringFlag{Name: "path", Usage: "For git registries: provide path if only the subfolder needs to be synced", Aliases: []string{"p"}},
 		&cli.StringFlag{Name: "filename", Aliases: []string{"f"}, Usage: "For git registries:  provide filename if yml file is not granted.yml", DefaultText: "granted.yml"},
 		&cli.IntFlag{Name: "priority", Usage: "The priority for the profile registry", Value: 0},
-		&cli.StringFlag{Name: "ref", Hidden: true},
+		&cli.StringFlag{Name: "ref", Usage: "Git ref (branch, tag, or commit) to checkout"},
 		&cli.BoolFlag{Name: "prefix-all-profiles", Aliases: []string{"pap"}, Usage: "Provide this flag if you want to append registry name to all profiles"},
 		&cli.BoolFlag{Name: "prefix-duplicate-profiles", Aliases: []string{"pdp"}, Usage: "Provide this flag if you want to append registry name to duplicate profiles"},
 		&cli.BoolFlag{Name: "write-on-sync-failure", Aliases: []string{"wosf"}, Usage: "Always overwrite AWS config, even if sync fails (DEPRECATED)"},
@@ -93,6 +93,7 @@ var AddCommand = cli.Command{
 			URL:          URL,
 			Path:         pathFlag,
 			Filename:     configFileName,
+			Ref:          ref,
 			RequiredKeys: requiredKey,
 			Interactive:  true,
 		})

pkg/granted/registry/add_test.go

@@ -0,0 +1,51 @@
+package registry
+
+import (
+	"testing"
+
+	"github.com/common-fate/granted/pkg/granted/registry/gitregistry"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRegistryBackwardCompatibility(t *testing.T) {
+	tests := []struct {
+		name    string
+		ref     string
+		wantErr bool
+	}{
+		{
+			name:    "existing flow without ref works",
+			ref:     "",
+			wantErr: false,
+		},
+		{
+			name:    "flow with ref works",
+			ref:     "master",
+			wantErr: false,
+		},
+		{
+			name:    "flow with invalid ref fails",
+			ref:     "invalid-branch",
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test that we can create a registry with the ref
+			opts := gitregistry.Opts{
+				Name:     "test-registry",
+				URL:      "https://github.com/octocat/Hello-World.git",
+				Filename: "README",
+				Ref:      tt.ref,
+			}
+
+			registry, err := gitregistry.New(opts)
+			assert.NoError(t, err)
+			assert.NotNil(t, registry)
+
+			// We can't directly test pull() as it's private and uses internal paths
+			// But we've verified that the registry can be created with or without ref
+		})
+	}
+}

pkg/granted/registry/gitregistry/gitregistry.go

@@ -20,6 +20,7 @@ type Opts struct {
 	URL          string
 	Path         string
 	Filename     string
+	Ref          string
 	RequiredKeys []string
 	Interactive  bool
 }

pkg/granted/registry/gitregistry/pull.go

@@ -10,11 +10,11 @@ import (
 func (r Registry) pull() error {
 	if _, err := os.Stat(r.clonedTo); err != nil {
 		// folder doesn't exist yet, so clone the repo and return early.
-		return git.Clone(r.opts.URL, r.clonedTo)
+		return git.CloneWithRef(r.opts.URL, r.clonedTo, r.opts.Ref)
 	}
 
 	// if we get here, the folder exists, so pull any changes.
-	err := git.Pull(r.clonedTo, false)
+	err := git.PullRef(r.clonedTo, r.opts.Ref, false)
 	if err != nil {
 		return err
 	}

pkg/granted/registry/gitregistry/pull_test.go

@@ -0,0 +1,52 @@
+package gitregistry
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRegistryWithRef(t *testing.T) {
+	tests := []struct {
+		name string
+		opts Opts
+	}{
+		{
+			name: "create registry without ref (backward compatibility)",
+			opts: Opts{
+				Name:     "test-registry",
+				URL:      "https://github.com/octocat/Hello-World.git",
+				Filename: "README",
+				// Ref is not set, testing backward compatibility
+			},
+		},
+		{
+			name: "create registry with empty ref",
+			opts: Opts{
+				Name:     "test-registry",
+				URL:      "https://github.com/octocat/Hello-World.git",
+				Filename: "README",
+				Ref:      "",
+			},
+		},
+		{
+			name: "create registry with ref",
+			opts: Opts{
+				Name:     "test-registry",
+				URL:      "https://github.com/octocat/Hello-World.git",
+				Filename: "README",
+				Ref:      "master",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Test that we can create a registry with or without ref
+			registry, err := New(tt.opts)
+			assert.NoError(t, err)
+			assert.NotNil(t, registry)
+			assert.Equal(t, tt.opts.Ref, registry.opts.Ref)
+		})
+	}
+}

pkg/granted/registry/registry.go

@@ -37,6 +37,7 @@ func GetProfileRegistries(interactive bool) ([]loadedRegistry, error) {
 				URL:         r.URL,
 				Path:        r.Path,
 				Filename:    r.Filename,
+				Ref:         r.Ref,
 				Interactive: interactive,
 			})