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.

Sign up for GitHub

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

Jump to bottom

Eliminate/prevent dependencies on extensions from core #9953

Closed
mergeconflict opened this issue Feb 6, 2020 · 7 comments · Fixed by #20614
Closed

Eliminate/prevent dependencies on extensions from core #9953

mergeconflict opened this issue Feb 6, 2020 · 7 comments · Fixed by #20614
Assignees
@alyssawilk
Labels
area/build help wanted Needs help!
Milestone
1.22.0

Comments

@mergeconflict
Copy link
Member

There are some cases where core Envoy code (or tests) depends on extensions. One example:

//source/extensions/access_loggers/grpc:http_config
//source/extensions/access_loggers/grpc:tcp_config

are used by:

//test/integration/http2_upstream_integration_test
//test/common/access_log/access_log_impl_test

This poses an issue for users like Google, who import only specific whitelisted extensions. We have to disable tests that depend on non-whitelisted extensions. I think we'd like to add some logic to check_format or similar, to catch these dependencies before they're introduced.

Not sure if this is related to #7903, likely seems related to #2910.
@mattklein123
Copy link
Member

+1 yes please. If you are going to fix this can you also potentially fix #6736? It's related in the sense that by default we build and link all extensions into some of the server integration tests. Optimally the core integration tests should depend on only a build with a very limited set of extensions.

@mattklein123 mattklein123 added this to the 1.15.0 milestone May 5, 2020
@mattklein123
Copy link
Member

I think @yanavlasov is working on this. Also discussing fixing #6736 as well.

@mattklein123 mattklein123 modified the milestones: 1.15.0, 1.16.0 Jun 24, 2020
@antoniovicente
Copy link
Contributor

Related: #12315 #12314

@alyssawilk alyssawilk mentioned this issue Jul 28, 2020
Closed
@alyssawilk alyssawilk self-assigned this Jul 28, 2020
@alyssawilk alyssawilk mentioned this issue Jul 28, 2020
Merged
@alyssawilk alyssawilk removed their assignment Jul 28, 2020
lizan pushed a commit that referenced this issue Aug 1, 2020
@alyssawilk
build: marking extensions as extension-only visible by default (#12337)
da403fa 
Risk Level: medium (of build breakage)
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Part of #9953

Signed-off-by: Alyssa Wilk <[email protected]>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this issue Aug 7, 2020
@alyssawilk
build: marking extensions as extension-only visible by default (envoy…
d2ebac4 
…proxy#12337)

Risk Level: medium (of build breakage)
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
chaoqin-li1123 pushed a commit to chaoqin-li1123/envoy that referenced this issue Aug 7, 2020
@alyssawilk
build: marking extensions as extension-only visible by default (envoy…
e30a45e 
…proxy#12337)

Risk Level: medium (of build breakage)
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: chaoqinli <[email protected]>
@mattklein123 mattklein123 modified the milestones: 1.16.0, 1.17.0 Oct 4, 2020
@tehnerd tehnerd mentioned this issue Jan 7, 2021
Merged
@mattklein123 mattklein123 modified the milestones: 1.17.0, 1.18.0 Jan 7, 2021
@ppadevski ppadevski mentioned this issue Jan 12, 2021
Open
@antoniovicente
Copy link
Contributor

Tests for core extension (TLS) depending on non-core extension with new cert verifier functionality -> broken build if you only want the transport_sockets TLS extension.

#14884

@mathetake mathetake mentioned this issue Mar 5, 2021
Merged
@mattklein123 mattklein123 modified the milestones: 1.18.0, 1.19.0 Apr 25, 2021
@alyssawilk alyssawilk mentioned this issue May 25, 2021
Merged
@mattklein123 mattklein123 removed this from the 1.19.0 milestone Jul 7, 2021
alyssawilk added a commit that referenced this issue Dec 14, 2021
@alyssawilk
test: fixing grpc access_logger test visibility (#19267)
4137a96 
part of #9953

Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk alyssawilk modified the milestones: 1.21.0, 1.22.0 Jan 10, 2022
@alyssawilk alyssawilk mentioned this issue Jan 12, 2022
Merged
alyssawilk added a commit that referenced this issue Jan 13, 2022
@alyssawilk
http: removing deprecated filter names (#19522)
b3285f0 
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: inline
part of #9953

Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk alyssawilk mentioned this issue Feb 2, 2022
Merged
htuch pushed a commit that referenced this issue Feb 9, 2022
@alyssawilk
test: fixing build dependencies (#19783)
489807b 
Part of #9953

Signed-off-by: Alyssa Wilk <[email protected]>
joshperry pushed a commit to joshperry/envoy that referenced this issue Feb 13, 2022
@alyssawilk
test: fixing grpc access_logger test visibility (envoyproxy#19267)
05c3ddc 
part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Josh Perry <[email protected]>
joshperry pushed a commit to joshperry/envoy that referenced this issue Feb 13, 2022
@alyssawilk
http: removing deprecated filter names (envoyproxy#19522)
be49615 
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: inline
part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Josh Perry <[email protected]>
joshperry pushed a commit to joshperry/envoy that referenced this issue Feb 13, 2022
@alyssawilk
test: fixing build dependencies (envoyproxy#19783)
19a9970 
Part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: Josh Perry <[email protected]>
@mattklein123
Copy link
Member

I think @alyssawilk has been picking away at this. @alyssawilk are we done with this or is there more to do?

@alyssawilk
Copy link
Contributor

Down from ~30 to ~6 but not compelte

source/extensions/filters/network/common/BUILD: # Used by core. TODO(#9953) clean up.
source/extensions/filters/http/buffer/BUILD: # Legacy test use. TODO(#9953) clean up.
source/extensions/filters/http/ip_tagging/BUILD: # TODO(#9953) clean up.
source/extensions/filters/listener/tls_inspector/BUILD: # TODO(#9953) clean up.
source/extensions/filters/listener/tls_inspector/BUILD: # TODO(#9953) clean up.
source/extensions/access_loggers/file/BUILD: # TODO(#9953) determine if this is core or should be cleaned up.

This was referenced Mar 29, 2022
Merged
Merged
Merged
Merged
Closed
alyssawilk added a commit that referenced this issue Mar 30, 2022
@alyssawilk
test: creating a test listener filter and moving the xds test over to…
2757eb9 
… it (#20571)

Part of #9953

Signed-off-by: Alyssa Wilk <[email protected]>
alyssawilk added a commit that referenced this issue Mar 30, 2022
@alyssawilk
builld: tweaking visibility rules (#20552)
0ab9440 
Risk Level: n/a (visibility only)
part of #9953
Signed-off-by: Alyssa Wilk <[email protected]>
alyssawilk added a commit that referenced this issue Mar 30, 2022
@alyssawilk
test: moving the tls inspector test to the extension directory (#20588)
f9d936e 
Part of #9953
Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk alyssawilk mentioned this issue Mar 31, 2022
Merged
alyssawilk added a commit that referenced this issue Mar 31, 2022
@alyssawilk
test: removing deps on the tls inspector extension (#20614)
f35221f 
Fixes #9953

Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk
Copy link
Contributor

DONE.

ravenblackx pushed a commit to ravenblackx/envoy that referenced this issue Jun 8, 2022
@alyssawilk @ravenblackx
test: creating a test listener filter and moving the xds test over to…
3c45ae0 
… it (envoyproxy#20571)

Part of envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
ravenblackx pushed a commit to ravenblackx/envoy that referenced this issue Jun 8, 2022
@alyssawilk @ravenblackx
builld: tweaking visibility rules (envoyproxy#20552)
a39849a 
Risk Level: n/a (visibility only)
part of envoyproxy#9953
Signed-off-by: Alyssa Wilk <[email protected]>
ravenblackx pushed a commit to ravenblackx/envoy that referenced this issue Jun 8, 2022
@alyssawilk @ravenblackx
test: moving the tls inspector test to the extension directory (envoy…
9a648ac 
…proxy#20588)

Part of envoyproxy#9953
Signed-off-by: Alyssa Wilk <[email protected]>
ravenblackx pushed a commit to ravenblackx/envoy that referenced this issue Jun 8, 2022
@alyssawilk @ravenblackx
test: removing deps on the tls inspector extension (envoyproxy#20614)
02fdd41 
Fixes envoyproxy#9953

Signed-off-by: Alyssa Wilk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alyssawilk alyssawilk

Labels
area/build help wanted Needs help!
Projects
None yet
Milestone
1.22.0
Development

Successfully merging a pull request may close this issue.

test: removing deps on the tls inspector extension alyssawilk/envoy
5 participants
@mergeconflict @mattklein123 @yanavlasov @alyssawilk @antoniovicente