github/hauler
1
0
Fork 0
mirror of https://github.com/hauler-dev/hauler.git synced 2026-08-18 03:46:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fixed command precedents for hauler store sync (#734)

Browse Source
This commit is contained in:
Zack Brady
2026-08-13 09:45:16 -04:00
committed by GitHub
parent bd70379158
commit 6d8a7cc9a1
10 changed files with 310 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/hauler/cli/store.go
+15 -17
View File
@@ -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
+21 -30
View File
@@ -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,7 +248,7 @@ 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,
@@ -256,7 +256,7 @@ func addImageVerifyConfig(o *flags.AddImageOpts) cosign.Config {
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)
@@ -777,11 +777,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 +795,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 +821,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 +1373,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,
cmd/hauler/cli/store/add_test.go
+65 -15
View File
@@ -1798,12 +1798,13 @@ func TestResolveChartJobs_ExcludeExtras(t *testing.T) {
tests := []struct {
name string
cli bool
cliChanged bool
annotation string
perChart bool
want bool
}{
{name: "nothing set", want: false},
{name: "CLI flag alone", cli: true, want: true},
{name: "CLI flag alone", cli: true, cliChanged: true, want: true},
{name: "annotation alone", annotation: "true", want: true},
{name: "per-chart alone", perChart: true, want: true},
{
@@ -1816,14 +1817,16 @@ func TestResolveChartJobs_ExcludeExtras(t *testing.T) {
// --exclude-extras back off; both are one-way switches.
name: "CLI flag survives an annotation that is not true",
cli: true,
cliChanged: true,
annotation: "false",
want: true,
},
{
name: "CLI flag survives a false per-chart field",
cli: true,
perChart: false,
want: true,
name: "CLI flag survives a false per-chart field",
cli: true,
cliChanged: true,
perChart: false,
want: true,
},
{
name: "annotation survives a false per-chart field",
@@ -1831,11 +1834,20 @@ func TestResolveChartJobs_ExcludeExtras(t *testing.T) {
perChart: false,
want: true,
},
{
// An explicit CLI --exclude-extras=false wins outright over an
// annotation/per-chart true.
name: "explicit CLI false overrides annotation and per-chart",
cliChanged: true,
annotation: "true",
perChart: true,
want: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
o := &flags.SyncOpts{ExcludeExtras: tc.cli}
o := &flags.SyncOpts{ExcludeExtras: tc.cli, ExcludeExtrasChanged: tc.cliChanged}
a := map[string]string{}
if tc.annotation != "" {
a[consts.ImageAnnotationExcludeExtras] = tc.annotation
@@ -1877,10 +1889,16 @@ func TestResolveChartJobs_Platform(t *testing.T) {
want: "linux/amd64",
},
{
name: "per-chart wins over both",
name: "CLI flag wins over annotation and per-chart",
cli: "linux/amd64",
annotation: "linux/arm64",
perChart: "linux/s390x",
want: "linux/amd64",
},
{
name: "per-chart wins over annotation when CLI flag unset",
annotation: "linux/arm64",
perChart: "linux/s390x",
want: "linux/s390x",
},
{
@@ -2155,7 +2173,6 @@ func TestResolveChartJobs_NoCharts(t *testing.T) {
// TestResolveChartJobs_CredentialFields pins that every TLS/verification
// field on v1.Chart reaches the job's ChartOpts unchanged.
func TestResolveChartJobs_CredentialFields(t *testing.T) {
insecure := true
ch := v1.Chart{
Name: "rancher",
Verify: true,
@@ -2164,7 +2181,7 @@ func TestResolveChartJobs_CredentialFields(t *testing.T) {
CertFile: "/certs/client.crt",
KeyFile: "/certs/client.key",
CaFile: "/certs/ca.crt",
InsecureSkipTLSVerify: &insecure,
InsecureSkipTLSVerify: true,
PlainHTTP: true,
}
@@ -2195,14 +2212,47 @@ func TestResolveChartJobs_CredentialFields(t *testing.T) {
if opts.CaFile != ch.CaFile {
t.Errorf("CaFile = %q, want %q", opts.CaFile, ch.CaFile)
}
if opts.InsecureSkipTLSVerify != derefInsecure(ch.InsecureSkipTLSVerify) {
t.Errorf("InsecureSkipTLSVerify = %v, want %v", opts.InsecureSkipTLSVerify, derefInsecure(ch.InsecureSkipTLSVerify))
if opts.InsecureSkipTLSVerify != ch.InsecureSkipTLSVerify {
t.Errorf("InsecureSkipTLSVerify = %v, want %v", opts.InsecureSkipTLSVerify, ch.InsecureSkipTLSVerify)
}
if opts.PlainHTTP != ch.PlainHTTP {
t.Errorf("PlainHTTP = %v, want %v", opts.PlainHTTP, ch.PlainHTTP)
}
}
func TestResolveChartJobs_CaFilePrecedence(t *testing.T) {
tests := []struct {
name string
cli string
annotation string
perChart string
want string
}{
{name: "annotation used when CLI and per-chart unset", annotation: "/ann/ca.crt", want: "/ann/ca.crt"},
{name: "per-chart wins over annotation", annotation: "/ann/ca.crt", perChart: "/chart/ca.crt", want: "/chart/ca.crt"},
{name: "CLI wins over per-chart and annotation", cli: "/cli/ca.crt", annotation: "/ann/ca.crt", perChart: "/chart/ca.crt", want: "/cli/ca.crt"},
{name: "none set stays empty", want: ""},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
o := &flags.SyncOpts{CaFile: tc.cli}
a := map[string]string{}
if tc.annotation != "" {
a[consts.ImageAnnotationCaFile] = tc.annotation
}
jobs, err := resolveChartJobs(o, a, "/manifests", []v1.Chart{{Name: "rancher", CaFile: tc.perChart}})
if err != nil {
t.Fatalf("resolveChartJobs: %v", err)
}
if got := jobs[0].opts.ChartOpts.CaFile; got != tc.want {
t.Errorf("CaFile = %q, want %q", got, tc.want)
}
})
}
}
// TestResolveChartJobs_CredentialEnv pins that UsernameEnv/PasswordEnv are
// resolved into ChartOpts.Username/Password via resolveChartCreds.
func TestResolveChartJobs_CredentialEnv(t *testing.T) {
@@ -2999,7 +3049,7 @@ func TestStoreImage_CAFileAndInsecure(t *testing.T) {
t.Run("bad caFile without insecure returns error and stores nothing", func(t *testing.T) {
s := newTestStore(t)
insecure := false
img := v1.Image{Name: ref, CaFile: missingCA, InsecureSkipTLSVerify: &insecure}
img := v1.Image{Name: ref, CaFile: missingCA, InsecureSkipTLSVerify: insecure}
err := storeImage(ctx, s, img, "", false,
defaultRootOpts(s.Root), defaultCliOpts(), "", "", false)
if err == nil {
@@ -3017,7 +3067,7 @@ func TestStoreImage_CAFileAndInsecure(t *testing.T) {
t.Fatal(err)
}
insecure := false
img := v1.Image{Name: ref, CaFile: junk, InsecureSkipTLSVerify: &insecure}
img := v1.Image{Name: ref, CaFile: junk, InsecureSkipTLSVerify: insecure}
err := storeImage(ctx, s, img, "", false,
defaultRootOpts(s.Root), defaultCliOpts(), "", "", false)
if err == nil {
@@ -3031,7 +3081,7 @@ func TestStoreImage_CAFileAndInsecure(t *testing.T) {
// ignored and the pull still succeeds. If caFile were read first, the
// pull would error and nothing would be stored.
insecure := true
img := v1.Image{Name: ref, CaFile: missingCA, InsecureSkipTLSVerify: &insecure}
img := v1.Image{Name: ref, CaFile: missingCA, InsecureSkipTLSVerify: insecure}
err := storeImage(ctx, s, img, "", false,
defaultRootOpts(s.Root), defaultCliOpts(), "", "", false)
if err != nil {
@@ -3043,7 +3093,7 @@ func TestStoreImage_CAFileAndInsecure(t *testing.T) {
t.Run("valid caFile without insecure is accepted", func(t *testing.T) {
s := newTestStore(t)
insecure := false
img := v1.Image{Name: ref, CaFile: writeCAFile(t), InsecureSkipTLSVerify: &insecure}
img := v1.Image{Name: ref, CaFile: writeCAFile(t), InsecureSkipTLSVerify: insecure}
err := storeImage(ctx, s, img, "", false,
defaultRootOpts(s.Root), defaultCliOpts(), "", "", false)
if err != nil {
cmd/hauler/cli/store/sync.go
+97 -107
View File
@@ -189,7 +189,7 @@ func SyncCmd(ctx context.Context, o *flags.SyncOpts, s *store.Layout, rso *flags
if strings.HasPrefix(haulPath, "http://") || strings.HasPrefix(haulPath, "https://") {
l.Debugf("detected remote manifest... starting download... [%s]", haulPath)
h := getter.NewHttp(derefInsecure(o.InsecureSkipTLSVerify), o.CaFile)
h := getter.NewHttp(o.InsecureSkipTLSVerify, o.CaFile)
parsedURL, err := url.Parse(haulPath)
if err != nil {
return err
@@ -241,7 +241,7 @@ func SyncCmd(ctx context.Context, o *flags.SyncOpts, s *store.Layout, rso *flags
if strings.HasPrefix(haulPath, "http://") || strings.HasPrefix(haulPath, "https://") {
l.Debugf("detected remote image.txt... starting download... [%s]", haulPath)
h := getter.NewHttp(derefInsecure(o.InsecureSkipTLSVerify), o.CaFile)
h := getter.NewHttp(o.InsecureSkipTLSVerify, o.CaFile)
parsedURL, err := url.Parse(haulPath)
if err != nil {
return err
@@ -287,28 +287,16 @@ func SyncCmd(ctx context.Context, o *flags.SyncOpts, s *store.Layout, rso *flags
return nil
}
// resolveInsecure applies precedence: cli > per-item > annotation.
// A non-nil per-item pointer wins outright — including an explicit false — so an
// individual file/image/chart can opt out of an insecure annotation or the global
// --insecure-skip-tls-verify flag. nil means "not set on the item", which falls
// through to the annotation, then the global flag.
func resolveInsecure(item *bool, ann map[string]string, global *bool) bool {
if global != nil {
return *global
// resolveBoolFlag applies CLI-first precedence for a plain-bool flag: an
// explicitly-set CLI flag wins outright (even when false); otherwise the flag
// is on if the resolved CLI value (e.g. from an env var), the per-item field,
// or the annotation is true. Plain bools have no unset state, so per-item and
// annotation can only turn a flag on, never force it back off.
func resolveBoolFlag(item, annTrue, global, cliChanged bool) bool {
if cliChanged {
return global
}
if item != nil {
return *item
}
if ann != nil && ann[consts.ImageAnnotationInsecureSkipTLSVerify] == "true" {
return true
}
return false
}
// derefInsecure is a nil-safe read of a *bool for logging/plumbing where a plain
// bool is needed. nil reads as false.
func derefInsecure(p *bool) bool {
return p != nil && *p
return global || item || annTrue
}
func processContent(ctx context.Context, fi *os.File, o *flags.SyncOpts, s *store.Layout, rso *flags.StoreRootOpts, ro *flags.CliRootOpts, targetStores map[string]*store.Layout) error {
@@ -347,11 +335,11 @@ func processContent(ctx context.Context, fi *os.File, o *flags.SyncOpts, s *stor
return err
}
a := cfg.GetAnnotations()
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores)
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores, o.StoreChanged)
if err != nil {
return err
}
docRso, err := resolveDocRetries(a, rso)
docRso, err := resolveDocRetries(a, rso, o.RetriesChanged)
if err != nil {
return err
}
@@ -374,11 +362,11 @@ func processContent(ctx context.Context, fi *os.File, o *flags.SyncOpts, s *stor
}
a := cfg.GetAnnotations()
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores)
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores, o.StoreChanged)
if err != nil {
return err
}
docRso, err := resolveDocRetries(a, rso)
docRso, err := resolveDocRetries(a, rso, o.RetriesChanged)
if err != nil {
return err
}
@@ -403,11 +391,11 @@ func processContent(ctx context.Context, fi *os.File, o *flags.SyncOpts, s *stor
return err
}
a := cfg.GetAnnotations()
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores)
docStore, err := resolveTargetStore(ctx, a, s, rso, ro, targetStores, o.StoreChanged)
if err != nil {
return err
}
docRso, err := resolveDocRetries(a, rso)
docRso, err := resolveDocRetries(a, rso, o.RetriesChanged)
if err != nil {
return err
}
@@ -433,7 +421,12 @@ func processContent(ctx context.Context, fi *os.File, o *flags.SyncOpts, s *stor
// resolveTargetStore picks a doc's store based on its hauler.dev/store annotation,
// falling back to def. Opens (or reuses, via targetStores) the target store otherwise.
func resolveTargetStore(ctx context.Context, a map[string]string, def *store.Layout, rso *flags.StoreRootOpts, ro *flags.CliRootOpts, targetStores map[string]*store.Layout) (*store.Layout, error) {
// An explicit CLI --store wins: cliStoreSet short-circuits to def, ignoring the annotation.
func resolveTargetStore(ctx context.Context, a map[string]string, def *store.Layout, rso *flags.StoreRootOpts, ro *flags.CliRootOpts, targetStores map[string]*store.Layout, cliStoreSet bool) (*store.Layout, error) {
if cliStoreSet {
return def, nil
}
target := a[consts.AnnotationTargetStore]
if target == "" {
return def, nil
@@ -466,8 +459,13 @@ func resolveTargetStore(ctx context.Context, a map[string]string, def *store.Lay
// resolveDocRetries returns a copy of rso with Retries overridden by a doc's
// hauler.dev/retries annotation, or rso unchanged if it's not set. Copy, not
// mutation, so it can't leak into a sibling doc.
func resolveDocRetries(a map[string]string, rso *flags.StoreRootOpts) (*flags.StoreRootOpts, error) {
// mutation, so it can't leak into a sibling doc. An explicit CLI --retries
// wins: cliRetriesSet returns rso unchanged, ignoring the annotation.
func resolveDocRetries(a map[string]string, rso *flags.StoreRootOpts, cliRetriesSet bool) (*flags.StoreRootOpts, error) {
if cliRetriesSet {
return rso, nil
}
v, ok := a[consts.AnnotationRetries]
if !ok || v == "" {
return rso, nil
@@ -604,10 +602,12 @@ type imageJob struct {
certGithubWorkflowRepository string
}
// resolveImageJobs applies the precedence rules (per-image > annotation >
// CLI, except registry relocation which is CLI > annotation) to every image
// in images, producing one imageJob per image. It is pure -- cosign
// verification happens later, inside the pull worker; see resolveAndVerify.
// resolveImageJobs applies the precedence rules (CLI > per-image > annotation)
// to every image in images, producing one imageJob per image. For the boolean
// flags (tlog, exclude-extras, insecure-skip-tls-verify) an explicit CLI flag
// wins outright; otherwise per-image or annotation can only turn them on (see
// resolveBoolFlag). It is pure -- cosign verification happens later, inside the
// pull worker; see resolveAndVerify.
func resolveImageJobs(o *flags.SyncOpts, a map[string]string, images []v1.Image) ([]imageJob, error) {
var jobs []imageJob
@@ -635,11 +635,8 @@ func resolveImageJobs(o *flags.SyncOpts, a map[string]string, images []v1.Image)
i.CaFile = o.CaFile
}
insecureSkipTLSVerify := false
if o.CaFile == "" {
insecureSkipTLSVerify = resolveInsecure(i.InsecureSkipTLSVerify, a, o.InsecureSkipTLSVerify)
}
i.InsecureSkipTLSVerify = &insecureSkipTLSVerify
// a CA file and skipping TLS verification are mutually exclusive: providing one forces verification on
i.InsecureSkipTLSVerify = o.CaFile == "" && resolveBoolFlag(i.InsecureSkipTLSVerify, a[consts.ImageAnnotationInsecureSkipTLSVerify] == "true", o.InsecureSkipTLSVerify, o.InsecureChanged)
if i.Local {
needsPubKeyVerification := a[consts.ImageAnnotationKey] != "" || o.Key != "" || i.Key != ""
@@ -669,71 +666,71 @@ func resolveImageJobs(o *flags.SyncOpts, a map[string]string, images []v1.Image)
if needsPubKeyVerification {
key := o.Key
if o.Key == "" && a[consts.ImageAnnotationKey] != "" {
expanded, err := homedir.Expand(a[consts.ImageAnnotationKey])
if err != nil {
return nil, err
if o.Key == "" {
if i.Key != "" {
expanded, err := homedir.Expand(i.Key)
if err != nil {
return nil, err
}
key = expanded
} else if a[consts.ImageAnnotationKey] != "" {
expanded, err := homedir.Expand(a[consts.ImageAnnotationKey])
if err != nil {
return nil, err
}
key = expanded
}
key = expanded
}
if i.Key != "" {
expanded, err := homedir.Expand(i.Key)
if err != nil {
return nil, err
}
key = expanded
}
tlog := o.Tlog
if !o.Tlog && a[consts.ImageAnnotationTlog] == "true" {
tlog = true
}
if i.Tlog {
tlog = i.Tlog
}
tlog := resolveBoolFlag(i.Tlog, a[consts.ImageAnnotationTlog] == "true", o.Tlog, o.TlogChanged)
job.needsPubKey = true
job.key = key
job.tlog = tlog
} else if needsKeylessVerificaton { //Keyless signature verification
certIdentityRegexp := o.CertIdentityRegexp
if o.CertIdentityRegexp == "" && a[consts.ImageAnnotationCertIdentityRegexp] != "" {
certIdentityRegexp = a[consts.ImageAnnotationCertIdentityRegexp]
}
if i.CertIdentityRegexp != "" {
certIdentityRegexp = i.CertIdentityRegexp
if o.CertIdentityRegexp == "" {
if i.CertIdentityRegexp != "" {
certIdentityRegexp = i.CertIdentityRegexp
} else if a[consts.ImageAnnotationCertIdentityRegexp] != "" {
certIdentityRegexp = a[consts.ImageAnnotationCertIdentityRegexp]
}
}
certIdentity := o.CertIdentity
if o.CertIdentity == "" && a[consts.ImageAnnotationCertIdentity] != "" {
certIdentity = a[consts.ImageAnnotationCertIdentity]
}
if i.CertIdentity != "" {
certIdentity = i.CertIdentity
if o.CertIdentity == "" {
if i.CertIdentity != "" {
certIdentity = i.CertIdentity
} else if a[consts.ImageAnnotationCertIdentity] != "" {
certIdentity = a[consts.ImageAnnotationCertIdentity]
}
}
certOidcIssuer := o.CertOidcIssuer
if o.CertOidcIssuer == "" && a[consts.ImageAnnotationCertOidcIssuer] != "" {
certOidcIssuer = a[consts.ImageAnnotationCertOidcIssuer]
}
if i.CertOidcIssuer != "" {
certOidcIssuer = i.CertOidcIssuer
if o.CertOidcIssuer == "" {
if i.CertOidcIssuer != "" {
certOidcIssuer = i.CertOidcIssuer
} else if a[consts.ImageAnnotationCertOidcIssuer] != "" {
certOidcIssuer = a[consts.ImageAnnotationCertOidcIssuer]
}
}
certOidcIssuerRegexp := o.CertOidcIssuerRegexp
if o.CertOidcIssuerRegexp == "" && a[consts.ImageAnnotationCertOidcIssuerRegexp] != "" {
certOidcIssuerRegexp = a[consts.ImageAnnotationCertOidcIssuerRegexp]
}
if i.CertOidcIssuerRegexp != "" {
certOidcIssuerRegexp = i.CertOidcIssuerRegexp
if o.CertOidcIssuerRegexp == "" {
if i.CertOidcIssuerRegexp != "" {
certOidcIssuerRegexp = i.CertOidcIssuerRegexp
} else if a[consts.ImageAnnotationCertOidcIssuerRegexp] != "" {
certOidcIssuerRegexp = a[consts.ImageAnnotationCertOidcIssuerRegexp]
}
}
certGithubWorkflowRepository := o.CertGithubWorkflowRepository
if o.CertGithubWorkflowRepository == "" && a[consts.ImageAnnotationCertGithubWorkflowRepository] != "" {
certGithubWorkflowRepository = a[consts.ImageAnnotationCertGithubWorkflowRepository]
}
if i.CertGithubWorkflowRepository != "" {
certGithubWorkflowRepository = i.CertGithubWorkflowRepository
if o.CertGithubWorkflowRepository == "" {
if i.CertGithubWorkflowRepository != "" {
certGithubWorkflowRepository = i.CertGithubWorkflowRepository
} else if a[consts.ImageAnnotationCertGithubWorkflowRepository] != "" {
certGithubWorkflowRepository = a[consts.ImageAnnotationCertGithubWorkflowRepository]
}
}
job.needsKeyless = true
@@ -745,11 +742,12 @@ func resolveImageJobs(o *flags.SyncOpts, a map[string]string, images []v1.Image)
}
platform := o.Platform
if o.Platform == "" && a[consts.ImageAnnotationPlatform] != "" {
platform = a[consts.ImageAnnotationPlatform]
}
if i.Platform != "" {
platform = i.Platform
if o.Platform == "" {
if i.Platform != "" {
platform = i.Platform
} else if a[consts.ImageAnnotationPlatform] != "" {
platform = a[consts.ImageAnnotationPlatform]
}
}
rewrite := ""
@@ -757,13 +755,7 @@ func resolveImageJobs(o *flags.SyncOpts, a map[string]string, images []v1.Image)
rewrite = i.Rewrite
}
excludeExtras := o.ExcludeExtras
if !o.ExcludeExtras && a[consts.ImageAnnotationExcludeExtras] == "true" {
excludeExtras = true
}
if i.ExcludeExtras {
excludeExtras = i.ExcludeExtras
}
excludeExtras := resolveBoolFlag(i.ExcludeExtras, a[consts.ImageAnnotationExcludeExtras] == "true", o.ExcludeExtras, o.ExcludeExtrasChanged)
job.platform = platform
job.rewrite = rewrite
@@ -795,7 +787,7 @@ func (j imageJob) verifyConfig() cosign.Config {
return cosign.Config{
Key: j.key,
Tlog: j.tlog,
InsecureSkipTLSVerify: derefInsecure(j.img.InsecureSkipTLSVerify),
InsecureSkipTLSVerify: j.img.InsecureSkipTLSVerify,
CaFile: j.img.CaFile,
}
case j.needsKeyless:
@@ -805,7 +797,7 @@ func (j imageJob) verifyConfig() cosign.Config {
CertOidcIssuer: j.certOidcIssuer,
CertOidcIssuerRegexp: j.certOidcIssuerRegexp,
CertGithubWorkflowRepository: j.certGithubWorkflowRepository,
InsecureSkipTLSVerify: derefInsecure(j.img.InsecureSkipTLSVerify),
InsecureSkipTLSVerify: j.img.InsecureSkipTLSVerify,
CaFile: j.img.CaFile,
}
default:
@@ -1152,9 +1144,10 @@ type fileJob struct {
file v1.File
}
// resolveFileJobs converts every v1.File in files into a fileJob, applying
// the caFile/insecure precedence rules (per-file > annotation > global) --
// see resolveInsecure. It is pure.
// resolveFileJobs converts every v1.File in files into a fileJob. caFile is
// CLI > per-file > annotation; insecureSkipTLSVerify is CLI-first (an explicit
// CLI flag wins, otherwise per-file or annotation can only turn it on -- see
// resolveBoolFlag). It is pure.
func resolveFileJobs(o *flags.SyncOpts, a map[string]string, files []v1.File) []fileJob {
jobs := make([]fileJob, 0, len(files))
for _, f := range files {
@@ -1166,11 +1159,8 @@ func resolveFileJobs(o *flags.SyncOpts, a map[string]string, files []v1.File) []
f.CaFile = o.CaFile
}
insecure := false
if o.CaFile == "" {
insecure = resolveInsecure(f.InsecureSkipTLSVerify, a, o.InsecureSkipTLSVerify)
}
f.InsecureSkipTLSVerify = &insecure
// a CA file and skipping TLS verification are mutually exclusive: providing one forces verification on
f.InsecureSkipTLSVerify = o.CaFile == "" && resolveBoolFlag(f.InsecureSkipTLSVerify, a[consts.ImageAnnotationInsecureSkipTLSVerify] == "true", o.InsecureSkipTLSVerify, o.InsecureChanged)
jobs = append(jobs, fileJob{file: f})
}
cmd/hauler/cli/store/sync_test.go
+95 -17
View File
@@ -245,7 +245,7 @@ func TestResolveDocRetries_NoAnnotation_ReturnsRsoUnchanged(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
rso.Retries = 5
got, err := resolveDocRetries(map[string]string{}, rso)
got, err := resolveDocRetries(map[string]string{}, rso, false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -258,7 +258,7 @@ func TestResolveDocRetries_Override_ReturnsCopyNotMutation(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
rso.Retries = 5
got, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "9"}, rso)
got, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "9"}, rso, false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -273,11 +273,27 @@ func TestResolveDocRetries_Override_ReturnsCopyNotMutation(t *testing.T) {
}
}
func TestResolveDocRetries_CLIWinsOverAnnotation(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
rso.Retries = 5
got, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "9"}, rso, true)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if got != rso {
t.Fatal("expected rso unchanged when --retries was set on the CLI")
}
if got.Retries != 5 {
t.Fatalf("got Retries %d, want the CLI value 5", got.Retries)
}
}
func TestResolveDocRetries_ZeroMeansDefault(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
rso.Retries = 5
got, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "0"}, rso)
got, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "0"}, rso, false)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@@ -289,7 +305,7 @@ func TestResolveDocRetries_ZeroMeansDefault(t *testing.T) {
func TestResolveDocRetries_Negative_ReturnsError(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
if _, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "-1"}, rso); err == nil {
if _, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "-1"}, rso, false); err == nil {
t.Fatal("expected an error for a negative hauler.dev/retries value, got nil")
}
}
@@ -297,7 +313,7 @@ func TestResolveDocRetries_Negative_ReturnsError(t *testing.T) {
func TestResolveDocRetries_NotANumber_ReturnsError(t *testing.T) {
rso := defaultRootOpts(t.TempDir())
if _, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "banana"}, rso); err == nil {
if _, err := resolveDocRetries(map[string]string{consts.AnnotationRetries: "banana"}, rso, false); err == nil {
t.Fatal("expected an error for a non-numeric hauler.dev/retries value, got nil")
}
}
@@ -943,10 +959,17 @@ func TestResolveImageJobs_KeyPrecedence(t *testing.T) {
wantKey: homeKey,
},
{
name: "per-image overrides annotation and CLI, expanded via homedir",
// CLI wins outright over both annotation and per-image.
name: "CLI overrides annotation and per-image",
cliKey: "/cli/key.pub",
annotation: "/annotation/key.pub",
imageKey: "~/mykey.pub",
wantKey: "/cli/key.pub",
},
{
name: "per-image overrides annotation when CLI key unset, expanded via homedir",
annotation: "/annotation/key.pub",
imageKey: "~/mykey.pub",
wantKey: homeKey,
},
}
@@ -982,25 +1005,36 @@ func TestResolveImageJobs_TlogPrecedence(t *testing.T) {
tests := []struct {
name string
cliTlog bool
cliChanged bool
annotation string
imageTlog bool
wantTlog bool
}{
{
name: "CLI true",
cliTlog: true,
wantTlog: true,
name: "CLI true",
cliTlog: true,
cliChanged: true,
wantTlog: true,
},
{
name: "annotation true overrides CLI false",
name: "annotation true when CLI unset",
annotation: "true",
wantTlog: true,
},
{
name: "per-image true overrides annotation/CLI false",
name: "per-image true when CLI unset",
imageTlog: true,
wantTlog: true,
},
{
// An explicit CLI flag wins outright, even false, over an
// annotation/per-image true.
name: "explicit CLI false overrides annotation and per-image",
cliChanged: true,
annotation: "true",
imageTlog: true,
wantTlog: false,
},
{
name: "all false stays false",
wantTlog: false,
@@ -1009,7 +1043,7 @@ func TestResolveImageJobs_TlogPrecedence(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
o := &flags.SyncOpts{Key: "/cli/key.pub", Tlog: tc.cliTlog}
o := &flags.SyncOpts{Key: "/cli/key.pub", Tlog: tc.cliTlog, TlogChanged: tc.cliChanged}
a := map[string]string{}
if tc.annotation != "" {
a[consts.ImageAnnotationTlog] = tc.annotation
@@ -1042,7 +1076,9 @@ func TestResolveImageJobs_PlatformPrecedence(t *testing.T) {
// Annotation only applies when the CLI platform is unset — it does not
// override an explicitly-set CLI platform.
{name: "annotation used when CLI platform unset", annotation: "linux/arm64", want: "linux/arm64"},
{name: "per-image overrides annotation and CLI", cliPlatform: "linux/amd64", annotation: "linux/arm64", imagePlatform: "linux/386", want: "linux/386"},
// CLI wins outright over both annotation and per-image.
{name: "CLI overrides annotation and per-image", cliPlatform: "linux/amd64", annotation: "linux/arm64", imagePlatform: "linux/386", want: "linux/amd64"},
{name: "per-image overrides annotation when CLI platform unset", annotation: "linux/arm64", imagePlatform: "linux/386", want: "linux/386"},
{name: "none set stays empty", want: ""},
}
@@ -1103,19 +1139,21 @@ func TestResolveImageJobs_ExcludeExtrasPrecedence(t *testing.T) {
tests := []struct {
name string
cliExcludeExtras bool
cliChanged bool
annotation string
imageExclude bool
want bool
}{
{name: "CLI true", cliExcludeExtras: true, want: true},
{name: "annotation true overrides CLI false", annotation: "true", want: true},
{name: "per-image true overrides annotation/CLI false", imageExclude: true, want: true},
{name: "CLI true", cliExcludeExtras: true, cliChanged: true, want: true},
{name: "annotation true when CLI unset", annotation: "true", want: true},
{name: "per-image true when CLI unset", imageExclude: true, want: true},
{name: "explicit CLI false overrides annotation and per-image", cliChanged: true, annotation: "true", imageExclude: true, want: false},
{name: "all false stays false", want: false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
o := &flags.SyncOpts{ExcludeExtras: tc.cliExcludeExtras}
o := &flags.SyncOpts{ExcludeExtras: tc.cliExcludeExtras, ExcludeExtrasChanged: tc.cliChanged}
a := map[string]string{}
if tc.annotation != "" {
a[consts.ImageAnnotationExcludeExtras] = tc.annotation
@@ -1136,6 +1174,46 @@ func TestResolveImageJobs_ExcludeExtrasPrecedence(t *testing.T) {
}
}
func TestResolveImageJobs_InsecurePrecedence(t *testing.T) {
tests := []struct {
name string
cliCaFile string
cliChanged bool
annotation string
imageIns bool
want bool
}{
{name: "per-image true when CLI unset", imageIns: true, want: true},
{name: "annotation true when CLI unset", annotation: "true", want: true},
{name: "explicit CLI false overrides annotation and per-image", cliChanged: true, annotation: "true", imageIns: true, want: false},
// a CA file forces verification on, overriding a per-image/annotation true
{name: "ca-file forces insecure off", cliCaFile: "/ca.pem", annotation: "true", imageIns: true, want: false},
{name: "all unset stays false", want: false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
o := &flags.SyncOpts{CaFile: tc.cliCaFile, InsecureChanged: tc.cliChanged}
a := map[string]string{}
if tc.annotation != "" {
a[consts.ImageAnnotationInsecureSkipTLSVerify] = tc.annotation
}
images := []v1.Image{{Name: "rancher/rancher:v2.9", InsecureSkipTLSVerify: tc.imageIns}}
jobs, err := resolveImageJobs(o, a, images)
if err != nil {
t.Fatalf("resolveImageJobs: %v", err)
}
if len(jobs) != 1 {
t.Fatalf("expected 1 job, got %d", len(jobs))
}
if jobs[0].img.InsecureSkipTLSVerify != tc.want {
t.Errorf("got insecure %v, want %v", jobs[0].img.InsecureSkipTLSVerify, tc.want)
}
})
}
}
func TestResolveImageJobs_NoOptions_MinimalJob(t *testing.T) {
o := &flags.SyncOpts{}
images := []v1.Image{{Name: "rancher/rancher:v2.9"}}
internal/flags/add.go
+2 -2
View File
@@ -22,7 +22,7 @@ type AddImageOpts struct {
ExcludeExtras bool
Local bool
CaFile string
InsecureSkipTLSVerify *bool
InsecureSkipTLSVerify bool
}
func (o *AddImageOpts) AddFlags(cmd *cobra.Command) {
@@ -39,7 +39,7 @@ func (o *AddImageOpts) AddFlags(cmd *cobra.Command) {
f.BoolVar(&o.ExcludeExtras, "exclude-extras", false, "(Optional) Exclude cosign signatures, attestations, SBOMs, and OCI referrers when pulling the image")
f.BoolVar(&o.Local, "local", false, "(Optional) Add image from the local Docker daemon instead of a remote registry")
f.StringVar(&o.CaFile, "ca-file", "", "(Optional) Location of CA Bundle to enable certification verification")
f.Bool("insecure-skip-tls-verify", false, "(Optional) Skip TLS certificate verification")
f.BoolVar(&o.InsecureSkipTLSVerify, "insecure-skip-tls-verify", false, "(Optional) Skip TLS certificate verification")
}
type AddFileOpts struct {
internal/flags/sync.go
+12 -2
View File
@@ -26,7 +26,17 @@ type SyncOpts struct {
Concurrency int
NoProgress bool
CaFile string
InsecureSkipTLSVerify *bool
InsecureSkipTLSVerify bool
// Whether each of these flags was explicitly set on the CLI, captured in
// sync's PreRunE. A plain bool (and a resolved store/retries value) has no
// "unset" state, so the resolvers use these markers to let an explicit CLI
// value win over per-item/annotation instead of only ever turning a flag on.
TlogChanged bool
ExcludeExtrasChanged bool
InsecureChanged bool
StoreChanged bool
RetriesChanged bool
}
func (o *SyncOpts) AddFlags(cmd *cobra.Command) {
@@ -50,5 +60,5 @@ func (o *SyncOpts) AddFlags(cmd *cobra.Command) {
f.IntVarP(&o.Concurrency, "concurrency", "j", consts.DefaultConcurrency, "(Optional) Maximum number of artifacts to fetch and store concurrently (1 = serial; also via HAULER_CONCURRENCY, explicit flag wins)")
f.BoolVar(&o.NoProgress, "no-progress", false, "(Optional) Disable the live progress display")
f.StringVar(&o.CaFile, "ca-file", "", "(Optional) Location of CA Bundle to enable certification verification")
f.Bool("insecure-skip-tls-verify", false, "(Optional) Skip TLS certificate verification")
f.BoolVar(&o.InsecureSkipTLSVerify, "insecure-skip-tls-verify", false, "(Optional) Skip TLS certificate verification")
}
pkg/apis/hauler.cattle.io/v1/chart.go
+1 -1
View File
@@ -41,6 +41,6 @@ type Chart struct {
CertFile string `json:"certFile,omitempty"`
KeyFile string `json:"keyFile,omitempty"`
CaFile string `json:"caFile,omitempty"`
InsecureSkipTLSVerify *bool `json:"insecureSkipTLSVerify,omitempty"`
InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"`
PlainHTTP bool `json:"plainHTTP,omitempty"`
}
pkg/apis/hauler.cattle.io/v1/file.go
+1 -1
View File
@@ -26,5 +26,5 @@ type File struct {
// TLS options for verifying the file contents for remote files.
// If not specified, the default system CA bundle will be used.
CaFile string `json:"ca-file"`
InsecureSkipTLSVerify *bool `json:"insecure-skip-tls-verify,omitempty"`
InsecureSkipTLSVerify bool `json:"insecure-skip-tls-verify"`
}
pkg/apis/hauler.cattle.io/v1/image.go
+1 -1
View File
@@ -43,5 +43,5 @@ type Image struct {
// TLS options for verifying the image signature. If not specified, the default system CA bundle will be used.
CaFile string `json:"ca-file"`
InsecureSkipTLSVerify *bool `json:"insecure-skip-tls-verify,omitempty"`
InsecureSkipTLSVerify bool `json:"insecure-skip-tls-verify"`
}