Ext_Authz duration dynamic metadata by llu94 · Pull Request #19316 · envoyproxy/envoy

llu94 · 2021-12-19T01:38:32Z

Risk Level: low
Testing: unit testing
Docs Changes: n/a
Release Notes: inline

Issue Link: #19255

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

repokitteh-read-only · 2021-12-19T01:38:36Z

Hi @llu94, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #19316 was opened by llu94.

see: more, trace.

repokitteh-read-only · 2021-12-19T01:38:41Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @adisuissa
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #19316 was opened by llu94.

see: more, trace.

adisuissa

Thanks for working on this.
Not sure why you get the API changes, but they LGTM.
Left a few comments on the rest of the code.
/lgtm api
/wait

source/extensions/filters/http/ext_authz/ext_authz.cc

api/envoy/config/filter/http/jwt_authn/v2alpha/config.proto

source/extensions/filters/network/ext_authz/ext_authz.cc

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

dio

Thanks for working on this. A couple of comments from me:

source/common/stream_info/utility.cc

source/extensions/filters/http/ext_authz/ext_authz.cc

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

jmarantz · 2022-01-07T15:24:18Z

@llu94 please do not force-push to github PRs that are in review: https://github.com/envoyproxy/envoy/blob/main/CONTRIBUTING.md has detail about why that's bad and what to do instead.

source/common/stream_info/utility.cc

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

source/extensions/filters/network/ext_authz/ext_authz.cc

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

adisuissa

LGTM, thanks.
/assign-from @envoyproxy/senior-maintainers

repokitteh-read-only · 2022-01-21T03:22:43Z

@envoyproxy/senior-maintainers assignee is @zuercher

🐱

Caused by: a #19316 (review) was submitted by @adisuissa.

see: more, trace.

zuercher

Thanks! Please see my comments below.

source/extensions/filters/network/ext_authz/ext_authz.cc

docs/root/configuration/http/http_filters/ext_authz_filter.rst

source/extensions/filters/http/ext_authz/ext_authz.cc

test/extensions/filters/http/ext_authz/ext_authz_test.cc

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 · 2022-01-28T19:07:55Z

@zuercher I have made some changes to the test in line with your suggestions using a simulated time system.

test/extensions/filters/network/ext_authz/ext_authz_test.cc

test/extensions/filters/http/ext_authz/ext_authz_test.cc

source/extensions/filters/http/ext_authz/ext_authz.h

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

zuercher

Thanks!

llu94 · 2022-02-02T18:46:47Z

No worries. Thanks to you and all of the other reviewers for guiding me through this first PR. I am by no means good with C++ and this code base is pretty daunting at times.

adisuissa

LGTM, thanks!

llu94 · 2022-02-03T02:34:16Z

I can't merge it. Not authorized. Can someone else do it for me?

Adds a dynamic metadata field named ext_authz_duration with the duration of completed ext_authz requests. Risk Level: low Testing: unit testing Docs Changes: documented new dynamic metadata Release Notes: n/a Fixes Issue: envoyproxy#19255 Signed-off-by: Lucas Lu <lucasludev@gmail.com> Signed-off-by: Josh Perry <josh.perry@mx.com>

llu94 added 3 commits December 18, 2021 00:22

http namespace ext_authz dynamic metadata done

a3f46c0

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

ext_authz for network namespace ready

8399867

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

code formatter changes

e3c90d3

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 requested a review from dio as a code owner December 19, 2021 01:38

repokitteh-read-only bot added v2-freeze api labels Dec 19, 2021

repokitteh-read-only bot assigned adisuissa Dec 19, 2021

junr03 assigned dio Dec 20, 2021

adisuissa requested changes Dec 21, 2021

View reviewed changes

repokitteh-read-only bot added waiting and removed api v2-freeze labels Dec 21, 2021

llu94 added 2 commits December 21, 2021 22:58

respond to PR feedback

f178ebd

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

respond to PR feedback and apply fix format diff

ad0727d

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

repokitteh-read-only bot removed the waiting label Dec 22, 2021

llu94 added 2 commits December 21, 2021 23:04

remove unnecessary function

9911284

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

apply formatting fix

14ab553

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

dio reviewed Dec 27, 2021

View reviewed changes

source/common/stream_info/utility.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/ext_authz/ext_authz.cc Outdated Show resolved Hide resolved

source/extensions/filters/http/ext_authz/ext_authz.cc Show resolved Hide resolved

llu94 added 2 commits December 29, 2021 16:13

make changes in line with PR feedback

530ad73

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

add formatter fix

b4345a0

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 force-pushed the authz_duration branch from c2663e4 to b4345a0 Compare December 30, 2021 03:35

llu94 requested review from adisuissa and dio January 4, 2022 18:19

adisuissa reviewed Jan 7, 2022

View reviewed changes

source/common/stream_info/utility.cc Outdated Show resolved Hide resolved

llu94 added 2 commits January 9, 2022 23:20

changes in response to PR feedback

7b72fd7

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

remove extra artifact

fcffd56

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 force-pushed the authz_duration branch from c980717 to fcffd56 Compare January 10, 2022 18:32

llu94 added 4 commits January 14, 2022 18:43

adjust test coverage

a333bd0

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

adjust test coverage

940c82b

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

formatting changes

9806b91

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

Adjust test coverage for network ext_authz

f874200

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

adisuissa reviewed Jan 18, 2022

View reviewed changes

source/extensions/filters/network/ext_authz/ext_authz.cc Outdated Show resolved Hide resolved

remove helper methods

bd15dd1

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

adisuissa previously approved these changes Jan 21, 2022

View reviewed changes

repokitteh-read-only bot assigned zuercher Jan 21, 2022

zuercher requested changes Jan 21, 2022

View reviewed changes

llu94 added 2 commits January 22, 2022 22:25

Documentation and timing implementation changes

c51a383

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

formatting changes

e3f576e

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 dismissed adisuissa’s stale review via e3f576e January 24, 2022 16:53

llu94 added 4 commits January 27, 2022 12:52

renaming/refactoring http ext_authz

c40a796

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

add tests to http ext authz

4427dda

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

reformatted ext_authz functions

a6ece49

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

add timing tests for network ext_authz. Reformatting

370b958

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

llu94 requested review from adisuissa and zuercher January 31, 2022 18:46

zuercher reviewed Feb 1, 2022

View reviewed changes

ext_authz filter tests adjusted; public time methods removed

e2a202a

Signed-off-by: Lucas Lu <lucasludev@gmail.com>

zuercher approved these changes Feb 2, 2022

View reviewed changes

adisuissa approved these changes Feb 3, 2022

View reviewed changes

zuercher merged commit 4d6ce6d into envoyproxy:main Feb 3, 2022

llu94 deleted the authz_duration branch March 27, 2022 02:25

Conversation

llu94 commented Dec 19, 2021

Uh oh!

repokitteh-read-only bot commented Dec 19, 2021

Uh oh!

repokitteh-read-only bot commented Dec 19, 2021

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmarantz commented Jan 7, 2022

Uh oh!

Uh oh!

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

repokitteh-read-only bot commented Jan 21, 2022

Uh oh!

zuercher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llu94 commented Jan 28, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuercher left a comment

Choose a reason for hiding this comment

Uh oh!

llu94 commented Feb 2, 2022

Uh oh!

adisuissa left a comment

Choose a reason for hiding this comment

Uh oh!

llu94 commented Feb 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants