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

extensions: Add extension categories in envoy_cc_extension #14744

Merged
merged 52 commits into from
Feb 5, 2021

Conversation

phlax
Copy link
Member

@phlax phlax commented Jan 19, 2021

Commit Message: extensions: Add extension categories in envoy_cc_extension
Additional Description:

This is a breakout PR from #14721

atm it just marks the category as SOMECAT as a marker. I will go through and add correct categories...

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

phlax added 10 commits January 19, 2021 12:57
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented Jan 20, 2021

@htuch hopefully this is doing the right thing - at least so far.

to figure out the categories i have used the following command:

$ git grep "std::string category() const override " | cut -d' ' -f9 | xargs -I{} bash -c "X={} echo \${X}" | sort | uniq
envoy.access_logger.extension_filters
envoy.access_loggers
envoy.bootstrap
envoy.clusters
envoy.compression.compressor
envoy.compression.decompressor
envoy.dubbo_proxy.filters
...

hopefully this is the ~canonical way to derive the categories.

ill start matching them now...

Signed-off-by: Ryan Northey <[email protected]>
@phlax phlax requested a review from cpakulski as a code owner January 20, 2021 13:11
phlax added 9 commits January 25, 2021 15:10
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
bazel/envoy_library.bzl Outdated Show resolved Hide resolved
Signed-off-by: Ryan Northey <[email protected]>
@phlax
Copy link
Member Author

phlax commented Jan 25, 2021

@htuch apologies for delay on this, i was fighting starlark syntax.

i think this is ready for final review.

the only one left with SOMECAT is the crypto (~not) extension

bazel/envoy_library.bzl Outdated Show resolved Hide resolved
Signed-off-by: Ryan Northey <[email protected]>
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM modulo TODOs and resolving the SSL implementation status with @lizan.

@phlax phlax requested a review from Shikugawa as a code owner February 5, 2021 15:32
@phlax
Copy link
Member Author

phlax commented Feb 5, 2021

@htuch this should be gtg once tests have passed

phlax added 2 commits February 5, 2021 16:26
Signed-off-by: Ryan Northey <[email protected]>
Signed-off-by: Ryan Northey <[email protected]>
"envoy.guarddog_actions",
"envoy.health_checkers",
"envoy.internal_redirect_predicates",
"envoy.io_socket",
Copy link
Member Author

Choose a reason for hiding this comment

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

@htuch i have added envoy.io_socket category reflecting what was in the source/extensions/extensions_build_config.bzl file

afaict there is not a factory for it tho, not sure of the implications

Copy link
Member

Choose a reason for hiding this comment

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

SGTM

@phlax
Copy link
Member Author

phlax commented Feb 5, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14744 (comment) was created by @phlax.

see: more, trace.

@lizan lizan merged commit 06f9002 into envoyproxy:main Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants