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

Setup environment variable collection #2963

Closed
wants to merge 54 commits into from

Conversation

BobanL
Copy link
Collaborator

@BobanL BobanL commented Nov 27, 2024

PULL REQUEST

Summary

  • Create new COLLECTION env variable to help support the different profiles
    • if anyone has other suggestions for names please let me know since I don't really care for COLLECTION
  • Use dotenvx to load all environment variables
    • One image is created and depending on the value of the environment variable COLLECTION, a different profile will be used to start the server
  • Update package scripts so COLLECTION and it's variables only need to be set in one place

Cleanup 🧹

  • Rename APP_ENV to NBS_AUTH
  • Combine Azure-storage and azure init into a single dockerfile so that convert-seed-data doesn't abort when the init is complete
  • Remove wrapping around S3 Client creation since undefined values will not effect the creation of the client
  • Fix conflicting IDs in LA's example eCRs.

Related Issue

Fixes #2873

Acceptance Criteria

  • collections created for each
    • AWS integrated
      • APP_ENV=test
      • NEXT_PUBLIC_NON_INTEGRATED_VIEWER=false
      • SOURCE=azure
      • METADATA_DATABASE_TYPE=undefined
      • METADATA_DATABASE_SCHEMA=undefined
    • Azure integrated
      • APP_ENV=test
      • NEXT_PUBLIC_NON_INTEGRATED_VIEWER=false
      • SOURCE=azure
      • METADATA_DATABASE_TYPE=undefined
      • METADATA_DATABASE_SCHEMA=undefined
    • AWS non-integrated
      • APP_ENV=prod
      • NEXT_PUBLIC_NON_INTEGRATED_VIEWER=true
      • SOURCE=s3
      • METADATA_DATABASE_TYPE=sqlserver
      • METADATA_DATABASE_SCHEMA=extended
    • Azure non-integrated
      • APP_ENV=prod
      • NEXT_PUBLIC_NON_INTEGRATED_VIEWER=true
      • SOURCE=azure
      • METADATA_DATABASE_TYPE=sqlserver
      • METADATA_DATABASE_SCHEMA=?
  • Setting the new configuration name environment variable causes the correct set of environment variables to be set on startup

Additional Information

Anything else the review team should know?

Checklist

  • If this code affects the other scrum team, have they been notified? (In Slack, as reviewers, etc.)

@BobanL BobanL marked this pull request as ready for review December 2, 2024 15:04
containers/ecr-viewer/env/.env.AWS_INTEGRATED Outdated Show resolved Hide resolved
containers/ecr-viewer/env/.env.AZURE_INTEGRATED Outdated Show resolved Hide resolved
containers/message-parser/app/default_schemas/core.json Outdated Show resolved Hide resolved
containers/orchestration/docker-compose.yml Outdated Show resolved Hide resolved
containers/ecr-viewer/.env.sample Outdated Show resolved Hide resolved
containers/ecr-viewer/docker-compose.yml Show resolved Hide resolved
@BobanL BobanL marked this pull request as ready for review December 11, 2024 21:50
@BobanL BobanL changed the title env variable collections Setup environment variable collection Dec 11, 2024
APP_ENV=test
NEXT_PUBLIC_NON_INTEGRATED_VIEWER=true
SOURCE=postgres
COLLECTION=AWS_SQLSERVER_NON_INTEGRATED
Copy link
Collaborator

Choose a reason for hiding this comment

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

spitballing:

  • ENV_FILE
  • ENV_COLLECTION
  • APP_CONFIG
  • CONFIG_NAME
  • CONFIG_COLLECTION

"clear-local": "docker compose -f ./docker-compose.yml --profile \"*\" down -v",
"convert-seed-data": "npx @dotenvx/dotenvx run -f .env.local -- sh -c 'docker compose -f ./seed-scripts/docker-compose-seed.yml --profile ${SOURCE} --profile ${METADATA_DATABASE_TYPE} --profile ecr-viewer up --abort-on-container-exit'",
"convert-seed-data:build": "npx @dotenvx/dotenvx run -f .env.local -- sh -c 'docker compose -f ./seed-scripts/docker-compose-seed.yml --profile ${SOURCE} --profile ${METADATA_DATABASE_TYPE} --profile ecr-viewer up --abort-on-container-exit --build'",
"clear-local": "docker compose -f ./docker-compose.yml --profile \"*\" down -v && docker compose -f ./seed-scripts/docker-compose-seed.yml --profile \"*\" down -v",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we'd only need the seed-script one since it includes the outer one

@@ -89,19 +89,25 @@ npm i

# Write env vars to .env.local
npm run setup-local-env
echo "NEXT_PUBLIC_NON_INTEGRATED_VIEWER=$IS_NON_INTEGRATED" >> .env.local
echo "NBS_AUTH=false" >> .env.local
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this always hold even when the collection has it set the opposite?

@BobanL BobanL marked this pull request as draft December 13, 2024 16:27
@BobanL BobanL closed this Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create environment variable collections
3 participants