Skip to content

Commit

Permalink
refactor: Make CLI argument names consistent (#2084)
Browse files Browse the repository at this point in the history
* fix: Wrong argument naming

* fix: tarPath as well

* Test

* fix: Fix tests

* np: Format markdown

* fix: Review changes
  • Loading branch information
gabyx authored Aug 22, 2022
1 parent 348018d commit 90e426b
Show file tree
Hide file tree
Showing 9 changed files with 750 additions and 494 deletions.
5 changes: 5 additions & 0 deletions .prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
printWidth: 80
tabWidth: 2
semi: false
singleQuote: false
overrides: [{ files: "*.md", options: { proseWrap: always } }]
25 changes: 16 additions & 9 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ To get started developing, see our [DEVELOPMENT.md](./DEVELOPMENT.md).

In this file you'll find info on:

- [The CLA](#contributor-license-agreement)
- [The code review process](#code-reviews)
- [Standards](#standards) around [commit messages](#commit-messages) and [code](#coding-standards)
- [Contributing to Kaniko](#contributing-to-kaniko)
- [Contributor License Agreement](#contributor-license-agreement)
- [Code reviews](#code-reviews)
- [Standards](#standards)
- [Commit Messages](#commit-messages)
- [Coding standards](#coding-standards)
- [Finding something to work on](#finding-something-to-work-on)
[code](#coding-standards)
- [Finding something to work on](#finding-something-to-work-on)

## Contributor License Agreement
Expand Down Expand Up @@ -36,12 +41,13 @@ This section describes the standards we will try to maintain in this repo.

### Commit Messages

All commit messages should follow [these best practices](https://chris.beams.io/posts/git-commit/),
specifically:
All commit messages should follow
[these best practices](https://chris.beams.io/posts/git-commit/), specifically:

- Start with a subject line
- Contain a body that explains _why_ you're making the change you're making
- Reference an issue number if one exists, closing it if applicable (with text such as
- Reference an issue number if one exists, closing it if applicable (with text
such as
["Fixes #245" or "Closes #111"](https://help.github.com/articles/closing-issues-using-keywords/))

Aim for [2 paragraphs in the body](https://www.youtube.com/watch?v=PJjmw9TRB7s).
Expand All @@ -61,10 +67,11 @@ The code in this repo should follow best practices, specifically:

## Finding something to work on

Thanks so much for considering contributing to our project!! We hope very much you can find something
interesting to work on:
Thanks so much for considering contributing to our project!! We hope very much
you can find something interesting to work on:

- To find issues that we particularly would like contributors to tackle, look for
- To find issues that we particularly would like contributors to tackle, look
for
[issues with the "help wanted" label](https://github.com/GoogleContainerTools/kaniko/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22).
- Issues that are good for new folks will additionally be marked with
["good first issue"](https://github.com/GoogleContainerTools/kaniko/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22).
2 changes: 1 addition & 1 deletion DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ steps:
args:
- --build-arg=NUM=${_COUNT}
- --no-push
- --snapshotMode=redo
- --snapshot-mode=redo
env:
- 'BENCHMARK_FILE=gs://$PROJECT_ID/gcb/benchmark_file'
```
Expand Down
1,072 changes: 641 additions & 431 deletions README.md

Large diffs are not rendered by default.

67 changes: 49 additions & 18 deletions cmd/executor/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,31 @@ func init() {
RootCmd.PersistentFlags().MarkDeprecated("whitelist-var-run", "Please use ignore-var-run instead.")
}

func valdiateFlags() {
checkNoDeprecatedFlags()

// Allow setting --registry-mirror using an environment variable.
if val, ok := os.LookupEnv("KANIKO_REGISTRY_MIRROR"); ok {
opts.RegistryMirrors.Set(val)
}

// Default the custom platform flag to our current platform, and validate it.
if opts.CustomPlatform == "" {
opts.CustomPlatform = platforms.Format(platforms.Normalize(platforms.DefaultSpec()))
}
if _, err := v1.ParsePlatform(opts.CustomPlatform); err != nil {
logrus.Fatalf("Invalid platform %q: %v", opts.CustomPlatform, err)
}
}

// RootCmd is the kaniko command that is run
var RootCmd = &cobra.Command{
Use: "executor",
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
if cmd.Use == "executor" {

valdiateFlags()

// Command line flag takes precedence over the KANIKO_DIR environment variable.
dir := config.KanikoDir
if opts.KanikoDir != constants.DefaultKanikoPath {
Expand All @@ -85,7 +104,7 @@ var RootCmd = &cobra.Command{
}

if !opts.NoPush && len(opts.Destinations) == 0 {
return errors.New("You must provide --destination, or use --no-push")
return errors.New("you must provide --destination, or use --no-push")
}
if err := cacheFlagsValid(); err != nil {
return errors.Wrap(err, "cache flags invalid")
Expand All @@ -97,10 +116,10 @@ var RootCmd = &cobra.Command{
return errors.Wrap(err, "error resolving dockerfile path")
}
if len(opts.Destinations) == 0 && opts.ImageNameDigestFile != "" {
return errors.New("You must provide --destination if setting ImageNameDigestFile")
return errors.New("you must provide --destination if setting ImageNameDigestFile")
}
if len(opts.Destinations) == 0 && opts.ImageNameTagDigestFile != "" {
return errors.New("You must provide --destination if setting ImageNameTagDigestFile")
return errors.New("you must provide --destination if setting ImageNameTagDigestFile")
}
// Update ignored paths
if opts.IgnoreVarRun {
Expand Down Expand Up @@ -184,8 +203,8 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().StringVarP(&ctxSubPath, "context-sub-path", "", "", "Sub path within the given context.")
RootCmd.PersistentFlags().StringVarP(&opts.Bucket, "bucket", "b", "", "Name of the GCS bucket from which to access build context as tarball.")
RootCmd.PersistentFlags().VarP(&opts.Destinations, "destination", "d", "Registry the final image should be pushed to. Set it repeatedly for multiple destinations.")
RootCmd.PersistentFlags().StringVarP(&opts.SnapshotMode, "snapshotMode", "", "full", "Change the file attributes inspected during snapshotting")
RootCmd.PersistentFlags().StringVarP(&opts.CustomPlatform, "customPlatform", "", "", "Specify the build platform if different from the current host")
RootCmd.PersistentFlags().StringVarP(&opts.SnapshotMode, "snapshot-mode", "", "full", "Change the file attributes inspected during snapshotting")
RootCmd.PersistentFlags().StringVarP(&opts.CustomPlatform, "custom-platform", "", "", "Specify the build platform if different from the current host")
RootCmd.PersistentFlags().VarP(&opts.BuildArgs, "build-arg", "", "This flag allows you to pass in ARG values at build time. Set it repeatedly for multiple values.")
RootCmd.PersistentFlags().BoolVarP(&opts.Insecure, "insecure", "", false, "Push to insecure registry using plain HTTP")
RootCmd.PersistentFlags().BoolVarP(&opts.SkipTLSVerify, "skip-tls-verify", "", false, "Push to insecure registry ignoring TLS verify")
Expand All @@ -194,7 +213,7 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().IntVar(&opts.PushRetry, "push-retry", 0, "Number of retries for the push operation")
RootCmd.PersistentFlags().IntVar(&opts.ImageFSExtractRetry, "image-fs-extract-retry", 0, "Number of retries for image FS extraction")
RootCmd.PersistentFlags().StringVarP(&opts.KanikoDir, "kaniko-dir", "", constants.DefaultKanikoPath, "Path to the kaniko directory, this takes precedence over the KANIKO_DIR environment variable.")
RootCmd.PersistentFlags().StringVarP(&opts.TarPath, "tarPath", "", "", "Path to save the image in as a tarball instead of pushing")
RootCmd.PersistentFlags().StringVarP(&opts.TarPath, "tar-path", "", "", "Path to save the image in as a tarball instead of pushing")
RootCmd.PersistentFlags().BoolVarP(&opts.SingleSnapshot, "single-snapshot", "", false, "Take a single snapshot at the end of the build.")
RootCmd.PersistentFlags().BoolVarP(&opts.Reproducible, "reproducible", "", false, "Strip timestamps out of the image to make it reproducible")
RootCmd.PersistentFlags().StringVarP(&opts.Target, "target", "", "", "Set the target build stage to build")
Expand Down Expand Up @@ -225,18 +244,10 @@ func addKanikoOptionsFlags() {
RootCmd.PersistentFlags().VarP(&opts.IgnorePaths, "ignore-path", "", "Ignore these paths when taking a snapshot. Set it repeatedly for multiple paths.")
RootCmd.PersistentFlags().BoolVarP(&opts.ForceBuildMetadata, "force-build-metadata", "", false, "Force add metadata layers to build image")

// Allow setting --registry-mirror using an environment variable.
if val, ok := os.LookupEnv("KANIKO_REGISTRY_MIRROR"); ok {
opts.RegistryMirrors.Set(val)
}

// Default the custom platform flag to our current platform, and validate it.
if opts.CustomPlatform == "" {
opts.CustomPlatform = platforms.Format(platforms.Normalize(platforms.DefaultSpec()))
}
if _, err := v1.ParsePlatform(opts.CustomPlatform); err != nil {
logrus.Fatalf("Invalid platform %q: %v", opts.CustomPlatform, err)
}
// Deprecated flags.
RootCmd.PersistentFlags().StringVarP(&opts.SnapshotModeDeprecated, "snapshotMode", "", "", "This flag is deprecated. Please use '--snapshot-mode'.")
RootCmd.PersistentFlags().StringVarP(&opts.CustomPlatformDeprecated, "customPlatform", "", "", "This flag is deprecated. Please use '--custom-platform'.")
RootCmd.PersistentFlags().StringVarP(&opts.TarPath, "tarPath", "", "", "This flag is deprecated. Please use '--tar-path'.")
}

// addHiddenFlags marks certain flags as hidden from the executor help text
Expand Down Expand Up @@ -272,6 +283,26 @@ func checkContained() bool {
return proc.GetContainerRuntime(0, 0) != proc.RuntimeNotFound
}

// checkNoDeprecatedFlags return an error if deprecated flags are used.
func checkNoDeprecatedFlags() {

// In version >=2.0.0 make it fail (`Warn` -> `Fatal`)
if opts.CustomPlatformDeprecated != "" {
logrus.Warn("Flag --customPlatform is deprecated. Use: --custom-platform")
opts.CustomPlatform = opts.CustomPlatformDeprecated
}

if opts.SnapshotModeDeprecated != "" {
logrus.Warn("Flag --snapshotMode is deprecated. Use: --snapshot-mode")
opts.SnapshotMode = opts.SnapshotModeDeprecated
}

if opts.TarPathDeprecated != "" {
logrus.Warn("Flag --tarPath is deprecated. Use: --tar-path")
opts.TarPath = opts.TarPathDeprecated
}
}

// cacheFlagsValid makes sure the flags passed in related to caching are valid
func cacheFlagsValid() error {
if !opts.Cache {
Expand Down
2 changes: 1 addition & 1 deletion integration/benchmark_fs/cloudbuild.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ steps:
args:
- --build-arg=NUM=${_COUNT}
- --no-push
- --snapshotMode=redo
- --snapshot-mode=redo
env:
- 'BENCHMARK_FILE=gs://$PROJECT_ID/gcb/benchmark_file_${_COUNT}'
timeout: 2400s
Expand Down
2 changes: 1 addition & 1 deletion integration/dockerfiles/Dockerfile_test_issue_1837
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ RUN yum --disableplugin=subscription-manager install -y iputils
RUN setcap cap_net_raw+ep /bin/ping || exit 1

FROM base
RUN [ ! -z "$(getcap /bin/ping)" ] || exit 1
RUN [ ! -z "$(getcap /bin/ping)" ] || exit 1
2 changes: 1 addition & 1 deletion integration/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ var additionalDockerFlagsMap = map[string][]string{
var additionalKanikoFlagsMap = map[string][]string{
"Dockerfile_test_add": {"--single-snapshot"},
"Dockerfile_test_run_new": {"--use-new-run=true"},
"Dockerfile_test_run_redo": {"--snapshotMode=redo"},
"Dockerfile_test_run_redo": {"--snapshot-mode=redo"},
"Dockerfile_test_scratch": {"--single-snapshot"},
"Dockerfile_test_maintainer": {"--single-snapshot"},
"Dockerfile_test_target": {"--target=second"},
Expand Down
67 changes: 35 additions & 32 deletions pkg/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,38 +47,41 @@ type RegistryOptions struct {
type KanikoOptions struct {
RegistryOptions
CacheOptions
Destinations multiArg
BuildArgs multiArg
Labels multiArg
Git KanikoGitOptions
IgnorePaths multiArg
DockerfilePath string
SrcContext string
SnapshotMode string
CustomPlatform string
Bucket string
TarPath string
KanikoDir string
Target string
CacheRepo string
DigestFile string
ImageNameDigestFile string
ImageNameTagDigestFile string
OCILayoutPath string
ImageFSExtractRetry int
SingleSnapshot bool
Reproducible bool
NoPush bool
NoPushCache bool
Cache bool
Cleanup bool
CompressedCaching bool
IgnoreVarRun bool
SkipUnusedStages bool
RunV2 bool
CacheCopyLayers bool
CacheRunLayers bool
ForceBuildMetadata bool
Destinations multiArg
BuildArgs multiArg
Labels multiArg
Git KanikoGitOptions
IgnorePaths multiArg
DockerfilePath string
SrcContext string
SnapshotMode string
SnapshotModeDeprecated string
CustomPlatform string
CustomPlatformDeprecated string
Bucket string
TarPath string
TarPathDeprecated string
KanikoDir string
Target string
CacheRepo string
DigestFile string
ImageNameDigestFile string
ImageNameTagDigestFile string
OCILayoutPath string
ImageFSExtractRetry int
SingleSnapshot bool
Reproducible bool
NoPush bool
NoPushCache bool
Cache bool
Cleanup bool
CompressedCaching bool
IgnoreVarRun bool
SkipUnusedStages bool
RunV2 bool
CacheCopyLayers bool
CacheRunLayers bool
ForceBuildMetadata bool
}

type KanikoGitOptions struct {
Expand Down

0 comments on commit 90e426b

Please sign in to comment.