-
Notifications
You must be signed in to change notification settings - Fork 233
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
Extend 'config print' command with additional options #415
Conversation
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
@xmudrii I just noticed that provider is not getting validated
|
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
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.
small tweaks are needed
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
Signed-off-by: Marko Mudrinić <mudrinic.mare@gmail.com>
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
/approve
LGTM label has been added. Git tree hash: 01ecf6cbdfffbd2269b5ce6f2f736a27dded1f5a
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kron4eg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/override pull-kubeone-e2e-aws-conformance-1.13.5 |
@xmudrii: Overrode contexts on behalf of xmudrii: pull-kubeone-e2e-aws-conformance-1.13.5, pull-kubeone-e2e-digitalocean-conformance-1.13.5, pull-kubeone-e2e-digitalocean-conformance-1.14.1, pull-kubeone-e2e-digitalocean-upgrade-1.13.5-1.14.1 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-kubeone-dependencies |
@xmudrii: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/override pull-kubeone-e2e-aws-conformance-1.14.1 |
@xmudrii: Overrode contexts on behalf of xmudrii: pull-kubeone-e2e-aws-conformance-1.14.1 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What this PR does / why we need it:
This is an experimental approach to the
config print
command. I tested and confirmed it works well. Theconfig print
command is now extended to support most of the options, so someone can build a configuration file without browsing GitHub and searching for theconfig.yaml.dist
manifest.The idea behind this is to improve the user experience, so the user can work with our API without having to browse GitHub to find
config.yaml.dist
or to browse the API types. Various tools have different approaches to this, so I can believe we can find a good one too.However, I have several concerns regarding this topic:
hosts
andenableOpenIDConnect
(so we have all features supported) and drop others as they're mostly like not-commonly used.set
flag that changes some property not covered by flags. That way the command is not overloaded with flags, but it's still possible to build usable configuration file.set
flag is that you still have to know the API.config print
command we can split them to two commands:config create
command that works like one in this PRconfig print
command that has two "modes":config.yaml.dist
config.yaml.dist
might overlap withconfig create
, so potentially we can drop one.yamled
is nice as we get a nice looking manifest without all the empty fields, but there is no strongly enforced type safety.kyaml.UnmarshalStrict
ensures that it'll not print invalid manifest, but in that case, there should be some unit test cases in the CI to check is it working.Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):xref #254
Release note:
/assign @kron4eg