cors: allow using regexes to match origins #3769
Conversation
Resolves envoyproxy#2526 Signed-off-by: Neri Marschik <codesuki@users.noreply.github.com>
| if (allowOriginRegexes() == nullptr) { | ||
| return false; | ||
| } | ||
| for (const auto& regex : *allowOriginRegexes()) { |
There was a problem hiding this comment.
Would have been great if for() just gets skipped when allowOriginRegexes is a nullptr, like in Go.
Signed-off-by: Neri Marschik <codesuki@users.noreply.github.com>
There was a problem hiding this comment.
This looks great. Just a couple things, one of which is that per #2832 and the recent announcement, we're not taking any further v1 API changes in advance of removing it altogether. Please revert the rds_json.cc change and the API v1 docs.
api/envoy/api/v2/route/route.proto
Outdated
| @@ -303,6 +303,9 @@ message CorsPolicy { | |||
|
|
|||
| // Specifies if CORS is enabled. Defaults to true. Only effective on route. | |||
| google.protobuf.BoolValue enabled = 7; | |||
|
|
|||
| // Specifies regex patterns that match allowed origins. | |||
| repeated string allow_origin_regex = 8; | |||
There was a problem hiding this comment.
I think it's worth moving this up, next to allow_origin since they're releated (and add // [#comment:next free field: 9] just above the message CorsPolicy).
I think it's also woth a comment to indicate that an origin that matches either allow_origin or allow_origin_regex will be allowed.
Signed-off-by: Neri Marschik <codesuki@users.noreply.github.com>
|
Done! I changed the links in the doc to be more helpful. |
| @@ -15,6 +15,7 @@ upstream cluster to route to or whether to perform a redirect. | |||
| "name": "...", | |||
| "domains": [], | |||
| "routes": [], | |||
| "cors": {}, | |||
There was a problem hiding this comment.
revert? Or was this missing previously?
There was a problem hiding this comment.
This was missing previously. We have a cors config on vhost and route, I noticed that field was missing in the docs.
There was a problem hiding this comment.
Below in this file we have
:ref:`cors <config_http_conn_man_route_table_cors>`
(optional, object) Specifies the virtual host's CORS policy.
Signed-off-by: Neri Marschik <codesuki@users.noreply.github.com>
Description: Adds
allow_origin_regexfield to the CORS config which is a list of regexes to match against. First the fixed strings inallow_originare checked and afterwards the regexes.If it's preferable to to only allow one regex or a boolean switch for the current
allow_origins, let me know.Risk Level: Medium
Testing: Unit and integration tests.
Docs Changes: Yes
Release Notes: cors: allow using regexes to match origins
Resolves #2526
Signed-off-by: Neri Marschik codesuki@users.noreply.github.com