Allow user env vars for GitLab subgroups #17831

geropl · 2023-06-05T15:01:21Z

Description

This PR allows to specify environment variable patterns for subgroups in GitLab. This is a long-standing issue that we always avoided to fix, but is now requested/assumed to be working by multiple customers.

We had multiple discussions in the past about deprecating this feature in the first place. However never did so, because it's a very convenient feature, that even if we had better secret support, adds value for users.

For examples of what's possible see the tests here and here.

The docs don't need to be changed, but could use a bit more explanation (see here).

Related Issue(s)

Fixes WEB-353

How to test

Resolution

add these user-level env vars to your user config: link
open two workspaces:
- project 1
- project 2
note that you only see the expected env vars only ✔️

Permissions 1

in one of the workspacex run gp env -u USER_GROUP_TEST, and see the "cannot delete" error ✔️

Permissions 2

in one of the workspaces, run:

gp env TEST_NEW=abc
gp env
gp env -u TEST_NEW
gp env

and see that it does what you'd expect without errors ✔️

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

🏷️ Name - gpl-env-var-1
🔗 URL - gpl-env-var-1.preview.gitpod-dev.com/workspaces.
📚 Documentation - See our internal documentation for information on how to interact with your preview environment.

Build Options

Build

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer

analytics=segment
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

/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
with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

/hold

components/gitpod-protocol/src/protocol.ts

svenefftinge · 2023-06-14T09:37:49Z

components/gitpod-protocol/src/protocol.ts

 export namespace UserEnvVar {
+    export const DELIMITER = "/";
+    export const WILDCARD_ASTERISK = "*";
+    const WILDCARD_SHARP = "#"; // TODO(gpl) Where does this come from? Bc we have/had patterns as part of URLs somewhere, maybe...?


Where does this come from?

there is a mention below https://github.dev/gitpod-io/gitpod/blob/2c1d74d479c10c68081d62c5a5f566ad1acd4e6d/components/gitpod-protocol/src/protocol.ts#L497-L498 not sure if this is still used and how/why a user would pass such a pattern through URL

It's from here: https://github.com/gitpod-io/gitpod/pull/17831/files#diff-957d9dd15b947e3969539ebd0dabcb8b40ce8b5564e5820cd9bb27d88be1dbc7L501

Could not find a mention in docs or tests anywhere. 🤷
I*'m inclined to remove it, but want to do that in a separate PR.

In that case I'd say let's remove and see who complains 😅

Will do, after this has landed 👍

components/gitpod-protocol/src/protocol.ts

svenefftinge · 2023-06-14T09:49:37Z

It seems a bit surprising that a single * would match any number of segments. I believe it would be better to allow users being explicit about that using a globstar **. Did you consider that?
We could even just use a glob library which would also make it easier to explain users how the pattern match. I believe it is compatible with the limited matching we have currently.

Edit: going with a lib might be a bit too much, but the supporting and requiring ** to match multiple segments would be good. translating the pattern to a regexp seems most straight forward.

components/gitpod-protocol/src/protocol.ts

geropl · 2023-06-14T10:09:31Z

We could even just use a glob library which would also make it easier to explain users how the pattern match.

Yes, considered that...

Edit: going with a lib might be a bit too much,

...but also felt it's a tad too much as well. 🙃

but the supporting and requiring ** to match multiple segments would be good.

Hm, so far I only thought about */* (required) vs * (does not work).
I like the idea, and will see how it pans out after lunch.

geropl · 2023-06-14T16:11:07Z

@svenefftinge I added the ** special case in 996620b and would love your feedback on the complexity.
Good news: it helped uncover a bug due to the additional tests 🙃

svenefftinge · 2023-06-14T17:10:43Z

I believe translating the pattern to a regexp (i.e. '**' becomes '.*' and '*' becomes '[^/]*') would be simpler.

geropl · 2023-06-15T12:09:41Z

@svenefftinge I did the regex version in 61f84cb but it a) does not remove that much code, and b) has other problems (escaping the pattern for other regex control characters .+, etc.).
As we have the other version here already and it's tested, I would like to go with that. Ok?

svenefftinge · 2023-06-15T12:22:09Z

components/gitpod-protocol/src/protocol.ts

+                    case WILDCARD_DOUBLE_ASTERISK:
+                        return ".*";
+                    default:
+                        return s;


asked chatGPT for replacing control patterns:

s.replace(/[-\/\\^$+?.()|[\]{}]/g, '\\$&')

didn't try 🤷

svenefftinge · 2023-06-15T12:25:08Z

components/gitpod-protocol/src/protocol.ts

+            return false;
+        }
+
+        const regexStr = joinRepositoryPattern(


the segments need to be joined using an escaped slash (\/)
Maybe there is less value in having a join function when gong with the regexp approach.
i.e. maybe just inlining .join('\/')?

svenefftinge · 2023-06-15T12:25:32Z

components/gitpod-protocol/src/protocol.ts

+                }
+            }),
+        );
+        const regex = new RegExp(regexStr);


new RegExp('^' + regexStr + '$')

svenefftinge · 2023-06-15T12:26:35Z

Cool! I find the code much simpler tbh as we no longer have the iteration over two arrays logic

geropl · 2023-06-15T14:51:45Z

/unhold

roboquat added do-not-merge/work-in-progress do-not-merge/hold size/XL labels Jun 5, 2023

[server] Move from owner/repo to potentially longer env var patterns

2c1d74d

geropl force-pushed the gpl/gitlab-env-var branch from 3286c4f to 2c1d74d Compare June 14, 2023 07:23

roboquat added size/L and removed size/XL labels Jun 14, 2023

geropl marked this pull request as ready for review June 14, 2023 09:13

geropl requested a review from a team June 14, 2023 09:13

geropl changed the title ~~[wip] Env var~~ Allow user env vars for GitLab subgroups Jun 14, 2023

github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 14, 2023

roboquat removed the do-not-merge/work-in-progress label Jun 14, 2023

svenefftinge reviewed Jun 14, 2023

View reviewed changes

components/gitpod-protocol/src/protocol.ts Outdated Show resolved Hide resolved

svenefftinge reviewed Jun 14, 2023

View reviewed changes

components/gitpod-protocol/src/protocol.ts Outdated Show resolved Hide resolved

svenefftinge reviewed Jun 14, 2023

View reviewed changes

components/gitpod-protocol/src/protocol.ts Outdated Show resolved Hide resolved

review suggestions

df46209

svenefftinge reviewed Jun 14, 2023

View reviewed changes

components/gitpod-protocol/src/protocol.ts Show resolved Hide resolved

fix and double-wildcard

996620b

roboquat added size/XL and removed size/L labels Jun 14, 2023

geropl force-pushed the gpl/gitlab-env-var branch from 4c8f2a1 to 61f84cb Compare June 15, 2023 10:09

svenefftinge reviewed Jun 15, 2023

View reviewed changes

geropl force-pushed the gpl/gitlab-env-var branch from 61f84cb to 996620b Compare June 15, 2023 12:37

Adjust TestsEnvVarService for double wild card

80b02f6

svenefftinge approved these changes Jun 15, 2023

View reviewed changes

roboquat removed the do-not-merge/hold label Jun 15, 2023

roboquat merged commit 1f42bd0 into main Jun 15, 2023

roboquat deleted the gpl/gitlab-env-var branch June 15, 2023 14:52

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 16, 2023

jeephacker mentioned this pull request Jun 21, 2023

Bug Report - Gitpod Not Fetching Variables from User Setting > Variables Menu #17975

Closed

This was referenced Jun 27, 2023

Bug / Feature Request: Add brace expansion, extended glob matching, and 'globstar' matching to user/project environment variable repo patterns #15744

Closed

Environment scope doesn't apply consistently for GitLab subgroups #8618

Closed

Allow user env vars for GitLab subgroups #17831

Allow user env vars for GitLab subgroups #17831

Uh oh!

Conversation

geropl commented Jun 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

How to test

Resolution

Permissions 1

Permissions 2

Documentation

Preview status

Build Options

Uh oh!

Uh oh!

Uh oh!

svenefftinge Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

svenefftinge Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

geropl Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

svenefftinge Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

geropl Jun 14, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svenefftinge commented Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

geropl commented Jun 14, 2023

Uh oh!

geropl commented Jun 14, 2023

Uh oh!

svenefftinge commented Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geropl commented Jun 15, 2023

Uh oh!

svenefftinge Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

svenefftinge Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svenefftinge Jun 15, 2023

Choose a reason for hiding this comment

Uh oh!

svenefftinge commented Jun 15, 2023

Uh oh!

geropl commented Jun 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

geropl commented Jun 5, 2023 •

edited

Loading

svenefftinge Jun 14, 2023 •

edited

Loading

svenefftinge commented Jun 14, 2023 •

edited

Loading

svenefftinge commented Jun 14, 2023 •

edited

Loading

svenefftinge Jun 15, 2023 •

edited

Loading