-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
test, oauth2: Make sure config test runs field validation #13496
Conversation
This makes sure config tests for oauth2 filter checks fields validations. This also handles nullptr values for token_secret and hmac_secret fields due to misconfigurations (for example: token_secret: {}). Example config in the docs is also updated. Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
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.
Thanks! Just a few comments
@@ -81,7 +81,7 @@ class HttpFilterNameValues { | |||
// AWS Lambda filter | |||
const std::string AwsLambda = "envoy.filters.http.aws_lambda"; | |||
// OAuth filter | |||
const std::string OAuth = "envoy.filters.http.oauth"; | |||
const std::string OAuth = "envoy.filters.http.oauth2"; |
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.
Is this safe to change? I think we prefer to use the proto type to identify the filter to create, so I think yes?
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.
Yes. This is for consistency since we print the name of extensions at startup, i.e. [info][main] [external/envoy/source/server/server.cc:309]: envoy.filters.http: ...
Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
@snowp I think have updated the patch following your comments. Please help to take a look. Thank you! |
My bad. One of the tests was failed since missing secret names. |
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.
Thanks!
/retest |
Retrying Azure Pipelines, to retry CircleCI checks, use |
* master: (22 commits) delay health checks until transport socket secrets are ready. (envoyproxy#13516) test, oauth2: Make sure config test runs field validation (envoyproxy#13496) [http] swap codec implementations to default new (envoyproxy#13579) wasm: update proxy-wasm-cpp-host (envoyproxy#13606) postgres: do not copy and linearize received data when it is not going to be used (envoyproxy#13393) configs: Update configs v2 -> v3 (envoyproxy#13562) http2: Remove RELEASE_ASSERTs in sendPendingFrames() error handling (envoyproxy#13546) dependencies: track untracked implied dependencies, wrapup dashboard. (envoyproxy#13571) listener: add match all filter chain (envoyproxy#13449) fix mistakes in docstrings (envoyproxy#13603) ratelimit: add route entry metadata to ratelimit actions (envoyproxy#13269) cluster manager: avoid immediate activation for dynamic inserted cluster when initialize (envoyproxy#12783) ext_authz: Avoid calling check multiple times (envoyproxy#13288) docs: Unexclude remaining configs from validation (envoyproxy#13534) build: update rules_rust to allow Rustc in RBE (envoyproxy#13595) docs: Update sphinxext.rediraffe (envoyproxy#13589) Deprecate moonjit support on Windows before beta (envoyproxy#13541) dependencies: bump LuaJIT to 2.1 branch HEAD @ e9af1ab. (envoyproxy#13474) docs: add TLS stats to cluster stats doc (envoyproxy#13561) ci: stop building alpine-debug images in favor of ubuntu-based debug image (envoyproxy#13598) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
Commit Message: This makes sure config tests for oauth2 filter checks field validations. This also handles possible
nullptr
values fortoken_secret
andhmac_secret
fields due to misconfigurations (for example:token_secret: {}
). The example config in the docs is also updated.Risk Level: Low
Testing: Updated
Docs Changes: Updated example config
Release Notes: N/A
Fixes #13023
Signed-off-by: Dhi Aurrahman dio@tetrate.io