HTTP Basic Auth filter#30079
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
d51179b to
947f186
Compare
There was a problem hiding this comment.
Sounds a reasonable contribution, thanks. But I think at lease one maintainer sponsor and two reviewers for this new core extension.
All extensions must be sponsored by an existing maintainer. Sponsorship means that the maintainer will shepherd the extension through design/code reviews. Maintainers can self-sponsor extensions if they are going to write them, shepherd them, and maintain them.
Each extension must have two reviewers proposed for reviewing PRs to the extension. Neither of the reviewers must be a senior maintainer. Existing maintainers (including the sponsor) and other contributors can count towards this number. The initial reviewers will be codified in the CODEOWNERS file for long term maintenance. These reviewers can be swapped out as needed.
See https://github.com/envoyproxy/envoy/blob/main/EXTENSION_POLICY.md for more detail.
If you are willing, I can help to review or even sponsor it (this extension should be simple and won't take too much bandwidth), but we still need another reviewer to ensure the quality of extension.
Or you can add this extension as contrib extension. contrib extension have a relatively loose requirement.
api/envoy/extensions/filters/http/basic_auth/v3/basic_auth.proto
Outdated
Show resolved
Hide resolved
mattklein123
left a comment
There was a problem hiding this comment.
Seems OK to me to add if @wbpcode is willing to review.
|
@wbpcode Since we agreed on the API, I converted this PR to draft and proceed with implementation. |
750e8b2 to
ac45d4c
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
ac45d4c to
51dc663
Compare
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
wbpcode
left a comment
There was a problem hiding this comment.
LGTM. Thanks.
Hi, cc @mattklein123 could you also give a quick check in case I am missing something?
cpakulski
left a comment
There was a problem hiding this comment.
Few typos.
Should integration test be added to make sure that it works e-2-e?
docs/root/configuration/http/http_filters/basic_auth_filter.rst
Outdated
Show resolved
Hide resolved
docs/root/configuration/http/http_filters/basic_auth_filter.rst
Outdated
Show resolved
Hide resolved
SGTM. But I have no strong opinion on this because this extension is pretty simple. |
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Should I add an e-2-e test? Looks like there are no e-2-e tests for HTTP filters in the test/integration directory, or I looked into the wrong place? |
|
Integration tests are placed in sub directory of filter itself. You can check |
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
|
@wbpcode @cpakulski Integration test has been added. PTAL. |
|
/retest |
The implementation for HTTP Basic Auth filter.
Description: HTTP Basic Authn is a frequently requested feature from users, I'd like to have Envoy to support it.
Related issue: #15365
Sponsor: wbpcode
Risk Level: low
Testing: Yes
Docs Changes: Yes
Release Notes: Yes
Platform Specific Features: No
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]