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

installer: always override -c config on default values #8401

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

Pothulapati
Copy link
Contributor

Description

Currently, Users run init command first, update it and
pass full config to render to generate Kubernetes manifests. The
passage of -c is a requirement here, and users can't skip it.

This PR makes the passage of config to render optional, and
flexible. This means

  • Users can skip -c entirely, in which case we use the default
    values on the default config version for that installer binary
  • Users can selectively override fields and thus no need to pass full
    config
    all the time. This means -c flag acts as a flag through
    which they can override the default fields.

For the second case, When a user explicitely sets the apiVersion
field in the passed config, we use the default values for that
version. If no apiVersion is passed, we override the passed config
onto the default values on the default config version for that
installer binary.

After this change, For users This means that they only store and use
the config that has their changes only (on the default config), and
not the entire config.

Signed-off-by: Tarun Pothulapati [email protected]

Related Issue(s)

Fixes #8267

How to test

Build the binary, with a speciifc version

VERSION="main.2514" make build

Run render no -c passed

./dist/gitpod-installer render

Run render with only domain set

echo "domain: gitpod-test.com" > config.yaml

./dist/gitpod-installer render -c ./config.yaml | grep gitpod-test

Release Notes

Make `-c` optional in installer, while allowing the passed config to be flexible

Documentation

@Pothulapati Pothulapati requested a review from a team February 23, 2022 06:42
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Feb 23, 2022
@Pothulapati Pothulapati force-pushed the tar/installer-default branch from e3cb65a to 386bd24 Compare February 23, 2022 06:43
@codecov
Copy link

codecov bot commented Feb 23, 2022

Codecov Report

Merging #8401 (a3232f0) into main (db835ef) will decrease coverage by 4.72%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             main   #8401      +/-   ##
=========================================
- Coverage   12.31%   7.59%   -4.73%     
=========================================
  Files          20      31      +11     
  Lines        1161    2172    +1011     
=========================================
+ Hits          143     165      +22     
- Misses       1014    2004     +990     
+ Partials        4       3       -1     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
install-installer-raw-app 4.58% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/auth.go
components/local-app/pkg/auth/pkce.go
...staller/pkg/components/ws-manager/networkpolicy.go 0.00% <0.00%> (ø)
...installer/pkg/components/ws-manager/rolebinding.go 0.00% <0.00%> (ø)
...l/installer/pkg/components/ws-manager/tlssecret.go 0.00% <0.00%> (ø)
install/installer/pkg/common/render.go 0.00% <0.00%> (ø)
install/installer/pkg/common/ca.go 0.00% <0.00%> (ø)
...nstall/installer/pkg/components/ws-manager/role.go 0.00% <0.00%> (ø)
.../installer/pkg/components/ws-manager/deployment.go 0.00% <0.00%> (ø)
install/installer/pkg/common/storage.go 0.00% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db835ef...a3232f0. Read the comment docs.

@mrsimonemms
Copy link
Contributor

mrsimonemms commented Feb 23, 2022

I think the domain is something that is must always be set - because no one will be running the app from gitpod.example.com, I think we have to enforce this property

As this is an update to the config, can you also run go run . init > example-config.yaml and commit any changes to the repo please?

@Pothulapati Pothulapati added this to the release/2022.02 milestone Feb 24, 2022
@Pothulapati Pothulapati force-pushed the tar/installer-default branch from 386bd24 to f6261b3 Compare February 24, 2022 11:52
Fixes #8267

Currently, Users run `init` command first, update it and
pass full config to `render` to generate Kubernetes manifests. The
passage of `-c` is a requirement here, and users can't skip it.

This PR makes the passage of `config` to `render` optional, and
flexible. This means
- Users can skip `-c` entirely, in which case we use the default
  values on the default config version for that installer binary
- Users can selectively override fields and *thus no need to pass full
  config* all the time. This means `-c` flag acts as a flag through
  which they can override the default fields.

For the second case, When a user explicitely sets the `apiVersion`
field in the passed config, we use the default values for that
version. If no `apiVersion` is passed, we override the passed config
onto the default values on the default config version for that
installer binary.

After this change, For users This means that they only store and use
the config that has their changes only (on the default config), and
not the entire config.

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati force-pushed the tar/installer-default branch from f6261b3 to a3232f0 Compare February 24, 2022 12:01
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm! Thanks Tarun! 🚀

@roboquat roboquat merged commit 63d893c into main Feb 24, 2022
@roboquat roboquat deleted the tar/installer-default branch February 24, 2022 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/M team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[installer] Make all config values with sensible defaults optional
4 participants