Improve google-pubsub test coverage #3910

[JiriOndrusek]

fixes https://github.com/apache/camel-quarkus/issues/3910

• Ordering test is disabled when the real account is used. Problem has to be inspected in detail, I reported this issue for the Camel, see https://issues.apache.org/jira/browse/CAMEL-18277
• I created a new test-support module for google-* extensions, which is similar to ..-aws2-support. In the future, more google extensions should use the test support module (which has to be extended for different google services) and therefore should have the same testing capabilities/functionality as the other google extensions
• There is a readme.adoc file, which describe how to use the real service for the test execution.

[jamesnetherton]

Maybe a flaky test:

2022-07-19T11:00:15.5493101Z [ERROR]   GooglePubsubTest.testOrdering:132 1 expectation failed.
2022-07-19T11:00:15.5493460Z Expected status code <201> but was <500>.

[JiriOndrusek]

I find some suspicious small issues - fixed and added some logging + test to wait for consumer to stop. I hope that these small changes will make tests successful.

[JiriOndrusek]

Another try, please do not merge (even if the result is successful)

[JiriOndrusek]

I still don't know why, but ordering test, as was created in the beginning, fails in CLI. The original test was sending 6 messages and receives them afterwards. In CLI there was a failure (without any obvious reason) with the 3rd message. Here is a log snippet:

022-07-27T13:37:14.9565463Z 2022-07-27 13:37:14,950 DEBUG [org.apa.cam.com.goo.pub.GooglePubsubProducer] (executor-thread-0) uploader thread/id: 126 / 64C35E7D9B618FD-0000000000000008. api call completed.
2022-07-27T13:37:14.9565962Z 2022-07-27 13:37:14,950 TRACE [org.apa.cam.pro.err.RedeliveryErrorHandler] (executor-thread-0) This exchange is not handled or continued so its marked as failed: Exchange[64C35E7D9B618FD-0000000000000008]
2022-07-27T13:37:14.9566895Z 2022-07-27 13:37:14,953 ERROR [org.apa.cam.pro.err.DefaultErrorHandler] (executor-thread-0) Failed delivery for (MessageId: 64C35E7D9B618FD-0000000000000008 on ExchangeId: 64C35E7D9B618FD-0000000000000008). Exhausted after delivery attempt: 1 caught: java.util.concurrent.ExecutionException: java.util.concurrent.CancellationException: Execution cancelled because executing previous runnable failed.

Ordering test in Camel Quarkus does not need to test functionality, but it should cover that CQ extension works with the requested configuration. In my POV it is enough to see that 2 messages are delivered during ordering test, because we can state, that it is possible to call ordering capability of the extension. Ordering itself is "executed" by the google pubsub provider.

I reported this "unknown" failure for further investigation as a ticket

I think that this PR could be merged and the ordering test is covering the ordering feature even with this stripped scenario.

[JiriOndrusek]

@aldettinger I fixed all your comments. I kept them not-resolved, to be able to find them (there are already some older resolved conversations)

[JiriOndrusek]

The CI failure in google-storage does not seem related to this changes (I checked both the files and run google-storage tests on local machine). I'm re-running the CI.

[aldettinger]

Checking the feature coverage:

• Producers => OK
• Consumers => OK
• Message body => OK
• AUTHENTICATION CONFIGURATION => I'm not sure ?
• ROLLBACK AND REDELIVERY => I think it is covered by the ACK test ?

Paying attention to the native test coverage of authentication might be a good investment. For instance, there are some reflections logic used in google's DefaultCredentialsProvider. I wonder if users could hit that ?

[JiriOndrusek]

Checking the feature coverage:

• Producers => OK
• Consumers => OK
• Message body => OK
• AUTHENTICATION CONFIGURATION => I'm not sure ?
• ROLLBACK AND REDELIVERY => I think it is covered by the ACK test ? est ?

yes

Paying attention to the native test coverage of authentication might be a good investment. For instance, there are some reflections logic used in google's DefaultCredentialsProvider. I wonder if users could hit that ?

I didn't check this google source, I agree that this code should be covered more carefully. I'll revisit authentication case again, because it could be possible, that I removed some coe, which was covering part of this use case during refactoring. (without noticing it is important)

[aldettinger]

Checking the feature coverage:

• Producers => OK
• Consumers => OK
• Message body => OK
• AUTHENTICATION CONFIGURATION => I'm not sure ?
• ROLLBACK AND REDELIVERY => I think it is covered by the ACK test ? est ?

yes

Paying attention to the native test coverage of authentication might be a good investment. For instance, there are some reflections logic used in google's DefaultCredentialsProvider. I wonder if users could hit that ?

I didn't check this google source, I agree that this code should be covered more carefully. I'll revisit authentication case again, because it could be possible, that I removed some coe, which was covering part of this use case during refactoring. (without noticing it is important)

@JiriOndrusek That could be the subject of another ticket ? What do you think ?

[JiriOndrusek]

~~As @aldettinger suggested in the above comment, authentication configuration should be covered more in detail to cover all possible scenarios (in native). Because the google authentication module is common in all google-* extensions, it makes sense to create another ticket covering this functionality. (and to let this PR to be merged). What do you think @jamesnetherton~~

EDITED

~~Use case from the camel doc is~~

By default this component aquires credentials using GoogleCredentials.getApplicationDefault(). This behavior can be disabled by setting authenticate option to false, in which case requests to Google API will be made without authentication details. This is only desirable when developing against an emulator. This behavior can be altered by supplying a path to a service account key file.

~~I debugged the executions and:~~

~~1. In emulated run Google API will be made without authentication details. is coverted 2. With real account, DefaultCredentialsProvider code is used. 3 There is another option ServiceAccountCredentials see camel code , which calls different google api. But it is not mentioned in the doc. We should cover it also, right? (@aldettinger , @jamesnetherton )~~

EDITED (2nd)

Let me investigate it once again, I miss-debug some calls, so my result are not correct (I'm sorry for disturbing you again and again)

[JiriOndrusek]

I looked into DefaultCredenbtialProvider

There are several ways of authentication for google clouds available, see the documentation.

There is no limitation listed nor in camel component doc neither in camel quarkus extension doc.

• Scenario with disabled authentication is covered via emulated tests.
• Basic scenario with GoogleCredentials.getApplicationDefault() using service account (via export of GOOGLE_APPLICATION_CREDENTIALS) is covered by using real account as is described in RADME.adoc file in the test module.

Which brings me to several findings:

1. It is not possible to cover all scenarios via emulated tests or via tests with real account. (which uses Accessing private data on behalf of a service account outside Google Cloud environments)
2. I think, that we can put into documentation, that only several ways of authentications are supported in native mode -> service account, by exporting GOOGLE_APPLICATION_CREDENTIALS
3. Another option is to manually cover different authentication ways mainly for native mode.
4. Current coverage is acceptable and there is no need to add more sub-scenarios.

I prefer option 2 - to list limitations for native mode.

What do you think? @aldettinger @jamesnetherton @ppalaga

[aldettinger]

@JiriOndrusek That's great investigation :+1: If we take it from camel-google-pusbsub component/endpoint options, then authenticate true/fase is covered ? However serviceAccountKey is not covered ?

If that's the case, then couldn't we document that serviceAccountKey is not yet supported in native mode. And then add support of serviceAccountKey in another ticket, maybe with a manual test.

[JiriOndrusek]

@JiriOndrusek That's great investigation +1 If we take it from camel-google-pusbsub component/endpoint options, then authenticate true/fase is covered ? However serviceAccountKey is not covered ?

If that's the case, then couldn't we document that serviceAccountKey is not yet supported in native mode. And then add support of serviceAccountKey in another ticket, maybe with a manual test.

I reported a ticket for missing authentications - https://github.com/apache/camel-quarkus/issues/3970

[JiriOndrusek]

Hi @aldettinger , do you see any remaining work on this PR or may it be approved/merged?

[aldettinger]

@JiriOndrusek Yes, there are still 2 unresolved conversations in this PR.

[JiriOndrusek]

@JiriOndrusek Yes, there are still 2 unresolved conversations in this PR.

I'm sorry, I missed them. I'll fix them both, thanks for reminding me

[JiriOndrusek]

@aldettinger I went through the all conversations and all are resolved now. (Once again I'm sorry for missing the opened ones)

[JiriOndrusek]

Work on this PR is done

[JiriOndrusek]

fixes https://github.com/apache/camel-quarkus/issues/3957

[JiriOndrusek]

This PR is ready to merge. I'm finishing another PR which depends on this one. Would it be possible to merge this PR? @jamesnetherton @ppalaga

(My last change which fixes https://github.com/apache/camel-quarkus/issues/3957 unfortunately contains also rebase to the current main branch, therefore the compare link shows a lot of affected files. You can see the changed files only in https://github.com/apache/camel-quarkus/commit/c77e6f2e4cb8a9917294fc83171b237ea9137d92

[jamesnetherton]

One final question. Are these dependencies needed in the virtualDependencies profile?

<dependency>
    <groupId>org.apache.camel.quarkus</groupId>
    <artifactId>camel-quarkus-mock</artifactId>
</dependency>
<dependency>
    <groupId>org.apache.camel.quarkus</groupId>
    <artifactId>camel-quarkus-direct</artifactId>
</dependency>

[JiriOndrusek]

One final question. Are these dependencies needed in the virtualDependencies profile?

<dependency>
    <groupId>org.apache.camel.quarkus</groupId>
    <artifactId>camel-quarkus-mock</artifactId>
</dependency>
<dependency>
    <groupId>org.apache.camel.quarkus</groupId>
    <artifactId>camel-quarkus-direct</artifactId>
</dependency>

I fixed it (both dependencies are in the "standard" dependencies and *-deployment are in virtualDependencies now)