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

Add session-cookie-keywords #601

Merged
merged 3 commits into from
Jun 15, 2020
Merged

Conversation

MartinKirchner
Copy link

Add option to configure the cookie options in more detail.
This might be necessary to let the backend server access the affinity cookie
or to turn off the httponly flag.

The default value is still indirect nocache httponly to be backwards
compatible.

Add option to configure the cookie options in more detail.
This might be necessary to let the backend server access the affinity cookie
or to turn off the `httponly` flag.

The default value is still `indirect nocache httponly` to be backwards
compatible.
@MartinKirchner
Copy link
Author

Closes #566

Copy link
Owner

@jcmoraisjr jcmoraisjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for supporting this controller! The approach is great, following just a few remarks in the docs.

@@ -198,6 +198,7 @@ The table below describes all supported configuration keys.
| [`session-cookie-name`](#affinity) | cookie name | Backend | |
| [`session-cookie-shared`](#affinity) | [true\|false] | Backend | `false` |
| [`session-cookie-strategy`](#affinity) | [insert\|prefix\|rewrite] | Backend | |
| [`session-cookie-keywords`](#affinity) | cookie options | Backend | `indirect nocache httponly` |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort alphabetically

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

| `session-cookie-name` | `Backend` | `INGRESSCOOKIE` | |
| `session-cookie-shared` | `Backend` | `false` | v0.8 |
| `session-cookie-strategy` | `Backend` | `insert` | |
| `session-cookie-keywords` | `Backend` | `indirect nocache httponly` | v0.11 |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort alphabetically as well

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -353,6 +355,7 @@ Configure if HAProxy should maintain client requests to the same backend server.
* `session-cookie-strategy`: the cookie strategy to use (insert, rewrite, prefix). `insert` is the default value if not declared.
* `session-cookie-shared`: defines if the persistence cookie should be shared between all domains that uses this backend. Defaults to `false`. If `true` the `Set-Cookie` response will declare all the domains that shares this backend, indicating to the HTTP agent that all of them should use the same backend server.
* `session-cookie-dynamic`: indicates whether or not dynamic cookie value will be used. With the default of `true`, a cookie value will be generated by HAProxy using a hash of the server IP address, TCP port, and dynamic cookie secret key. When `false`, the server name will be used as the cookie name. Note that setting this to `false` will have no impact if [use-resolver](#dns-resolvers) is set.
* `session-cookie-keywords`: options to the `cookie` option like `nocache`, `preserve`, `httponly`. `indirect nocache httponly` is the default if not declared to be backwards compatible to earlier versions.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a reference to the default value only if insert strategy is used.

try also to rephrase the last sentence to something like "the default is ..." in order to separate the former list of keywords to the later one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -110,6 +110,7 @@ const (
BackSessionCookieName = "session-cookie-name"
BackSessionCookieShared = "session-cookie-shared"
BackSessionCookieStrategy = "session-cookie-strategy"
BackSessionCookieKeywords = "session-cookie-keywords"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort alphabetically

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jcmoraisjr
Copy link
Owner

Merging, thanks!

@jcmoraisjr jcmoraisjr merged commit 11a4c0e into jcmoraisjr:master Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants