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.

Sign up for GitHub

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

Jump to bottom

Support Application-Based Cookies #60

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

Support Application-Based Cookies #60

wants to merge 4 commits into 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
nitrocode
nitrocode previously requested changes Dec 7, 2021
View reviewed changes
variables.tf Outdated Show resolved Hide resolved
@lgants @nitrocode
cleanup variable description
d159d63 
Co-authored-by: nitrocode <nitrocode@users.noreply.github.com>
@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
View reviewed changes
@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nitrocode nitrocode nitrocode left review comments

@dotCipher dotCipher Awaiting requested review from dotCipher dotCipher is a code owner automatically assigned from cloudposse/contributors

@woz5999 woz5999 Awaiting requested review from woz5999 woz5999 is a code owner automatically assigned from cloudposse/contributors

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
triage Needs triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lgants @nitrocode @goruha @hans-d @cloudpossebot