Skip to content

Support custom CA certificates in Helm #6590

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
wants to merge 1 commit into from
Closed

Support custom CA certificates in Helm #6590

wants to merge 1 commit into from

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Nov 6, 2021

Description

Adds support to Helm charts for custom CA certificates.
Origin PR is #2984 Thanks to @jgallucci32

  1. split ca bundle to (full-ca-bundle and extra-ca-bundle)
    A. full-ca-bundle used for normal components, like ws-daemon, server etc.. it contain full chain ca bundle
    B. extra-ca-bundle is only contain custom ca bundle, which is used for workspace, use supervisor to inject exist ca-certificates.crt
  2. implement supervisor inject

Related Issue(s)

Fixes #2615

How to test

Release Notes

NONE

Documentation

@roboquat
Copy link
Contributor

roboquat commented Nov 6, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign corneliusludmann, mrsimonemms after the PR has been reviewed.
You can assign the PR to them by writing /assign @corneliusludmann @mrsimonemms in a comment when ready.

Associated issue: #2984

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat added team: IDE team: workspace Issue belongs to the Workspace team size/L and removed size/M labels Nov 6, 2021
@aledbf
Copy link
Member

aledbf commented Nov 6, 2021

/hold

@aledbf
Copy link
Member

aledbf commented Nov 6, 2021

@iQQBot thank you for the PR, but unfortunately, we are in the process to deprecate and replace the helm chart with a go installer https://github.com/gitpod-io/gitpod/tree/main/installer

@iQQBot
Copy link
Contributor Author

iQQBot commented Nov 6, 2021

I see that, this is a part for #6563
a self-sign CA can make this easily

@iQQBot
Copy link
Contributor Author

iQQBot commented Nov 6, 2021

No stable version of installer has been released yet, and helm seems to have better control if it is used as a development debug stage @aledbf

@aledbf
Copy link
Member

aledbf commented Nov 6, 2021

@iQQBot I understand that but merging anything to the chart would be a mistake at this stage.

@iQQBot
Copy link
Contributor Author

iQQBot commented Nov 6, 2021

OK, Another idea, I can make this a patch file, so that new contributors can apply this patch on their own dev environment and get the quick dev environment, is this OK? @aledbf

@iQQBot iQQBot marked this pull request as ready for review November 7, 2021 15:48
@iQQBot
Copy link
Contributor Author

iQQBot commented Nov 17, 2021

/werft run

👍 started the job as gitpod-build-pd-custom-ca-fork.0

@iQQBot
Copy link
Contributor Author

iQQBot commented Nov 17, 2021

just want to have a online image ref to test

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #6590 (31b4c42) into main (c009480) will increase coverage by 19.19%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #6590       +/-   ##
===========================================
+ Coverage   19.04%   38.24%   +19.19%     
===========================================
  Files           2       32       +30     
  Lines         168     8067     +7899     
===========================================
+ Hits           32     3085     +3053     
- Misses        134     4736     +4602     
- Partials        2      246      +244     
Flag Coverage Δ
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-supervisor-app 37.05% <0.00%> (?)
components-ws-manager-app 39.80% <0.00%> (?)

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

Impacted Files Coverage Δ
components/supervisor/pkg/supervisor/supervisor.go 5.98% <0.00%> (ø)
components/ws-manager/pkg/manager/create.go 78.97% <0.00%> (ø)
components/local-app/pkg/auth/auth.go
components/local-app/pkg/auth/pkce.go
components/supervisor/pkg/supervisor/config.go 4.23% <0.00%> (ø)
components/supervisor/pkg/ports/exposed-ports.go 0.00% <0.00%> (ø)
components/ws-manager/pkg/manager/metrics.go 11.11% <0.00%> (ø)
components/ws-manager/pkg/manager/imagespec.go 0.00% <0.00%> (ø)
components/supervisor/pkg/terminal/service.go 32.38% <0.00%> (ø)
components/supervisor/pkg/supervisor/tasks.go 58.56% <0.00%> (ø)
... and 26 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 5dafe5c...31b4c42. Read the comment docs.

@csweichel
Copy link
Contributor

I reckon you could achieve the same effect without us adding an additional feature. By mounting a secret using the workspace template, you would just have the CA cert available in the workspace.
That should work with the Helm chart today, as well as with the installer.

@jgallucci32
Copy link
Contributor

@csweichel The issue is multiple components need to have custom volume and volumeMount parameters. The ws-daemon for example needs it but does not support custom mounts.

The approach Bitnami took is that every component has the ability to have a custom mount, and there is a global way to do that for all pods. If this was implemented that way it's possible. The only caveat is the server pod needs to have an env var set for node.js as volume-only mount does not fix the SSL issue.

@stale
Copy link

stale bot commented Nov 29, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Nov 29, 2021
@iQQBot iQQBot closed this Dec 3, 2021
@ThatDeveloper
Copy link

@aledbf Is there an update on the installer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution do-not-merge/hold meta: stale This issue/PR is stale and will be closed soon release-note-none size/L team: IDE team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Self-Hosted] Support for own CA certificates
7 participants