Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to change configuration variables at runtime #536

Merged
merged 17 commits into from
Apr 9, 2024

Conversation

podliashanyk
Copy link
Contributor

@podliashanyk podliashanyk commented Jan 30, 2024

Fixes #456?

The implementation here is heavily inspired by the global config option described here https://profinit.eu/en/blog/build-once-deploy-many-in-react-dynamic-configuration-properties/

Implemented changes:

  • Configuration can now be changes at runtime in dev and prod environments via variables provided in runtime-config.json file
  • Updated CHANGELOG
  • Updated docs on frontend

Expected behaviour:

  • Default config values are always used in test env.
  • Default config values are used in dev env unless /public/runtime-config.json is added. Updating /public/runtime-config.json at runtime will immediately trigger changes to the application when running the application in dev via docker-compose up.
  • There is no default config in prod env. %FRONTEND_BASE_URL%/runtime-config.json file must be served or else the app will not run.

Following is to be expected when runtime-config.json is provided:

  • backendUrl and cookieDomain are required, otherwise the app will fail.
  • if frontendVersion or apiVersion values are provided, they are silently ignored.
  • if any other unknown variables are provided, they are silently ignored.

Further work:

@podliashanyk podliashanyk added usability Improves ease of use refactoring No functional changes labels Jan 30, 2024
@podliashanyk podliashanyk self-assigned this Jan 30, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024

Codecov Report

Attention: 32 lines in your changes are missing coverage. Please review.

Comparison is base (27001ea) 57.55% compared to head (72ae301) 57.30%.
Report is 4 commits behind head on master.

Files Patch % Lines
src/index.tsx 0.00% 17 Missing ⚠️
src/api/client.ts 22.22% 7 Missing ⚠️
src/config.tsx 90.32% 3 Missing ⚠️
src/services/RealtimeService.ts 0.00% 2 Missing ⚠️
src/components/alertsnackbar/index.tsx 50.00% 1 Missing ⚠️
src/components/incident/IncidentFilterToolbar.tsx 75.00% 1 Missing ⚠️
src/utils.tsx 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
- Coverage   57.55%   57.30%   -0.25%     
==========================================
  Files          86       86              
  Lines        3652     3687      +35     
  Branches      816      836      +20     
==========================================
+ Hits         2102     2113      +11     
- Misses       1541     1565      +24     
  Partials        9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lunkwill42
Copy link
Member

An attempt to summarize an offline discussion:

This doesn't seem to translate into an actual runtime configuration file. The values of runtime-config.json are built into the JS bundles at compile time (as the require directive is a compile-time directive), so this suggested solution seems to be essentially identicalto the current solution (i.e. it fetches build-time config from a file rather than from the environment, but it is still build-time configuration).

What we want is an actual separate file with only configuration variables, which is loaded from the web server as the React application starts. This file should be editable during runtime, using any editor, like e.g. vim, and the changes should take effect merely by hard-reloading the application in your browser.

When deployed as a container, said configuration file should be generated from the container's environment variables as the container starts. An example of a current production-oriented deployment can be found in https://github.com/Uninett/argus-docker/blob/master/frontend/Dockerfile . This is a two-stage Dockerfile. The first stage uses NPM to build the Argus frontend, having the configuration variables come from build-time arguments. The second stage is a simple nginx-based container, where the static files to be served are copied from the first container (so we have a resulting minimal image without NPM, just with a web server and some static files). Ideally, the config variables that come as build ARGs in the first stage should rather come as ENV vars in the second stage. The run command then needs to be a two-stage script that first generates the static config file from these environment variables (maybe using a simple templating command like envsubst, and then launches nginx as before.

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

I have tested this with argus-docker, and in our PaaS environment by updating the K8s deployment definitions for a review site, and things seem to be working smoothly - and everything has fewer deploy steps now that the same docker image can be used for all deployments.

Updating docs and ensuring there is good error feedback if the runtime-config is missing or broken is all that is left to do, IMO.

@lunkwill42
Copy link
Member

lunkwill42 commented Feb 15, 2024

I've added Uninett/argus-docker#3 to track this when it hits a new release (presumably as 1.13.0)

podliashanyk and others added 8 commits February 16, 2024 14:26
This updates the docker production image to be a fully operational nginx
server which serves the statically built Argus front-end.  Now that
runtime configuration is supported, the updated image will produce a
runtime config file from the same environment variables that were
originally expected by the frontend build process, and ensure it's in
the correct location before starting nginx.
For handling of fixed values, required and optional values and easy stripping of unknown config values provided by users
in DEV env: config file can still be added to overwrite defaults but is not required anymore
It is no longer required in dev
@podliashanyk podliashanyk marked this pull request as ready for review February 16, 2024 14:24
@lunkwill42 lunkwill42 changed the title Make it possible to change environment variables at runtime Make it possible to change configuration variables at runtime Feb 20, 2024
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
lunkwill42 and others added 4 commits February 20, 2024 10:25
The image runtime has significantly changed, simplifying deployment.
Co-authored-by: Morten Brekkevold <morten.brekkevold@sikt.no>
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

I guess we're there now! Time for a 1.13 release, @hmpf! :)

@podliashanyk podliashanyk merged commit 8570175 into master Apr 9, 2024
3 checks passed
@podliashanyk podliashanyk deleted the config-build branch April 9, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring No functional changes usability Improves ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Research alternative ways to configure the frontend application
3 participants