Skip to content
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

gp env to respect workspace env vars #16414

Merged
merged 1 commit into from
Feb 20, 2023
Merged

gp env to respect workspace env vars #16414

merged 1 commit into from
Feb 20, 2023

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Feb 15, 2023 •
edited
Loading

Description

gp env reflects workspace env vars, not only user env vars, i.e. as well coming from projects and context URL.

Related Issue(s)

fix #12208

How to test

  • Setup different project and user envs, for instance fork https://github.com/akosyakov/parcel-demo/tree/tes_gp_env
  • Test that gp env reflects visible project and context env vars.
  • Test that starting/restart prebuild picks up all project env vars, but not for user.
  • Test that starting/restart workspace picks up user, visible project and context env vars.

Release Notes

`gp env` reflects workspace env vars, not only user env vars, i.e. as well coming from projects and context URL.

Documentation

Build Options:

  • /werft with-github-actions
    Experimental feature to run the build with GitHub Actions (and not in Werft).
  • 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-slow-database
  • 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

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-ak-gp-env-project.2 because the annotations in the pull request description changed
(with .werft/ from main)

@akosyakov akosyakov changed the title fix #12208: in workspace project env var management in workspace project env var management Feb 15, 2023
@akosyakov akosyakov force-pushed the ak/gp_env_project branch 3 times, most recently from 7c5cd41 to 4c51d0e Compare February 16, 2023 11:30
@akosyakov akosyakov changed the title in workspace project env var management gp env to respect workspace env vars Feb 16, 2023
@roboquat roboquat added size/XXL and removed size/XL labels Feb 16, 2023
@roboquat roboquat added size/XL and removed size/XXL labels Feb 16, 2023
@akosyakov
Copy link
Member Author

akosyakov commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

@svenefftinge @geropl I have not tested yet, going after the lunch, but I've refactored it that now there is EnvVarService.resolve which hides the merge logic of env vars from different sources given user (optional) or workspace. All code in Server is using it, i.e. for create/start workspace, run/rerun prebuilds, and resolve env vars for the running workspace. I also added tests for the merge logic. Could you have a look please? 🙏

@akosyakov
Copy link
Member Author

akosyakov commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-clean-slate-deployment=true recreate-vm=true

👍 started the job as gitpod-build-ak-gp-env-project.10
(with .werft/ from main)

@roboquat roboquat added size/XXL and removed size/XL labels Feb 16, 2023
@akosyakov akosyakov force-pushed the ak/gp_env_project branch 2 times, most recently from 9cc5607 to eac186c Compare February 16, 2023 14:10
akosyakov
akosyakov commented Feb 16, 2023
View reviewed changes
components/server/src/workspace/workspace-starter.ts
const res = new Map<string, string>();
const imageAuth = projectEnvVars.find((e) => e.name === "GITPOD_IMAGE_AUTH");
const imageAuth = envVars.project.find((e) => e.name === "GITPOD_IMAGE_AUTH");
Copy link
Member Author

Choose a reason for hiding this comment

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

@svenefftinge this is the major reason why we need to return all, although we could change that this method itself fetches a required env var.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this env var needs to be set on project-level and not anyhow.

Copy link
Member Author

Choose a reason for hiding this comment

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

@svenefftinge I removed other layers, but kept this one for now. I think someone else can address it later if needed. Looking at the original PR and docs it seems to be that intent was to allow it only on project env vars.

@akosyakov
Copy link
Member Author

akosyakov commented Feb 16, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-ak-gp-env-project.15
(with .werft/ from main)

@roboquat roboquat added size/XL and removed size/XXL labels Feb 17, 2023
@akosyakov
Copy link
Member Author

akosyakov commented Feb 17, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-ak-gp-env-project.19
(with .werft/ from main)

@akosyakov
Copy link
Member Author

akosyakov commented Feb 17, 2023 •
edited
Loading

Output of gp env with this PR:

  • start after prebuild

Screenshot 2023-02-17 at 11 49 47

  • restart

Screenshot 2023-02-17 at 11 51 57

  • start after prebuild with context env vars

Screenshot 2023-02-17 at 11 54 17

Output of env reflects the same.

@akosyakov akosyakov marked this pull request as ready for review February 17, 2023 10:55
@akosyakov akosyakov requested review from a team February 17, 2023 10:55
@github-actions github-actions bot added team: webapp Issue belongs to the WebApp team team: IDE labels Feb 17, 2023
@akosyakov
Copy link
Member Author

akosyakov commented Feb 17, 2023 •
edited by werft-gitpod-dev-com bot
Loading

/hold

just want @geropl to double check, can someone from @gitpod-io/engineering-ide to review 🙏

geropl
geropl approved these changes Feb 20, 2023
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

LGTM

@akosyakov
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 532513a into main Feb 20, 2023
@roboquat roboquat deleted the ak/gp_env_project branch February 20, 2023 17:36
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed: IDE IDE change is running in production deployed Change is completely running in production labels Feb 21, 2023
@kenibrewer kenibrewer mentioned this pull request Mar 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geropl geropl geropl approved these changes

@iQQBot iQQBot iQQBot approved these changes

@svenefftinge svenefftinge svenefftinge approved these changes

Assignees
No one assigned
Labels
deployed: IDE IDE change is running in production deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/XL team: IDE team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support altering project-level environment variables with gp env
5 participants
@akosyakov @svenefftinge @iQQBot @geropl @roboquat