Inconsistency in credentials specification for CPS Publisher and Subscriber

[kamalaboulhosn]

Both the Publisher and the Subscriber class state "If no credentials are provided, the [Publisher|Subscriber] will use application default creentials through GoogleCredentials#getApplicationDefault. However, while Subscriber.Builder has a setCredentials method, Publisher.Builder does not. Moreover, from what I can see, Subscriber does not actually use the credentials set via Subscriber.Builder.setCredentials. If we are going to allow one to set credentials this way, we should be consistent between the Publisher and Subscriber and ensure we use the provided credentials. Otherwise, we should remove the method from the Subscriber.Builder.

[was2]

I am seeing the same behavior with Subscriber.Builder.setCrecentials - even when valid creds are provided, I get an error complaining that app default credentials are not available. This appears to have started in 0.13.0.

[garrettjonesgoogle]

gRPC has a way to provide credentials through CallOptions, which is currently marked with @ExperimentalApi, but that will be removed soon. This enables us to change how credentials are provided to GAPIC clients: we could move CredentialsProvider fromChannelProvider up to ClientSettings (so that it is a sibling of ChannelProvider). This would make the code to provide custom credentials a bit simpler. Then, this same design could be used in Publisher/Subscriber, where credentials are provided separately from the channel.

Current code:

CredentialsProvider credentialsProvider =
    FixedCredentialsProvider
        .create(ServiceAccountCredentials.fromStream(new FileInputStream("credentials.json")));
ChannelProvider channelProvider =
    TopicAdminSettings.defaultChannelProviderBuilder()
        .setCredentialsProvider(credentialsProvider).build();
TopicName topicName = TopicName.create("my-project-id", "my-topic-id");
Publisher publisher =
    Publisher.defaultBuilder(topicName).setChannelProvider(channelProvider).build();

Proposed:

CredentialsProvider credentialsProvider =
    FixedCredentialsProvider
        .create(ServiceAccountCredentials.fromStream(new FileInputStream("credentials.json")));
TopicName topicName = TopicName.create("my-project-id", "my-topic-id");
Publisher publisher =
    Publisher.defaultBuilder(topicName).setCredentialsProvider(credentialsProvider).build();

@pongad what are your thoughts?

[pongad]

This was missed when we migrate from ChannelBuilder to ChannelProvider. The method setCredentials weren't removed but calling it is essentially a noop.

@garrettjonesgoogle My understanding of credentials is that we put an interceptor into the channel, and the interceptor insert credentials into calls. If the CredentialsProvider is a sibling of the ChannelProvider, how does the ChannelProvider create a channel without knowing what the credentials are? In general, I think Subscriber/Publisher should use the same "credential inserter" as gapic clients. Or do we plan to change those too?

In either case, I think we can all agree that the doc and method are misleading and should be removed. I'll make a PR for this.

[garrettjonesgoogle]

@pongad we would no longer use an interceptor; we would add a new UnaryCallable in the stack which adds the credentials to CallOptions before calling the next UnaryCallable. In that way, the credentials and the channel are decoupled.

I agree that Subscriber/Publisher should use the same mechanism as GAPIC clients.

[pongad]

Ah OK. I assume this new layer hasn't been written yet? I'll make a PR to remove this old code. When the layer is ready, we can migrate en masse.

[garrettjonesgoogle]

@pongad correct, support for this is not written yet into GAX or toolkit.