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

[Issue #2278] Attempting to add another S3 bucket for API usage #2740

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

mdragon
Copy link
Collaborator

@mdragon mdragon commented Nov 5, 2024

Summary

Fixes #2278

Time to review: 3 mins

Changes proposed

Adding a 2nd bucket to store non-published Documents/Attachments associated with Opportunities.

@mdragon mdragon linked an issue Nov 5, 2024 that may be closed by this pull request
Copy link
Collaborator

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

It's close! My second comment is the only thing blocking deploys.

Also, can you narrow the filename to the use case a bit more, eg. infra/api/service/draft_documents.tf

@@ -0,0 +1,83 @@
resource "aws_s3_bucket" "documents_draft" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
resource "aws_s3_bucket" "documents_draft" {
resource "aws_s3_bucket" "draft_documents" {

small opinion

@@ -18,6 +18,7 @@ locals {
{ name : "PORT", value : tostring(var.container_port) },
{ name : "AWS_REGION", value : data.aws_region.current.name },
{ name : "S3_BUCKET_ARN", value : aws_s3_bucket.general_purpose.arn },
{ name : "DRAFT_S3_BUCKET_ARN", value : aws_s3_bucket.documents_draft.arn },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move this into infra/api/service/main.tf line 143

Like so:

  extra_environment_variables = merge(
    local.service_config.extra_environment_variables,
    { "ENVIRONMENT" : var.environment_name },
    { "DRAFT_S3_BUCKET_ARN" : aws_s3_bucket.documents_draft.arn }
  )

@mdragon mdragon force-pushed the mdragon/2278-infra-draft-documents branch from a533db2 to 6031041 Compare November 6, 2024 15:59
@github-actions github-actions bot added the ci/cd label Nov 6, 2024
@mdragon mdragon force-pushed the mdragon/2278-infra-draft-documents branch from 40d7655 to 8a15f1d Compare November 6, 2024 16:12
@mdragon mdragon changed the title Attempting to add another S3 bucket for API usage [Issue #2278] Attempting to add another S3 bucket for API usage Nov 6, 2024
Comment on lines +12 to +22
workflow_dispatch:
inputs:
environment:
description: "target environment"
required: true
default: "dev"
type: choice
options:
- dev
- staging
- prod
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏼 !

@mdragon mdragon marked this pull request as ready for review November 18, 2024 17:13
@mdragon mdragon requested a review from acouch as a code owner November 18, 2024 17:13
@mdragon mdragon force-pushed the mdragon/2278-infra-draft-documents branch from 9db8202 to 863d31d Compare November 18, 2024 17:13
Copy link
Collaborator

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

🚢

@mdragon mdragon merged commit a711b0d into main Nov 18, 2024
6 checks passed
@mdragon mdragon deleted the mdragon/2278-infra-draft-documents branch November 18, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup Infrastructure for storing/serving NOFOs
2 participants