Skip to content

feat#90: billing bucket#91

Open
sh1220 wants to merge 2 commits intomainfrom
feat/#90
Open

feat#90: billing bucket#91
sh1220 wants to merge 2 commits intomainfrom
feat/#90

Conversation

@sh1220
Copy link
Contributor

@sh1220 sh1220 commented Jul 17, 2025

#️⃣ Related Issues

#90

📝 Work Summary

billing을 담을 s3 bucket 생성

Screenshot (Optional)

💬 Review Notes (Optional)

Add any specific points you would like the reviewers to focus on.

@sh1220 sh1220 requested a review from Copilot July 17, 2025 12:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements an S3 bucket for storing AWS billing reports as part of issue #90. The implementation creates the necessary infrastructure to allow AWS billing service to upload cost and usage reports to a dedicated S3 bucket.

  • Creates an S3 bucket specifically for billing reports with appropriate naming
  • Configures bucket policy to allow AWS billing service to upload objects
  • Sets up Terraform state management with S3 backend and DynamoDB locking

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
management-team-account/billing/s3/main.tf Creates S3 bucket and policy for billing reports
management-team-account/billing/s3/outputs.tf Exports bucket ID and ARN for use by other modules
management-team-account/billing/s3/backend.tf Configures Terraform state backend with S3 and DynamoDB


# billing을 담을 bucket 생성
resource "aws_s3_bucket" "billing" {
bucket = "billing-report-bucket"
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The bucket name is hardcoded and may not be unique across AWS accounts. Consider using a variable or adding a random suffix to ensure uniqueness and avoid potential naming conflicts.

Copilot uses AI. Check for mistakes.
# billing을 담을 bucket 생성
resource "aws_s3_bucket" "billing" {
bucket = "billing-report-bucket"
force_destroy = true
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

Setting force_destroy = true on a billing bucket is risky as it allows Terraform to delete the bucket even when it contains objects. This could lead to accidental loss of billing data. Consider setting this to false for production environments.

Suggested change
force_destroy = true
force_destroy = false

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +18
Action = "s3:PutObject",
Resource = "${aws_s3_bucket.billing.arn}/*"
Copy link

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The bucket policy is missing the GetBucketAcl and GetBucketPolicy actions that are typically required by AWS billing service. The policy should include these permissions for the billing service to function properly.

Suggested change
Action = "s3:PutObject",
Resource = "${aws_s3_bucket.billing.arn}/*"
Action = ["s3:PutObject", "s3:GetBucketAcl", "s3:GetBucketPolicy"],
Resource = "${aws_s3_bucket.billing.arn}/*"
},
{
Effect = "Allow",
Principal = { Service = "billingreports.amazonaws.com" },
Action = ["s3:GetBucketAcl", "s3:GetBucketPolicy"],
Resource = "${aws_s3_bucket.billing.arn}"

Copilot uses AI. Check for mistakes.
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.

2 participants