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.

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

env vars with commas in value do not work #495

Closed
brianzinn opened this issue Mar 28, 2024 · 4 comments · Fixed by #496
Closed

env vars with commas in value do not work #495

brianzinn opened this issue Mar 28, 2024 · 4 comments · Fixed by #496
Labels
bug Something isn't working

Comments

@brianzinn
Copy link

TL;DR

Unable to include environment variables with commas

Expected behavior

I was expecting the commas to be allowed. A delimiter would be introduced based on the input or otherwise we could supply one.

Observed behavior

The commas cause the environment to be split out - using parts of the values as keys for new entries.

Action YAML

id: 'deploy'
   uses: 'google-github-actions/deploy-cloudrun@v2.2.0'
   with:
          service: 'my-name'
          image: 'us.gcr.io/my-image'
          region: 'my-region'
          # these values are actually from github variables
          env_vars: |
            EMAIL_ADDRESSES=name 1 <name1@email.com>,name 2 <name2@email.com>,name 3 <name3@email.com>
            VAR2=test

Log output

gcloud run deploy ... \
   --update-env-vars EMAIL_ADDRESSES=name 1 <name1@email.com>,name 2 <name2@email.com>=,name 3 <name3@email.com>=,VAR2=TEST

Additional information

see #469 the environment values are being split out and new keys are being created from the values.
https://cloud.google.com/workflows/docs/use-environment-variables#set_multiple_environment_variables
That PR that was not merged I was suspecting would convert to:
gcloud run deploy ...
--set-env-vars ^@^KEY1=VALUE1,VALUE2,VALUE3@KEY2=VALUE2

So, if we were able to provide the delimiter as per the PR then it would be possible to have commas in the values? Otherwise if you are using a non-comma delimiter then you need to change how you are sending it...

I was unable to provide a full example since I have a lot of variables, so hopefully that makes a clean repro on your side.

@brianzinn brianzinn added the bug Something isn't working label Mar 28, 2024
sethvargo added a commit that referenced this issue Mar 28, 2024
This requires changing the test inputs to expect JSON instead of K=V
pairs in our e2e runner.

- Fixes #495 
- Fixes #492
@brianzinn
Copy link
Author

uses: 'google-github-actions/deploy-cloudrun@68504f3206b0ad0856bfeb602d0f7aca7a00e213'
with:
  env_vars: |
    KEY1=VALUE1
    KEY2=VALUE2,VALUE3,VALUE4,VALUE5

Seems not fixed. Here is current output (notice the "ca1" was generated as a delimiter):

gcloud run deploy warehouse-portal --quiet --image us.gcr.io/***/...:latest \
  --update-env-vars ^ca1^KEY1=VALUE1ca1KEY2=VALUE2ca1VALUE3=ca1VALUE4=ca1VALUE5=...

@sethvargo
Copy link
Member

@brianzinn
Copy link
Author

thansk @sethvargo - it does work with escaping the commas. However, there is a contradiction in my view below where you state:

You do not have to escape YAML or JSON.

I think as an aside that my example should work as-is. Unfortunately, if you don't fix this now then it will be a breaking change to fix in the future.

@sethvargo
Copy link
Member

Hi @brianzinn - sorry, I'm not following... You have to read the full context, not just the diff: https://github.com/google-github-actions/deploy-cloudrun?tab=readme-ov-file#inputs. Notably, the point about "You do not have to escape YAML or JSON." is under the env_vars_file variable, which can accept a text file, YAML file, or JSON file.

I think as an aside that my example should work as-is. Unfortunately, if you don't fix this now then it will be a breaking change to fix in the future.

Thank you for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants