Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/vendir/config/directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ type DirectoryContentsGit struct {
SkipInitSubmodules bool `json:"skipInitSubmodules,omitempty"`
Depth int `json:"depth,omitempty"`
ForceHTTPBasicAuth bool `json:"forceHTTPBasicAuth,omitempty"`
// OriginalRef holds the ref before it was replaced by a locked SHA.
// Not serialized; set by Lock() to enable targeted fetching in locked mode.
OriginalRef string `json:"-" yaml:"-"`
}

type DirectoryContentsGitVerification struct {
Expand Down Expand Up @@ -348,6 +351,7 @@ func (c *DirectoryContentsGit) Lock(lockConfig *LockDirectoryContentsGit) error
if len(lockConfig.SHA) == 0 {
return fmt.Errorf("Expected git SHA to be non-empty")
}
c.OriginalRef = c.Ref
c.Ref = lockConfig.SHA
return nil
}
Expand Down
65 changes: 62 additions & 3 deletions pkg/vendir/fetch/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,43 @@ func (t *Git) fetch(dstPath string, tempArea ctlfetch.TempArea, bundle string) e
}
}

argss = append(argss, []string{"config", "remote.origin.tagOpt", "--tags"})
// Suppress tag fetching when a targeted refspec fetch is possible:
// - named ref (tag/branch): targeted fetch by name
// - locked SHA with a known named original ref: fetch by original ref name
// RefSelection needs all tags for semver matching. A bare SHA with no
// OriginalRef has no named ref to target and must fall back to a full fetch.
canTargetFetch := (len(t.opts.Ref) > 0 && !isHexSHA(t.opts.Ref) && t.opts.RefSelection == nil) ||
(isHexSHA(t.opts.Ref) && len(t.opts.OriginalRef) > 0 && !isHexSHA(t.opts.OriginalRef))
if canTargetFetch {
argss = append(argss, []string{"config", "remote.origin.tagOpt", "--no-tags"})
} else {
argss = append(argss, []string{"config", "remote.origin.tagOpt", "--tags"})
}

if bundle != "" {
argss = append(argss, []string{"bundle", "unbundle", bundle})
}

// When fetching a single named ref, git places it in FETCH_HEAD only —
// no local ref is created. Track this so we can use FETCH_HEAD for checkout.
var useFetchHead bool
{
fetchArgs := []string{"fetch", "origin"}
if strings.HasPrefix(t.opts.Ref, "origin/") {
// only fetch the exact ref we're seeking
switch {
case strings.HasPrefix(t.opts.Ref, "origin/"):
// only fetch the exact remote branch we're seeking
fetchArgs = append(fetchArgs, t.opts.Ref[7:])
case len(t.opts.Ref) > 0 && !isHexSHA(t.opts.Ref):
// fetch only the named tag or branch; SHAs must use a full fetch
fetchArgs = append(fetchArgs, t.opts.Ref, "--no-tags")
useFetchHead = true
case isHexSHA(t.opts.Ref) && len(t.opts.OriginalRef) > 0 && !isHexSHA(t.opts.OriginalRef):
// locked mode: fetch by the original named ref so we avoid a full fetch.
// Strip any "origin/" prefix — that prefix is a local tracking convention
// and is not a valid remote refspec.
originalRef := strings.TrimPrefix(t.opts.OriginalRef, "origin/")
fetchArgs = append(fetchArgs, originalRef, "--no-tags")
useFetchHead = true
}
if t.opts.Depth > 0 {
fetchArgs = append(fetchArgs, "--depth", strconv.Itoa(t.opts.Depth))
Expand All @@ -194,6 +220,24 @@ func (t *Git) fetch(dstPath string, tempArea ctlfetch.TempArea, bundle string) e
return err
}

// In locked mode we fetch by the original named ref for performance, but must
// check out the exact locked SHA. Verify FETCH_HEAD resolves to that SHA before
// checkout so a force-pushed tag is caught early.
if useFetchHead && isHexSHA(t.opts.Ref) {
out, _, err := t.cmdRunner.Run([]string{"rev-parse", "FETCH_HEAD^{commit}"}, nil, dstPath)
if err != nil {
return err
}
if strings.TrimSpace(out) != t.opts.Ref {
return fmt.Errorf("Locked SHA %s does not match fetched commit %s — tag may have been moved", t.opts.Ref, strings.TrimSpace(out))
}
// Use the locked SHA for checkout so mutable refs (e.g. origin/main) don't
// cause locked syncs to track the branch tip instead of the pinned commit.
ref = t.opts.Ref
} else if useFetchHead {
ref = "FETCH_HEAD"
}

if t.opts.Verification != nil {
err := Verification{dstPath, *t.opts.Verification, t.refFetcher}.Verify(ref)
if err != nil {
Expand Down Expand Up @@ -245,6 +289,21 @@ func (t *Git) tags(dstPath string) ([]string, error) {
return strings.Split(out, "\n"), nil
}

// isHexSHA reports whether s looks like a full or abbreviated git commit SHA
// (7–40 hex characters, case-insensitive). SHAs cannot be fetched by refspec
// name and require a full fetch to be resolved.
func isHexSHA(s string) bool {
if len(s) < 7 || len(s) > 40 {
return false
}
for _, c := range s {
if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) {
return false
}
}
return true
}

type CommandRunner interface {
RunMultiple(argss [][]string, env []string, dstPath string) error
Run(args []string, env []string, dstPath string) (string, string, error)
Expand Down
179 changes: 179 additions & 0 deletions pkg/vendir/fetch/git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"carvel.dev/vendir/pkg/vendir/config"
"carvel.dev/vendir/pkg/vendir/fetch"
"carvel.dev/vendir/pkg/vendir/fetch/git"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -65,10 +66,183 @@ func TestGit_Retrieve(t *testing.T) {
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.ErrorContains(t, err, "Username/password authentication is only supported for https remotes")
})

t.Run("Named tag ref uses targeted fetch with --no-tags", func(t *testing.T) {
runner := &cmdRunnerLocal{commandsToRun: [][]string{}}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: "v1.2.3",
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs, "expected a fetch command")
assert.Contains(t, fetchArgs, "v1.2.3", "expected specific ref in fetch")
assert.Contains(t, fetchArgs, "--no-tags", "expected --no-tags for named ref fetch")

tagOptArgs := findConfigArgs(runner.commandsToRun, "remote.origin.tagOpt")
require.NotNil(t, tagOptArgs, "expected tagOpt config command")
assert.Equal(t, "--no-tags", tagOptArgs[len(tagOptArgs)-1], "expected --no-tags tagOpt for named ref")

checkoutArgs := findCheckoutArgs(runner.commandsToRun)
require.NotNil(t, checkoutArgs, "expected a checkout command")
assert.Contains(t, checkoutArgs, "FETCH_HEAD", "expected FETCH_HEAD checkout for targeted fetch")
})

t.Run("origin/ branch ref uses branch-scoped fetch without --no-tags on refspec", func(t *testing.T) {
runner := &cmdRunnerLocal{commandsToRun: [][]string{}}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: "origin/main",
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs, "expected a fetch command")
assert.Contains(t, fetchArgs, "main", "expected branch name in fetch for origin/ ref")
assert.NotContains(t, fetchArgs, "--no-tags", "expected no --no-tags on the refspec for origin/ branch")

checkoutArgs := findCheckoutArgs(runner.commandsToRun)
require.NotNil(t, checkoutArgs, "expected a checkout command")
assert.Contains(t, checkoutArgs, "main", "expected branch name checkout for origin/ ref")
assert.NotContains(t, checkoutArgs, "FETCH_HEAD", "expected direct checkout for origin/ ref, not FETCH_HEAD")
})

t.Run("Locked SHA with OriginalRef uses OriginalRef for targeted fetch", func(t *testing.T) {
lockedSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"
runner := &cmdRunnerLocal{
commandsToRun: [][]string{},
// rev-parse FETCH_HEAD^{commit} must return the locked SHA for verification to pass
responses: map[string]string{"rev-parse FETCH_HEAD^{commit}": lockedSHA},
}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: lockedSHA,
OriginalRef: "v1.2.3",
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs, "expected a fetch command")
assert.Contains(t, fetchArgs, "v1.2.3", "expected OriginalRef in fetch for locked SHA mode")
assert.Contains(t, fetchArgs, "--no-tags", "expected --no-tags for locked SHA with OriginalRef")

tagOptArgs := findConfigArgs(runner.commandsToRun, "remote.origin.tagOpt")
require.NotNil(t, tagOptArgs)
assert.Equal(t, "--no-tags", tagOptArgs[len(tagOptArgs)-1], "expected --no-tags tagOpt for locked SHA with OriginalRef")

// Checkout must use the locked SHA directly, not FETCH_HEAD, so mutable
// refs (e.g. origin/main) don't cause locked sync to track the branch tip.
checkoutArgs := findCheckoutArgs(runner.commandsToRun)
require.NotNil(t, checkoutArgs)
assert.Contains(t, checkoutArgs, lockedSHA, "expected locked SHA checkout in locked SHA mode")
assert.NotContains(t, checkoutArgs, "FETCH_HEAD", "expected locked SHA, not FETCH_HEAD, for checkout")

// Verification: rev-parse FETCH_HEAD^{commit} must be called before checkout
revParseArgs := findCommandArgs(runner.commandsToRun, "rev-parse")
require.NotNil(t, revParseArgs, "expected rev-parse for SHA verification")
assert.Contains(t, revParseArgs, "FETCH_HEAD^{commit}")
})

t.Run("Locked SHA with origin/ OriginalRef strips prefix before fetch", func(t *testing.T) {
lockedSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2"
runner := &cmdRunnerLocal{
commandsToRun: [][]string{},
responses: map[string]string{"rev-parse FETCH_HEAD^{commit}": lockedSHA},
}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: lockedSHA,
OriginalRef: "origin/main", // common in vendir configs
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs)
assert.Contains(t, fetchArgs, "main", "expected origin/ prefix stripped from OriginalRef")
assert.NotContains(t, fetchArgs, "origin/main", "expected origin/ prefix stripped before fetch refspec")
})

t.Run("Plain SHA ref without OriginalRef uses full fetch with --tags", func(t *testing.T) {
runner := &cmdRunnerLocal{commandsToRun: [][]string{}}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", // 40-char hex SHA, no OriginalRef
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs, "expected a fetch command")
// full fetch: only "fetch" and "origin", no extra refspec
assert.Equal(t, []string{"fetch", "origin"}, fetchArgs, "expected bare full fetch for plain SHA")

tagOptArgs := findConfigArgs(runner.commandsToRun, "remote.origin.tagOpt")
require.NotNil(t, tagOptArgs)
assert.Equal(t, "--tags", tagOptArgs[len(tagOptArgs)-1], "expected --tags tagOpt for plain SHA ref")
})

t.Run("Depth flag is appended after refspec for named ref targeted fetch", func(t *testing.T) {
runner := &cmdRunnerLocal{commandsToRun: [][]string{}}
gitRetriever := git.NewGitWithRunner(config.DirectoryContentsGit{
URL: "https://some.git/repo",
Ref: "v1.2.3",
Depth: 1,
}, os.Stdout, &fetch.SingleSecretRefFetcher{}, runner)
_, err := gitRetriever.Retrieve("", &tmpFolder{t}, "")
require.NoError(t, err)

fetchArgs := findCommandArgs(runner.commandsToRun, "fetch")
require.NotNil(t, fetchArgs)
fetchStr := strings.Join(fetchArgs, " ")
assert.Contains(t, fetchStr, "v1.2.3 --no-tags --depth 1", "expected refspec, --no-tags, then --depth in order")
})
}

// findCommandArgs finds the first command in commandsToRun whose first arg matches firstArg.
func findCommandArgs(commands [][]string, firstArg string) []string {
for _, args := range commands {
if len(args) > 0 && args[0] == firstArg {
return args
}
}
return nil
}

// findCheckoutArgs finds the git checkout command (issued as "-c advice.detachedHead=false checkout ...").
func findCheckoutArgs(commands [][]string) []string {
for _, args := range commands {
for i, a := range args {
if a == "checkout" {
return args[i:]
}
}
}
return nil
}

// findConfigArgs finds the git config command that sets the given key.
func findConfigArgs(commands [][]string, key string) []string {
for _, args := range commands {
if len(args) >= 2 && args[0] == "config" {
for _, a := range args[1:] {
if a == key {
return args
}
}
}
}
return nil
}

type cmdRunnerLocal struct {
commandsToRun [][]string
// responses maps "arg0 arg1 ..." to the stdout value Run should return.
responses map[string]string
}

func (c *cmdRunnerLocal) RunMultiple(argss [][]string, _ []string, _ string) error {
Expand All @@ -78,6 +252,11 @@ func (c *cmdRunnerLocal) RunMultiple(argss [][]string, _ []string, _ string) err

func (c *cmdRunnerLocal) Run(args []string, _ []string, _ string) (string, string, error) {
c.commandsToRun = append(c.commandsToRun, args)
if c.responses != nil {
if out, ok := c.responses[strings.Join(args, " ")]; ok {
return out, "", nil
}
}
return "", "", nil
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/vendir/fetch/http/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ func (t *Sync) addAuth(req *http.Request) error {

// Basic auth — password is optional, defaults to empty string
if hasUser {
password := string(secret.Data[
ctlconf.SecretK8sCorev1BasicAuthPasswordKey])
password := string(secret.Data[ctlconf.SecretK8sCorev1BasicAuthPasswordKey])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result of go fmt from hack/build.sh

req.SetBasicAuth(
string(secret.Data[ctlconf.SecretK8sCorev1BasicAuthUsernameKey]),
password,
Expand Down
Loading