Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/cip: Default to non-production runs with --confirm #285

Merged
merged 3 commits into from
Sep 14, 2021
Merged
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
7 changes: 0 additions & 7 deletions cmd/cip/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions cmd/cip/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion legacy/cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

type RootOptions struct {
LogLevel string
DryRun bool
}

func printVersion() {
Expand Down
25 changes: 13 additions & 12 deletions legacy/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type RunOptions struct {
Threads int
MaxImageSize int
SeverityThreshold int
DryRun bool
Confirm bool
JSONLogSummary bool
ParseOnly bool
MinimalSnapshot bool
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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) **********")
}
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
48 changes: 19 additions & 29 deletions legacy/dockerregistry/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion legacy/dockerregistry/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test-e2e/cip-auditor/cip-auditor-e2e.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func (t *E2ETest) clearRepositories() error {
{Registries: t.Registries},
},
10,
false,
true,
true)
if err != nil {
return err
Expand Down
23 changes: 9 additions & 14 deletions test-e2e/cip/e2e.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -132,18 +130,16 @@ 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
}

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)
Expand All @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down