Skip to content

Commit 9496e8e

Browse files
apply copilot reviews (#689)
1 parent 7fb3f77 commit 9496e8e

2 files changed

Lines changed: 45 additions & 8 deletions

File tree

cmd/kosli/snapshotK8S.go

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,18 @@ func newSnapshotK8SCmd(out io.Writer) *cobra.Command {
161161
Example: snapshotK8SExample,
162162
Args: func(cmd *cobra.Command, args []string) error {
163163
configFileFlag := cmd.Flags().Lookup("config-file")
164-
hasConfigFile := configFileFlag != nil && configFileFlag.Changed
165-
if hasConfigFile && len(args) > 0 {
164+
if configFileFlag != nil && configFileFlag.Changed && o.configFilePath == "" {
165+
return fmt.Errorf("cannot use '--config-file' with an empty value")
166+
}
167+
useConfigFile := o.configFilePath != ""
168+
if useConfigFile && len(args) > 0 {
166169
return fmt.Errorf("cannot use '--config-file' together with a positional environment name argument")
167170
}
168-
if !hasConfigFile && len(args) == 0 {
171+
if !useConfigFile && len(args) == 0 {
169172
return fmt.Errorf("requires either a positional environment name argument or --config-file")
170173
}
171-
if !hasConfigFile && len(args) > 1 {
172-
return fmt.Errorf("accepts at most 1 arg(s), received %d", len(args))
174+
if !useConfigFile && len(args) > 1 {
175+
return fmt.Errorf("accepts 1 arg(s), received %d", len(args))
173176
}
174177
return nil
175178
},
@@ -180,8 +183,11 @@ func newSnapshotK8SCmd(out io.Writer) *cobra.Command {
180183
}
181184

182185
configFileFlag := cmd.Flags().Lookup("config-file")
183-
hasConfigFile := configFileFlag != nil && configFileFlag.Changed
184-
if hasConfigFile {
186+
if configFileFlag != nil && configFileFlag.Changed && o.configFilePath == "" {
187+
return ErrorBeforePrintingUsage(cmd, "cannot use '--config-file' with an empty value")
188+
}
189+
useConfigFile := o.configFilePath != ""
190+
if useConfigFile {
185191
namespaceFlagNames := []string{"namespaces", "exclude-namespaces", "namespaces-regex", "exclude-namespaces-regex"}
186192
for _, flagName := range namespaceFlagNames {
187193
if f := cmd.Flags().Lookup(flagName); f != nil && f.Changed {
@@ -191,7 +197,20 @@ func newSnapshotK8SCmd(out io.Writer) *cobra.Command {
191197
return nil
192198
}
193199

194-
return MuXRequiredFlags(cmd, []string{"namespaces", "exclude-namespaces"}, false)
200+
// Include vs exclude namespace flags mutual exclusion (all combinations)
201+
if err := MuXRequiredFlags(cmd, []string{"namespaces", "exclude-namespaces"}, false); err != nil {
202+
return err
203+
}
204+
if err := MuXRequiredFlags(cmd, []string{"namespaces", "exclude-namespaces-regex"}, false); err != nil {
205+
return err
206+
}
207+
if err := MuXRequiredFlags(cmd, []string{"namespaces-regex", "exclude-namespaces"}, false); err != nil {
208+
return err
209+
}
210+
if err := MuXRequiredFlags(cmd, []string{"namespaces-regex", "exclude-namespaces-regex"}, false); err != nil {
211+
return err
212+
}
213+
return nil
195214
},
196215
RunE: func(cmd *cobra.Command, args []string) error {
197216
clientset, err := kube.NewK8sClientSet(o.kubeconfig)

cmd/kosli/snapshotK8S_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,30 @@ func (suite *SnapshotK8STestSuite) TestSnapshotK8SCmd() {
3838
cmd: fmt.Sprintf(`snapshot k8s %s --namespaces default --exclude-namespaces default %s`, suite.envName, suite.defaultKosliArguments),
3939
golden: "Error: only one of --namespaces, --exclude-namespaces is allowed\n",
4040
},
41+
{
42+
wantError: true,
43+
name: "snapshot K8S fails if both --namespaces-regex and --exclude-namespaces are set",
44+
cmd: fmt.Sprintf(`snapshot k8s %s --namespaces-regex "^default" --exclude-namespaces kube-system %s`, suite.envName, suite.defaultKosliArguments),
45+
golden: "Error: only one of --namespaces-regex, --exclude-namespaces is allowed\n",
46+
},
47+
{
48+
wantError: true,
49+
name: "snapshot K8S fails if both --namespaces and --exclude-namespaces-regex are set",
50+
cmd: fmt.Sprintf(`snapshot k8s %s --namespaces default --exclude-namespaces-regex "^kube-" %s`, suite.envName, suite.defaultKosliArguments),
51+
golden: "Error: only one of --namespaces, --exclude-namespaces-regex is allowed\n",
52+
},
4153
{
4254
wantError: true,
4355
name: "snapshot K8S fails if no args and no --config-file",
4456
cmd: fmt.Sprintf(`snapshot k8s %s`, suite.defaultKosliArguments),
4557
golden: "Error: requires either a positional environment name argument or --config-file\n",
4658
},
59+
{
60+
wantError: true,
61+
name: "snapshot K8S fails if --config-file is set to empty value",
62+
cmd: fmt.Sprintf(`snapshot k8s --config-file "" %s`, suite.defaultKosliArguments),
63+
golden: "Error: cannot use '--config-file' with an empty value\n",
64+
},
4765
{
4866
wantError: true,
4967
name: "snapshot K8S fails if --config-file and positional arg are both provided",

0 commit comments

Comments
 (0)