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

filepromoter: Prefer Confirm instead of DryRun #437

Merged
merged 2 commits into from
Oct 2, 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
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.2.1
3.3.0-beta.0
2 changes: 1 addition & 1 deletion cloudbuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ substitutions:
# vYYYYMMDD-hash, and can be used as a substitution
_GIT_TAG: '12345'
_PULL_BASE_REF: 'dev'
_IMG_VERSION: 'v3.2.1-1'
_IMG_VERSION: 'v3.3.0-beta.0-1'

tags:
- 'kpromo'
Expand Down
2 changes: 1 addition & 1 deletion cmd/kpromo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Usage:
kpromo run files [flags]

Flags:
--dry-run test run promotion without modifying any filestore (default true)
--confirm initiate a PRODUCTION artifact promotion
--files files path to the files manifest
--filestores filestores path to the filestores promoter manifest
-h, --help help for files
Expand Down
8 changes: 4 additions & 4 deletions cmd/kpromo/cmd/run/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ func init() {

// TODO: Consider moving this to the root command
filesCmd.PersistentFlags().BoolVar(
&filesOpts.DryRun,
"dry-run",
filesOpts.DryRun,
"test run promotion without modifying any filestore",
&filesOpts.Confirm,
"confirm",
filesOpts.Confirm,
"initiate a PRODUCTION artifact promotion",
)

filesCmd.PersistentFlags().BoolVar(
Expand Down
4 changes: 2 additions & 2 deletions dependencies.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
dependencies:
# Release version
- name: "repo release version"
version: 3.2.1
version: 3.3.0-beta.0
refPaths:
- path: VERSION

Expand Down Expand Up @@ -57,7 +57,7 @@ dependencies:
match: go \d+.\d+

- name: "k8s.gcr.io/artifact-promoter/kpromo"
version: v3.2.1-1
version: v3.3.0-beta.0-1
refPaths:
- path: cloudbuild.yaml
match: "_IMG_VERSION: 'v((([0-9]+)\\.([0-9]+)\\.([0-9]+)(?:-([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?)(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?)-([0-9]+)'"
Expand Down
15 changes: 8 additions & 7 deletions filepromoter/filestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ type FilestorePromoter struct {

Files []api.File

DryRun bool
// Confirm, if set, will trigger a PRODUCTION artifact promotion.
Confirm bool

// UseServiceAccount must be true, for service accounts to be used
// This gives some protection against a hostile manifest.
Expand All @@ -60,7 +61,7 @@ type syncFilestore interface {
func openFilestore(
ctx context.Context,
filestore *api.Filestore,
useServiceAccount, dryRun bool,
useServiceAccount, confirm bool,
) (syncFilestore, error) {
u, err := url.Parse(filestore.Base)
if err != nil {
Expand All @@ -79,7 +80,7 @@ func openFilestore(
)
}

withAuth, err := useStorageClientAuth(filestore, useServiceAccount, dryRun)
withAuth, err := useStorageClientAuth(filestore, useServiceAccount, confirm)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -125,7 +126,7 @@ func openFilestore(

func useStorageClientAuth(
filestore *api.Filestore,
useServiceAccount, dryRun bool,
useServiceAccount, confirm bool,
) (bool, error) {
withAuth := false

Expand All @@ -135,7 +136,7 @@ func useStorageClientAuth(
return withAuth, nil
}

if !dryRun {
if confirm {
if filestore.ServiceAccount == "" {
return withAuth, fmt.Errorf("cannot execute a production file promotion without a service account")
}
Expand Down Expand Up @@ -237,7 +238,7 @@ func (p *FilestorePromoter) BuildOperations(
ctx,
p.Source,
p.UseServiceAccount,
p.DryRun,
p.Confirm,
)
if err != nil {
return nil, err
Expand All @@ -250,7 +251,7 @@ func (p *FilestorePromoter) BuildOperations(
ctx,
p.Dest,
p.UseServiceAccount,
p.DryRun,
p.Confirm,
)
if err != nil {
return nil, err
Expand Down
16 changes: 8 additions & 8 deletions filepromoter/filestore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Test_useStorageClientAuth(t *testing.T) {
type args struct {
filestore *api.Filestore
useServiceAccount bool
dryRun bool
confirm bool
}
tests := []struct {
name string
Expand All @@ -42,7 +42,7 @@ func Test_useStorageClientAuth(t *testing.T) {
filestore: &api.Filestore{
ServiceAccount: "[email protected]",
},
dryRun: false,
confirm: true,
},
want: true,
wantErr: false,
Expand All @@ -51,7 +51,7 @@ func Test_useStorageClientAuth(t *testing.T) {
name: "production without service account",
args: args{
filestore: &api.Filestore{},
dryRun: false,
confirm: true,
},
want: false,
wantErr: true,
Expand All @@ -62,7 +62,7 @@ func Test_useStorageClientAuth(t *testing.T) {
filestore: &api.Filestore{
Src: true,
},
dryRun: false,
confirm: true,
},
want: false,
wantErr: false,
Expand All @@ -71,7 +71,7 @@ func Test_useStorageClientAuth(t *testing.T) {
name: "non-production",
args: args{
filestore: &api.Filestore{},
dryRun: true,
confirm: false,
},
want: false,
wantErr: false,
Expand All @@ -80,7 +80,7 @@ func Test_useStorageClientAuth(t *testing.T) {
name: "non-production with service account failure",
args: args{
filestore: &api.Filestore{},
dryRun: true,
confirm: false,
useServiceAccount: true,
},
want: false,
Expand All @@ -92,7 +92,7 @@ func Test_useStorageClientAuth(t *testing.T) {
filestore: &api.Filestore{
ServiceAccount: "[email protected]",
},
dryRun: true,
confirm: false,
useServiceAccount: true,
},
want: true,
Expand All @@ -104,7 +104,7 @@ func Test_useStorageClientAuth(t *testing.T) {
got, err := useStorageClientAuth(
tt.args.filestore,
tt.args.useServiceAccount,
tt.args.dryRun,
tt.args.confirm,
)

if tt.wantErr {
Expand Down
5 changes: 3 additions & 2 deletions filepromoter/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import (
type ManifestPromoter struct {
Manifest *api.Manifest

DryRun bool
// Confirm, if set, will trigger a PRODUCTION artifact promotion.
Confirm bool

// UseServiceAccount must be true, for service accounts to be used
// This gives some protection against a hostile manifest.
Expand Down Expand Up @@ -57,7 +58,7 @@ func (p *ManifestPromoter) BuildOperations(
Source: source,
Dest: filestore,
Files: p.Manifest.Files,
DryRun: p.DryRun,
Confirm: p.Confirm,
UseServiceAccount: p.UseServiceAccount,
}
ops, err := fp.BuildOperations(ctx)
Expand Down
28 changes: 16 additions & 12 deletions promobot/promotefiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ type PromoteFilesOptions struct {
// └── red.yaml
ManifestsPath string

// DryRun (if set) will not perform operations, but print them instead
DryRun bool
// Confirm, if set, will trigger a PRODUCTION artifact promotion.
Confirm bool

// UseServiceAccount must be true, for service accounts to be used
// This gives some protection against a hostile manifest.
Expand All @@ -75,7 +75,7 @@ type PromoteFilesOptions struct {

// PopulateDefaults sets the default values for PromoteFilesOptions
func (o *PromoteFilesOptions) PopulateDefaults() {
o.DryRun = true
o.Confirm = false
o.UseServiceAccount = false
o.Out = os.Stdout
}
Expand All @@ -87,21 +87,23 @@ func RunPromoteFiles(ctx context.Context, options PromoteFilesOptions) error {
return err
}

if options.DryRun {
if options.Confirm {
fmt.Fprintf(
options.Out,
"********** START (DRY RUN) **********\n")
"********** START **********\n",
)
} else {
fmt.Fprintf(
options.Out,
"********** START **********\n")
"********** START (DRY RUN) **********\n",
)
}

var ops []filepromoter.SyncFileOp
for _, manifest := range manifests {
promoter := &filepromoter.ManifestPromoter{
Manifest: manifest,
DryRun: options.DryRun,
Confirm: options.Confirm,
UseServiceAccount: options.UseServiceAccount,
}

Expand All @@ -125,7 +127,7 @@ func RunPromoteFiles(ctx context.Context, options PromoteFilesOptions) error {
)
}

if !options.DryRun {
if options.Confirm {
if err := op.Run(ctx); err != nil {
logrus.Warnf("error copying file: %v", err)
errors = append(errors, err)
Expand All @@ -144,14 +146,16 @@ func RunPromoteFiles(ctx context.Context, options PromoteFilesOptions) error {
return errors[0]
}

if options.DryRun {
if options.Confirm {
fmt.Fprintf(
options.Out,
"********** FINISHED (DRY RUN) **********\n")
"********** FINISHED **********\n",
)
} else {
fmt.Fprintf(
options.Out,
"********** FINISHED **********\n")
"********** FINISHED (DRY RUN) **********\n",
)
}

return nil
Expand Down Expand Up @@ -210,7 +214,7 @@ func ReadManifests(options PromoteFilesOptions) ([]*api.Manifest, error) {
prjOpts := &PromoteFilesOptions{
FilestoresPath: filestores,
FilesPath: files,
DryRun: options.DryRun,
Confirm: options.Confirm,
UseServiceAccount: options.UseServiceAccount,
Out: options.Out,
}
Expand Down
2 changes: 1 addition & 1 deletion workspace_status.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ p_ IMG_REGISTRY gcr.io
p_ IMG_REPOSITORY k8s-staging-artifact-promoter
p_ IMG_NAME kpromo
p_ IMG_TAG "${image_tag}"
p_ IMG_VERSION v3.2.1-1
p_ IMG_VERSION v3.3.0-beta.0-1
p_ TEST_AUDIT_PROD_IMG_REPOSITORY us.gcr.io/k8s-gcr-audit-test-prod
p_ TEST_AUDIT_STAGING_IMG_REPOSITORY gcr.io/k8s-gcr-audit-test-prod
p_ TEST_AUDIT_PROJECT_ID k8s-gcr-audit-test-prod
Expand Down