fix(server): fix version parsing #1586
Conversation
src/shadowbox/webpack.config.js
Outdated
| @@ -31,7 +31,9 @@ const config = { | |||
| }, | |||
| plugins: [ | |||
| // Used by server/version.ts. | |||
| process.env.SB_VERSION ? new webpack.DefinePlugin({'__VERSION__': JSON.stringify(process.env.SB_VERSION)}): undefined, | |||
| process.env.VERSION | |||
There was a problem hiding this comment.
The fix is OK, but I worry that such a generic env var may collide with other things running on the developer's machine, causing accidental injection.
There was a problem hiding this comment.
I think your interpretation of the Task change is wrong. We are not using an environment variable at all. We have a Task VERSION variable that gets interpolated in the command. There's no env VAR for VERSION. we set the docker --build-arg flag.
There was a problem hiding this comment.
I meant that the webpack build step is reading from an environment variable (process.env.X), but we stopped providing it in our Docker build.
But I was wrong about VERSION being available. It was working for me locally, but maybe I polluted my env while testing the build stages. I also just noticed that the build arg is in the wrong place anyway. It needs to be inside the FROM build stage to be usable, which is causing the label to not be set.
PTAL. I'm changing the Node.js env name back to SB_VERSION and set it as an ENV variable for the Node.js build step. I think that's the correct solution.
By the way, what does ${BUILD_ENV:+--mode="${BUILD_ENV} do on the npx webpack line? I can't find any references to it.
There was a problem hiding this comment.
That's a bash expansion that expands only if BUILD_ENV is defined. That's because we need it to expand to a --mode= flag.
There was a problem hiding this comment.
Ah it's https://webpack.js.org/configuration/mode/? For some reason I couldn't find it. Thanks.
src/shadowbox/webpack.config.js
Outdated
| @@ -31,7 +31,9 @@ const config = { | |||
| }, | |||
| plugins: [ | |||
| // Used by server/version.ts. | |||
| process.env.SB_VERSION ? new webpack.DefinePlugin({'__VERSION__': JSON.stringify(process.env.SB_VERSION)}): undefined, | |||
| process.env.VERSION | |||
There was a problem hiding this comment.
That's a bash expansion that expands only if BUILD_ENV is defined. That's because we need it to expand to a --mode= flag.
src/shadowbox/Taskfile.yml
Outdated
| @@ -91,7 +94,7 @@ tasks: | |||
| cmds: | |||
| - rm -rf '{{.IMAGE_ROOT}}' | |||
| - mkdir -p '{{.IMAGE_ROOT}}' | |||
| - {task: build, vars: {TARGET_OS: linux, TARGET_ARCH: '{{.TARGET_ARCH}}', TARGET_DIR: '{{joinPath .IMAGE_ROOT "/opt/outline-server"}}'}} | |||
| - {task: build, vars: {SB_VERSION: '{{.VERSION}}', TARGET_OS: linux, TARGET_ARCH: '{{.TARGET_ARCH}}', TARGET_DIR: '{{joinPath .IMAGE_ROOT "/opt/outline-server"}}'}} | |||
There was a problem hiding this comment.
Use VERSION for task variables.
#1535 changed how we build the Shadowbox server image. Nothing is setting the
SB_VERSIONenvironment variable that the webpack config parses to determine the server's version.This now sets the correct version again: