diff --git a/cmd/cip/cmd/root.go b/cmd/cip/cmd/root.go index b132695d..60d93414 100644 --- a/cmd/cip/cmd/root.go +++ b/cmd/cip/cmd/root.go @@ -52,13 +52,6 @@ func init() { "info", fmt.Sprintf("the logging verbosity, either %s", log.LevelNames()), ) - - rootCmd.PersistentFlags().BoolVar( - &rootOpts.DryRun, - "dry-run", - rootOpts.DryRun, - "test run promotion without modifying any registry", - ) } func initLogging(*cobra.Command, []string) error { diff --git a/cmd/cip/cmd/run.go b/cmd/cip/cmd/run.go index c86e4ef1..fafb5698 100644 --- a/cmd/cip/cmd/run.go +++ b/cmd/cip/cmd/run.go @@ -49,6 +49,13 @@ var runOpts = &cli.RunOptions{} // TODO: Function 'init' is too long (171 > 60) (funlen) // nolint: funlen func init() { + runCmd.PersistentFlags().BoolVar( + &runOpts.Confirm, + "confirm", + runOpts.Confirm, + "initiate a PRODUCTION image promotion", + ) + // TODO: Move this into a default options function in pkg/promobot runCmd.PersistentFlags().StringVar( &runOpts.Manifest, diff --git a/legacy/cli/root.go b/legacy/cli/root.go index ae489103..e689bd30 100644 --- a/legacy/cli/root.go +++ b/legacy/cli/root.go @@ -24,7 +24,6 @@ import ( type RootOptions struct { LogLevel string - DryRun bool } func printVersion() { diff --git a/legacy/cli/run.go b/legacy/cli/run.go index 16543b42..99916cc6 100644 --- a/legacy/cli/run.go +++ b/legacy/cli/run.go @@ -40,7 +40,7 @@ type RunOptions struct { Threads int MaxImageSize int SeverityThreshold int - DryRun bool + Confirm bool JSONLogSummary bool ParseOnly bool MinimalSnapshot bool @@ -142,7 +142,7 @@ func RunPromoteCmd(opts *RunOptions) error { sc, err = reg.MakeSyncContext( mfests, opts.Threads, - opts.DryRun, + opts.Confirm, opts.UseServiceAcct, ) if err != nil { @@ -159,8 +159,9 @@ func RunPromoteCmd(opts *RunOptions) error { sc, err = reg.MakeSyncContext( mfests, opts.Threads, - opts.DryRun, - opts.UseServiceAcct) + opts.Confirm, + opts.UseServiceAcct, + ) if err != nil { logrus.Fatal(err) } @@ -210,10 +211,10 @@ So a 'fixable' vulnerability may not necessarily be immediately actionable. For example, even though a fixed version of the binary is available, it doesn't necessarily mean that a new version of the image layer is available.`, ) - } else if opts.DryRun { - logrus.Info("********** START (DRY RUN) **********") - } else { + } else if opts.Confirm { logrus.Info("********** START **********") + } else { + logrus.Info("********** START (DRY RUN) **********") } } @@ -249,7 +250,7 @@ necessarily mean that a new version of the image layer is available.`, sc, err = reg.MakeSyncContext( mfests, opts.Threads, - opts.DryRun, + opts.Confirm, opts.UseServiceAcct, ) if err != nil { @@ -302,7 +303,7 @@ necessarily mean that a new version of the image layer is available.`, } // Check the pull request - if opts.DryRun { + if !opts.Confirm { err = sc.RunChecks([]reg.PreCheck{}) if err != nil { return errors.Wrap(err, "running prechecks before promotion") @@ -362,10 +363,10 @@ necessarily mean that a new version of the image layer is available.`, if opts.SeverityThreshold >= 0 { logrus.Info("********** FINISHED (VULN CHECK) **********") - } else if opts.DryRun { - logrus.Info("********** FINISHED (DRY RUN) **********") - } else { + } else if opts.Confirm { logrus.Info("********** FINISHED **********") + } else { + logrus.Info("********** FINISHED (DRY RUN) **********") } return nil diff --git a/legacy/dockerregistry/inventory.go b/legacy/dockerregistry/inventory.go index 48d82cc0..f976a1fb 100644 --- a/legacy/dockerregistry/inventory.go +++ b/legacy/dockerregistry/inventory.go @@ -60,11 +60,11 @@ func GetSrcRegistry(rcs []RegistryContext) (*RegistryContext, error) { func MakeSyncContext( mfests []Manifest, threads int, - dryRun, useSvcAcc bool, + confirm, useSvcAcc bool, ) (SyncContext, error) { sc := SyncContext{ Threads: threads, - DryRun: dryRun, + Confirm: confirm, UseServiceAccount: useSvcAcc, Inv: make(MasterInventory), InvIgnore: []ImageName{}, @@ -2012,10 +2012,10 @@ func MKPopulateRequestsForPromotionEdges( return } - if sc.DryRun { - logrus.Info("---------- BEGIN PROMOTION (DRY RUN) ----------") - } else { + if sc.Confirm { logrus.Info("---------- BEGIN PROMOTION ----------") + } else { + logrus.Info("---------- BEGIN PROMOTION (DRY RUN) ----------") } for promoteMe := range toPromote { @@ -2282,24 +2282,19 @@ func (sc *SyncContext) Promote( captured := make(CapturedRequests) - if sc.DryRun { + if sc.Confirm { + processRequest = processRequestReal + } else { processRequestDryRun := MkRequestCapturer(&captured) processRequest = processRequestDryRun - } else { - processRequest = processRequestReal } if customProcessRequest != nil { processRequest = *customProcessRequest } - err := sc.ExecRequests(populateRequests, processRequest) - - if sc.DryRun { - sc.PrintCapturedRequests(&captured) - } - - return err + sc.PrintCapturedRequests(&captured) + return sc.ExecRequests(populateRequests, processRequest) } // PrintCapturedRequests pretty-prints all given PromotionRequests. @@ -2470,25 +2465,22 @@ func (sc *SyncContext) GarbageCollect( captured := make(CapturedRequests) - if sc.DryRun { + if sc.Confirm { + processRequest = processRequestReal + } else { processRequestDryRun := MkRequestCapturer(&captured) processRequest = processRequestDryRun - } else { - processRequest = processRequestReal } if customProcessRequest != nil { processRequest = *customProcessRequest } + sc.PrintCapturedRequests(&captured) err := sc.ExecRequests(populateRequests, processRequest) if err != nil { logrus.Info(err) } - - if sc.DryRun { - sc.PrintCapturedRequests(&captured) - } } func supportedMediaType(v string) (ggcrV1Types.MediaType, error) { @@ -2594,17 +2586,19 @@ func (sc *SyncContext) ClearRepository( captured := make(CapturedRequests) - if sc.DryRun { + if sc.Confirm { + processRequest = processRequestReal + } else { processRequestDryRun := MkRequestCapturer(&captured) processRequest = processRequestDryRun - } else { - processRequest = processRequestReal } if customProcessRequest != nil { processRequest = *customProcessRequest } + sc.PrintCapturedRequests(&captured) + // TODO: These variables can likely be condensed into a single function var ( isEqualTo = func(want ggcrV1Types.MediaType) func(ggcrV1Types.MediaType) bool { @@ -2633,10 +2627,6 @@ func (sc *SyncContext) ClearRepository( if err != nil { logrus.Info(err) } - - if sc.DryRun { - sc.PrintCapturedRequests(&captured) - } } // GetWriteCmd generates a gcloud command that is used to make modifications to diff --git a/legacy/dockerregistry/types.go b/legacy/dockerregistry/types.go index 7e075593..959c8642 100644 --- a/legacy/dockerregistry/types.go +++ b/legacy/dockerregistry/types.go @@ -78,7 +78,7 @@ type CollectedLogs struct { // SyncContext is the main data structure for performing the promotion. type SyncContext struct { Threads int - DryRun bool + Confirm bool UseServiceAccount bool Inv MasterInventory InvIgnore []ImageName diff --git a/test-e2e/cip-auditor/cip-auditor-e2e.go b/test-e2e/cip-auditor/cip-auditor-e2e.go index b3d90ab4..42c7129a 100644 --- a/test-e2e/cip-auditor/cip-auditor-e2e.go +++ b/test-e2e/cip-auditor/cip-auditor-e2e.go @@ -328,7 +328,7 @@ func (t *E2ETest) clearRepositories() error { {Registries: t.Registries}, }, 10, - false, + true, true) if err != nil { return err diff --git a/test-e2e/cip/e2e.go b/test-e2e/cip/e2e.go index 9cb61732..bea86ce2 100644 --- a/test-e2e/cip/e2e.go +++ b/test-e2e/cip/e2e.go @@ -85,16 +85,14 @@ func main() { // Loop through each e2e test case. for _, t := range ts { fmt.Printf("\n===> Running e2e test '%s'...\n", t.Name) - err := testSetup(*repoRootPtr, &t) - if err != nil { + if err := testSetup(*repoRootPtr, &t); err != nil { logrus.Fatalf("error with test setup: %q", err) } fmt.Println("checking snapshots BEFORE promotion:") for _, snapshot := range t.Snapshots { - err := checkSnapshot(snapshot.Name, snapshot.Before, *repoRootPtr, t.Registries) - if err != nil { - logrus.Fatalf("error checking snapshot for %s: %q", snapshot.Name, err) + if err := checkSnapshot(snapshot.Name, snapshot.Before, *repoRootPtr, t.Registries); err != nil { + logrus.Fatalf("error checking snapshot before promotion for %s: %q", snapshot.Name, err) } } @@ -132,10 +130,8 @@ func checkSnapshot( diff := cmp.Diff(got, expected) if diff != "" { - return errors.Errorf( - "expected equivalent image sets, but the following diff exists: %s", - diff, - ) + fmt.Printf("the following diff exists: %s", diff) + return errors.Errorf("expected equivalent image sets") } return nil @@ -143,7 +139,7 @@ func checkSnapshot( func testSetup(repoRoot string, t *E2ETest) error { if err := t.clearRepositories(); err != nil { - return err + return errors.Wrap(err, "cleaning test repository") } goldenPush := fmt.Sprintf("%s/test-e2e/golden-images/push-golden.sh", repoRoot) @@ -158,7 +154,6 @@ func testSetup(repoRoot string, t *E2ETest) error { std, err := cmd.RunSuccessOutput() fmt.Println(std.Output()) fmt.Println(std.Error()) - return err } @@ -167,7 +162,7 @@ func runPromotion(repoRoot string, t *E2ETest) error { "run", fmt.Sprintf("%s/cmd/cip/main.go", repoRoot), "run", - "--dry-run=false", + "--confirm", "--log-level=debug", "--use-service-account", // There is no need to use -key-files=... because we already activated @@ -251,10 +246,10 @@ func (t *E2ETest) clearRepositories() error { {Registries: t.Registries}, }, 10, - false, + true, true) if err != nil { - return err + return errors.Wrap(err, "trying to create sync context") } sc.ReadRegistries(