Skip to content

[ws-manager-mk2] Protect tokens #16806

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

Merged
merged 16 commits into from
Mar 15, 2023
Merged

[ws-manager-mk2] Protect tokens #16806

merged 16 commits into from
Mar 15, 2023

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Mar 10, 2023

Description

Store sensitive workspace data (tokens) in short lived Kubernetes secrets. When a workspace start is requested we create a secret (named [workspaceid]-tokens) that contains the tokens extracted from the content initializer. The tokens will be removed from the content initializer.

The secret itself will live in a separate namespace so that we can narrow down the scope of permissions we need to give to ws-daemon so that it can access the secrets. When the content init happens in ws-daemon the secrets will be injected into the initializer again and the secret will be deleted once the workspace reaches the running phase but latest when it reaches the stopped phase.

Related Issue(s)

n.a.

How to test

  • Open workspace for a private repository

Release Notes

None

Build Options:

  • /werft with-werft
    Run the build with werft instead of GHA
  • leeway-no-cache
    leeway-target=components:all
  • /werft no-test
    Run Leeway with --dont-test
Publish Options
  • /werft publish-to-npm
  • /werft publish-to-jb-marketplace
Installer Options
  • with-ee-license
  • with-dedicated-emulation
  • [] with-ws-manager-mk2
  • workspace-feature-flags
    Add desired feature flags to the end of the line above, space separated

Preview Environment Options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • [] /werft with-preview
  • [] /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@Furisto Furisto added team: workspace Issue belongs to the Workspace team feature: ws-manager-mk2 labels Mar 10, 2023
@Furisto Furisto requested a review from a team March 10, 2023 15:10
@Furisto Furisto self-assigned this Mar 10, 2023
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-fo-protected-tokens-4.1 because the annotations in the pull request description changed
(with .werft/ from main)

Copy link
Member

@WVerlaek WVerlaek left a comment

Choose a reason for hiding this comment

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

lgtm, tested in preview env. Added hold for 2 comments

@@ -52,7 +52,7 @@ MATCHES="$(grep -c -- --- k8s.yaml)"
# get the read number of K8s manifest docs
# K8s object names and kinds are duplicated in a config map to faciliate deletion
# subtract one (the config map) and then divide by 2 to get the actual # of docs we'll loop through
DOCS="$((((MATCHES - 1) / 2) + 1))"
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Had an out of bounds error without it.

Comment on lines 362 to 365
err = r.deleteSecret(ctx, fmt.Sprintf("%s-%s", ws.Name, "tokens"), r.Config.SecretsNamespace)
if err != nil {
log.Error(err, "could not delete token secret", "workspace", ws.Name)
}
Copy link
Member

Choose a reason for hiding this comment

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

still think we should return an error when we fail to delete the secret, and not remove the finalizer if there's an error, so we try again until all secrets have been cleaned up

Copy link
Member Author

Choose a reason for hiding this comment

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

We retry the deletion of the secret now and will not delete the workspace CR until the deletion of the secret suceeds.

@roboquat roboquat added size/XXL and removed size/XL labels Mar 14, 2023
@Furisto Furisto force-pushed the fo/protected-tokens-4 branch from 529036d to ef697aa Compare March 14, 2023 14:30
@Furisto
Copy link
Member Author

Furisto commented Mar 14, 2023

/werft run with-preview

👍 started the job as gitpod-build-fo-protected-tokens-4.4
(with .werft/ from main)

@Furisto
Copy link
Member Author

Furisto commented Mar 14, 2023

/werft run with-preview recreate-vm

👍 started the job as gitpod-build-fo-protected-tokens-4.5
(with .werft/ from main)

@Furisto
Copy link
Member Author

Furisto commented Mar 15, 2023

/werft run with-preview recreate-vm

👍 started the job as gitpod-build-fo-protected-tokens-4.7
(with .werft/ from main)

@Furisto Furisto force-pushed the fo/protected-tokens-4 branch from 0fe78e2 to 8a5d725 Compare March 15, 2023 11:01
@roboquat roboquat merged commit c800175 into main Mar 15, 2023
@roboquat roboquat deleted the fo/protected-tokens-4 branch March 15, 2023 11:57
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production feature: ws-manager-mk2 release-note-none size/XXL team: workspace Issue belongs to the Workspace team
Projects
No open projects
Status: In Validation
Development

Successfully merging this pull request may close these issues.

3 participants