Skip to content
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

Introduce shared gRPC authorization interceptor #5515

Merged
merged 4 commits into from
Jun 23, 2021

Conversation

axw
Copy link
Member

@axw axw commented Jun 22, 2021

Motivation/summary

Introduce a new gRPC authorization interceptor which is used for both OTLP and Jaeger. This pulls authorization up to the interceptor level, and out of individual methods (in the case of Jaeger). This will enable us to introduce another interceptor for rate limiting anonymous agents.

The new authorization interceptor dispatches to method-specific handlers. All OTLP methods use the "authorization" metadata approach, whereas Jaeger has different approaches depending on the method: extract process tags for PostSpans, and anonymous auth only for GetSamplingStrategy.

How to test these changes

Non-functional change.

Related issues

#5347

Introduce a new gRPC authorization interceptor which
is used for both OTLP and Jaeger. This new interceptor
dispatches to method-specific handlers. All OTLP methods
use the "authorization" metadata, whereas Jaeger has
different approaches depending on the method: extract
process tags for PostSpans, and anonymous auth only for
GetSamplingStrategy.
@axw axw added the v7.14.0 label Jun 22, 2021
@axw axw requested a review from a team June 22, 2021 09:43
@apmmachine
Copy link
Contributor

apmmachine commented Jun 22, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #5515 updated

  • Start Time: 2021-06-22T11:50:11.473+0000

  • Duration: 37 min 43 sec

  • Commit: fbba82a

Test stats 🧪

Test Results
Failed 0
Passed 6138
Skipped 120
Total 6258

Trends 🧪

Image of Build Times

Image of Tests

axw added 3 commits June 22, 2021 18:43
The response error message has changed, update the
test to expect the new one.
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how clean the handling with the interceptors is; great changes!

@axw axw merged commit 92335c5 into elastic:master Jun 23, 2021
@axw axw deleted the grpc-auth-interceptor branch June 23, 2021 01:18
mergify bot pushed a commit that referenced this pull request Jun 23, 2021
* Introduce shared gRPC authorization interceptor

Introduce a new gRPC authorization interceptor which
is used for both OTLP and Jaeger. This new interceptor
dispatches to method-specific handlers. All OTLP methods
use the "authorization" metadata, whereas Jaeger has
different approaches depending on the method: extract
process tags for PostSpans, and anonymous auth only for
GetSamplingStrategy.

* beater/otlp: revert metrics change

* tests/system: update Jaeger system test

The response error message has changed, update the
test to expect the new one.

* beater/jaeger: fix logger name

(cherry picked from commit 92335c5)
axw added a commit that referenced this pull request Jun 23, 2021
* Introduce shared gRPC authorization interceptor

Introduce a new gRPC authorization interceptor which
is used for both OTLP and Jaeger. This new interceptor
dispatches to method-specific handlers. All OTLP methods
use the "authorization" metadata, whereas Jaeger has
different approaches depending on the method: extract
process tags for PostSpans, and anonymous auth only for
GetSamplingStrategy.

* beater/otlp: revert metrics change

* tests/system: update Jaeger system test

The response error message has changed, update the
test to expect the new one.

* beater/jaeger: fix logger name

(cherry picked from commit 92335c5)

Co-authored-by: Andrew Wilkins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants