Skip to content

[installer] Rename and print OR remove --danger-use-unsupported-config flag #8107

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

Closed
sagor999 opened this issue Feb 8, 2022 · 4 comments · Fixed by #8477
Closed

[installer] Rename and print OR remove --danger-use-unsupported-config flag #8107

sagor999 opened this issue Feb 8, 2022 · 4 comments · Fixed by #8477
Assignees
Labels
team: delivery Issue belongs to the self-hosted team

Comments

@sagor999
Copy link
Contributor

sagor999 commented Feb 8, 2022

Bug description

when using experimental settings in installer, error message is not very helpful.
would be good to tell user how to enable experimental options in that message.
Also, after digging through source code I did find the switch for it:
-danger-use-unsupported-config enable use of unsupported config
which doesn't mention anything that it enables experimental settings.

Steps to reproduce

./gitpod-installer render --config overrride-values.yaml > manifests.yaml
config contains experimental options - ignoring them

Workspace affected

No response

Expected behavior

No response

Example repository

No response

Anything else?

No response

@metcalfc
Copy link
Contributor

metcalfc commented Feb 8, 2022

This is a rough UX. Can we change to flag to something that connects it to experimental ?

--enable-experimental-config

The user of the installer is considered someone who is ok with low-level access and is often other Gitpod folks. I think the safety is good but we should still make it easy. To go with this, when we warn folks lets tell them how to fix it, ie add the flag vs just ignore it.

If folks have experimental config sections in their YAML, chances are they want to use them. Should we just bail at this point and emit the warning that they should re-run with the flag or remove the experimental config?

@corneliusludmann
Copy link
Contributor

If folks have experimental config sections in their YAML, chances are they want to use them. Should we just bail at this point and emit the warning that they should re-run with the flag or remove the experimental config?

Given that the main config surface for most people out there should be the Replicated UI and that the term “experimental” in the config file should be warning enough, I would also vote for removing the flag entirely.

@corneliusludmann corneliusludmann changed the title [installer] config contains experimental options - ignoring them [installer] Rename and print OR remove --danger-use-unsupported-config flag Feb 17, 2022
@Pothulapati Pothulapati self-assigned this Feb 28, 2022
@Pothulapati Pothulapati moved this from 🤝Proposed to ⚒In Progress in 🚚 Security, Infrastructure, and Delivery Team (SID) Feb 28, 2022
Pothulapati added a commit that referenced this issue Feb 28, 2022
Fixes #8107

Users when they explicitely add the `experimental` section into
their config, probably know that they are using it. We might add a
section in the [config documentation](#8107)
to make it a bit more explicit that it might be dangerous to use.

But having to use a flag doesn't really make sense, as discussed in the
issue. This PR removes that flag.

Signed-off-by: Tarun Pothulapati <[email protected]>
@mrsimonemms
Copy link
Contributor

I would also vote for removing the flag entirely.

I think I would respectfully disagree. The experimental config is not for most users (truthfully, it's mostly for SaaS) and there's a danger that people read "experimental" as "beta" or "this is some brilliant stuff that's coming in the future - use it now to be bleeding-edge" which it absolutely isn't.

It's unsupported configuration which allows users to go outside the tracks of the Installer's framework

@corneliusludmann
Copy link
Contributor

OK, convinced me. In particular, I think @csweichel's argument is key:

Say you get config sent to you, don't really understand it in detail, then this flag will make (hopefully) make you stop and think for a second.

Pothulapati added a commit that referenced this issue Feb 28, 2022
Fixes #8107

This PR updates the flag descriptions, and the `render` display
notes to be more explicit on what experimental config is.

This flag is required, to make sure users are opting in explicitely/
knowlingly to use the experimental config.

Signed-off-by: Tarun Pothulapati <[email protected]>
Pothulapati added a commit that referenced this issue Mar 2, 2022
Fixes #8107

This PR updates the flag descriptions, and the `render` display
notes to be more explicit on what experimental config is.

This flag is required, to make sure users are opting in explicitely/
knowlingly to use the experimental config.

Signed-off-by: Tarun Pothulapati <[email protected]>
@corneliusludmann corneliusludmann moved this from ⚒In Progress to 🕶In Review / Measuring in 🚚 Security, Infrastructure, and Delivery Team (SID) Mar 2, 2022
roboquat pushed a commit that referenced this issue Mar 2, 2022
Fixes #8107

This PR updates the flag descriptions, and the `render` display
notes to be more explicit on what experimental config is.

This flag is required, to make sure users are opting in explicitely/
knowlingly to use the experimental config.

Signed-off-by: Tarun Pothulapati <[email protected]>
Repository owner moved this from 🕶In Review / Measuring to ✨Done in 🚚 Security, Infrastructure, and Delivery Team (SID) Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team: delivery Issue belongs to the self-hosted team
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants