Skip to content

Enable protected secrets by default #14083

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 24, 2022
Merged

Enable protected secrets by default #14083

merged 3 commits into from
Oct 24, 2022

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Oct 21, 2022 •
edited by sagor999
Loading

Description

Enable protected secrets by default. This is the part for the workspace side. Once this change is deployed we can also change it on web app side.

Related Issue(s)

Related to #13634

How to test

  • Open workspace in preview environment and check that protected secrets are still used

Release Notes

None

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=workspace
    Valid options are all, workspace, webapp, ide

@Furisto Furisto added the team: workspace Issue belongs to the Workspace team label Oct 21, 2022
@Furisto Furisto requested a review from a team October 21, 2022 11:51
@Furisto Furisto self-assigned this Oct 21, 2022
@werft-gitpod-dev-com
Copy link

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

@werft-gitpod-dev-com
Copy link

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

@sagor999
Copy link
Contributor

/hold
would like to run this through integration tests just to be safe

sagor999
sagor999 approved these changes Oct 21, 2022
View reviewed changes
Copy link
Contributor

@sagor999 sagor999 left a comment

Choose a reason for hiding this comment

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

changes LGTM, but placed hold to run through integration tests

@werft-gitpod-dev-com
Copy link

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

@werft-gitpod-dev-com
Copy link

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

@sagor999
Copy link
Contributor

sagor999 commented Oct 21, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-protected-secrets.9
(with .werft/ from main)

@sagor999
Copy link
Contributor

@Furisto it seems like image builder test repeatedly fails. Could be related to your changes? 🤔

@Furisto
Copy link
Member Author

Furisto commented Oct 24, 2022

@sagor999 Good catch! The image builder did not set the feature flag, so image builds were still using the old method (but imagebuilder set a secret itself, so we are clear here). With the new default behavior the secret set by imagebuilder was overwritten by protected secrets. Fixed with 54c2aa2

@Furisto
[ws-manager] Ensure values are not overwritten
f457b67 
If a variable is already sourced from a reference do not overwrite it.
jenting
jenting approved these changes Oct 24, 2022
View reviewed changes
Copy link
Contributor

@jenting jenting left a comment

Choose a reason for hiding this comment

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

LGTM

@Furisto
Copy link
Member Author

Furisto commented Oct 24, 2022

/unhold

@roboquat roboquat merged commit 9fd3e3b into main Oct 24, 2022
@roboquat roboquat deleted the protected-secrets branch October 24, 2022 11:56
@kylos101
Copy link
Contributor

@Furisto I added this to our project, as it is being deployed, but doesn't resolve the related issue. This way, we can validate the PR.

@Furisto Furisto mentioned this pull request Oct 31, 2022
Merged
4 tasks
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagor999 sagor999 sagor999 approved these changes

@jenting jenting jenting approved these changes

Assignees

@Furisto Furisto

Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note-none size/XL team: workspace Issue belongs to the Workspace team
Projects
No open projects
🌌 Workspace Team
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Furisto @sagor999 @kylos101 @jenting @roboquat