cli validate cluster name in cmd and config #6789
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
eks-distro-bot
added
the
size/M
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6789 +/- ##
==========================================
- Coverage 71.78% 71.75% -0.03%
==========================================
Files 531 531
Lines 41295 41261 -34
==========================================
- Hits 29642 29607 -35
- Misses 9993 9996 +3
+ Partials 1660 1658 -2
☔ View full report in Codecov by Sentry. |
| @@ -65,7 +66,8 @@ func init() { | |||
| flags.MarkRequired(createClusterCmd.Flags(), flags.ClusterConfig.Name) | |||
| } | |||
|
|
|||
| func (uc *upgradeClusterOptions) upgradeCluster(cmd *cobra.Command) error { | |||
| // nolint:gocyclo | |||
There was a problem hiding this comment.
upgradeCluster is already a high cyclomatic complexity function. If we take validation out, what remains are still above 10 cyclomatic complexity, plus we need to pass clusterSpec and clusterConfig from validation func or generate it again. So adding nolint here
| @@ -91,6 +93,10 @@ func (uc *upgradeClusterOptions) upgradeCluster(cmd *cobra.Command) error { | |||
| if _, err := uc.commonValidations(ctx); err != nil { | |||
| return fmt.Errorf("common validations failed due to: %v", err) | |||
| } | |||
|
|
|||
| if err := validations.ValidateClusterNameFromCommandAndConfig(args, clusterConfig.Name); err != nil { | |||
There was a problem hiding this comment.
We technically don't support upgrading the cluster without passing in a file with the -f flag. So we never actually use the cluster name itself if they pass one in as an arg in the command.
But this makes sense if we just want to catch cases where they think they are running the upgrade plan command 👍
There was a problem hiding this comment.
yes, this is to prevent customer mistyping upgrade plan and plan upgrade
|
/lgtm |
Issue #, if available:
#6716
Description of changes:
add validation checking cluster name provided in command matches with config
Testing (if applicable):
unit test
Documentation added/planned (if applicable):
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.