-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add a testing environment for Cognito #656
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing resource needs to be imported again once the configuration change is merged into master.
This tripped me up again, and I had to recall how I did it previously. Nice to document here to head off the memory lookup again in the future.
The counterpart to our login.nextstrain.org custom domain. Including in our Terraform configuration is helpful so we can use the same structure for other environments too.
In preparation for adding a second configuration as env/testing/. Multiple configurations using local modules are the primary and recommended¹ way to have multiple environments that are similar but not identical. Terraform "workspaces" are another way manage multiple environments, but they're intended for multiple instances of a single configuration.² You can parameterize the configuration on the workspace name, but it's harder to express larger scale divergence (different resources, different modules) with just parameterization due to the lack of any top-level conditional constructs. The directory name is "env", not "terraform", because I expect these directories to house additional non-Terraform files needed for each environment. For example, similar to how our Terraform modules under aws/ currently house non-Terraform assets. ¹ c.f. <https://developer.hashicorp.com/terraform/tutorials/modules/organize-configuration> ² <https://developer.hashicorp.com/terraform/cli/workspaces#use-cases>
Nested modules run counter to the recommended patterns of module composition¹, but it didn't pose issues when we had a single root configuration. Now that we're going to have more than one configuration, it makes expressing optional module usage a lot harder. (Just like the docs warned me of!) The "moved" blocks let the old resource names in the current state migrate to the new resource names when `terraform apply` is run. ¹ <https://developer.hashicorp.com/terraform/language/modules/develop/composition>
These may contain sensitive information, and besides, we never want to commit them anyway.
nextstrain-bot
temporarily deployed
to
nextstrain-s-trs-testin-fvuz4s
February 11, 2023 01:16
Inactive
We will use this for CI (e.g. to test group creation/modification in automated tests) and also local development.
Values still need to be set as environment variables for server processes, but they're no longer hardcoded in the source code.
Massages `terraform output` into a shell snippet suitable for `source` or a directory suitable for `envdir`. I wrote an initial version of this to help with my local testing, and this bit-more-refined version might as well live here for ad-hoc use in future development.
This avoids the manual overhead of having to arrange on every server invocation for the definition of several environment variables with static values. Going forward, we can start to use this config file for other, non-Cognito constants as well.
The new config module marks the beginning of having any concept of a collective singular configuration for this app rather than a bunch of individual variables scattered around. This lets us start to use shared-by-reference definitions rather than shared-by-copy definitions. Move our most pervasive of variables, PRODUCTION, into this new way.
tsibley
force-pushed
the
trs/testing-env
branch
from
February 11, 2023 01:17
99e6322
to
c8dd762
Compare
Base automatically changed from
trs/bypass-cloudfront-for-manifest
to
master
February 13, 2023 19:45
Since this PR is dev-facing only and is something where I want to minimize divergence from |
…esting keys This requires either a) out-of-band S3 operations to move the state objects or b) `terraform init -migrate-state`, but it yields more sensible and clear keys.
This was referenced Feb 15, 2023
victorlin
reviewed
Feb 25, 2023
tsibley
added a commit
to nextstrain/cli
that referenced
this pull request
Apr 24, 2023
…ent variables Useful for development/testing against non-production infrastructure. These aren't ergonomic to use, but they at least make it possible to test without changing the CLI's source code. Developer ergonomics can come later if warranted. Related-to: <nextstrain/nextstrain.org#656>
2 tasks
tsibley
added a commit
to nextstrain/cli
that referenced
this pull request
Apr 24, 2023
…ent variables Useful for development/testing against non-production infrastructure. These aren't ergonomic to use, but they at least make it possible to test without changing the CLI's source code. Developer ergonomics can come later if warranted. Related-to: <nextstrain/nextstrain.org#656>
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commit messages for details.
Spurred by #581 (comment).
Testing
terraform -chdir=env/production plan
reports only minor expected changesterraform -chdir=env/testing apply
worked (resources now exist)