Skip to content

Commit

Permalink
Merge pull request #437 from justaugustus/330
Browse files Browse the repository at this point in the history
filepromoter: Prefer `Confirm` instead of `DryRun`
  • Loading branch information
k8s-ci-robot authored Oct 2, 2021
2 parents 98f769a + e23a787 commit e43d7d2
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 39 deletions.
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

0 comments on commit e43d7d2

Please sign in to comment.