Merge branch 'main' into added-hauler-copy-cmd

Author: zackbradys

cmd/hauler/cli/store.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"fmt"
 	"os"
-	"strconv"
 
 	"github.com/spf13/cobra"
 	"helm.sh/helm/v4/pkg/action"
@@ -78,11 +77,18 @@ func addStoreSync(rso *flags.StoreRootOpts, ro *flags.CliRootOpts) *cobra.Comman
 			if o.CaFile == "" {
 				o.CaFile = os.Getenv(consts.CaFile)
 			}
-			if o.InsecureSkipTLSVerify == nil {
-				if v := os.Getenv(consts.InsecureSkipTLSVerify); v != "" {
-					b, _ := strconv.ParseBool(v)
-					o.InsecureSkipTLSVerify = &b
-				}
+
+			// record which precedence-carrying flags the user explicitly set, so
+			// the resolvers can let an explicit CLI value win over per-item/annotation
+			o.TlogChanged = cmd.Flags().Changed("use-tlog-verify")
+			o.ExcludeExtrasChanged = cmd.Flags().Changed("exclude-extras")
+			o.InsecureChanged = cmd.Flags().Changed("insecure-skip-tls-verify")
+			o.StoreChanged = cmd.Flags().Changed("store")
+			o.RetriesChanged = cmd.Flags().Changed("retries")
+
+			// env var only applies when the flag wasn't set, so an explicit --insecure-skip-tls-verify=false still wins
+			if !o.InsecureChanged && os.Getenv(consts.InsecureSkipTLSVerify) == "true" {
+				o.InsecureSkipTLSVerify = true
 			}
 
 			// --dry-run requires --products
@@ -124,12 +130,6 @@ func addStoreSync(rso *flags.StoreRootOpts, ro *flags.CliRootOpts) *cobra.Comman
 			}
 			rso.BlobConcurrency = bc
 
-			// resolve *bool: nil unless the user explicitly passed the flag
-			if cmd.Flags().Changed("insecure-skip-tls-verify") {
-				v, _ := cmd.Flags().GetBool("insecure-skip-tls-verify")
-				o.InsecureSkipTLSVerify = &v
-			}
-
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -411,11 +411,9 @@ func addStoreAddImage(rso *flags.StoreRootOpts, ro *flags.CliRootOpts) *cobra.Co
 			if o.CaFile == "" {
 				o.CaFile = os.Getenv(consts.CaFile)
 			}
-			if o.InsecureSkipTLSVerify == nil {
-				if v := os.Getenv(consts.InsecureSkipTLSVerify); v != "" {
-					b, _ := strconv.ParseBool(v)
-					o.InsecureSkipTLSVerify = &b
-				}
+			// env var only applies when the flag wasn't set, so an explicit --insecure-skip-tls-verify=false still wins
+			if !cmd.Flags().Changed("insecure-skip-tls-verify") && os.Getenv(consts.InsecureSkipTLSVerify) == "true" {
+				o.InsecureSkipTLSVerify = true
 			}
 			return nil
 		},

cmd/hauler/cli/store/add.go

@@ -59,7 +59,7 @@ func AddFileCmd(ctx context.Context, o *flags.AddFileOpts, s *store.Layout, refe
 	cfg := v1.File{
 		Path:                  reference,
 		CaFile:                o.CaFile,
-		InsecureSkipTLSVerify: &o.InsecureSkipTLSVerify,
+		InsecureSkipTLSVerify: o.InsecureSkipTLSVerify,
 	}
 	if len(o.Name) > 0 {
 		cfg.Name = o.Name
@@ -83,7 +83,7 @@ func storeFile(ctx context.Context, s *store.Layout, fi v1.File, ro *flags.CliRo
 
 	copts := getter.ClientOptions{
 		NameOverride:          fi.Name,
-		InsecureSkipTLSVerify: derefInsecure(fi.InsecureSkipTLSVerify),
+		InsecureSkipTLSVerify: fi.InsecureSkipTLSVerify,
 		CAFile:                fi.CaFile,
 	}
 
@@ -248,15 +248,15 @@ func AddImageCmd(ctx context.Context, o *flags.AddImageOpts, s *store.Layout, re
 func addImageVerifyConfig(o *flags.AddImageOpts) cosign.Config {
 	switch {
 	case o.Key != "":
-		return cosign.Config{Key: o.Key, Tlog: o.Tlog, InsecureSkipTLSVerify: derefInsecure(o.InsecureSkipTLSVerify), CaFile: o.CaFile}
+		return cosign.Config{Key: o.Key, Tlog: o.Tlog, InsecureSkipTLSVerify: o.InsecureSkipTLSVerify, CaFile: o.CaFile}
 	case o.CertIdentityRegexp != "" || o.CertIdentity != "":
 		return cosign.Config{
 			CertIdentity:                 o.CertIdentity,
 			CertIdentityRegexp:           o.CertIdentityRegexp,
 			CertOidcIssuer:               o.CertOidcIssuer,
 			CertOidcIssuerRegexp:         o.CertOidcIssuerRegexp,
 			CertGithubWorkflowRepository: o.CertGithubWorkflowRepository,
-			InsecureSkipTLSVerify:        derefInsecure(o.InsecureSkipTLSVerify),
+			InsecureSkipTLSVerify:        o.InsecureSkipTLSVerify,
 			CaFile:                       o.CaFile,
 		}
 	default:
@@ -480,7 +480,7 @@ func storeImage(ctx context.Context, s *store.Layout, i v1.Image, platform strin
 		return err
 	}
 
-	insecureSkipTLSVerify := derefInsecure(i.InsecureSkipTLSVerify)
+	insecureSkipTLSVerify := i.InsecureSkipTLSVerify
 	caFile := i.CaFile
 
 	log.BaseFromContext(ctx).Debugf("resolving image [%s] (verified=%t, platform=%q, excludeExtras=%t, insecureSkipTLSVerify=%t, caFile=%q, rewrite=%q, digest=%q)", i.Name, verified, platform, excludeExtras, insecureSkipTLSVerify, caFile, rewrite, pinnedDigest)
@@ -619,7 +619,13 @@ func rewriteReference(ctx context.Context, s *store.Layout, oldRef name.Referenc
 	// index.docker.io. Preserve the original registry when the source is non-docker.
 	if newRegistry == "index.docker.io" && !strings.HasPrefix(rawRewrite, "docker.io") && !strings.HasPrefix(rawRewrite, "index.docker.io") {
 		newRegistry = oldRegistry
-		newRepo = strings.TrimPrefix(newRepo, "library/") //if rewrite has library/ prefix in path it is stripped off unless registry specified in rewrite
+		rewriteRepo := strings.TrimPrefix(rawRewrite, "/")
+		if i := strings.LastIndex(rewriteRepo, ":"); i != -1 {
+			rewriteRepo = rewriteRepo[:i]
+		}
+		if !strings.HasPrefix(rewriteRepo, "library/") {
+			newRepo = strings.TrimPrefix(newRepo, "library/")
+		}
 	}
 	oldTotal := oldRepo + ":" + oldTag
 	newTotal := newRepo + ":" + newTag
@@ -777,11 +783,11 @@ type chartJob struct {
 //
 // The three precedence rules are not uniform. registry is CLI > annotation.
 // excludeExtras is a one-way switch that any of the three sources can flip on
-// and none can flip off. platform is per-chart > CLI > annotation: an explicit
-// --platform is run-time intent and outranks manifest metadata. That last rule
-// must stay identical to resolveImageJobs's, or a single `hauler store sync`
-// run would pull a chart's discovered images for a different platform than the
-// manifest's own Images section.
+// and none can flip off, since a plain bool has no unset state. platform is
+// CLI > per-chart > annotation. That last rule must stay identical to
+// resolveImageJobs's, or a single `hauler store sync` run would pull a
+// chart's discovered images for a different platform than the manifest's own
+// Images section.
 //
 // Every job allocates its own *action.ChartPathOptions. flags.AddChartOpts
 // holds that as a pointer, so copying the struct alone would leave sibling
@@ -795,20 +801,15 @@ func resolveChartJobs(o *flags.SyncOpts, annotations map[string]string, manifest
 
 	jobs := make([]chartJob, 0, len(charts))
 	for _, ch := range charts {
-		excludeExtras := o.ExcludeExtras
-		if !o.ExcludeExtras && annotations[consts.ImageAnnotationExcludeExtras] == "true" {
-			excludeExtras = true
-		}
-		if ch.ExcludeExtras {
-			excludeExtras = ch.ExcludeExtras
-		}
+		excludeExtras := resolveBoolFlag(ch.ExcludeExtras, annotations[consts.ImageAnnotationExcludeExtras] == "true", o.ExcludeExtras, o.ExcludeExtrasChanged)
 
 		platform := o.Platform
-		if o.Platform == "" && annotations[consts.ImageAnnotationPlatform] != "" {
-			platform = annotations[consts.ImageAnnotationPlatform]
-		}
-		if ch.Platform != "" {
-			platform = ch.Platform
+		if o.Platform == "" {
+			if ch.Platform != "" {
+				platform = ch.Platform
+			} else if annotations[consts.ImageAnnotationPlatform] != "" {
+				platform = annotations[consts.ImageAnnotationPlatform]
+			}
 		}
 
 		var valuesFiles []string
@@ -826,16 +827,13 @@ func resolveChartJobs(o *flags.SyncOpts, annotations map[string]string, manifest
 		if caFile == "" {
 			if ch.CaFile != "" {
 				caFile = ch.CaFile
-			} else if annotations[consts.ImageAnnotationCaFile] == "true" {
+			} else if annotations[consts.ImageAnnotationCaFile] != "" {
 				caFile = annotations[consts.ImageAnnotationCaFile]
 			}
 		}
 
-		insecureSkipTLSVerify := false
-		if o.CaFile == "" {
-			insecureSkipTLSVerify = resolveInsecure(ch.InsecureSkipTLSVerify, annotations, o.InsecureSkipTLSVerify)
-		} else {
-		}
+		// a CA file and skipping TLS verification are mutually exclusive: providing one forces verification on
+		insecureSkipTLSVerify := o.CaFile == "" && resolveBoolFlag(ch.InsecureSkipTLSVerify, annotations[consts.ImageAnnotationInsecureSkipTLSVerify] == "true", o.InsecureSkipTLSVerify, o.InsecureChanged)
 
 		jobs = append(jobs, chartJob{
 			cfg: ch,
@@ -1381,12 +1379,11 @@ func fetchChart(ctx context.Context, s *store.Layout, j chartJob, tempRoot strin
 			// there is no separate per-discovered-image TLS knob in a chart
 			// manifest, so the registry a chart's images live in is assumed
 			// to share the chart repo's trust configuration.
-			chartInsecure := j.opts.ChartOpts.InsecureSkipTLSVerify
 			imageJobs = append(imageJobs, imageJob{
 				img: v1.Image{
 					Name:                  relocated,
 					CaFile:                j.opts.ChartOpts.CaFile,
-					InsecureSkipTLSVerify: &chartInsecure,
+					InsecureSkipTLSVerify: j.opts.ChartOpts.InsecureSkipTLSVerify,
 				},
 				platform:      j.opts.Platform,
 				excludeExtras: j.opts.ExcludeExtras,
@@ -1461,6 +1458,7 @@ func fetchChart(ctx context.Context, s *store.Layout, j chartJob, tempRoot strin
 // rewrite. A rewrite that omits a tag inherits ref's.
 func rewriteChartReference(ctx context.Context, s *store.Layout, ref name.Reference, rewrite string) error {
 	rewrite = strings.TrimPrefix(rewrite, "/")
+	rawRewrite := rewrite
 	newRef, err := name.ParseReference(rewrite)
 	if err != nil {
 		// error... don't continue with a bad reference
@@ -1483,6 +1481,13 @@ func rewriteChartReference(ctx context.Context, s *store.Layout, ref name.Refere
 	// rename chart name in store
 	oldRepo := ref.Context().RepositoryStr()
 	newRepo := newRef.Context().RepositoryStr()
+	rewriteRepo := rawRewrite
+	if i := strings.LastIndex(rewriteRepo, ":"); i != -1 {
+		rewriteRepo = rewriteRepo[:i]
+	}
+	if !strings.HasPrefix(rewriteRepo, "library/") {
+		newRepo = strings.TrimPrefix(newRepo, "library/")
+	}
 	newTag := newRef.Identifier()
 	if tag, ok := newRef.(name.Tag); ok {
 		newTag = tag.TagStr()