Implement Project-level environment variables #7295

jankeromnes · 2021-12-17T16:41:45Z

Description

Introduce Project-level environment variables:

[dashboard] Unify project-getting code into a ProjectContext helper
Implement a Variables page in Project Settings
Implement Project-level variables in server and gitpod-db
Expose Project-level variables in prebuilds
Expose Project-level variables in workspaces...
...but also hiding certain variables in workspaces
...and censoring them in logs too (image builds, prebuild logs, etc. in order to mitigate accidental leaks of secrets) (EDIT: follow-up)
~~Drive-by: Export server's censor function via gitpod-protocol (in order to use it in gitpod-db)~~ (EDIT: Commit removed, censor code copy/pasted and specialized)
~~Drive-by: Always use the actual function name (arguments.callee.name) in checkUser and checkAndBlockUser (instead of re-typing/copy-pasting the function name and sometimes making mistakes)~~ (EDIT: Commit removed again, arguments.callee is deprecated 🙄)

Related Issue(s)

How to test

Log in to https://jx-project-variables.staging.gitpod-dev.com/projects
In your User Settings, create an environment variable called USER with value user and pattern */*
Next, create a project
Navigate to Project Settings > Variables, and create a new variable (e.g. PROJECT with value project, visible in both Prebuilds and Workspaces)
In the Project repository, create a new branch containing a .gitpod.yml file that looks like this:

tasks:
  - init: echo "USER: $USER, PROJECT: $PROJECT"
    command: echo "USER: $USER, PROJECT: $PROJECT"

Verify that, in the triggered prebuild (from the new branch created in 5.), only the PROJECT variable is defined (USER should still be undefined in the prebuild)
When you open a workspace for that new branch, both USER and PROJECT should be defined (not in the printed Prebuild logs, but in the interactive command & also in live Terminals)

Open question: If you open a workspace for the new branch as a new user who doesn't have access to the Project (i.e. the repository is public, and you open a workspace for it, which gives you a prebuild, but you have no access to the Gitpod Project) -- should you be able to see the Project variables? (Prebuild/non-prebuild consistency says "yes", secrets say "no", maybe this should be configurable for each variable, see #4456 (comment)) (EDIT: Visibility is now configurable)

Release Notes

Introduce Project-level environment variables

Documentation

/werft with-clean-slate-deployment

/uncc

jankeromnes · 2021-12-17T16:42:00Z

/werft run with-clean-slate-deployment

👍 started the job as gitpod-build-jx-project-variables.3

jankeromnes · 2021-12-30T13:58:26Z

/werft run

👍 started the job as gitpod-build-jx-project-variables.10

codecov · 2022-01-04T17:05:24Z

Codecov Report

Merging #7295 (1f6b435) into main (2773b35) will increase coverage by 1.68%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            main    #7295      +/-   ##
=========================================
+ Coverage   8.69%   10.38%   +1.68%     
=========================================
  Files         33       18      -15     
  Lines       2323      992    -1331     
=========================================
- Hits         202      103      -99     
+ Misses      2117      888    -1229     
+ Partials       4        1       -3

Flag	Coverage Δ
components-gitpod-cli-app	`10.38% <ø> (ø)`
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	`?`
installer-raw-app	`?`

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

Impacted Files	Coverage Δ
installer/pkg/common/render.go
components/local-app/pkg/auth/pkce.go
installer/pkg/components/ws-manager/rolebinding.go
installer/pkg/common/storage.go
installer/pkg/common/display.go
installer/pkg/common/objects.go
installer/pkg/components/ws-manager/configmap.go
components/local-app/pkg/auth/auth.go
installer/pkg/components/ws-manager/tlssecret.go
installer/pkg/common/common.go
... 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 2773b35...1f6b435. Read the comment docs.

jankeromnes · 2022-01-05T15:51:12Z

components/dashboard/src/components/Modal.tsx

@@ -41,7 +41,8 @@ export default function Modal(props: {
        return () => {
            window.removeEventListener('keydown', handler);
        };
-    }, []); // Empty array ensures that effect is only run on mount and unmount
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [props.onClose, props.onEnter]);


Note: This was required because apparently state variables (like name: string and value: string in AddVariableModal) get "captured" in the onClose / onEnter lambdas, thus causing these functions to always use initial state values (i.e. empty string) instead of updated state values (i.e. whatever name/value a user typed).

The solution was to make this effect depend on onClose and onEnter, such that, if these functions change (they do change every time a captured state variable changes), the handler gets re-registered properly.

jankeromnes · 2022-01-05T15:57:08Z

Okay, calling this ready for review 🎉

@geropl and/or @AlexTugarev, could you please take a look at the code changes?

@gtsiolis could you please take a look at the new Project Variables page UX?

(Happy to discuss / answer any question sync or async 🙏)

gtsiolis · 2022-01-14T15:00:20Z

Taking another look at the UX changes here! 👀

geropl · 2022-01-14T15:08:51Z

@jankeromnes Thx for putting in all this effort. Love the explicitness of the new messages! ❤️

/lgtm
/hold

So @gtsiolis can review as well

roboquat · 2022-01-14T15:08:55Z

LGTM label has been added.

Git tree hash: 34d504ab9541adf0333b4aec043af2382ba2786d

roboquat · 2022-01-14T15:08:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: geropl, gtsiolis

Associated issue: #4456

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~components/dashboard/OWNERS~~ [geropl,gtsiolis]
~~components/gitpod-db/OWNERS~~ [geropl,gtsiolis]
~~components/gitpod-protocol/OWNERS~~ [geropl,gtsiolis]
~~components/server/OWNERS~~ [geropl,gtsiolis]

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

gtsiolis

This took slightly longer than expected. 💭

@jankeromnes I've left some final comments below regarding UX changes. Feel free to create follow up issues for each one of them if needed. 🏀

gtsiolis · 2022-01-14T15:26:50Z

components/dashboard/src/projects/ProjectVariables.tsx

+    }
+
+    return <Modal visible={true} onClose={props.onClose} onEnter={() => { addVariable(); return false; }}>
+        <h3 className="mb-4">Add Variable</h3>


suggestion: Even if we don't have the update operations here, renaming this as described in /7295/files#r781502096 to match the voice and tone used in user-specific environment variables would be nice!

Suggested change

<h3 className="mb-4">Add Variable</h3>

<h3 className="mb-4">New Variable</h3>

components/dashboard/src/projects/ProjectVariables.tsx

gtsiolis · 2022-01-14T17:09:45Z

components/dashboard/src/projects/ProjectVariables.tsx

+    return <Modal visible={true} onClose={props.onClose} onEnter={() => { addVariable(); return false; }}>
+        <h3 className="mb-4">Add Variable</h3>
+        <div className="border-t border-b border-gray-200 dark:border-gray-800 -mx-6 px-6 py-4 flex flex-col">
+            <AlertBox><strong>Project variables might be accessible by anyone with read access to your repository.</strong><br/>Secret values can be exposed if they are printed in the logs, persisted to the file system, or made visible in workspaces.</AlertBox>


issue(non-blocking): I've been trying to propose some new colors for the alert component as seen in the designs in #7295 (comment) but let's not increase the scope of this PR with this. Added #7613 regarding alert component colors, etc.

Many thanks for filing this follow-up!

More than happy to implement the adjustments -- really like the new design. Please simply post a spec to the issue + remove needs design label + schedule, as usual 😊 (you could even directly assign me to it 🚀 I'm looking forward to have this fixed)

Je vous remercie, @jankeromnes!

gtsiolis · 2022-01-14T17:23:47Z

components/dashboard/src/projects/ProjectVariables.tsx

+                            <ItemField className="flex justify-end">
+                                <ItemFieldContextMenu menuEntries={[
+                                    {
+                                        title: 'Delete',


thought: Still not entirely sure if not being able to edit or update the variable is better but I understand the security concerns. From what I've seen there are two approaches:

🅰️ Systems do not allow editting such values but only allowing to delete them.
🅱️ Systems allow users with specific permissions to read, update and delete such values.

Since we do have two different user roles (OWNER, MEMBER), we could hide the variable menu or disable actions on variable for MEMBERS.

suggestion: I'd be in favor of providing more user control and allowing read and update permissions, however, I agree @jankeromnes, let's go with a boring (simple) solution to disable editing the variables for now and come back to this if users find this idea useful.

gtsiolis · 2022-01-14T17:25:48Z

components/dashboard/src/projects/ProjectVariables.tsx

+                            <ItemField className="flex justify-end">
+                                <ItemFieldContextMenu menuEntries={[
+                                    {
+                                        title: 'Delete',


issue: We should probably ask for user confirmation before deleting, as we do with user-specific environment variables.

Thanks for noticing! I personally think we don't need to be overly cautious here (i.e. clicking once on a red Delete button is likely already sufficient intent, and this action doesn't have surprising chain reactions as consequence).

Leaving this as an optional follow-up if we really feel we need the extra user confirmation.

components/dashboard/src/projects/ProjectVariables.tsx

gtsiolis · 2022-01-14T18:07:38Z

components/dashboard/src/projects/ProjectVariables.tsx

+    return <Modal visible={true} onClose={props.onClose} onEnter={() => { addVariable(); return false; }}>
+        <h3 className="mb-4">Add Variable</h3>
+        <div className="border-t border-b border-gray-200 dark:border-gray-800 -mx-6 px-6 py-4 flex flex-col">
+            <AlertBox><strong>Project variables might be accessible by anyone with read access to your repository.</strong><br/>Secret values can be exposed if they are printed in the logs, persisted to the file system, or made visible in workspaces.</AlertBox>


thought: It took me a while to wrap my head around the security cases here. 😇

Some improvements could include:

Making the alert title brief and short in one line.

Avoiding complex state handling for users by default.

suggestion: For the second point I still think that having a sensible default where variables can be exposed everywhere by default instead of requiring users to think everytime could be better. Here's how the copy could look like although I could be wrong on terminal access only:

BEFORE AFTER

Let me know what you think!

The message at the top could be a little clearer about how prebuild variables could leak into workspaces.

The current default to Hide in workspaces is good IMO because it errs on the side of less exposure.

I think we can leave out the extra instruction for what it means to uncheck the checkbox.

Suggest the following:

Warning
Even with Hide in Workspaces checked, a project-specific environment variable may become exposed to anyone who can start a workspace e.g. if logged or persisted to the file system.

Name
[ _ _ _ ]

Value
[ _ _ _ ]

☑️ Hide in Workspaces
Project-specific environment variables are always visible in prebuilds.

(after unchecking)

NOTE
This environment variable will be visible to anyone who starts a Gitpod workspace on this repository.

Thanks for the excellent feedback here! 🙏

Phew. After splitting a few more hairs on this, I think I've reached an optimum consensus solution on the wording:

Going to ship this 🚢 (but happy to further split hairs in follow-up PRs 😅)

gtsiolis

Three last comments. Promise! 🙂

components/dashboard/src/projects/ProjectVariables.tsx

jldec · 2022-01-14T20:13:52Z

Awesome work @jankeromnes - thanks for going the extra mile with hiding in workspaces 🙏

roboquat · 2022-01-17T10:01:38Z

New changes are detected. LGTM label has been removed.

…ge component

…Enter} are changed

… out of Workspaces

Co-authored-by: George Tsiolis <[email protected]> Co-authored-by: jldec <[email protected]>

jankeromnes · 2022-01-17T18:17:06Z

Just waiting for a last build to triple-check everything, then shipping this. 🚢

(Manually carrying over pre-nit / pre-rebase LGTM, keeping hold for now.)

jankeromnes · 2022-01-17T18:36:30Z

Everything seems to still work as expected! (Tested the variable management UI, but couldn't test prebuilds/workspaces because they're currently broken in core-dev (internal)).

Thus, releasing the breaks! 🚀 ✨

/unhold

shaal · 2022-01-17T22:19:41Z

Thank you for working on this!
When will we be able to test Project-level environment variables in gitpod.io ?

jankeromnes · 2022-01-18T07:22:20Z

@shaal Sure, my pleasure! This should go live in about an hour with the regular twice-a-week deployment. 📅📅

EDIT: Deployment got delayed a bit due to technical issues. Should still go live ~~today~~ tomorrow morning though.

jankeromnes added the approved label Dec 17, 2021

roboquat added do-not-merge/work-in-progress release-note team: webapp Issue belongs to the WebApp team size/XXL labels Dec 17, 2021

jankeromnes force-pushed the jx/project-variables branch from 1d069fd to 015ca6f Compare December 17, 2021 17:15

roboquat added size/XL and removed size/XXL labels Dec 17, 2021

jankeromnes force-pushed the jx/project-variables branch 3 times, most recently from 4d73d88 to a056a54 Compare December 24, 2021 17:34

jankeromnes force-pushed the jx/project-variables branch 2 times, most recently from 600a708 to 8585c0d Compare December 30, 2021 13:40

jankeromnes force-pushed the jx/project-variables branch 6 times, most recently from 2ceb652 to 641a6bc Compare January 4, 2022 16:50

roboquat added size/XXL and removed size/XL labels Jan 4, 2022

jankeromnes force-pushed the jx/project-variables branch 2 times, most recently from b5e00eb to 3801563 Compare January 5, 2022 14:30

jankeromnes commented Jan 5, 2022

View reviewed changes

jankeromnes marked this pull request as ready for review January 5, 2022 15:57

roboquat removed the do-not-merge/work-in-progress label Jan 5, 2022

roboquat assigned geropl Jan 14, 2022

roboquat added the lgtm label Jan 14, 2022

gtsiolis mentioned this pull request Jan 14, 2022

Improve warning alert component design #7613

Closed

gtsiolis reviewed Jan 14, 2022

View reviewed changes

components/dashboard/src/projects/ProjectVariables.tsx Outdated Show resolved Hide resolved

components/dashboard/src/projects/ProjectVariables.tsx Outdated Show resolved Hide resolved

components/dashboard/src/projects/ProjectVariables.tsx Show resolved Hide resolved

jankeromnes force-pushed the jx/project-variables branch from d466a9f to 40b579a Compare January 17, 2022 10:01

roboquat removed the lgtm label Jan 17, 2022

jankeromnes force-pushed the jx/project-variables branch from 98155d0 to 5bebd98 Compare January 17, 2022 16:39

jankeromnes and others added 5 commits January 17, 2022 18:14

[dashboard] Implement a ProjectContext and refactor ProjectSettingsPa…

feb1bcf

…ge component

[dashboard] Update Modal keydown event handler when props.{onClose,on…

928dbe1

…Enter} are changed

[db][server][dashboard] Implement Project-level environment variables

ebb1ad4

[db][server][dashboard] Allow censoring Project environment variables…

1e870bc

… out of Workspaces

Address code review feedback

1f6b435

Co-authored-by: George Tsiolis <[email protected]> Co-authored-by: jldec <[email protected]>

jankeromnes force-pushed the jx/project-variables branch from 5bebd98 to 1f6b435 Compare January 17, 2022 18:14

jankeromnes added the lgtm label Jan 17, 2022

roboquat removed the do-not-merge/hold label Jan 17, 2022

roboquat merged commit bb80946 into main Jan 17, 2022

roboquat deleted the jx/project-variables branch January 17, 2022 18:37

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jan 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Project-level environment variables #7295

Implement Project-level environment variables #7295

jankeromnes commented Dec 17, 2021 •

edited

Loading

jankeromnes commented Dec 17, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Dec 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading

jankeromnes Jan 5, 2022

jankeromnes commented Jan 5, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Jan 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

geropl commented Jan 14, 2022

roboquat commented Jan 14, 2022

roboquat commented Jan 14, 2022

gtsiolis left a comment

gtsiolis Jan 14, 2022

gtsiolis Jan 14, 2022

jankeromnes Jan 17, 2022

gtsiolis Jan 17, 2022

gtsiolis Jan 14, 2022

gtsiolis Jan 14, 2022

jankeromnes Jan 17, 2022

gtsiolis Jan 14, 2022 •

edited

Loading

jldec Jan 14, 2022

jankeromnes Jan 17, 2022

gtsiolis left a comment

jldec commented Jan 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

roboquat commented Jan 17, 2022

jankeromnes commented Jan 17, 2022 •

edited

Loading

jankeromnes commented Jan 17, 2022 •

edited by werft-gitpod-dev-com bot

Loading

shaal commented Jan 17, 2022

jankeromnes commented Jan 18, 2022 •

edited by gtsiolis

Loading

	<h3 className="mb-4">Add Variable</h3>
	<h3 className="mb-4">New Variable</h3>

Implement Project-level environment variables #7295

Implement Project-level environment variables #7295

Conversation

jankeromnes commented Dec 17, 2021 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

jankeromnes commented Dec 17, 2021 • edited by werft-gitpod-dev-com bot Loading

jankeromnes commented Dec 30, 2021 • edited by werft-gitpod-dev-com bot Loading

codecov bot commented Jan 4, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

jankeromnes commented Jan 5, 2022 • edited by werft-gitpod-dev-com bot Loading

gtsiolis commented Jan 14, 2022 • edited by werft-gitpod-dev-com bot Loading

geropl commented Jan 14, 2022

roboquat commented Jan 14, 2022

roboquat commented Jan 14, 2022

gtsiolis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis Jan 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gtsiolis left a comment

Choose a reason for hiding this comment

jldec commented Jan 14, 2022 • edited by werft-gitpod-dev-com bot Loading

roboquat commented Jan 17, 2022

jankeromnes commented Jan 17, 2022 • edited Loading

jankeromnes commented Jan 17, 2022 • edited by werft-gitpod-dev-com bot Loading

shaal commented Jan 17, 2022

jankeromnes commented Jan 18, 2022 • edited by gtsiolis Loading

jankeromnes commented Dec 17, 2021 •

edited

Loading

jankeromnes commented Dec 17, 2021 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Dec 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading

jankeromnes commented Jan 5, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis commented Jan 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis Jan 14, 2022 •

edited

Loading

jldec commented Jan 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Jan 17, 2022 •

edited

Loading

jankeromnes commented Jan 17, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jankeromnes commented Jan 18, 2022 •

edited by gtsiolis

Loading