diff --git a/pkg/renew/renew.go b/pkg/renew/renew.go index 19ddf40..5f7e26f 100644 --- a/pkg/renew/renew.go +++ b/pkg/renew/renew.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" @@ -96,24 +97,37 @@ func NewCmdRenew(ctx context.Context, ioStreams genericclioptions.IOStreams) *co // Validate validates the provided options func (o *Options) Validate(cmd *cobra.Command, args []string) error { - if len(o.LabelSelector) > 0 && len(args) > 0 { - return errors.New("cannot specify Certificate names in conjunction with label selectors") + // The --all, --selector and args are mutually exclusive. + // - --all is equivalent to a match-all label selector + // - --selector filters by label selector + // - args are an explicit list of Certificate names + // However, there must always be one of the three specified. + + var flags []string + if len(args) > 0 { + flags = append(flags, fmt.Sprintf("the Certificate resource names %q", args)) } - - if len(o.LabelSelector) > 0 && o.All { - return errors.New("cannot specify label selectors in conjunction with --all flag") + if o.All { + flags = append(flags, "the --all flag") + } + if len(o.LabelSelector) > 0 { + flags = append(flags, "a label selector") } - if o.All && len(args) > 0 { - return errors.New("cannot specify Certificate names in conjunction with --all flag") + // Ensure that only one of the three flags are specified + if len(flags) > 1 { + return fmt.Errorf("cannot specify %s in conjunction with %s", flags[0], strings.Join(flags[1:], " and ")) } - if o.All && cmd.PersistentFlags().Changed("namespace") { - return errors.New("cannot specify --namespace flag in conjunction with --all flag") + if len(flags) == 0 { + return errors.New("please either supply one or more Certificate resource names, label selectors, or use the --all flag to renew all Certificate resources") } - if !o.All && len(args) == 0 { - return errors.New("please supply one or more Certificate resource names or use the --all flag to renew all Certificate resources") + // The --all-namespaces flag overrides the --namespace flag. + // Additionally, we can only use --all-namespaces when not specifying a list of Certificate names + + if o.AllNamespaces && len(args) > 0 { + return errors.New("cannot specify Certificate names in conjunction with --all-namespaces flag") } return nil diff --git a/pkg/renew/renew_test.go b/pkg/renew/renew_test.go index b93f228..92957e3 100644 --- a/pkg/renew/renew_test.go +++ b/pkg/renew/renew_test.go @@ -41,12 +41,18 @@ func TestValidate(t *testing.T) { args: []string{"abc"}, expErr: true, }, + "If there are arguments, as well as --all-namespaces, error": { + options: &Options{ + AllNamespaces: true, + }, + args: []string{"abc"}, + expErr: true, + }, "If there are all certificates selected, as well as label selector, error": { options: &Options{ LabelSelector: "foo=bar", All: true, }, - args: []string{""}, expErr: true, }, "If there are all certificates selected, as well as arguments, error": { @@ -63,14 +69,14 @@ func TestValidate(t *testing.T) { }, expErr: false, }, - "If --namespace and --all namespace specified, error": { + "If --namespace and --all specified, don't error": { options: &Options{ All: true, }, setStringFlags: []stringFlag{ {name: "namespace", value: "foo"}, }, - expErr: true, + expErr: false, }, "If --namespace specified without arguments, error": { options: &Options{}, @@ -95,6 +101,29 @@ func TestValidate(t *testing.T) { }, expErr: false, }, + "If --label-selector specified with --all, error": { + options: &Options{ + LabelSelector: "foo=bar", + All: true, + }, + expErr: true, + }, + "If --label-selector specified with --all-namespaces, don't error": { + options: &Options{ + AllNamespaces: true, + LabelSelector: "foo=bar", + }, + expErr: false, + }, + "If --label-selector specified with --namespace, don't error": { + options: &Options{ + LabelSelector: "foo=bar", + }, + setStringFlags: []stringFlag{ + {name: "namespace", value: "foo"}, + }, + expErr: false, + }, } for name, test := range tests {