-
Notifications
You must be signed in to change notification settings - Fork 2
Prepare for integration with infra template #40
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
Changes from 10 commits
d5c5049
a565be7
8290022
271e1b6
4126860
0e7f6dd
43252f8
fc415c3
3edc69a
3bc7c0e
0ca92ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,9 +26,9 @@ AWS_DEFAULT_REGION=<FILL ME IN> | |
# Auth | ||
############################ | ||
|
||
AWS_COGNITO_USER_POOL_ID=<FILL ME IN> | ||
AWS_COGNITO_CLIENT_ID=<FILL ME IN> | ||
AWS_COGNITO_CLIENT_SECRET=<FILL ME IN> | ||
COGNITO_USER_POOL_ID=<FILL ME IN> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you also mean to remove Also, maybe take it out of the AWS Services section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Thank you! |
||
COGNITO_CLIENT_ID=<FILL ME IN> | ||
COGNITO_CLIENT_SECRET=<FILL ME IN> | ||
|
||
############################ | ||
# Database | ||
|
@@ -37,4 +37,4 @@ AWS_COGNITO_CLIENT_SECRET=<FILL ME IN> | |
DB_HOST=127.0.0.1 | ||
DB_NAME=app_rails | ||
DB_USER=app_rails | ||
DB_PASSWORD=secret123 | ||
DB_PASSWORD=secret123 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,6 @@ We have are using [UUIDs for primary keys](https://guides.rubyonrails.org/active | |
|
||
- ⚠️ Using ActiveRecord functions like `Foo.first` and `Foo.last` have unreliable results | ||
- Generating new models or scaffolds requires passing the `--primary-key-type=uuid` flag. For instance, `make rails-generate GENERATE_COMMAND="model Foo --primary-key-type=uuid"` | ||
- `pgcrypto` is a required dependency | ||
|
||
#### Enums | ||
|
||
|
@@ -61,7 +60,7 @@ To preview email views in the browser, visit: `/rails/mailers` | |
|
||
To test AWS SES email sending locally: | ||
|
||
1. Set the `AWS_*` env var in your `.env` file | ||
1. Set the "AWS services" env var in your `.env` file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. env vars? If aws bucket (now just bucket) is moved out of AWS services section, is it needed here for SES to work properly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe so. |
||
1. Set the `SES_EMAIL` env var to a verified sending identity | ||
1. Restart the server | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
# Deployment to cloud environments | ||
|
||
## Requirements | ||
|
||
- External service access: The application must be able to make network calls to the public internet. | ||
- AWS Cognito: The application comes with AWS Cognito enabled by default, so AWS Cognito must be set up before deploying the application. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AWS Cognito has a lot of settings/defaults, and it's not clear from this how to set up a cognito user pool to satisfy the application. A terrafrom module would go a long way to describing what an example cognito/user pool config should look like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I'll make a note. |
||
- Custom domain name with HTTPS support: AWS Cognito requires HTTPS for callback urls to function. | ||
- Access to write to temporary directory: Rails needs to be able to write out temporary files. | ||
- Environment variables: You must provide the application with the environment variables listed in [local.env.example](/app-rails/local.env.example). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to say more about this specifically for cloud deployments? I know we're setting them as aws secrets and importing them to the ecs containers via the task definition, but if I'm using this repo and not using the infra template, that context would be really helpful. |
||
- Secrets: You must provide the application with a secret `SECRET_KEY_BASE`. For more information, see the [Rails Guide on Security](https://guides.rubyonrails.org/v7.1/security.html#environmental-security). | ||
|
||
## Deploying using the Platform Infrastructure Template | ||
|
||
*Note: The following will be true once https://github.com/navapbc/template-infra/pull/650 is merged.* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feels like it belongs in a release note, and not a deployment.md file? Hopefully it won't be an issue for long. |
||
|
||
This template can be deployed using the [Nava Platform Infrastructure Template](https://github.com/navapbc/template-infra). Using the infrastructure template will handle creating and configuring all of the resources required in AWS. | ||
|
||
While following the [infrastructure template installation instructions](https://github.com/navapbc/template-infra?tab=readme-ov-file#installation) and [setup instructions](https://github.com/navapbc/template-infra/blob/main/infra/README.md), use the following configuration: | ||
|
||
1. Rename `/infra/app` to `/infra/<APP_NAME>` so that it matches the directory name of your application. By default, this is `app-rails`. | ||
1. In `/infra/<APP_NAME>/app-config/main.tf`: | ||
1. Set `has_external_non_aws_service` to `true`. | ||
2. Set `enable_identity_provider` to `true`. | ||
1. In `/infra/<APP_NAME>/app-config/<ENVIRONMENT>.tf`: | ||
1. Set the `domain_name`. | ||
2. Set `enable_https` to `true`. | ||
3. Set `enable_command_execution` to `true`: This is necessary temporarily until a temporary file system can be enabled. Otherwise, ECS will run with read-only root filesystem, which will cause rails to error. | ||
1. In `/infra/<APP_NAME>/app-config/env-config/environment-variables.tf`: | ||
1. Add an entry to `secrets`: | ||
```terraform | ||
SECRET_KEY_BASE = { | ||
manage_method = "generated" | ||
secret_store_name = "/${var.app_name}-${var.environment}/service/rails-secret-key-base" | ||
} | ||
``` | ||
1. In `/infra/networks/main.tf`: | ||
1. Modify the `app_config` module to change the path to match the directory name of your application. By default, this is `app-rails`. | ||
```terraform | ||
module "app_config" { | ||
source = "../<APP_NAME>/app-config" | ||
} | ||
``` | ||
1. Follow the infrastructure template instructions to configure [custom domains](https://github.com/navapbc/template-infra/blob/main/docs/infra/set-up-custom-domains.md) and [https support](https://github.com/navapbc/template-infra/blob/main/docs/infra/https-support.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this is the right rails way, but could we set this client_id, along with the user_pool_id, from the ENV at the top of the class and reuse the variable, instead of resetting it in each method to be more DRY?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good change, but I'd like to break it out into a separate issue.