feat: Support env var interpolation in baremetal deploy.toml

[jaiakt]

This is based on #3708.

Context aka Why?

Some settings in the deploy config might need to be different based on what machine is deploying (for example the private key path). This adds env variable interpolation similar to what redwood.toml parsing does currently so that some fields can be loaded from .env.

What does this do?

Adds support for env var interpolation. Example syntax:

[[production.servers]]
host = '${HOST}'
repo = '${REPO:git://github.com}'
path = '${PATH:/var/www/app}'

This will do the following:

set value of host from the env var $HOST, and interpolate it
read the value of repo from $REPO, but fallback to git://github.com if not present. Note that the fallback has to be a string
read path from $PATH but fallback to the default if not present

Note: This could be potentially breaking for deploy configs with strings of the format ${}. I've gated this behavior behind the yarg interpretEnvVariables which can be defaulted with the environment variable DEPLOY_INTERPRET_ENV_VARIABLES. If this is too complicated, I can simplify it 😄

[jaiakt]

Hey @thedavidprice this is my first contribution to redwood. Love the project 😄. Not sure why all the checks are broken. Did I skip a step?

[thedavidprice]

@jaiakt well, in that case, I'm extra excited about this PR. Grateful you'd take the time to contribute.

CI: there are steps we need, as maintainers, manually take with every new PR. For the most part, pay attention to CI but don't focus much on it until the code review is underway. And there's a lot going on, so don't hesitate to ask for direction and/or help! (Note: in this case I'm working on yarn.lock being out of sync. Updating shortly)

[jaiakt]

One thing I was thinking about is that this could potentially be breaking (for just the deploy process) if any users have strings that look like env variable interpolations (although it would be strange to have strings of the format ${} in constants) 😓. Not sure what the best way to avoid this is. Maybe we want to add an optional parameter which enables this behavior?

[cannikin]

This is amazing, thanks so much! We'd love to have this update discussed in the docs as well, how do you feel about adding a note or two in there about this new functionality?

[replay-io]

15 replays were recorded for 2a44d33.

0 Failed

15 Passed

• useAuth hook, auth redirects checks
• Check that a specific blog post is prerendered
• Check that about is prerendered
• Check that homepage is prerendered
• Check that meta-tags are rendering the correct dynamic data
• Check that you can navigate from home page to specific blog post
• Waterfall prerendering (nested cells)
• RBAC: Admin user should be able to delete contacts
• RBAC: Should not be able to delete contact as non-admin user
• Smoke test with dev server
• Smoke test with rw serve
• Loads Cell mocks when Cell is nested in another story
• Loads Cell Stories
• Loads MDX Stories
• Mocks current user, and updates UI while dev server is running

View test run on Replay ↗︎

[jaiakt]

@cannikin added a note in the docs. LMK if that looks good

[cannikin]

This is amazing, I've wanted this feature for a while now but didn't have time to look into adding it! Thanks so much.

What do you think about having interpolation be enabled by default, if not just always there (no option needed to disable)? I feel like you wouldn't accidentally add ${} to your strings. And you'd never NOT want to interpolate them, leaving a random ${} in a string.

Unless maybe people have some CI system somewhere where they'd want to put a variable in deploy.toml that their CI system should replace, before it gets to yarn actually doing the deploy?

[cannikin]

You'll see in our checks that the first one is failing, about "Check test project fixture." Can you run this command and commit any changes? (You can ignore changes to any db migration files):

yarn build:test-project --rebuild-fixture

[jaiakt]

This is amazing, I've wanted this feature for a while now but didn't have time to look into adding it! Thanks so much.

What do you think about having interpolation be enabled by default, if not just always there (no option needed to disable)? I feel like you wouldn't accidentally add ${} to your strings. And you'd never NOT want to interpolate them, leaving a random ${} in a string.

Unless maybe people have some CI system somewhere where they'd want to put a variable in deploy.toml that their CI system should replace, before it gets to yarn actually doing the deploy?

I personally think this should be enabled by default (and probably always there). I was just a bit nervous adding a potentially breaking change. I do agree that the chances of someone having ${} isn't super high. I'll remove the yarn for now.

[jaiakt]

You'll see in our checks that the first one is failing, about "Check test project fixture." Can you run this command and commit any changes? (You can ignore changes to any db migration files):

yarn build:test-project --rebuild-fixture

Hmm I ran this command and am now changing a lot of files that probably shouldn't be touched by this PR. I'm assuming they are stale as of previous changes?

[cannikin]

Hmm I ran this command and am now changing a lot of files that probably shouldn't be touched by this PR. I'm assuming they are stale as of previous changes?

Hmm, possibly. I just merged in the latest main, maybe try again and verify? You can ignore changes to any files in api/db/migrations (they change because the filename contains a timestamp of when they were created) and any *.scenarios.js files (those change all the time because they're populated with random values).

If anything else shows as changed can you screenshot me a list, or maybe paste a diff into https://gist.github.com ?

[jaiakt]

@cannikin the changed files are in the changed files of this PR. It seems like some package versions in __fixtures__/test-project/web/package.json are being updated which causes the tailwind classnames to be reordered. I don't think this is a huge issue though 🤔
This could potentially be because I'm adding the new package "string-env-interpolation": "1.0.1" to packages/cli/package.json

[jaiakt]

Oh nice just updated to main and it looks like those file changes are gone!

[cannikin]

Hmm, getting a failing test in CI:

[jaiakt]

Ah yeah just learned how to run tests lol. Fixed the test!

[cannikin]

Thanks so much! This should auto-merge once the CI is done running!