#pr-reviews (2022-01)

Pull Request Reviews for Cloud Posse Projects

2022-01-27

David Lozano avatar
David Lozano

Hi everyone. I’m a little bit confused with the bridgecrew’s comments on the below PR. Do I need to validate that the new alias records are pointing to resources created in my account? I didn’t see this validation in the repo before my changes.

Are these crewbridge’s comments the reason because the PR is blocked?

https://github.com/cloudposse/terraform-aws-route53-alias/pull/37

create records using for_each instead of count by 1david5 · Pull Request #37 · cloudposse/terraform-aws-route53-alias attachment image

what Modify default and ipv6 aws_route53_record resources to use for_each instead of count. why Prevent destroying and recreating DNS records when removing elements from aliases list.

2022-01-26

2022-01-25

Frank avatar
Frank

Turns out the fix I made earlier does not work in all cases. This one should deal with that. https://github.com/cloudposse/terraform-aws-cloudfront-s3-cdn/pull/213

fix: handle viewer_certificate.ssl_support_method with CF default certificate by syphernl · Pull Request #213 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Sets ssl_support_method to null when default certificate is being used instead of setting it to sni-only in all cases. why Setting this to an explicit value (sni-only) in conjunction with t…

1
Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)

See comments

fix: handle viewer_certificate.ssl_support_method with CF default certificate by syphernl · Pull Request #213 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Sets ssl_support_method to null when default certificate is being used instead of setting it to sni-only in all cases. why Setting this to an explicit value (sni-only) in conjunction with t…

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)

Approved

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)

Going to release now as a hotfix.

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)

@ I’m wrestling with Terratest here a bit. Will merge once I get tests to pass.

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)

merged

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)
[main.tf] Updated conditions with included cache_policy_id variable by eboboshka · Pull Request #210 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Fixed conditions when using cache_policy_id why When using cache_policy_id, the module tries to change ttl values from zero to the default values. ~ default_cache_behavior { …

2022-01-18

2022-01-11

Frank avatar
Frank
fix: set viewer_certificate.ssl_support_method to a non-empty value by syphernl · Pull Request #208 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Sets the viewer_certificate.ssl_support_method to a non-empty value why Since AWS Provider 3.71.0 the viewer_certificate.ssl_support_method is being validated on plan-time and can no longer…

feat: add origin-shield by syphernl · Pull Request #207 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Add variables to enable the Origin Shield for the CloudFront distribution why Using Origin Shield can help reduce the load on your origin. references https://registry.terraform.io/provide

Yonatan Koren (Cloud Posse) avatar
Yonatan Koren (Cloud Posse)
feat: add origin-shield by syphernl · Pull Request #207 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Add variables to enable the Origin Shield for the CloudFront distribution why Using Origin Shield can help reduce the load on your origin. references https://registry.terraform.io/provide

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)
feat: add origin-shield by syphernl · Pull Request #207 · cloudposse/terraform-aws-cloudfront-s3-cdn attachment image

what Add variables to enable the Origin Shield for the CloudFront distribution why Using Origin Shield can help reduce the load on your origin. references https://registry.terraform.io/provide

Erik Osterman (Cloud Posse) avatar
Erik Osterman (Cloud Posse)

We are trying to prevent breaking the interface and without the default(...) function we can’t solve it. We are avoiding the experimental features due to the big disclaimer that the state object may change/break.

    keyboard_arrow_up