Fix retry logic for pushing git repos (#1067)

Remove stale state from the failed push so that we can attempt to push
the repository.

Co-authored-by: Wayne Starr <[email protected]>
Co-authored-by: Megamind <[email protected]>

src/config/lang/english.go

@@ -199,3 +199,4 @@ const (
 
 // ErrInitNotFound
 var ErrInitNotFound = errors.New("this command requires a zarf-init package, but one was not found on the local system. Re-run the last command again without '--confirm' to download the package")
+var ErrNotAServiceURL = errors.New("the provided URL does not match service url format of http://{SERVICE_NAME}.{NAMESPACE}.svc.cluster.local:{PORT}")

src/internal/cluster/tunnel.go

@@ -7,7 +7,6 @@ package cluster
 // Forked from https://github.com/gruntwork-io/terratest/blob/v0.38.8/modules/k8s/tunnel.go
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -22,6 +21,7 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/defenseunicorns/zarf/src/config/lang"
 	"github.com/defenseunicorns/zarf/src/types"
 
 	"github.com/defenseunicorns/zarf/src/config"
@@ -43,6 +43,9 @@ const (
 	ZarfLogging  = "LOGGING"
 	ZarfGit      = "GIT"
 	ZarfInjector = "INJECTOR"
+
+	// See https://regex101.com/r/OWVfAO/1
+	serviceURLPattern = `^(?P<name>[^\.]+)\.(?P<namespace>[^\.]+)\.svc\.cluster\.local$`
 )
 
 // Tunnel is the main struct that configures and manages port forwading tunnels to Kubernetes resources.
@@ -86,6 +89,21 @@ func (c *Cluster) PrintConnectTable() error {
 	return nil
 }
 
+// IsServiceURL will check if the provided string is a valid serviceURL based on if it properly matches a validating regexp
+func IsServiceURL(serviceURL string) bool {
+	parsedURL, err := url.Parse(serviceURL)
+	if err != nil {
+		return false
+	}
+
+	// Match hostname against local cluster service format
+	pattern := regexp.MustCompile(serviceURLPattern)
+	matches := pattern.FindStringSubmatch(parsedURL.Hostname())
+
+	// If incomplete match, return an error
+	return len(matches) == 3
+}
+
 // NewTunnelFromServiceURL takes a serviceURL and parses it to create a tunnel to the cluster. The string is expected to follow the following format:
 // Example serviceURL: http://{SERVICE_NAME}.{NAMESPACE}.svc.cluster.local:{PORT}
 func NewTunnelFromServiceURL(serviceURL string) (*Tunnel, error) {
@@ -101,13 +119,12 @@ func NewTunnelFromServiceURL(serviceURL string) (*Tunnel, error) {
 	}
 
 	// Match hostname against local cluster service format
-	// See https://regex101.com/r/OWVfAO/1
-	pattern := regexp.MustCompile(`^(?P<name>[^\.]+)\.(?P<namespace>[^\.]+)\.svc\.cluster\.local$`)
+	pattern := regexp.MustCompile(serviceURLPattern)
 	matches := pattern.FindStringSubmatch(parsedURL.Hostname())
 
 	// If incomplete match, return an error
 	if len(matches) != 3 {
-		return nil, errors.New("url does not match service url format http://{SERVICE_NAME}.{NAMESPACE}.svc.cluster.local:{PORT}")
+		return nil, lang.ErrNotAServiceURL
 	}
 
 	// Use the matched values to create a new tunnel

src/internal/packager/git/push.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"path/filepath"
 
-	"github.com/defenseunicorns/zarf/src/internal/cluster"
 	"github.com/defenseunicorns/zarf/src/pkg/message"
 	"github.com/go-git/go-git/v5"
 	goConfig "github.com/go-git/go-git/v5/config"
@@ -18,15 +17,6 @@ import (
 )
 
 func (g *Git) PushRepo(localPath string) error {
-	// If this is a serviceURL, create a port-forward tunnel to that resource
-	if tunnel, err := cluster.NewTunnelFromServiceURL(g.Server.Address); err != nil {
-		return err
-	} else {
-		tunnel.Connect("", false)
-		defer tunnel.Close()
-		g.Server.Address = fmt.Sprintf("http://%s", tunnel.Endpoint())
-	}
-
 	spinner := message.NewProgressSpinner("Processing git repo at %s", localPath)
 	defer spinner.Stop()
 
@@ -90,6 +80,9 @@ func (g *Git) prepRepoForPush() (*git.Repository, error) {
 		return nil, fmt.Errorf("unable to transform the git url: %w", err)
 	}
 
+	// Remove any preexisting offlineRemotes (happens when a retry is triggered)
+	_ = repo.DeleteRemote(offlineRemoteName)
+
 	_, err = repo.CreateRemote(&goConfig.RemoteConfig{
 		Name: offlineRemoteName,
 		URLs: []string{targetUrl},
@@ -135,7 +128,7 @@ func (g *Git) push(repo *git.Repository, spinner *message.Spinner) error {
 	} else if errors.Is(err, git.NoErrAlreadyUpToDate) {
 		message.Debugf("Repo already up-to-date, skipping fetch...")
 	} else if err != nil {
-		message.Warnf("unable to fetch remote cleanly prior to push: %w", err)
+		return fmt.Errorf("unable to fetch the git repo prior to push: %w", err)
 	}
 
 	// Push all heads and tags to the offline remote

src/internal/packager/images/push.go

@@ -28,11 +28,9 @@ func (i *ImgConfig) PushToZarfRegistry() error {
 		defer tunnel.Close()
 
 		registryURL = tunnel.Endpoint()
-	} else {
-		registryURL = i.RegInfo.Address
-
+	} else if cluster.IsServiceURL(i.RegInfo.Address) {
 		// If this is a serviceURL, create a port-forward tunnel to that resource
-		if tunnel, err := cluster.NewTunnelFromServiceURL(registryURL); err != nil {
+		if tunnel, err := cluster.NewTunnelFromServiceURL(i.RegInfo.Address); err != nil {
 			return err
 		} else {
 			tunnel.Connect("", false)

src/pkg/packager/deploy.go

@@ -377,19 +377,34 @@ func (p *Packager) pushImagesToRegistry(componentImages []string, noImgChecksum
 
 // Push all of the components git repos to the configured git server
 func (p *Packager) pushReposToRepository(reposPath string, repos []string) error {
-	// Try repo push up to 3 times
 	for _, repoURL := range repos {
-		gitClient := git.New(p.cfg.State.GitServer)
-		repoPath, err := gitClient.TransformURLtoRepoName(repoURL)
-		if err != nil {
-			return fmt.Errorf("unable to get the repo name from the URL %s: %w", repoURL, err)
+
+		// Create an anonymous function to push the repo to the Zarf git server
+		tryPush := func() error {
+			gitClient := git.New(p.cfg.State.GitServer)
+
+			// If this is a serviceURL, create a port-forward tunnel to that resource
+			if cluster.IsServiceURL(gitClient.Server.Address) {
+				if tunnel, err := cluster.NewTunnelFromServiceURL(gitClient.Server.Address); err != nil {
+					return err
+				} else {
+					tunnel.Connect("", false)
+					defer tunnel.Close()
+					gitClient.Server.Address = fmt.Sprintf("http://%s", tunnel.Endpoint())
+				}
+			}
+
+			// Convert the repo URL to a Zarf-formatted repo name
+			if repoPath, err := gitClient.TransformURLtoRepoName(repoURL); err != nil {
+				return fmt.Errorf("unable to get the repo name from the URL %s: %w", repoURL, err)
+			} else {
+				return gitClient.PushRepo(filepath.Join(reposPath, repoPath))
+			}
 		}
 
-		err = utils.Retry(func() error {
-			return gitClient.PushRepo(filepath.Join(reposPath, repoPath))
-		}, 3, 5*time.Second)
-		if err != nil {
-			return fmt.Errorf("unable to push repo %s to the Git Server: %w", repoPath, err)
+		// Try repo push up to 3 times
+		if err := utils.Retry(tryPush, 3, 5*time.Second); err != nil {
+			return fmt.Errorf("unable to push repo %s to the Git Server: %w", repoURL, err)
 		}
 	}