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

Fix: ordered_cache_behavior forwarded_values dynamic block #188

Conversation

neilbartley
Copy link

@neilbartley neilbartley commented Oct 19, 2021

what

  • Corrects logic for ordered_cache_behavior -> forwarded_values
  • Should be If a cache policy or origin request policy is specified, we cannot include a 'forwarded_values' block at all in the API request

why

  • If I only specify one of cache_policy_id or origin_request_policy_id then the forwarded_values block would still be added, triggering an API error:

Error: error updating CloudFront Distribution (XXXXXXXXXXXXXX): InvalidArgument: The parameter ForwardedValues cannot be used when a cache policy is associated to the cache behavior.

  • Pre PR functionality:
cache_policy_id supplied? origin_request_policy_id supplied? forwarded_values block generated? correct behaviour?

references

@neilbartley neilbartley requested review from a team as code owners October 19, 2021 16:35
@max-lobur
Copy link
Contributor

max-lobur commented Dec 9, 2021

Thanks for this @neilbartley !

Do you think it might be easier if we

  • add an explicit boolean (on/off) to forwarded_values
  • or make a forwarded_values group of variables a separate object inside cache object

@korenyoni korenyoni requested a review from a team as a code owner December 13, 2021 15:56
@korenyoni korenyoni requested review from max-lobur and RothAndrew and removed request for a team December 13, 2021 15:56
@korenyoni
Copy link
Member

Added a needs-discussion label while awaiting a response on #188 (comment)

@josh-onchain
Copy link

Yeah, I think I am running into a similar issue. This module has been a lifesaver and for what it's worth it's the only issue I have run into so far. While creating ordered_cache it is hard to tell what depends on what (by aws design it seems) and then it makes specify a lambda function arn even though I don't want to lol. Although I may just be doing something incorrect.

@mergify
Copy link

mergify bot commented Jan 27, 2022

This pull request is now in conflict. Could you fix it @neilbartley? 🙏

@neilbartley
Copy link
Author

Sorry, this fell off my radar. Looks like @korenyoni fixed in #210 🙌

@neilbartley neilbartley deleted the ordered-cache-forwarded-values-fix branch March 14, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants