feat: add ECR enhanced scanning and fix S3 logging configuration#52
Merged
feat: add ECR enhanced scanning and fix S3 logging configuration#52
Conversation
alismx
force-pushed
the
alis/fix-logging-path
branch
2 times, most recently
from
ad92ce8 to
c3a44ca
Compare
…ging The previous ARN pattern with `AWSLogs/.../*` path was too restrictive and prevented the S3 bucket policy from accepting PutObject requests from CloudTrail. Changed to allow PutObject on any object within the logging bucket.
alismx
force-pushed
the
alis/fix-logging-path
branch
2 times, most recently
from
ed32135 to
82f6777
Compare
…ging The previous ARN pattern with `AWSLogs/.../*` path was too restrictive and prevented the S3 bucket policy from accepting PutObject requests from CloudTrail. Changed to allow PutObject on any object within the logging bucket.
alismx
force-pushed
the
alis/fix-logging-path
branch
from
82f6777 to
5646a24
Compare
- Add aws_ecr_registry_scanning_configuration with ENHANCED scan type and SCAN_ON_PUSH frequency - Relaxed S3 logging policy to allow ALB access (add ALB ARN as principal) - Changed S3 action from PutObject to * to allow all necessary S3 operations for logging
alismx
changed the title
- Reverting ALB principal addition as it's not needed for S3 logging
- Add enable_enhanced_ecr_registry_scanning variable for optional enhanced scanning - Add depends_on for ECS service to wait for ALB creation - Condition ECR scanning configuration on both disable_ecr and enhanced scanning flag
alismx
force-pushed
the
alis/fix-logging-path
branch
3 times, most recently
from
03e5f94 to
1f2f31d
Compare
…rincipal - Change from s3:* to s3:PutObject for ALB access (principle of least privilege) - Add AWSLogDeliveryWrite statement with Service principal for elasticloadbalancing.amazonaws.com - This allows ALB to write logs while using least-privileged permissions
alismx
force-pushed
the
alis/fix-logging-path
branch
from
1f2f31d to
aa49c4c
Compare
…tion ALB creation was failing with 'Access Denied' when trying to write to the logging bucket because the S3 bucket policy was not fully propagated before the ALB tried to use it. The time_sleep resource adds a 10-second delay after the bucket policy is created/updated, ensuring AWS has time to apply the policy before the ALB attempts to write access logs. - Added time_sleep.wait_for_s3_bucket_policy in s3.tf - Added time provider in provider.tf - Updated alb.tf depends_on to include time_sleep resource
alismx
force-pushed
the
alis/fix-logging-path
branch
5 times, most recently
from
cff6652 to
7bb6418
Compare
S3 bucket policies do not support service principals like 'elasticloadbalancing.amazonaws.com'. This is a common confusion - service principals only work in resource-based policies that explicitly support them (SQS, SNS, Lambda). For S3 bucket policies, you must use the account-specific ELB service account ARN from data.aws_elb_service_account.elb_account_id.arn. - Removed AWSLogDeliveryWrite statement using service principal - Changed AWSLogDeliveryAclCheck to use AWS type with ELB service account Fixes: ALB creation failing with 'Access Denied' for bucket
alismx
force-pushed
the
alis/fix-logging-path
branch
from
7bb6418 to
f0d8122
Compare
…policy-propagation-delay.md
alismx
changed the title
- Remove unused hashicorp/time TerraForm provider dependency - Add checkov and trivy skip comments for CKV_AWS_145 and AVD-AWS-0132 on ALB logging S3 bucket (expected - ALB logging is not compatible with customer managed KMS keys) Related: alis/fix-logging-path
alismx
force-pushed
the
alis/fix-logging-path
branch
from
bf53320 to
8c67333
Compare
- Add checkov skip comment for CKV_AWS_145 on aws_s3_bucket.logging (ALB logging not fully compatible with customer managed KMS keys) - Regenerate README.md and configuration.png via tfutil Related: alis/fix-logging-path
alismx
changed the title
shanice-skylight
approved these changes
Apr 1, 2026
Collaborator
shanice-skylight
left a comment
shanice-skylight
left a comment
There was a problem hiding this comment.
Reviewed all file changes and ran all test plan actions that successfully passed. Terraform plan results here. LGTG
| sid = "AllowALBAccess" | ||
| effect = "Allow" | ||
| actions = ["s3:PutObject", "s3:PutObjectAcl"] | ||
| actions = ["s3:PutObject"] |
Collaborator
There was a problem hiding this comment.
Did you intentional remove the s3:PutObjectAcl action?
Collaborator
Author
There was a problem hiding this comment.
Yes, turns out it wasn't needed.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
This PR adds ECR registry scanning configuration, fixes the S3 logging bucket policy to use ARN references instead of hardcoded paths, and addresses compatibility issues with ALB logging.
Changes
aws_ecr_registry_scanning_configurationresource for enhanced ECR scanning (configurable viaenable_enhanced_ecr_registry_scanningvariable)aws_s3_bucket_server_side_encryption_configurationresourcedepends_onfor S3 logging policy and encryption configurationenable_enhanced_ecr_registry_scanningvariableTest plan
terraform planto verify changesterraform validateto check syntax (passed)terraform fmt -checkto verify formatting (passed)