fix: environment variable build context filtering fix
🎉 Thanks for submitting a pull request! 🎉
Summary
The pull request addresses the issue below.
- https://answers.netlify.com/t/build-step-injects-env-vars-only-when-using-same-value-for-all-contexts-since-today/122796
The getEnv function within the file packages/config/src/env/envelope.ts was filtering out env variables that did not belong to the all or dev contexts.
More information on how to replicate and how we isolated the issue is below.
Understanding and Isolating the Problem
In order to replicate the issue, we did the following steps.
- We created a simple vite react app, you can find the app for this test. here.
- https://github.com/wconrad265/env-test
- ran
npm installto install everything needed
- ran
netlify sites:createto create a new site and link the repo to it - We created two different env variables, on the netlify website.
-
VITE_All_ContextThis env variable has the same value in all contexts. -
VITE_Hellothisenvvariable has different values depending on the context
-
- We placed the
envvariables within the main react component and ran the following commands-
envvariables have to start withVITEin order for vite to recognize them
-
Next, we ran a series of deploy commands to see the result
Next we created second environment variable declared for all contexts to understand how varaibles declared within all contexts were being handled across different values passed to the --context flag.
command netlify deploy --build --prod --context
| VITE_Hello | VITE_All_Context | —build | —prod | —context |
|---|---|---|---|---|
| no | yes | x | x | n/a |
| yes | yes | x | x | dev |
| no | yes | x | x | production |
| no | yes | x | x | deploy-preview |
| no | yes | x | x | branch-deploy |
Next we examined the handling of environment variables in netlify dev
netlity dev
| VITE_Hello | VITE_All_Context | —context |
|---|---|---|
| yes | yes | n/a |
| yes | yes | dev |
| yes | yes | production |
| yes | yes | deploy-preview |
| yes | yes | branch-deploy |
We dug further to isolate the problem by running netlify build
netlify build context --dev
As we can see below the value of the env variable when using the dev context is the correct value.
The VITE_All_Contextvariable is the correct value
netlify build context --production
As we can see below, the value of the VITE_Hello when using the production context is void, however All_Context env variable is showing the correct value
the context of --branch-deploy and deploy-preview are the same as above. This led us to believe the issue is in the netlify build command
Note this is only an issue when deploying from the terminal with the above commands. When deploying from the website, the environment variables were injected correctly.
Solution
The netlify build command gathers environment variables by running getEnv().
When running this function, if siteInfo.use_envelope is a truthy value, it will invoke getEnvelope . We isolated this function as the source of env variable loss. Environment variables not belonging to the dev or all contexts were filtered out.
We updated the getEnvelopeto filter by all and context (the argument passed by user associated with the —context flag) and reran our isolated tests with the vite project above.
Results:
netlify deploy --build --prod
| VITE_Hello | VITE_All_Context | —build | —prod | —context |
|---|---|---|---|---|
| yes | yes | x | x | n/a |
| yes | yes | x | x | dev |
| yes | yes | x | x | production |
| yes | yes | x | x | deploy-preview |
| yes | yes | x | x | branch-deploy |
Also siteId was not passed in the function getEnvelope(), so we adjusted the function to make sure it was passed in.
For us to review and ship your PR efficiently, please perform the following steps:
- [x] Open a bug/issue before writing your code 🧑💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
- [x] Read the contribution guidelines 📖. This ensures your code follows our style guide and passes our tests.
- [x] Update or add tests (if any source code was changed or added) 🧪
- [x] Update or add documentation (if features were changed or added) 📝
- [x] Make sure the status checks below are successful ✅
A picture of a cute animal (not mandatory, but encouraged)
Nice catch! Looks good to me
Thanks! Do you know why the circleCi tests are failing?
@wconrad265 if you look at the logs for the failing test you can see your change introduced a discrepancy in one of the tests
@wconrad265 if you look at the logs for the failing test you can see your change introduced a discrepancy in one of the tests
Thanks, I will look into why those tests are failing. I think we need to update the tests, since they were always returning the dev environment variables.
Also, the test I was looking into was this, before the other tests had run.
https://app.circleci.com/pipelines/github/netlify/build/7538/workflows/b783faf8-d6cd-4a18-a5e9-a5f608458a07/jobs/10768
Updated the test that were failed with @tlane25.
Previously, the DEV variable was always returned, since the getEnvelope function was always returning the dev environment variables regardless of the context. Because the tests run in the production context, the test has been updated to return the correct corresponding env variable.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
