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
Feat: Match allowed cookies with optional character #71047
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one minor comment regarding readability but it can be ignored
Let me know about that and I will update it quickly ;) |
that's weird, the message got lost, let me re-send it 🤔 |
pkg/util/proxyutil/proxyutil.go
Outdated
// match all | ||
if v == "[]" { | ||
keepCookies[c.Name] = c | ||
continue | ||
} | ||
|
||
l := len(v) - 2 | ||
if v[l:] == "[]" { | ||
if len(c.Name) >= l && c.Name[:l] == v[:l] { | ||
keepCookies[c.Name] = c | ||
} | ||
} else { | ||
// look for exact match | ||
if c.Name == v { | ||
keepCookies[c.Name] = c | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested to use the strings
package to be a bit more readable:
if strings.HasSuffix(v, "[]") {
// match prefix
pattern := strings.TrimSuffix(v, "[]")
if strings.HasPrefix(c.Name, pattern) {
keepCookies[c.Name] = c
}
} else {
// exact match
if c.Name == v {
keepCookies[c.Name] = c
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* Match allowed cookies with optional character * Use strings package
What is this feature?
This adds a new option for "Allowed Cookies". Allowed cookies can be determined via their name or an optional part.
This optional part will be matched with
[]
. If the allowed cookie name ends with[]
we match all cookies that start with the given string.Example:
Allowed Cookie:
cookie[]
Matched Cookies:
cookie1, cookie2, cookie__special, cookieeeeee
NOT Matched Cookies:
supercookie1, super_cookie2, special_cookie__special
More details are here: https://github.com/grafana/observability-metrics-squad/issues/124
This PR is an up-to-date version of this PR. The old one had some weird change list so I created a new PR to prevent confusion.
Why do we need this feature?
To have an easier matcher for allowed cookies.
Which issue(s) does this PR fix?:
Fixes https://github.com/grafana/observability-metrics-squad/issues/124
Fixes https://github.com/grafana/observability-metrics-squad/issues/102
Special notes for your reviewer:
Please check that: