Use environment variables from @netlify/config

[eduardoboucas]

- Summary

This PR is the CLI counterpart of netlify/build#2040. It introduces the updated environment variables object coming from @netlify/build to various commands.

Copying across this list created by @ehmicky in the build PR. I'll use it to track the progress on the various commands.

• netlify build:
  • Do not pass the env option to @netlify/build anymore
  • This means getBuildEnv() can be removed.
• netlify env:list and netlify env:get:
  • Use the env return value from @netlify/config instead of config.build.environment.
    We might want to select only specific environment variables categories, and merge them.
• netlify dev, netlify dev:exec, netlify functions:create:
  • Use the env return value from @netlify/config. Note: this will also add support to build.environment.
  • The dev.environment property in netlify.toml should be merged to them: { ...env.all, ...dev.environment }.
  • Do not use getSiteInformation() anymore, except for the addonsUrl and dotenv logic
  • Adjust logs in addEnvVariables() to dissociate between netlify.toml and UI site environment variables
  • Pass --offline CLI flag to @netlify/config. netlify dev:exec and netlify functions:create might not have this flag yet, but netlify dev does.

- Test plan

Added new tests that assert the capture of environment variables from [build.environment]. Rest of the test suite unchanged.

- Description for the changelog

Mimic the environment variables used in production

- A picture of a cute animal (not mandatory but encouraged)

[erezrokah]

Thanks @eduardoboucas. Looks good and I love all the code removal.
It's great the CLI doesn't have to resolve those env variables.
Added a few comments/questions.
Please let me know what you think

[eduardoboucas]

@erezrokah I addressed your comments in 320d074, except for #1722 (comment) (removing this is causing the tests to fail, I'm investigating ⏳ )

Note that in 320d074#diff-98719cf70cb4c9ae11108e4f1dc3b629caf2e0f429a9ad4dee054596f8f69950L24-R22 I removed the filtering for process.env variables, because we're already doing that in injectEnvVariables. If we don't remove this, variables in .env that also exist in process.env would never reach injectEnvVariables and therefore wouldn't get logged.

As a result of this, I had to remove the corresponding test. I'll try to add a similar test in a more appropriate place, to assert that process.env is still taking precedence over .env files.

[erezrokah]

Note that in 320d074#diff-98719cf70cb4c9ae11108e4f1dc3b629caf2e0f429a9ad4dee054596f8f69950L24-R22 I removed the filtering for process.env variables, because we're already doing that in injectEnvVariables. If we don't remove this, variables in .env that also exist in process.env would never reach injectEnvVariables and therefore wouldn't get logged.

As a result of this, I had to remove the corresponding test. I'll try to add a similar test in a more appropriate place, to assert that process.env is still taking precedence over .env files.

Great catch! Removing the filtering logic makes sense.

[ehmicky]

This is looking great, thanks @eduardoboucas!

Everything is looking neat ✌️

The CI tests are failing, but I'm not quite sure this is related to this PR considering different tests are failing, and not for every OS and Node.js version... @erezrokah What do you think?

[erezrokah]

The CI tests are failing, but I'm not quite sure this is related to this PR considering different tests are failing, and not for every OS and Node.js version... @erezrokah What do you think?

Yes, everything looks good. Looks the deploy command tests are especially flaky, Seems unrelated. I'm working to improve those too.

[eduardoboucas]

Contrary to our internal discussion, I ended up including the addition of the --offline flag in this PR, via 1c49d95. I hope it's okay, as since it's literally a one-line change and it made my testing process a bit easier.

@erezrokah @ehmicky if you're okay with that, and once the tests finish running, what's our plan for releasing? Do we release @netlify/build and @netlify/config, and then merge this PR and release CLI?

Thanks! 🙌🏻

[erezrokah]

@erezrokah @ehmicky if you're okay with that, and once the tests finish running, what's our plan for releasing? Do we release @netlify/build and @netlify/config, and then merge this PR and release CLI?

I'm good with it. The release process you described is 💯

[ehmicky]

@eduardoboucas and I will pair to release this and the related Netlify Build PR tomorrow.

[ehmicky]

@eduardoboucas This should be ready for merge, after @netlify/build and @netlify/config are updated to the latest versions in package.json.