Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accessing OriginShieldProperty in cloudfront.distribution #12872

Closed
MEGrimshaw opened this issue Feb 4, 2021 · 4 comments · Fixed by #15453
Closed

Accessing OriginShieldProperty in cloudfront.distribution #12872

MEGrimshaw opened this issue Feb 4, 2021 · 4 comments · Fixed by #15453
Labels
effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p1

Comments

@MEGrimshaw
Copy link

MEGrimshaw commented Feb 4, 2021

https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.CfnDistribution.OriginShieldProperty.html

I am trying to add OriginShielding to a Cloudfront Distribution via CDK and the docs are very unclear about the approach.

To access origin shield within CDK you have to call cfnDistribution, however that doesn't appear to be compatible with the cloudfront distribution construct, nor cloudfront-origins.

Do I need to use cfnDistribution for the whole distribution?

It would be very helpful to have more thorough examples of a Cloudfront Distribution setup, especially after deprecating CloudFrontWebDistribution.

@MEGrimshaw MEGrimshaw added documentation This is a problem with documentation. feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2021
@rix0rrr rix0rrr assigned njlynch and unassigned rix0rrr Feb 8, 2021
@njlynch
Copy link
Contributor

njlynch commented Feb 9, 2021

Origin shield support has not yet been added to either Distribution or CloudFrontWebDistribution; the two new properties would need to be added to both the OriginProps (https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-cloudfront/lib/origin.ts#L56) and SourceConfiguration (https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts#L134).

Contributions welcome!

In the mean time, you can use escape hatch raw overrides to set the properties. For example:

  const cfnDist = dist.node.defaultChild as CfnDistribution;
  cfnDist.addPropertyOverride('DistributionConfig.Origins.0.OriginShield', {
    Enabled: true,
    OriginShieldRegion: 'us-east-2',
  });

@njlynch njlynch added effort/small Small work item – less than a day of effort p1 and removed documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Feb 9, 2021
@MEGrimshaw
Copy link
Author

MEGrimshaw commented Feb 9, 2021

@njlynch Thanks for the response! I'll take a look at contributing to getting this into cf.distribution.

In the meantime here is approach I was looking at (from https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.CfnDistribution.OriginShieldProperty.html):

    const EgDist = new cloudfront.CfnDistribution(this, 'CfDistribution', {
      distributionConfig: {
        enabled: true,
        priceClass: "Use All Edge Locations (Best Performance)",
        comment: "Example CF Distro",
        originShield: {
          enabled: true,
          originShieldRegion: envVars.REGION,
        },

I haven't tested the above yet, and the docs aren't clear if originShield should be under distributionConfig, or under origin like so:

    const EgDist = new cloudfront.CfnDistribution(this, 'CfDistribution', {
      distributionConfig: {
        enabled: true,
        priceClass: "Use All Edge Locations (Best Performance)",
        comment: "CF Distro",
        origin: {
          domainName: envVars.ORIGIN_DOMAIN,
          originShield: {
            enabled: true,
            originShieldRegion: envVars.REGION, 
        },

Would either of these work & simply be another way to do approach this?
I'll try it with your approach as well.
Thanks!

@MEGrimshaw
Copy link
Author

MEGrimshaw commented Feb 18, 2021

@njlynch Wanted to circle back here to close the loop if others were looking for possible solutions to this issue. I was able to get this to work with CfnDistribution, specifically within distributionConfig:

        origins: [{
          domainName: envVars.ORIGIN_DOMAIN,
          id: envVars.ORIGIN_ID,
          customOriginConfig: {
            originProtocolPolicy: "https-only",
            originSslProtocols: ["TLSv1.2"],
          },
          originShield: {
            enabled: true,
            originShieldRegion: envVars.REGION,
          },
        }],

FWIW, even with these issues, I'm loving CDK.

@ericzbeard ericzbeard added the feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs label Apr 2, 2021
@mergify mergify bot closed this as completed in #15453 Jul 30, 2021
mergify bot pushed a commit that referenced this issue Jul 30, 2021
**Closes [#12872](#12872 - Origin Shield is a part of the CfnDistribution resources but has not yet been added to the relevant CloudFront constructs. These changes add `originShieldRegion` fields to `OriginProps` and `SourceConfiguration` as specified [here](#12872 (comment)) as well as update the ReadMe and all the necessary tests.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Aug 3, 2021
**Closes [aws#12872](aws#12872 - Origin Shield is a part of the CfnDistribution resources but has not yet been added to the relevant CloudFront constructs. These changes add `originShieldRegion` fields to `OriginProps` and `SourceConfiguration` as specified [here](aws#12872 (comment)) as well as update the ReadMe and all the necessary tests.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
**Closes [aws#12872](aws#12872 - Origin Shield is a part of the CfnDistribution resources but has not yet been added to the relevant CloudFront constructs. These changes add `originShieldRegion` fields to `OriginProps` and `SourceConfiguration` as specified [here](aws#12872 (comment)) as well as update the ReadMe and all the necessary tests.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants