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

Use exported environment vars during pack build #4950

Merged
merged 5 commits into from
Jan 16, 2022

Conversation

emdienn
Copy link
Contributor

@emdienn emdienn commented Nov 25, 2021

Hey all, in using the builder-pack packer, I came across the scenario where my apps' config variables weren't being drawn into the build process like the documentation intimated it would/should.

Digging into it, I discovered that in the builder-pack build script, the config_export is called and stored in a file, but then that file is never touched again.

Figuring this was an oversight, I made a fix and would like to share it back into the codebase.

Why did I use sed on the "pretty" format instead of just use the "envfile" format?

Great question.

The tl:dr; is that the "envfile" format provided by config is actually incorrect: it surrounds the value of the variable in single quotes, ie.

APPLICATION_ID='49b2195e-da01-5f1c-b1c8-61455a695d93'

causing the value to include the quotes, because environment variables should* be specified as

APPLICATION_ID=49b2195e-da01-5f1c-b1c8-61455a695d93

After digging through the config subcommand codebase, I determined that this is being done by a dependency package (namely github.com/joho/godotenv) - rather than start wrangling the format (which might have implications elsewhere) I opted for a more localised solution.

*I couldn't find an authoritative source for the exact semantics of a dotenv file, but every other platform and system I have ever seen or used does not wrap the value in quotes. I'm happy to be wrong, but pack requires no-quote syntax regardless.


Anyway. Love dokku, hope you find this PR helpful.

plugins/builder-pack/builder-build Outdated Show resolved Hide resolved
emdienn and others added 3 commits January 15, 2022 06:54
Config was exported, but the result never used.
Docker accepts its flags as --env=FOO but pack requires --env FOO, which is why ENV_ARGS_VALUES is iterated into ENV_ARGS, then pack run through eval
Also add a test for env vars during build time for pack-built applications.
@josegonzalez josegonzalez merged commit 3c68114 into dokku:master Jan 16, 2022
github-actions bot pushed a commit that referenced this pull request Jan 18, 2022
# History

## 0.26.7

Install/update via the bootstrap script:

```shell
wget https://raw.githubusercontent.com/dokku/dokku/v0.26.7/bootstrap.sh
sudo DOKKU_TAG=v0.26.7 bash bootstrap.sh
```

### Bug Fixes

- #5003: @josegonzalez Respect pre-existing .env files when preparing the herokuish buildenv
- #4950: @emdienn Use exported environment vars during pack build
- #4999: @elhu Purge cache for all apps when calling buildpacks:set-property with --global flag
- #4988: @fomojola Conditionally restart NGINX
- #4922: @josegonzalez Gitignore more built plugin triggers in apps plugin

### New Features

- #4964: @josegonzalez Add log message when running state is false
- #4970: @josegonzalez Log the string error and not bytes when there is an issue updating the cron schedule
- #4924: @josegonzalez Update devcontainer to include aliased go packages
- #4920: @josegonzalez Ensure related go source and vscode plugins are installed in devcontainer

### Documentation

- #4949: @IlyaSemenov docs: update herokuish upgrade instructions
- #4992: @anthonyshew Typo for deploying using image SHA
- #4976: @josegonzalez Update link in header to dokku pro
- #4972: @Dam-Buty Clarify cron:report command
- #4943: @Stormheg Fix typo in registry management documentation
- #4935: @josegonzalez Reword note about env vars in nginx config templates

### Other

- #4995: @dependabot[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 203 to 205 in /tests/apps/php
- #4978: @dependabot[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 201 to 203 in /tests/apps/php
- #4936: @dependabot[bot] chore(deps-dev): bump heroku/heroku-buildpack-php from 200 to 201 in /tests/apps/php
- #4937: @dependabot[bot] chore(deps): bump socket.io from 4.3.2 to 4.4.0 in /tests/apps/.websocket.disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants