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

Environment-variables and secure-environment-variables now parsed according to action.yml #8

Closed
wants to merge 2 commits into from

Conversation

andrioid
Copy link

Motivation for this change

I've spent 2 days trying to get my Container Instance to talk to a Postgres server where I have to pass a PG connection string. I was unsure if the problem was with Github secrets, Github actions, This action or with the Azure API.

The connection string is defined in the format of PG_DSN=postgres://user:pass@hostname/dbname?sslmode=require or PG_DSN=host=hostname user=myuser password=mypassword

Both of these strings cause issues for the naive string.split(' ') and string.split('=') approach used currently.

'List of secure environment variables for the container. Space seperated values in "key=value" format'

The action.yml says that if you use spaces, you should add quotes, but I can't see that these quotes are handled anywhere.

Changes

  • I've added parseForPairs() and parsePair() utility functions and changed how the environment variables are parsed.

Comments

I just hope this will help the next person trying to use database connection strings with this action. Please give it a read and see if this can be used.

@msftclas
Copy link

msftclas commented May 24, 2020

CLA assistant check
All CLA requirements met.

@andrioid andrioid changed the title environment-variables and secure-environment-variables now parsed according to action.yml Environment-variables and secure-environment-variables now parsed according to action.yml May 24, 2020
@IslamHeggy
Copy link

IslamHeggy commented Aug 30, 2020

I have been trying to assign some environment variables to connection strings and it keeps parsing the = and ends up having incomplete string.

I think we need this fix, Otherwise I will be using the az client instead of using limited action.

gunnbr added a commit to gunnbr/GthxNetBot that referenced this pull request Sep 12, 2020
It has broken parsing of environment variables and a fix hasn't been resolved since May [Azure/aci-deploy#8](Azure/aci-deploy#8).
Thanks for linking to that bug, @IslamHeggy , so I could see your workaround for the problem.

(Note to anyone else looking: I don't know that this is the fully working fix yet since I don't know if I have to single, double, or not quote strings with spaces, so you may need to look at the full history of this to see how I ended up solving this.)
@mitsha-microsoft
Copy link
Member

I think #14 fixes your issue

@andrioid andrioid closed this Oct 20, 2020
@jesseward
Copy link
Contributor

hey @mitsha-microsoft , thanks for the pointer re #14 . Is there an ETA to cut a new release of the action? Simply pointing the workflow at aci-deploy@master seems to fail as module deps do not appear to be included in master branch.

@mitsha-microsoft
Copy link
Member

@jesseward yes simply pointing to aci-deploy@master won't work since it's not a good advice to point directly to the master branch. Forwarding your issue to @narula0781 and @ashishonce who can release a new version of this action with the updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants