fix(kubernetes_logs source): Handle OK response from api_watcher with embedded desync elegantly

[eeyun]

Handles the case where a watcher desync response returns embedded inside a 200 OK.

We've refactored our API here to remove the k8s_openapi::WatchResponse type as in our investigation it became clear that the extra abstraction was unnecessary for us. In removing that type we've provider our own internal Response type which allows us to simplify the redirection here.

Closes #5877
Closes #5846
Signed-off-by: Ian Henry ianjhenry00@gmail.com

[binarylogic]

Cc @tustvold in case you want to review.

[binarylogic]

@eeyun does this close an issue?

[MOZGIII]

All looks good, but we need more tests cases at API Watcher to cover the rest of the input kinds, as discussed.

• non-desync status error
• invalid json
• unexpected resource kind (i.e. Pod expected but Namespace was sent)

Also, ideally:

• network error at the stream level
• network error at the invocation level

[MOZGIII]

I think we want to be able to trigger and test more of the possible stream item variants.

What's already covered:

• Ok(WatchResponse::Ok(WatchEvent::Added { ... }))
• Err(watcher::stream::Error::Desync { ... })) - should assert the underlying value (the ... should be a concrete value matching the test input to assert the data is porperly passed)
• Err(watcher::stream::Error::Other { ... })) - same as above
• Ok(WatchResponse::Ok(WatchEvent::ErrorStatus(Status { ... }))) (covered by the non-desync status error case)

What we probably should cover as well:

• Ok(WatchResponse::Ok(WatchEvent::ErrorOther(RawExtension { ... }))) - pass the response event with some non-standard type (i.e. extension_test)
• Ok(WatchResponse::Other(Ok(Some(serde_json::Value { ... }))) - pass a valid JSON that is not a valid WatchResponse - sth like {"a":"b"} should do
• Ok(WatchResponse::Other(Ok(None)) - pass nothing?
• Ok(WatchResponse::Other(Err(...)) - I presume this should be the value when passing an invalid JSON? It would be great to add a test that illustrates how to trigger this error

What we don't care about:

• other "heppy" WatchEvent variants: WatchEvent:: Deleted, WatchEvent::Modified, WatchEvent:: Bookmark - although, for completeness, we could add them too.

With this, we'll have pretty good test coverage.

[eeyun]

This is ready to be tested! @tustvold (or anyone interested in testing this case) if you're curious to test I've created a docker image from this PR that can be pulled down like so:

docker pull eeyun/vector-k8s-test:error_misfire

[MOZGIII]

Great job!

[uthng]

Hello,

Thanks a lot for the correction. But anyone can tell me when the new release including this is planed please ?