-
Notifications
You must be signed in to change notification settings - Fork 162
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
Add --print-config flag #171
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
--print_config Instead of installing ASM, print all of | ||
the compiled YAML to stdout. All other | ||
output will be written to stderr, even if | ||
it would normally go to stdout. Skip all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I agree about "skip all validations" in this case. Is that standard/idiomatic for this kind of command? We can discuss offline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it's a trade-off. In the context of the linked issue (GitOps / CI/CD integration) I thought it was safer for now to disable it by default. I don't know if the CI/CD user/service account is going to have the access to check everything, if the cluster names may be templated in later, etc. @therealmitchconnors do you have any guidance one way or the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this validation against user-supplied config, or against runtime values in the cluster or project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Runtime values. The CLI options/dependencies are still verified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then it does seem like a runtime validation might not be possible in some CI scenarios. Perhaps we should have a separate flag to control this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Added #176 to track the extra flag.
Instead of installing anything, just print the config that would have been applied. This also skips verification and cluster/project setup. Closes #169.
Sample output (citadel was chosen to include an additional yaml file to overlay):