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

Improve Configuration for Docker #2916

Closed
qcaas-nhs-sjt opened this issue Mar 8, 2024 · 1 comment · Fixed by #2917
Closed

Improve Configuration for Docker #2916

qcaas-nhs-sjt opened this issue Mar 8, 2024 · 1 comment · Fixed by #2917
Milestone

Comments

@qcaas-nhs-sjt
Copy link
Contributor

At present, docker configuration is extremely limited, with only really the ability to change WebAPI URL. This means that if docker users want to amend any of the configurations, they are forced to override the current settings by creating their own images, or dynamically injecting the config file (which could introduce security concerns) rather than being able to use the default image and config file and override based on the environmental variables.

The same environmental variable substitution that is used for WEBAPI_URL can also provide for many other customisations, making the solution much more customisable for docker users.

qcaas-nhs-sjt added a commit to lsc-sde/fork-ohdsi-atlas that referenced this issue Mar 8, 2024
- added configuration via environment variables to Dockerfile
- amended authors list on container metadata
- amended envsubst command to include all environment variables
- extended config-local.js to include the majority of configuration options available in app.js
- Added comment to app.js to remind future contributors to add configuration to Dockerfile and config-local.js
@qcaas-nhs-sjt
Copy link
Contributor Author

Added Attached PR, this will effectively expose the majority of configurable options in the app. These will be overridden:

For those familiar with K8S, we've been able to then customise our implementation using the following:

  env:
  # Dynamic WebAPI will use /WebAPI on the address that was sent by the request, rather than having to define up-front
  # So if it receives a request at https://sandbox-ohdsi.lscsde.nhs.uk it will then look for the WebAPI at 
  # https://sandbox-ohdsi.lscsde.nhs.uk/WebAPI/ instead. This was adapted from the work provided by @chgl in
  # https://github.com/chgl/charts/tree/master/charts/ohdsi#override-config-localjs-for-atlas
  - name: USE_DYNAMIC_WEBAPI_URL
    value: "true"
  
  # Enable user authentication using Entra ID via the Open ID provider
  - name: USER_AUTHENTICATION
    value: "true"
  - name: OID_PROVIDER_ENABLED
    value: "true"
  - name: OID_PROVIDER_NAME
    value: "Microsoft Entra ID"

  # Customisations for our environment
  - name: SUPPORT_MAIL
    value: "[email protected]"
  - name: APP_NAME
    value: "LSCSDE - OHDSI Atlas"

  # Clear local storage before loading config
  - name: CLEAR_LOCAL_STORAGE
    value: "true"

As you can see this gives a lot more flexibility

qcaas-nhs-sjt added a commit to lsc-sde/fork-ohdsi-atlas that referenced this issue Mar 14, 2024
- amended variables that exist to match those on Broadsea implementation
qcaas-nhs-sjt added a commit to lsc-sde/fork-ohdsi-atlas that referenced this issue Apr 10, 2024
- added configuration via environment variables to Dockerfile
- amended authors list on container metadata
- amended envsubst command to include all environment variables
- extended config-local.js to include the majority of configuration options available in app.js
- Added comment to app.js to remind future contributors to add configuration to Dockerfile and config-local.js
qcaas-nhs-sjt added a commit to lsc-sde/fork-ohdsi-atlas that referenced this issue Apr 10, 2024
- amended variables that exist to match those on Broadsea implementation
@anthonysena anthonysena added this to the v2.15 milestone May 14, 2024
anthonysena pushed a commit that referenced this issue May 14, 2024
…2917)

* #2916
- added configuration via environment variables to Dockerfile
- amended authors list on container metadata
- amended envsubst command to include all environment variables
- extended config-local.js to include the majority of configuration options available in app.js
- Added comment to app.js to remind future contributors to add configuration to Dockerfile and config-local.js

* #2916
- amended variables that exist to match those on Broadsea implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants