Skip to content

[WIP] Replace config scripts - #232

Closed
roschaefer wants to merge 1 commit into
developfrom
improve_configuration
Closed

[WIP] Replace config scripts#232
roschaefer wants to merge 1 commit into
developfrom
improve_configuration

Conversation

@roschaefer

@roschaeferroschaefer commented Aug 13, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor
  • no bash scripts
    • error prone
    • platform dependent)

appinteractive
appinteractive reviewed Aug 14, 2018
View reviewed changes

@appinteractiveappinteractive left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We still need a way to include environment variables on the server at runtime without compiling it to the frontend bundle.
    My take on that were the dotenv files that are written by the shell scripts that have access to the environment vars at that point.

  2. All tries to access the process.env at runtime results in static values from buildtime as nuxt replaces them for security resons. Do your approach handles that case?

Comment threadconfig/production.json Outdated
@@ -0,0 +1,4 @@
{
"SENTRY_DNS_PRIVATE": "SENTRY_DNS_PRIVATE",

@appinteractiveappinteractiveAug 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roschaefer how do you prevent this variable to be bundled to the client?

Comment threadnuxt.config.js Outdated
API_PORT: config.get('API_PORT'),
BUILD_COMMIT: config.get('BUILD_COMMIT'),
DEPLOY_DATE: config.get('DEPLOY_DATE'),
SENTRY_DNS_PRIVATE: config.get('SENTRY_DNS_PRIVATE'),

@appinteractiveappinteractiveAug 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roschaefer EMBED_API_TOKEN & SENTRY_DNS_PRIVATE should no be bundled on the client as they are secrets!

@roschaeferroschaefer changed the title Complete rewrite of configuration[WIP] Replace nuxt-dotenv with configAug 14, 2018
@roschaefer
roschaeferforce-pushed the improve_configuration branch from c6a9240 to ee4a406CompareAugust 14, 2018 23:46
@roschaeferroschaefer changed the title [WIP] Replace nuxt-dotenv with config[WIP] Replace config scriptsAug 16, 2018
roschaefer
roschaefer commented Aug 16, 2018
View reviewed changes
Comment threadnuxt.config.js Outdated
'EMBED_API_URL',
'SENTRY_DNS_PUBLIC',
'MAPBOX_TOKEN',
'MAINTENANCE',

@roschaeferroschaeferAug 16, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@appinteractive this will expose only public environment variables to the client.

@roschaefer

roschaefer commented Aug 16, 2018

Copy link
Copy Markdown
ContributorAuthor

@appinteractive when I do console.log(process.env.SENTRY_DNS_PRIVATE) I see the value on the server. So I guess, using process.env.SOME_SECRET will be fine as long as it is run on the server only.

@roschaefer
roschaeferforce-pushed the improve_configuration branch 2 times, most recently from cc2ab5d to 47de0a4CompareAugust 19, 2018 11:16
@roschaefer
Complete rewrite of configuration
47de0a4
- no bash scripts
* error prone
* platform dependent)
@roschaefer

roschaefer commented Aug 19, 2018

Copy link
Copy Markdown
ContributorAuthor

OK, this branch cannot be merged without touching the Dockerfile, so I'll close this in favour of #233

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@appinteractiveappinteractiveappinteractive left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@roschaefer@appinteractive