Skip to content

Commit bc2d626

Browse files
committed
fix(webacl-conflicts): [PE1-4466] Ignoring webacl overrides on existing distributions when acl isn't declared
1 parent 756cc13 commit bc2d626

8 files changed

Lines changed: 45 additions & 14 deletions

File tree

.env.example

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ CDN_CLASS="default"
44
ENABLE_DELETION="false"
55
CF_DEFAULT_ORIGIN_DOMAIN="my.default.origin"
66
CF_PRICE_CLASS="PriceClass_All"
7-
CF_AWS_WAF="arn:aws:wafv2:us-east-1:123456789012:global/webacl/ExampleWebACL/473e64fd-f30b-4765-81a0-62ad96dd167a"
87
CF_CUSTOM_SSL_CERT="arn:aws:acm:us-east-1:123456789012:certificate/473e64fd-78bc-4244-bc34-5eba7ad17fd7"
98
CF_SECURITY_POLICY="TLSv1.2_2021"
109
CF_ENABLE_LOGGING="true"

README.md

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,24 @@ In CloudFront, these would result in the following order:
161161
- `/en-us/foo` -> en-us specific origin
162162
- `/*/foo` -> catch all origin
163163

164+
## WebACL Associations
165+
166+
When multiple ingresses share the same CloudFront distribution, the controller determines which AWS WAF WebACL (if any) to associate based on the following rules:
167+
168+
1. **Priority to Explicit WebACLs:**
169+
- If any ingress in the group specifies a WebACL ARN using the annotation `cdn-origin-controller.gympass.com/cf.web-acl-arn`, that WebACL will be associated with the distribution.
170+
- If multiple ingresses specify different WebACL ARNs, the controller will prioritize one (typically the first found, but this is not guaranteed—ensure consistency across your ingresses).
171+
2. **No Annotation or Empty Value:**
172+
- If no ingress in the group specifies a WebACL ARN, or if the annotation is present but empty, the controller will retain the current WebACL association on the distribution.
173+
- This means the WebACL will not be removed automatically if you remove or clear the annotation from your ingresses.
174+
3. **Manual Removal Required:**
175+
- The controller **will not** remove a WebACL from an existing distribution during reconciliation. If you want to disassociate a WebACL, you must do so manually via the AWS Console or CLI.
176+
177+
**Best Practices:**
178+
- Always specify the same WebACL ARN on all ingresses in a group to avoid ambiguity.
179+
- To change the WebACL, update the annotation on at least one ingress in the group to the new ARN.
180+
- To remove a WebACL from a distribution, remove the annotation from all ingresses and then manually disassociate the WebACL in AWS.
181+
164182
## Function Associations
165183

166184
In order to associate [Cloudfront Functions](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/cloudfront-functions.html) and [Lambda@Edge Functions](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-at-the-edge.html) to your Ingress-based origins, add the `cdn-origin-controller.gympass.com/cf.function-associations` annotation.
@@ -330,7 +348,6 @@ Use the following environment variables to change the controller's behavior:
330348

331349
| Env var key | Required | Description | Default |
332350
|---------------------------|----------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------|
333-
| CF_AWS_WAF | No | The Web ACL which should be associated with the distributions. Use the ID for WAF v1 and the ARN for WAF v2. | "" |
334351
| CF_CUSTOM_TAGS | No | Comma-separated list of custom tags to be added to distributions. Example: "foo=bar,bar=foo" | "" |
335352
| CF_DEFAULT_ORIGIN_DOMAIN | Yes | Domain of the default origin each distribution must have to route traffic to in case no custom behaviors match the request. | "" |
336353
| CF_DESCRIPTION_TEMPLATE | No | Template of the distribution's description. Currently a single field can be accessed, `{{group}}`, which matches the CDN group under which the distribution was provisioned. | "Serve contents for {{group}} group." |

internal/cloudfront/distribution.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,6 @@ func NewDistributionBuilder(group string, cfg config.Config) DistributionBuilder
125125
customOrigins: make(map[string]Origin),
126126
priceClass: cfg.CloudFrontPriceClass,
127127
group: group,
128-
webACLID: cfg.CloudFrontWAFARN,
129128
cfg: cfg,
130129
}
131130
}

internal/cloudfront/distribution_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ func (s *DistributionTestSuite) SetupTest() {
4343
DefaultOriginDomain: "test.default.origin",
4444
CloudFrontDescriptionTemplate: "test description: {{group}}",
4545
CloudFrontPriceClass: "test price class",
46-
CloudFrontWAFARN: "default-web-acl",
4746
CloudFrontDefaultCachingPolicyID: "4135ea2d-6df8-44a3-9df3-4b5a84be39ad",
4847
CloudFrontDefaultCacheRequestPolicyID: "216adef6-5c7f-47e4-b989-5492eafa07d3",
4948
CloudFrontDefaultPublicOriginAccessRequestPolicyID: "216adef6-5c7f-47e4-b989-5492eafa07d3",
@@ -78,7 +77,6 @@ func (s *DistributionTestSuite) TestDistributionBuilder_New() {
7877
s.Equal("test.default.origin", dist.DefaultOrigin.Host)
7978
s.Equal("test description: test group", dist.Description)
8079
s.Equal("test price class", dist.PriceClass)
81-
s.Equal("default-web-acl", dist.WebACLID)
8280
s.Equal("true", dist.Tags["cdn-origin-controller.gympass.com/owned"])
8381
s.Equal("test group", dist.Tags["cdn-origin-controller.gympass.com/cdn.group"])
8482
}

internal/cloudfront/repository.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ type DistributionRepository interface {
5656
ARNByGroup(group string) (string, error)
5757
// Create creates the given Distribution on CloudFront. Returns the created dist.
5858
Create(Distribution) (Distribution, error)
59+
// Gets the Distribution configuration by ID.
60+
DistributionConfigByID(id string) (*awscloudfront.GetDistributionConfigOutput, error)
5961
// Sync ensures the given Distribution is correctly configured on CloudFront. Returns synced dist.
6062
Sync(Distribution) (Distribution, error)
6163
// Delete deletes the Distribution at AWS
@@ -131,7 +133,7 @@ func (r DistRepository) Create(d Distribution) (Distribution, error) {
131133

132134
func (r DistRepository) Sync(d Distribution) (Distribution, error) {
133135
config := newAWSDistributionConfig(d, r.CallerRef, r.Cfg)
134-
output, err := r.distributionConfigByID(d.ID)
136+
output, err := r.DistributionConfigByID(d.ID)
135137
if err != nil {
136138
return Distribution{}, fmt.Errorf("getting distribution config: %v", err)
137139
}
@@ -176,7 +178,7 @@ func (r DistRepository) Sync(d Distribution) (Distribution, error) {
176178
}
177179

178180
func (r DistRepository) Delete(d Distribution) error {
179-
output, err := r.distributionConfigByID(d.ID)
181+
output, err := r.DistributionConfigByID(d.ID)
180182
if err != nil {
181183
return cdnaws.IgnoreErrorCodef("getting distribution config: %v", err, awscloudfront.ErrCodeNoSuchDistribution)
182184
}
@@ -251,7 +253,7 @@ func (r DistRepository) distributionTags(d Distribution) *awscloudfront.Tags {
251253
return &awsTags
252254
}
253255

254-
func (r DistRepository) distributionConfigByID(id string) (*awscloudfront.GetDistributionConfigOutput, error) {
256+
func (r DistRepository) DistributionConfigByID(id string) (*awscloudfront.GetDistributionConfigOutput, error) {
255257
input := &awscloudfront.GetDistributionConfigInput{
256258
Id: aws.String(id),
257259
}

internal/cloudfront/repository_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ func (s *DistributionRepositoryTestSuite) SetupTest() {
105105
s.cfg = config.Config{
106106
DefaultOriginDomain: "default.origin",
107107
CloudFrontPriceClass: awscloudfront.PriceClassPriceClass100,
108-
CloudFrontWAFARN: "default-web-acl",
109108
}
110109
}
111110

internal/cloudfront/service.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,11 @@ func (s *Service) newDistribution(ingresses []k8s.CDNIngress, group string, shar
247247

248248
if len(shared.WebACLARN) > 0 {
249249
b = b.WithWebACL(shared.WebACLARN)
250+
} else {
251+
b, err = s.keepCurrentWebACLConfig(b, distARN)
252+
if err != nil {
253+
return Distribution{}, fmt.Errorf("setting webacl config: %v", err)
254+
}
250255
}
251256

252257
if len(distARN) > 0 {
@@ -256,6 +261,23 @@ func (s *Service) newDistribution(ingresses []k8s.CDNIngress, group string, shar
256261
return b.Build()
257262
}
258263

264+
// keepCurrentWebACLConfig checks the current WebACL configuration on distribution and updates the desired state accordingly.
265+
func (s *Service) keepCurrentWebACLConfig(b DistributionBuilder, distARN string) (DistributionBuilder, error) {
266+
distibutionID := b.extractID(distARN)
267+
268+
config, err := s.DistRepo.DistributionConfigByID(distibutionID)
269+
if err != nil {
270+
return b, fmt.Errorf("getting distribution config by ID (%s): %v", distibutionID, err)
271+
}
272+
273+
// If there's a WebACLId in the existing distribution config, set it in the builder, otherwise keep the default (no WebACL).
274+
if config.DistributionConfig.WebACLId != nil && len(*config.DistributionConfig.WebACLId) > 0 {
275+
b = b.WithWebACL(*config.DistributionConfig.WebACLId)
276+
}
277+
278+
return b, nil
279+
}
280+
259281
// discoverCert returns the first found ACM Certificate that matches any Alternate Domain Name of the input Ingresses
260282
func (s *Service) discoverCert(ingresses []k8s.CDNIngress) (certificate.Certificate, error) {
261283
var alternateDomains []string

internal/config/config.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ const (
3535
enableDeletionKey = "enable_deletion"
3636
cfDefaultOriginDomainKey = "cf_default_origin_domain"
3737
cfPriceClassKey = "cf_price_class"
38-
cfWafArnKey = "cf_aws_waf"
3938
cfSecurityPolicyKey = "cf_security_policy"
4039
cfEnableLoggingKey = "cf_enable_logging"
4140
cfS3BucketLogKey = "cf_s3_bucket_log"
@@ -61,7 +60,6 @@ func initDefaults() {
6160
viper.SetDefault(enableDeletionKey, "false")
6261
viper.SetDefault(cfDefaultOriginDomainKey, "")
6362
viper.SetDefault(cfPriceClassKey, awscloudfront.PriceClassPriceClassAll)
64-
viper.SetDefault(cfWafArnKey, "")
6563
viper.SetDefault(cfSecurityPolicyKey, "")
6664
viper.SetDefault(cfEnableLoggingKey, "false")
6765
viper.SetDefault(cfS3BucketLogKey, "")
@@ -99,8 +97,6 @@ type Config struct {
9997
// CloudFrontPriceClass determines how many edge locations CloudFront will use for your distribution.
10098
// ref: https://docs.aws.amazon.com/sdk-for-go/api/service/cloudfront/
10199
CloudFrontPriceClass string
102-
// CloudFrontWAFARN the Web ACL ARN.
103-
CloudFrontWAFARN string
104100
// CloudFrontSecurityPolicy the minimum SSL/TLS protocol that CloudFront can use to communicate with viewers.
105101
// ref: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-viewercertificate.html
106102
CloudFrontSecurityPolicy string
@@ -175,7 +171,6 @@ func Parse() (Config, error) {
175171
DefaultOriginDomain: viper.GetString(cfDefaultOriginDomainKey),
176172
DeletionEnabled: viper.GetBool(enableDeletionKey),
177173
CloudFrontPriceClass: viper.GetString(cfPriceClassKey),
178-
CloudFrontWAFARN: viper.GetString(cfWafArnKey),
179174
CloudFrontSecurityPolicy: viper.GetString(cfSecurityPolicyKey),
180175
CloudFrontEnableLogging: viper.GetBool(cfEnableLoggingKey),
181176
CloudFrontS3BucketLog: viper.GetString(cfS3BucketLogKey),

0 commit comments

Comments
 (0)