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

Support for Viewer Protocol Policy in Behavior for Cloudfront #7086

Closed
prax0724 opened this issue Mar 31, 2020 · 13 comments
Closed

Support for Viewer Protocol Policy in Behavior for Cloudfront #7086

prax0724 opened this issue Mar 31, 2020 · 13 comments
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@prax0724
Copy link

The Behavior interface do not have any property to set Viewer Protocol Policy , although it can be set from AWS console.
Please add support for this property.
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.Behavior.html

@prax0724 prax0724 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 31, 2020
@SomayaB SomayaB added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Mar 31, 2020
@bpcrao
Copy link

bpcrao commented Apr 1, 2020

@prax0724 we can use 'viewerProtocolPolicy' attribute and set it to one of
ViewerProtocolPolicy {
HTTPS_ONLY = "https-only",
REDIRECT_TO_HTTPS = "redirect-to-https",
ALLOW_ALL = "allow-all"
}

@prax0724
Copy link
Author

prax0724 commented Apr 1, 2020

@bpcrao
Is ViewerProtocolPolicy available for Behavior? I can't find it.

@bpcrao
Copy link

bpcrao commented Apr 6, 2020

yes @prax0724

@iliapolo
Copy link
Contributor

@prax0724 It is available as a property of CloudFrontWebDistribution. Its slightly different then the CloudFormation spec per se.

Is this solution suffice or is this issue still relevant?

@iliapolo iliapolo added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 12, 2020
@prax0724
Copy link
Author

@iliapolo

It is available at CloudFrontWebDistribution but not for Behavior. On the console we have the option of setting it for each individual Behavior and not at Cloudfront level. For our use case we are not impacted by this but just in case for others.

@iliapolo
Copy link
Contributor

@prax0724 Got it - thanks for reporting 👍

@iliapolo iliapolo added effort/small Small work item – less than a day of effort and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Apr 13, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Apr 14, 2020
@ivawzh
Copy link

ivawzh commented Jul 27, 2020

I think this is a bug.

allowedMethods, defaultTtl and viewerProtocolPolicy are documented everywhere at https://docs.aws.amazon.com/cdk/api/latest/docs/aws-cloudfront-readme.html. E.g.

const myWebDistribution = new cloudfront.Distribution(this, 'myDist', {
  defaultBehavior: {
    origin: cloudfront.Origin.fromBucket(myBucket),
    allowedMethods: AllowedMethods.ALLOW_ALL,
    viewerProtocolPolicy: ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
  }
});

But it's not in type declarations

export interface AddBehaviorOptions {
    /**
     * HTTP methods to allow for this behavior.
     *
     * @default - GET and HEAD
     */
    readonly allowedMethods?: AllowedMethods;
    /**
     * Whether CloudFront will forward query strings to the origin.
     * If this is set to true, CloudFront will forward all query parameters to the origin, and cache
     * based on all parameters. See `forwardQueryStringCacheKeys` for a way to limit the query parameters
     * CloudFront caches on.
     *
     * @default false
     */
    readonly forwardQueryString?: boolean;
    /**
     * A set of query string parameter names to use for caching if `forwardQueryString` is set to true.
     *
     * @default []
     */
    readonly forwardQueryStringCacheKeys?: string[];
}
/**
 * Options for creating a new behavior.
 *
 * @experimental
 */
export interface BehaviorOptions extends AddBehaviorOptions {
    /**
     * The origin that you want CloudFront to route requests to when they match this behavior.
     */
    readonly origin: Origin;
}

@iliapolo iliapolo added the p1 label Aug 3, 2020
@iliapolo
Copy link
Contributor

iliapolo commented Aug 3, 2020

Just to clarify, the feature request was opened in relation to the CloudFrontWebDistribution construct, which indeed does not have a way to set ViewerProtocolPolicy on the behavior level.

What @ivawzh is referring to is the new Distribution construct, which actually documents that its available, but the code says differently.

@njlynch Can you confirm there is indeed a documentation in the new construct?

In any case, this will be added to the new Distribution construct as part of this issue: #9107.

As far as supporting this for the old CloudFrontWebDistribution construct, we are currently debating this and i'll update here.

Thanks

njlynch added a commit that referenced this issue Aug 3, 2020
…er protocol, and smooth streaming

Adds support for many of the missing properties for controlling behaviors on
the new Distribution construct. Also removed (currently unavailable) properties
from the README.

The remaining properties will come in a follow-up PR. They were not included
in this PR due to either being blocked by the latest CloudFormation spec merge,
or are still being prioritized (e.g., fieldLevelEncryption).

related #7086
related #9107
@njlynch
Copy link
Contributor

njlynch commented Aug 3, 2020

ivawzh@ / iliapolo@ - The documentation for the new Distribution construct was referencing the ViewerProtocolPolicy prior to it being available; rather than update the documentation, I've added the missing properties in #9411 (and updated the README where it still references missing attributes).

mergify bot pushed a commit that referenced this issue Aug 3, 2020
…er protocol, and smooth streaming (#9411)

Adds support for many of the missing properties for controlling behaviors on
the new Distribution construct. Also removed (currently unavailable) properties
from the README.

The remaining properties will come in a follow-up PR. They were not included
in this PR due to either being blocked by the latest CloudFormation spec merge,
or are still being prioritized (e.g., fieldLevelEncryption).

related #7086
related #9107


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
eladb pushed a commit that referenced this issue Aug 10, 2020
…er protocol, and smooth streaming (#9411)

Adds support for many of the missing properties for controlling behaviors on
the new Distribution construct. Also removed (currently unavailable) properties
from the README.

The remaining properties will come in a follow-up PR. They were not included
in this PR due to either being blocked by the latest CloudFormation spec merge,
or are still being prioritized (e.g., fieldLevelEncryption).

related #7086
related #9107


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
curtiseppel pushed a commit to curtiseppel/aws-cdk that referenced this issue Aug 11, 2020
…er protocol, and smooth streaming (aws#9411)

Adds support for many of the missing properties for controlling behaviors on
the new Distribution construct. Also removed (currently unavailable) properties
from the README.

The remaining properties will come in a follow-up PR. They were not included
in this PR due to either being blocked by the latest CloudFormation spec merge,
or are still being prioritized (e.g., fieldLevelEncryption).

related aws#7086
related aws#9107


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@iliapolo iliapolo assigned njlynch and unassigned iliapolo Aug 19, 2020
@jiegec
Copy link
Contributor

jiegec commented Sep 4, 2020

As far as supporting this for the old CloudFrontWebDistribution construct, we are currently debating this and i'll update here.

I encountered the same problem and is there any update? The new Distribution construct does not support IAM certificates, so we can't migrate to it.

EDIT:

A hacking solution:

    let dist = distribution.node.defaultChild as cloudfront.CfnDistribution;
    let conf = dist.distributionConfig as cloudfront.CfnDistribution.DistributionConfigProperty;
    let cacheBehaviors = conf.cacheBehaviors as cloudfront.CfnDistribution.CacheBehaviorProperty[];
    let behavior = cacheBehaviors[0];
    cacheBehaviors[0] = {
      ...cacheBehaviors[0],
      viewerProtocolPolicy: cloudfront.ViewerProtocolPolicy.ALLOW_ALL,
    } as cloudfront.CfnDistribution.CacheBehaviorProperty;

@jiegec
Copy link
Contributor

jiegec commented Sep 6, 2021

It seems that this issue won't be fixed?

@jiegec
Copy link
Contributor

jiegec commented Sep 6, 2021

A possible fix for this with API changes:

diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts
index 29a63fd681bb..ab2fbbd44b03 100644
--- a/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts
+++ b/packages/@aws-cdk/aws-cloudfront/lib/web-distribution.ts
@@ -454,6 +454,12 @@ export interface Behavior {
    */
   readonly functionAssociations?: FunctionAssociation[];
 
+  /**
+   * The viewer policy for this behavior.
+   *
+   * @default - the distribution wide viewer protocol policy will be used
+   */
+  readonly viewerProtocolPolicy?: ViewerProtocolPolicy;
 }
 
 export interface LambdaFunctionAssociation {
@@ -992,7 +998,7 @@ export class CloudFrontWebDistribution extends cdk.Resource implements IDistribu
       trustedKeyGroups: input.trustedKeyGroups?.map(key => key.keyGroupId),
       trustedSigners: input.trustedSigners,
       targetOriginId: input.targetOriginId,
-      viewerProtocolPolicy: protoPolicy || ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
+      viewerProtocolPolicy: input.viewerProtocolPolicy || protoPolicy || ViewerProtocolPolicy.REDIRECT_TO_HTTPS,
     };
     if (!input.isDefaultBehavior) {
       toReturn = Object.assign(toReturn, { pathPattern: input.pathPattern });

mergify bot pushed a commit that referenced this issue Sep 30, 2021
…r CloudFrontWebDistribution (#16389)


This pr fixes issue #7086 by allowing user to set viewer protocol policy in Behavior.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
njlynch pushed a commit that referenced this issue Oct 11, 2021
…r CloudFrontWebDistribution (#16389)


This pr fixes issue #7086 by allowing user to set viewer protocol policy in Behavior.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…r CloudFrontWebDistribution (aws#16389)


This pr fixes issue aws#7086 by allowing user to set viewer protocol policy in Behavior.

----

*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

github-actions bot commented Sep 6, 2022

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

7 participants