From 201f88d7f32b5c1345f43fa28a0d48d0edb4fbb2 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Tue, 24 Oct 2023 19:49:52 +0100 Subject: [PATCH 1/2] feat: Validate empty values for required flags --- .../create/helmbundle/helm_bundle.go | 12 +++++++ .../create/imagebundle/image_bundle.go | 12 +++++++ cmd/mindthegap/flags/validate.go | 32 +++++++++++++++++++ .../importcmd/imagebundle/image_bundle.go | 12 +++++++ cmd/mindthegap/push/bundle/bundle.go | 11 +++++++ cmd/mindthegap/serve/bundle/bundle.go | 12 +++++++ 6 files changed, 91 insertions(+) create mode 100644 cmd/mindthegap/flags/validate.go diff --git a/cmd/mindthegap/create/helmbundle/helm_bundle.go b/cmd/mindthegap/create/helmbundle/helm_bundle.go index acde0d26..73ff849c 100644 --- a/cmd/mindthegap/create/helmbundle/helm_bundle.go +++ b/cmd/mindthegap/create/helmbundle/helm_bundle.go @@ -18,6 +18,7 @@ import ( "github.com/mesosphere/mindthegap/archive" "github.com/mesosphere/mindthegap/cleanup" + "github.com/mesosphere/mindthegap/cmd/mindthegap/flags" "github.com/mesosphere/mindthegap/cmd/mindthegap/utils" "github.com/mesosphere/mindthegap/config" "github.com/mesosphere/mindthegap/docker/registry" @@ -34,6 +35,17 @@ func NewCommand(out output.Output) *cobra.Command { cmd := &cobra.Command{ Use: "helm-bundle", Short: "Create a Helm chart bundle", + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return err + } + + if err := flags.ValidateRequiredFlagValues(cmd, "helm-charts-file"); err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if !overwrite { out.StartOperation("Checking if output file already exists") diff --git a/cmd/mindthegap/create/imagebundle/image_bundle.go b/cmd/mindthegap/create/imagebundle/image_bundle.go index 934390b3..c28c48d4 100644 --- a/cmd/mindthegap/create/imagebundle/image_bundle.go +++ b/cmd/mindthegap/create/imagebundle/image_bundle.go @@ -23,6 +23,7 @@ import ( "github.com/mesosphere/mindthegap/archive" "github.com/mesosphere/mindthegap/cleanup" + "github.com/mesosphere/mindthegap/cmd/mindthegap/flags" "github.com/mesosphere/mindthegap/cmd/mindthegap/utils" "github.com/mesosphere/mindthegap/config" "github.com/mesosphere/mindthegap/docker/registry" @@ -43,6 +44,17 @@ func NewCommand(out output.Output) *cobra.Command { cmd := &cobra.Command{ Use: "image-bundle", Short: "Create an image bundle", + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return err + } + + if err := flags.ValidateRequiredFlagValues(cmd, "images-file"); err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if !overwrite { out.StartOperation("Checking if output file already exists") diff --git a/cmd/mindthegap/flags/validate.go b/cmd/mindthegap/flags/validate.go new file mode 100644 index 00000000..6946aa17 --- /dev/null +++ b/cmd/mindthegap/flags/validate.go @@ -0,0 +1,32 @@ +// Copyright 2023 D2iQ, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package flags + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +func ValidateRequiredFlagValues(cmd *cobra.Command, requiredFlagsWithValues ...string) error { + fs := cmd.Flags() + + missingFlagValues := []string{} + for _, flagName := range requiredFlagsWithValues { + foundFlag := fs.Lookup(flagName) + if foundFlag == nil { + continue + } + + if foundFlag.Value.String() == "" || foundFlag.Value.String() == "[]" { + missingFlagValues = append(missingFlagValues, flagName) + } + } + + if len(missingFlagValues) > 0 { + return fmt.Errorf(`required flag value(s) "%s" not set`, strings.Join(missingFlagValues, `", "`)) + } + return nil +} diff --git a/cmd/mindthegap/importcmd/imagebundle/image_bundle.go b/cmd/mindthegap/importcmd/imagebundle/image_bundle.go index 94321746..f83b2bb8 100644 --- a/cmd/mindthegap/importcmd/imagebundle/image_bundle.go +++ b/cmd/mindthegap/importcmd/imagebundle/image_bundle.go @@ -19,6 +19,7 @@ import ( "github.com/mesosphere/dkp-cli-runtime/core/output" "github.com/mesosphere/mindthegap/cleanup" + "github.com/mesosphere/mindthegap/cmd/mindthegap/flags" "github.com/mesosphere/mindthegap/cmd/mindthegap/utils" "github.com/mesosphere/mindthegap/containerd" "github.com/mesosphere/mindthegap/docker/registry" @@ -34,6 +35,17 @@ func NewCommand(out output.Output) *cobra.Command { cmd := &cobra.Command{ Use: "image-bundle", Short: "Import images from image bundles into Containerd", + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return err + } + + if err := flags.ValidateRequiredFlagValues(cmd, "image-bundle"); err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { cleaner := cleanup.NewCleaner() defer cleaner.Cleanup() diff --git a/cmd/mindthegap/push/bundle/bundle.go b/cmd/mindthegap/push/bundle/bundle.go index 4fde30b2..df2fbeb8 100644 --- a/cmd/mindthegap/push/bundle/bundle.go +++ b/cmd/mindthegap/push/bundle/bundle.go @@ -64,6 +64,17 @@ func NewCommand(out output.Output, bundleCmdName string) *cobra.Command { cmd := &cobra.Command{ Use: bundleCmdName, Short: "Push from bundles into an existing OCI registry", + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return err + } + + if err := flags.ValidateRequiredFlagValues(cmd, bundleCmdName, "to-registry"); err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { cleaner := cleanup.NewCleaner() defer cleaner.Cleanup() diff --git a/cmd/mindthegap/serve/bundle/bundle.go b/cmd/mindthegap/serve/bundle/bundle.go index 85c5cb1f..58896ebc 100644 --- a/cmd/mindthegap/serve/bundle/bundle.go +++ b/cmd/mindthegap/serve/bundle/bundle.go @@ -14,6 +14,7 @@ import ( "github.com/mesosphere/dkp-cli-runtime/core/output" "github.com/mesosphere/mindthegap/cleanup" + "github.com/mesosphere/mindthegap/cmd/mindthegap/flags" "github.com/mesosphere/mindthegap/cmd/mindthegap/utils" "github.com/mesosphere/mindthegap/config" "github.com/mesosphere/mindthegap/docker/registry" @@ -36,6 +37,17 @@ func NewCommand( cmd = &cobra.Command{ Use: bundleCmdName, Short: "Serve an OCI registry from previously created bundles", + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := cmd.ValidateRequiredFlags(); err != nil { + return err + } + + if err := flags.ValidateRequiredFlagValues(cmd, bundleCmdName); err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { cleaner := cleanup.NewCleaner() defer cleaner.Cleanup() From a59e49a8d6bfe06b25317ac5e48de196f95b84d1 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Wed, 25 Oct 2023 11:20:34 +0100 Subject: [PATCH 2/2] fixup! refactor: Rename func and better handle slice values --- .../create/helmbundle/helm_bundle.go | 2 +- .../create/imagebundle/image_bundle.go | 2 +- cmd/mindthegap/flags/validate.go | 19 +++++++++++++++---- .../importcmd/imagebundle/image_bundle.go | 2 +- cmd/mindthegap/push/bundle/bundle.go | 2 +- cmd/mindthegap/serve/bundle/bundle.go | 2 +- 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/cmd/mindthegap/create/helmbundle/helm_bundle.go b/cmd/mindthegap/create/helmbundle/helm_bundle.go index 73ff849c..1378ffd6 100644 --- a/cmd/mindthegap/create/helmbundle/helm_bundle.go +++ b/cmd/mindthegap/create/helmbundle/helm_bundle.go @@ -40,7 +40,7 @@ func NewCommand(out output.Output) *cobra.Command { return err } - if err := flags.ValidateRequiredFlagValues(cmd, "helm-charts-file"); err != nil { + if err := flags.ValidateFlagsThatRequireValues(cmd, "helm-charts-file"); err != nil { return err } diff --git a/cmd/mindthegap/create/imagebundle/image_bundle.go b/cmd/mindthegap/create/imagebundle/image_bundle.go index c28c48d4..73a0d684 100644 --- a/cmd/mindthegap/create/imagebundle/image_bundle.go +++ b/cmd/mindthegap/create/imagebundle/image_bundle.go @@ -49,7 +49,7 @@ func NewCommand(out output.Output) *cobra.Command { return err } - if err := flags.ValidateRequiredFlagValues(cmd, "images-file"); err != nil { + if err := flags.ValidateFlagsThatRequireValues(cmd, "images-file"); err != nil { return err } diff --git a/cmd/mindthegap/flags/validate.go b/cmd/mindthegap/flags/validate.go index 6946aa17..acf8ac40 100644 --- a/cmd/mindthegap/flags/validate.go +++ b/cmd/mindthegap/flags/validate.go @@ -8,9 +8,11 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/spf13/pflag" + "k8s.io/utils/strings/slices" ) -func ValidateRequiredFlagValues(cmd *cobra.Command, requiredFlagsWithValues ...string) error { +func ValidateFlagsThatRequireValues(cmd *cobra.Command, requiredFlagsWithValues ...string) error { fs := cmd.Flags() missingFlagValues := []string{} @@ -20,13 +22,22 @@ func ValidateRequiredFlagValues(cmd *cobra.Command, requiredFlagsWithValues ...s continue } - if foundFlag.Value.String() == "" || foundFlag.Value.String() == "[]" { - missingFlagValues = append(missingFlagValues, flagName) + if sv, ok := foundFlag.Value.(pflag.SliceValue); ok { + if len(slices.Filter(nil, sv.GetSlice(), func(s string) bool { return s != "" })) == 0 { + missingFlagValues = append(missingFlagValues, flagName) + } + } else { + if foundFlag.Value.String() == "" { + missingFlagValues = append(missingFlagValues, flagName) + } } } if len(missingFlagValues) > 0 { - return fmt.Errorf(`required flag value(s) "%s" not set`, strings.Join(missingFlagValues, `", "`)) + return fmt.Errorf( + `the following flags require value(s) to be specified: "%s"`, + strings.Join(missingFlagValues, `", "`), + ) } return nil } diff --git a/cmd/mindthegap/importcmd/imagebundle/image_bundle.go b/cmd/mindthegap/importcmd/imagebundle/image_bundle.go index f83b2bb8..63eadaae 100644 --- a/cmd/mindthegap/importcmd/imagebundle/image_bundle.go +++ b/cmd/mindthegap/importcmd/imagebundle/image_bundle.go @@ -40,7 +40,7 @@ func NewCommand(out output.Output) *cobra.Command { return err } - if err := flags.ValidateRequiredFlagValues(cmd, "image-bundle"); err != nil { + if err := flags.ValidateFlagsThatRequireValues(cmd, "image-bundle"); err != nil { return err } diff --git a/cmd/mindthegap/push/bundle/bundle.go b/cmd/mindthegap/push/bundle/bundle.go index df2fbeb8..033c835a 100644 --- a/cmd/mindthegap/push/bundle/bundle.go +++ b/cmd/mindthegap/push/bundle/bundle.go @@ -69,7 +69,7 @@ func NewCommand(out output.Output, bundleCmdName string) *cobra.Command { return err } - if err := flags.ValidateRequiredFlagValues(cmd, bundleCmdName, "to-registry"); err != nil { + if err := flags.ValidateFlagsThatRequireValues(cmd, bundleCmdName, "to-registry"); err != nil { return err } diff --git a/cmd/mindthegap/serve/bundle/bundle.go b/cmd/mindthegap/serve/bundle/bundle.go index 58896ebc..933555de 100644 --- a/cmd/mindthegap/serve/bundle/bundle.go +++ b/cmd/mindthegap/serve/bundle/bundle.go @@ -42,7 +42,7 @@ func NewCommand( return err } - if err := flags.ValidateRequiredFlagValues(cmd, bundleCmdName); err != nil { + if err := flags.ValidateFlagsThatRequireValues(cmd, bundleCmdName); err != nil { return err }