Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide instant feedback for invalid arguments #970

Closed
cPu1 opened this issue Jul 3, 2019 · 2 comments
Closed

Provide instant feedback for invalid arguments #970

cPu1 opened this issue Jul 3, 2019 · 2 comments
Labels
area/general-cli kind/feature New feature or request priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases

Comments

@cPu1
Copy link
Collaborator

cPu1 commented Jul 3, 2019

Why do you want this feature?
Some commands like eksctl utils describe-stacks, eksctl utils write-kubeconfig and eksctl delete iamidentitymapping (non-exhaustive list) authenticate with AWS first (using sts.GetCallerIdentity) before validating arguments for correctness. Since the auth check is an HTTP call, it takes some time to complete and any invalid arguments are reported only after that check is complete.

What feature/behavior/change do you want?
To provide instant feedback, eksctl commands should report any invalid arguments that can be validated locally before making any auth calls.

@cPu1 cPu1 added kind/feature New feature or request area/general-cli labels Jul 3, 2019
@errordeveloper
Copy link
Contributor

Most command actually do this. Which arguments/flags are you thinking of specifically?

Also, do keep in mind that bulk of parameters will lie in config files and we'd hope to reduce the number of flags over time. I believe that in most of the case we do validation and defaulting of config files before we do the auth check.

https://github.com/weaveworks/eksctl/blob/0b50182498491ff0ed3643fac222ec288af5d322/pkg/ctl/create/cluster.go#L89-L102

It would be best to focus on getting config file loading in a better shape. It would be great if we can get to a place where error messages around config file fields or flag usage don't need to be coded as very specific error message that refer to either field path or flag name.
We have something that works now, but there is still a bunch of stuff that can be cleaned-up and refactored. I think we should look at this with long-term goal of creating a library (see #813). We also have #488 that needs some attention long term. Any short term improvements would need to relate to those long term goals.

@martina-if martina-if added priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases technical debt labels Sep 15, 2020
@aclevername
Copy link
Contributor

Closing due to staleness

torredil pushed a commit to torredil/eksctl that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/general-cli kind/feature New feature or request priority/important-longterm Important over the long term, but may not be currently staffed and/or may require multiple releases
Projects
None yet
Development

No branches or pull requests

4 participants