udpa: filesystem list collection support for inline entries. #13028
Conversation
This is the first PR towards filesystem support for file:/// URLs compatible with envoyproxy#11264. Currently it plumbs in only LDS filesystem support for list collections with only inline entries. Risk level: Low (opt in) Testing: WiP, will finalize PR when these are ready. Signed-off-by: Harvey Tuch <[email protected]>
|
@chaoqin-li1123 FYI re: common plumbing from LDS to subscription factory. |
Signed-off-by: Harvey Tuch <[email protected]>
|
/cc stevenzzzz |
Signed-off-by: Harvey Tuch <[email protected]>
htuch
changed the title
Signed-off-by: Harvey Tuch <[email protected]>
|
The |
| return std::make_unique<Config::FilesystemCollectionSubscriptionImpl>( | ||
| dispatcher_, path, callbacks, resource_decoder, stats, validation_visitor_, api_); | ||
| } | ||
| default: |
There was a problem hiding this comment.
Is this checked by schema? If not, maybe better to not use default and specify all of the options so we get a compile error if something is added?
There was a problem hiding this comment.
This is an enum, so you have weird values like ResourceLocator_Scheme_ResourceLocator_Scheme_INT_MIN_SENTINEL_DO_NOT_USE_
| @@ -79,6 +81,222 @@ TEST_F(FilesystemSubscriptionImplTest, UpdateTimeChangedOnUpdateSuccess) { | |||
| EXPECT_TRUE(statsAre(3, 2, 0, 0, 0, TEST_TIME_MILLIS + 1, 7148434200721666028, "0")); | |||
| } | |||
|
|
|||
| // TODO(htuch): Add generic test harness support for collection subscriptions so that we can test | |||
| // gRPC/HTTP transports similar to below. | |||
| class FilesystemCollectionSubscriptionImplTest : public testing::Test, | |||
There was a problem hiding this comment.
I'm wondering, per my comment above in the prof code about shared code, could the tests here actually be a TEST_P that somehow creates a collection or not and just runs all the same tests for the most part to verify the same stats, etc.?
There was a problem hiding this comment.
I plan on a followup PR to address this as per the TODO.
There was a problem hiding this comment.
OK, seems like a lot of duplication but up to you.
There was a problem hiding this comment.
The more time I spent looking at these tests and what coverage they provide, the more I realized they don't actually do a whole lot for some of the unhappy path cases that I've been adding in this PR. I definitely want to reform this entire set of tests before moving forward with any further UDPA PRs, since they are key to being able to validate implementations for both filesystem and gRPC. This is probably going to be a series of PRs that don't have a ton to do with the filesystem list collection support per se, but they would benefit from having it landed as a forcing function.
There was a problem hiding this comment.
Sure sounds fine to do as followup cleanups if you are tracking.
test/config/integration/server_xds.lds.udpa.list_collection.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
Signed-off-by: Harvey Tuch <[email protected]>
…code * upstream/master: lint: add more linters for using absl:: over std:: (envoyproxy#13043) udpa: filesystem list collection support for inline entries. (envoyproxy#13028) filter: http: jwt: implement matching for HTTP CONNECT (envoyproxy#13064) [fuzz] split http filter logic into a fuzzing class (envoyproxy#13016) xds: allow empty delta update (envoyproxy#12699) CacheFilter: parses the allowed_vary_headers from the cache config. (envoyproxy#12928) router: extend HTTP CONNECT route matching criteria (envoyproxy#13056) docs: clarify use of Extended CONNECT for h/2 (envoyproxy#13051) build: shellcheck tools/ (envoyproxy#13007) [fuzz] Refactored Health Checker Impl Tests (envoyproxy#13017) Signed-off-by: Lihao Luo <[email protected]>
This is the first PR towards filesystem support for file:/// URLs
compatible with #11264. Currently it plumbs in only LDS filesystem
support for list collections with only inline entries.
Risk level: Low (opt in)
Testing: Unit and integration tests added.
Signed-off-by: Harvey Tuch [email protected]