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 Application-Based Cookies #60

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lgants
Copy link

@lgants lgants commented Nov 21, 2021

what

  • This PR adds support for the optional cookie_name stickiness block parameter
  • This PR also adds a verification check in the variables file - this functionality requires terraform version 0.13, which is consistent with existing version requirements for this module

why

  • This module should support application-based cookies. The AWS documentation states that, "Application Load Balancers support both duration-based cookies and application-based cookies. The key to managing sticky sessions is determining how long your load balancer should consistently route the user's request to the same target. Sticky sessions are enabled at the target group level. You can use a combination of duration-based stickiness, application-based stickiness, and no stickiness across all of your target groups." - source: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/sticky-sessions.html.

references

ALB Terraform ALB target group stickiness configuration: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_target_group#stickiness

@lgants lgants requested review from a team as code owners November 21, 2021 21:23
@lgants lgants changed the title added cookie name Support Application-Based Cookies Nov 21, 2021
@lgants lgants requested a review from a team as a code owner November 21, 2021 21:24
@lgants lgants requested review from dotCipher and woz5999 November 21, 2021 21:24
variables.tf Outdated Show resolved Hide resolved
@lgants lgants requested a review from nitrocode December 8, 2021 04:00
@mergify mergify bot dismissed nitrocode’s stale review December 8, 2021 04:00

This Pull Request has been updated, so we're dismissing all reviews.

nitrocode
nitrocode previously approved these changes Jul 6, 2022
@nitrocode
Copy link
Member

/test all

@nitrocode nitrocode dismissed their stale review March 17, 2023 11:43

test errors

@nitrocode
Copy link
Member

/test all

@goruha
Copy link
Member

goruha commented Jul 29, 2023

/terratest

@hans-d hans-d added the stale This PR has gone stale label Mar 2, 2024
@mergify mergify bot added triage Needs triage and removed stale This PR has gone stale labels Apr 1, 2024
@abeluck
Copy link

abeluck commented Dec 6, 2024

We also need this feature for some applications, is there a reason this wasn't merged originally?

@osterman @goruha @nitrocode could we please get a review of this again?

I've rebased the changes in my own branch here and it works fine!

main...abeluck:terraform-aws-alb-ingress:feat/application-cookies

I could open up a new PR if @lgants isn't around to update this PR, just let me know.

@nitrocode
Copy link
Member

@abeluck there are conflicts in this PR. If the author can resolve the conflicts, someone can review. Or the pr can be recreated with the conflicts fixed.

@abeluck
Copy link

abeluck commented Dec 10, 2024

@abeluck there are conflicts in this PR. If the author can resolve the conflicts, someone can review. Or the pr can be recreated with the conflicts fixed.

@nitrocode Thanks for getting back! Since I'm not the original author, I've opened a new PR with the same changes here (less merge conflicts of course).

#95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants