Add a cookie strategy#89
Conversation
jcmoraisjr
left a comment
jcmoraisjr
left a comment
There was a problem hiding this comment.
Hi, thanks for supporting this controller! Just a few docs and logging changes.
| ||`ingress.kubernetes.io/secure-backends`|[true\|false]|-| | ||
| ||`ingress.kubernetes.io/secure-verify-ca-secret`|secret name|-| | ||
| ||[`ingress.kubernetes.io/session-cookie-name`](#affinity)|cookie name|-| | ||
| |`[0]`|[`ingress.kubernetes.io/session-cookie-strategy`](#affinity)|cookie strategy|-| |
There was a problem hiding this comment.
This PR belongs to v0.6, so use [1] instead and create also a * ``[1]`` only in ``snapshot`` tag entry just before this table.
Change also cookie strategy text to a list of options. Have a look at eg forwardfor configmap option.
| ss, err := parser.GetStringAnnotation(annotationAffinityCookieStrategy, ing) | ||
|
|
||
| if err != nil || !affinityCookieStrategyRegex.MatchString(ss) { | ||
| glog.V(3).Infof("Invalid or no annotation value found in Ingress %v: %v. Setting it to default %v", ing.Name, annotationAffinityCookieStrategy, defaultAffinityCookieStrategy) |
There was a problem hiding this comment.
This means an invalid configuration, so change the log level to glog.Warningf.
There was a problem hiding this comment.
I changed to a warning if there's an invalid strategy specified, but left as an info if there is no strategy specified since the intent was to have backwards compatibility and not require people to use this option. Otherwise, anyone who was using affinity but didn't set this would start getting warnings. Hope that's ok, but can change if necessary.
| } | ||
|
|
||
| if nginxAffinity.CookieConfig.Strategy != "insert" { | ||
| t.Errorf("expected route as sticky-strategy but returned %v", nginxAffinity.CookieConfig.Strategy) |
There was a problem hiding this comment.
...expected insert as sticky-...
|
Thanks for the feedback! |
jcmoraisjr
left a comment
jcmoraisjr
left a comment
There was a problem hiding this comment.
Just one new issue I just found, everything else is fine.
|
|
||
| var ( | ||
| affinityCookieHashRegex = regexp.MustCompile(`^(index|md5|sha1)$`) | ||
| affinityCookieStrategyRegex = regexp.MustCompile(`^(insert|prefix|rewrite)`) |
There was a problem hiding this comment.
Will fix, thanks.
|
Thank you very much, merged! |
2 participants
This is an additional annotation that will allow the user to select a different HAProxy cookie strategy other than insert. Supported additions are prefix and rewrite.