spring-attic / spring-cloud-gcp

Integration for Google Cloud Platform APIs with Spring
Apache License 2.0
705 stars 693 forks source link

Pub/Sub client doesn't reconnect after a permission denied error #2504

Closed olivierboudet closed 3 years ago

olivierboudet commented 4 years ago

Versions :

Is your feature request related to a problem? Please describe.

When removing the role 'PubSub Subscriber' to the service account used by the application, an expected exception PermissionDeniedException is thrown after few secondes or minutes. Then, if the role is re-added to the service account, the connection is not reopened by the client.

Steps to reproduce

  1. Start an application listening on a subscription
  2. Remove the role 'PubSub Subscriber' to the service account used by the application
  3. Wait for the PermissionDeniedException exception
  4. Add the role 'PubSub Subscriber' to the service account used by the application
  5. Publish a message in the corresponding topic

The message is never consumed by the application. It needs to be restarted.

Describe the solution you'd like It could be useful if the client can reconnect after a change in the IAM configuration (errors happens... :) ). I initially thought that the option keep-alive-interval-minutes: 5 could be used on permission denied error but it has no effect.

meltsufin commented 4 years ago

We simply delegate to the Pub/Sub client library com.google.cloud.pubsub.v1.Subscriber to handle the subscription. So, we probably need to see if the same issue happens when you use that class directly.

olivierboudet commented 4 years ago

I test it directly with the client library in this sample : https://github.com/olivierboudet/test-pubsub-gcp-issue Same issue happens so I suppose we can close issue here and I should open it in https://github.com/googleapis/java-pubsub ?

    @Bean
    public Subscriber pubsub(TransportChannelProvider transportChannelProvider) throws IOException {
        Credentials credentials = new Credentials();
        credentials.setEncodedKey(this.pubsubKey);

        ProjectSubscriptionName subscription = ProjectSubscriptionName.of(projectId, subscriptionName);
        Subscriber subscriber = Subscriber.newBuilder(subscription, (message, consumer) -> {
            System.err.println(message);
        })
                .setChannelProvider(transportChannelProvider)
                .setCredentialsProvider(new DefaultCredentialsProvider(() -> credentials)).build();
        subscriber.startAsync();
        return subscriber;

    }

    @Bean
    public TransportChannelProvider transportChannelProvider() {
        return InstantiatingGrpcChannelProvider.newBuilder()
                .setKeepAliveTime(Duration.ofMinutes(this.keepAlive))
                .build();
    }
meltsufin commented 4 years ago

@olivierboudet Yes, please open an issue in java-pubsub repo. Many thanks!

olivierboudet commented 4 years ago

Thanks, I have opened https://github.com/googleapis/java-pubsub/issues/330

olivierboudet commented 3 years ago

Hello @meltsufin , I never took the time to comment here since last year but the issue in java-pubsub is closed because they do not consider 403 errors as retryable. Do you think this is the role of spring-cloud-gcp to handle that or it must be directly in the application code ?

meltsufin commented 3 years ago

@olivierboudet If it doesn't make sense for the client library to do it, I'm not sure why it would make sense for Spring Cloud GCP. This might be just a requirement relatively unique to your application. In that case, I guess it would make sense to keep in the application code.

elefeint commented 3 years ago

@olivierboudet If this is a common scenario in your application, you may want to look into defining your own beans with @RefreshScope, and refreshing them after a known configuration change or on a regular basis.

Which beans should be @RefreshScope depends on your needs. Here is an example using Spring Cloud GCP's Spring Integration support to re-subscribe upon credential refresh: https://github.com/elefeint/examples/tree/master/pubsub-rotate-credentials

elefeint commented 3 years ago

Closing the issue as working as intended.