fix: escape env variables

With this change, we avoid the problem of having to escape env variables altogether. Previously, env variables were loaded both by bash (in development) and docker-compose (in production). This meant that we had to deal with escaping in both contexts, which was the source of trouble. We resolve this by loading env vars only with bash. Close overhangio#61.
ghassanmas · Nov 23, 2022 · 6860962 · 6860962
1 parent 9bf192a
commit 6860962
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 29 deletions.
diff --git a/tutormfe/patches/local-docker-compose-dev-services b/tutormfe/patches/local-docker-compose-dev-services
@@ -11,15 +11,4 @@
     tty: true
     volumes:
         - ../plugins/mfe/apps/mfe/webpack.dev-tutor.config.js:/openedx/app/webpack.dev-tutor.config.js:ro
-    env_file:
-        - ../plugins/mfe/build/mfe/env/production
-        - ../plugins/mfe/build/mfe/env/development
-    environment:
-        - "PORT={{ app['port'] }}"
-        {%- for key, value in app.get("env", {}).get("production", {}).items() %}
-        - "{{ key }}={{ value }}"
-        {% endfor %}
-        {%- for key, value in app.get("env", {}).get("development", {}).items() %}
-        - "{{ key }}={{ value }}"
-        {%- endfor %}
 {% endfor %}
diff --git a/tutormfe/templates/mfe/build/mfe/Dockerfile b/tutormfe/templates/mfe/build/mfe/Dockerfile
@@ -22,9 +22,8 @@ RUN echo "copying i18n data" \
   && mkdir -p /openedx/i18n/{{ app["name"] }} \
   {%- endfor %}
   echo "done."
-
 {% for app in iter_values_named(suffix="MFE_APP") %}
-
+################ {{ app["name"] }} MFE
 ######## {{ app["name"] }} (src)
 FROM base AS {{ app["name"] }}-src
 RUN git clone {{ app["repository"] }} --branch {{ app.get("version", MFE_COMMON_VERSION) }} --depth 1 .
@@ -37,15 +36,15 @@ COPY --from=i18n /openedx/i18n/{{ app["name"] }} /openedx/i18n/{{ app["name"] }}
 COPY --from=i18n /openedx/i18n/i18n-merge.js /openedx/i18n/i18n-merge.js
 RUN /openedx/i18n/i18n-merge.js /openedx/app/src/i18n/messages /openedx/i18n/{{ app["name"] }} /openedx/app/src/i18n/messages
 
-######## {{ app["name"] }} (dev)
-FROM base AS {{ app["name"] }}-dev
+######## {{ app["name"] }} (common)
+FROM base AS {{ app["name"] }}-common
 COPY --from={{ app["name"] }}-src /openedx/app/package.json /openedx/app/package.json
 COPY --from={{ app["name"] }}-src /openedx/app/package-lock.json /openedx/app/package-lock.json
 ARG NPM_REGISTRY={{ NPM_REGISTRY }}
 {{ patch("mfe-dockerfile-pre-npm-install") }}
-{# Required for building optipng on M1 #}
+{#- Required for building optipng on M1 #}
 ENV CPPFLAGS=-DPNG_ARM_NEON_OPT=0
-{# We define this environment variable to bypass an issue with the installation of pact https://github.com/pact-foundation/pact-js-core/issues/264 #}
+{#- We define this environment variable to bypass an issue with the installation of pact https://github.com/pact-foundation/pact-js-core/issues/264 #}
 ENV PACT_SKIP_BINARY_INSTALL=true
 RUN npm clean-install --no-audit --no-fund --registry=$NPM_REGISTRY \
   && rm -rf ~/.npm
@@ -54,22 +53,41 @@ COPY --from={{ app["name"] }}-src /openedx/app /openedx/app
 COPY --from={{ app["name"] }}-i18n /openedx/app/src/i18n/messages /openedx/app/src/i18n/messages
 ENV PUBLIC_PATH='/{{ app["name"] }}/'
 EXPOSE {{ app['port'] }}
-CMD ["npm", "run", "start", "---", "--config", "./webpack.dev-tutor.config.js"]
-
-{% endfor %}
-
-{% for app in iter_values_named(suffix="MFE_APP") %}
 
-######## {{ app["name"] }} (production)
-FROM {{ app["name"] }}-dev AS {{ app["name"] }}
 COPY ./env/production /openedx/env/production
-RUN touch /openedx/env/production.override \
+{%- set overrides = app.get("env", {}).get("production", {}) %}
+{%- if overrides %}
+RUN echo "setting production overrides..." \
   {%- for key, value in app.get("env", {}).get("production", {}).items() %}
-  && echo "{{ key }}='{{ value }}'" >> /openedx/env/production.override \
+  && echo "{{ key }}='{{ value }}'" >> /openedx/env/production \
   {%- endfor %}
   && echo "done setting production overrides"
-RUN bash -c "set -a && source /openedx/env/production && source /openedx/env/production.override && npm run build"
+{%- endif %}
+
+######## {{ app["name"] }} (dev)
+FROM {{ app["name"] }}-common AS {{ app["name"] }}-dev
+COPY ./env/development /openedx/env/development
+{%- set overrides = app.get("env", {}).get("development", {}) %}
+{%- if overrides %}
+RUN echo "setting development overrides..." \
+  {%- for key, value in overrides.items() %}
+  && echo "{{ key }}='{{ value }}'" >> /openedx/env/development \
+  {%- endfor %}
+  && echo "done setting development overrides"
+{%- endif %}
+CMD ["/bin/bash", "-c", "set -a && \
+  source /openedx/env/production && \
+  source /openedx/env/development && \
+  npm run start --- --config ./webpack.dev-tutor.config.js"]
+{% endfor %}
 
+# Production images are last to accelerate dev image building
+{%- for app in iter_values_named(suffix="MFE_APP") %}
+######## {{ app["name"] }} (production)
+FROM {{ app["name"] }}-common AS {{ app["name"] }}-prod
+RUN bash -c "set -a && \
+  source /openedx/env/production && \
+  npm run build"
 {% endfor %}
 
 ####### final production image with all static assets
@@ -79,7 +97,7 @@ RUN mkdir -p /openedx/dist
 
 # Copy static assets
 {% for app in iter_values_named(suffix="MFE_APP") %}
-COPY --from={{ app["name"] }} /openedx/app/dist /openedx/dist/{{ app["name"] }}
+COPY --from={{ app["name"] }}-prod /openedx/app/dist /openedx/dist/{{ app["name"] }}
 {% endfor %}
 
 # Copy caddy config file

diff --git a/tutormfe/templates/mfe/build/mfe/env/production b/tutormfe/templates/mfe/build/mfe/env/production
@@ -19,7 +19,7 @@ NODE_ENV=production
 PUBLISHER_BASE_URL=
 REFRESH_ACCESS_TOKEN_ENDPOINT={% if ENABLE_HTTPS %}https{% else %}http{% endif %}://{{ LMS_HOST }}/login_refresh
 SEGMENT_KEY=
-SITE_NAME={{ PLATFORM_NAME|replace("'", "'\\''")|replace(" ", "\ ") }}
+SITE_NAME="{{ PLATFORM_NAME }}"
 STUDIO_BASE_URL={{ "https" if ENABLE_HTTPS else "http" }}://{{ CMS_HOST }}
 USER_INFO_COOKIE_NAME=user-info