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: make OpenVSX URL configurable in the openvsx proxy #8266

Merged
merged 1 commit into from
Feb 22, 2022

Conversation

Pothulapati
Copy link
Contributor

Description

This PR adds a new OpenVSX object into the top-level Config field
to support configurations of the openvsx-proxy component. Currently,
Only URL field is present. This is needed to support air-gap
instlalations where people are expected to host their own open-vsx.

This config is top-level and not under the workspace or IDE as this
configures the proxy, but not the IDE.

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

Related Issue(s)

Fixes #8224

How to test

Set openVSX.url to something else and do a ./installer render -c ./config.yaml | grep <something>

Release Notes

Make Open VSX upstream URL configurable in the installer for air-gap installations

Documentation

@Pothulapati Pothulapati requested review from a team February 17, 2022 08:23
@github-actions github-actions bot added team: IDE team: delivery Issue belongs to the self-hosted team labels Feb 17, 2022
Copy link
Contributor

@mrsimonemms mrsimonemms left a comment

Choose a reason for hiding this comment

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

What happens if you run it with a config that doesn't have the openVSX object created? I'm just thinking about user's who create a new installation without updating their config

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #8266 (3813f39) into main (ddc80c1) will decrease coverage by 4.72%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             main   #8266      +/-   ##
=========================================
- 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
...installer/pkg/components/ws-manager/rolebinding.go 0.00% <0.00%> (ø)
...staller/pkg/components/ws-manager/networkpolicy.go 0.00% <0.00%> (ø)
install/installer/pkg/common/storage.go 0.00% <0.00%> (ø)
...components/ws-manager/unpriviledged-rolebinding.go 0.00% <0.00%> (ø)
install/installer/pkg/common/objects.go 0.00% <0.00%> (ø)
...nstall/installer/pkg/components/ws-manager/role.go 0.00% <0.00%> (ø)
...l/installer/pkg/components/ws-manager/tlssecret.go 0.00% <0.00%> (ø)
.../installer/pkg/components/ws-manager/deployment.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 ddc80c1...3813f39. Read the comment docs.

@corneliusludmann
Copy link
Contributor

What happens if you run it with a config that doesn't have the openVSX object created? I'm just thinking about user's who create a new installation without updating their config

In my opinion, we should make all config values that have sensible default values optional in the config file. E.g., it should be totally fine to use a config like this:

apiVersion: v1
domain: gitpod.my-domain.example

That would also make generating config values for e.g. replicated easier.

I'll create an issue for this and we can discuss this there. When we agree on this, we can leave this pull request as is and handle your concern with a more general solution of using sensible default values of missing config properties.

@corneliusludmann
Copy link
Contributor

Looks good to me. @Pothulapati Would you mind squashing the commits into one commit?

(By the way: I usually amend changes to the previous commit and force push them directly to the PR branch so that the Git history is always clean and ready to merge. You still see the history of changes in the PR on the Gitpod PR page and everyone can see diffs for each step even when you force push.)

@Pothulapati Pothulapati force-pushed the tar/installer-openvsx branch from a7757ec to d19ae75 Compare February 19, 2022 13:09
Fixes #8224

This PR adds a new `OpenVSX` object into the top-level `Config` field
to support configurations of the `openvsx-proxy` component. Currently,
Only `URL` field is present. This is needed to support air-gap
instlalations where people are expected to host their own open-vsx.

This config is top-level and not under the workspace or IDE as this
configures the proxy, but not the IDE.

The URL is validated by using the validate go tags.

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

Took the liberty and resolved the conflicts after we merged #8345. Hope, you're fine with that, @Pothulapati. 🙏

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

LGTM

@roboquat roboquat merged commit d9b9f30 into main Feb 22, 2022
@roboquat roboquat deleted the tar/installer-openvsx branch February 22, 2022 13:02
@Pothulapati
Copy link
Contributor Author

Thanks @corneliusludmann for doing that!

@roboquat roboquat added the deployed: IDE IDE change is running in production label Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production release-note size/S team: delivery Issue belongs to the self-hosted team team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[installer] Make OpenVSX URL configurable
6 participants