feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup #32740

jamiepmullan · 2025-01-04T21:50:55Z

Issue # (if applicable)

Closes #.

Reason for this change

add origin selection criteria to L2 Distribution & L2 OriginGroup
add unit + integration tests

Description of changes

Add Selection Criteria to Distribution L2 construct. CFN docs can be found here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-cloudfront-distribution-origingroup.html#cfn-cloudfront-distribution-origingroup-selectioncriteria

Description of how you validated changes

Added unit + integration tests for this new feature - by default it can be undefined, so shouldn't impact other configurations.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

add integration test

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

codecov · 2025-01-04T22:05:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.52%. Comparing base (c57debb) to head (739b6f9).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32740   +/-   ##
=======================================
  Coverage   81.52%   81.52%           
=======================================
  Files         222      222           
  Lines       13715    13715           
  Branches     2417     2417           
=======================================
  Hits        11181    11181           
  Misses       2254     2254           
  Partials      280      280

Flag	Coverage Δ
suite.unit	`81.52% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.97% <ø> (ø)`
packages/aws-cdk-lib/core	`82.09% <ø> (ø)`

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

… better with existing interfaces

lpizzinidev

Thanks 👍 Left some comments for adjustments

lpizzinidev · 2025-01-06T16:16:11Z

packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts

+ *
+ * @default - none


Suggested change

*

* @default - none

enum declarations don't need a @default

my bad! removed

lpizzinidev · 2025-01-06T16:16:57Z

packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts

+  DEFAULT='default',
+  /**
+   * Selection based on media quality.
+   */
+  MEDIA_QUALITY_BASED='media-quality-based',


Suggested change

DEFAULT='default',

/**

* Selection based on media quality.

*/

MEDIA_QUALITY_BASED='media-quality-based',

DEFAULT = 'default',

/**

* Selection based on media quality.

*/

MEDIA_QUALITY_BASED = 'media-quality-based',

Formatting.

lpizzinidev · 2025-01-06T16:18:31Z

packages/aws-cdk-lib/aws-cloudfront/lib/origin.ts

+   * The failover selection for the Distribution.
+   *
+   * @default - nothing is returned


Suggested change

* The failover selection for the Distribution.

*

* @default - nothing is returned

* The selection criteria for the origin group.

*

* @see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/high_availability_origin_failover.html#concept_origin_groups.creating

*

* @default - OriginSelectionCriteria.DEFAULT

I assume CFN will enforce default if not specified by CDK?
Can you please align the documentation in the other variable declarations as well?

Yeah - if it is undefined, then it'll default to default - even if we pass undefined, do you want the docs to reflect the "default" setting?

lpizzinidev · 2025-01-06T16:24:34Z

packages/aws-cdk-lib/aws-cloudfront/test/selection-criteria-origin.test.ts

I think these test cases should be added to the existing distribution.test.ts file.
Also, the Ensure default is undefined case seems redundant.

I've moved them to distribution.test.ts - i removed that test, but have added in a test where "default" is explicitly set.

lpizzinidev

Thanks 👍 Please update the documentation for the variable in origin-group.ts as well

lpizzinidev · 2025-01-07T16:17:56Z

packages/aws-cdk-lib/aws-cloudfront-origins/lib/origin-group.ts

+  /**
+   * The selection criteria for the origin group.
+   *
+   * @default - nothing is returned


Suggested change

* @default - nothing is returned

* @see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/high_availability_origin_failover.html#concept_origin_groups.creating

*

* @default - OriginSelectionCriteria.DEFAULT

done! thanks!

aws-cdk-automation · 2025-01-07T16:55:31Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 739b6f9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

add origin selection criteria to L2 Distribution

e8d0f65

add integration test

github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Jan 4, 2025

aws-cdk-automation requested a review from a team January 4, 2025 21:51

github-actions bot added the p2 label Jan 4, 2025

aws-cdk-automation previously requested changes Jan 4, 2025

View reviewed changes

jamiepmullan marked this pull request as draft January 4, 2025 22:05

Add readme

e8f669a

jamiepmullan marked this pull request as ready for review January 4, 2025 22:43

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 4, 2025

rename OriginGroupSelectionCriteria to OriginSelectionCriteria to fit…

6fb2474

… better with existing interfaces

jamiepmullan changed the title ~~feat(cloudfront): add origin group selection criteria to L2 Distribution~~ feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup Jan 5, 2025

lpizzinidev suggested changes Jan 6, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 6, 2025

jamiepmullan added 2 commits January 6, 2025 17:21

fixes based on review feedback

7ec5343

Merge branch 'main' into feat/origin-group-selection-criteria

7b81e82

jamiepmullan requested a review from lpizzinidev January 6, 2025 18:50

lpizzinidev approved these changes Jan 7, 2025

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 7, 2025

jamiepmullan added 2 commits January 7, 2025 16:23

added docs

3c90fa5

Merge branch 'main' into feat/origin-group-selection-criteria

739b6f9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup #32740

feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup #32740

jamiepmullan commented Jan 4, 2025 •

edited

Loading

aws-cdk-automation left a comment

codecov bot commented Jan 4, 2025 •

edited

Loading

lpizzinidev left a comment

lpizzinidev Jan 6, 2025

jamiepmullan Jan 6, 2025

lpizzinidev Jan 6, 2025

jamiepmullan Jan 6, 2025

lpizzinidev Jan 6, 2025

jamiepmullan Jan 6, 2025

lpizzinidev Jan 6, 2025

jamiepmullan Jan 6, 2025

lpizzinidev left a comment

lpizzinidev Jan 7, 2025

jamiepmullan Jan 7, 2025

aws-cdk-automation commented Jan 7, 2025

-   * @default - nothing is returned
+   * @see https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/high_availability_origin_failover.html#concept_origin_groups.creating
+   *
+   * @default - OriginSelectionCriteria.DEFAULT

feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup #32740

Are you sure you want to change the base?

feat(cloudfront): add origin group selection criteria to L2 Distribution and L2 OriginGroup #32740

Conversation

jamiepmullan commented Jan 4, 2025 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 4, 2025 • edited Loading

Codecov Report

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 7, 2025

AWS CodeBuild CI Report

jamiepmullan commented Jan 4, 2025 •

edited

Loading

codecov bot commented Jan 4, 2025 •

edited

Loading