Add support for storage->pubsub notifications

[tseaver]

See the concepts doc and the registering object changes doc.

Pull requests target the 3956-storage-notifications branch

Steps:

• Add a BucketNotification class, mapping the notification resource. Note that a BucketNotification may point to a topic which belongs to a different project than the one which owns its bucket. (Add 'BucketNotification' class. #3957)
• Add Bucket.notification factory. returning new BucketNotification. (Add 'Bucket.notification' factory #3958)
• Add BucketNotification.create method, mapping notifications/insert. Update the bucket's properties from the response (e.g., to get its server-generated ID). Should we barf if the ID is already set? (Add 'BucketNotification.create' API wrapper. #3966)
• Add BucketNotification.delete method, mapping notifications/delete. Presumably barf if the ID is not set. (Add 'BucketNotification.delete' API wrapper. #3983)
• Add BucketNotification.exists method, mapping notifications/get onto boolean (False for NotFound). Presumably barf if the ID is not set. (Add 'BucketNotification.exists' API wrapper. #3984)
• Add BucketNotification.reload method, mapping notifications/get, Presumably barf if the ID is not set. (Add 'BucketNotification.reload' API wrapper. #3986)
• Add Bucket.list_notifications method, mapping notifications/list. Return an iterator of BucketNotification instances. (Add 'Bucket.list_notifications' API wrapper. #3990)
• Add a system test which exercises the new API wrapper methods. (Add system tests for bucket notifications. #4021)

Questions:

• Do we need to support de novo creation of BucketNotfication instances w/ a user-supplied ID? The spec says that the ID is generated during notifications/insert, which suggests not.
• I assume that access to the new features does not depend on being able to import google.cloud.pubsub. The counter-example would be that BucketNotification.create must verify the existence of its topic, and maybe create a new one. Or does the back-end do that? The docs say that a topic is created if it does not exist.
• There is no documented support for update / patch of existing notifications. Is that intentional?
• What are the intended semantics of passing {'payload_format`: 'NONE'}? I don't think subscribers will find message with an empty payload useful, so I must be misunderstanding something. Do the non-payload attributes provide enough information for the subscriber to do something useful?

/cc @lukesneeringer, perhaps to loop in the appropriate GCS back-end contact.

[tseaver]

Another question: do we need a "round trip" system test? E.g., one which verifies that notifications arrive at the subscriber? Or can we get away with just doing BucketNotification CRUD in the system tests?

[BrandonY]

Hi @tseaver,

Answers to questions:

Do we need to support de novo creation of BucketNotfication instances w/ a user-supplied ID? The spec says that the ID is generated during notifications/insert, which suggests not.

Indeed, GCS will generate IDs for you.

I assume that access to the new features does not depend on being able to import google.cloud.pubsub. The counter-example would be that BucketNotification.create must verify the existence of its topic, and maybe create a new one. Or does the back-end do that? The docs say that a topic is created if it does not exist.

GCS will not create a Cloud Pub/Sub topic for you if one does not exist. It will, though, check to see if the topic you specify exists and permissions have been appropriately configured for GCS to be able to publish topics to it, and the call to create a subscription will fail if either of these two requirements are not met. The "gsutil" command-line utility will, on the client side, create a topic if it does not exist.

I suggest a similar, single "subscribe()" command exist, that fetches the correct robot name, attaches appropriate permissions for it to a Pub/Sub topic (and maybe creates that topic), and then creates the subscription. Otherwise pretty much every user will have to write out that same chain of commands.

There is no documented support for update / patch of existing notifications. Is that intentional?

Yes. Subscriptions cannot be modified. They can only be deleted and created.

What are the intended semantics of passing {'payload_format`: 'NONE'}? I don't think subscribers will find message with an empty payload useful, so I must be misunderstanding something. Do the non-payload attributes provide enough information for the subscriber to do something useful?

The attributes provide enough information to accomplish most tasks. They tell you the event that happened (an object was created, an object was deleted), and they also tell you which object it happened to and when. The payload may be useful if you want to see, for instance, the object's type or its size, but it's not necessary in many cases.

Another question: do we need a "round trip" system test? E.g., one which verifies that notifications arrive at the subscriber? Or can we get away with just doing BucketNotification CRUD in the system tests?

A round trip test would be ideal, but getting back a successful "subscription created" message from GCS is a good sign that you've done things correctly.

[tseaver]

@lukesneeringer Working on system tests for the feature, and running into permissions errors:

Traceback (most recent call last):
  File "/home/tseaver/projects/agendaless/Google/src/google-cloud-python/storage/tests/system.py", line 609, in test_notification_minimal
    self.assertTrue(notification.exists())
  File "/home/tseaver/projects/agendaless/Google/src/google-cloud-python/storage/.nox/sys-3-6/lib/python3.6/site-packages/test_utils/retry.py", line 95, in wrapped_function
    return to_wrap(*args, **kwargs)
  File "/home/tseaver/projects/agendaless/Google/src/google-cloud-python/storage/google/cloud/storage/notification.py", line 243, in create
    data=properties,
  File "/home/tseaver/projects/agendaless/Google/src/google-cloud-python/storage/.nox/sys-3-6/lib/python3.6/site-packages/google/cloud/_http.py", line 293, in api_request
    raise exceptions.from_http_response(response)
google.api.core.exceptions.Forbidden: 403 POST https://www.googleapis.com/storage/v1/b/notification-minimal-1505764404665/notificationConfigs: The service account '[email protected]' does not have permission to publish messages to to the Cloud Pub/Sub topic '//pubsub.googleapis.com/projects/precise-truck-742/topics/notification-1505764383884', or that topic does not exist.

Does the feature require whitelisting? The docs say that the notifications/insert API will create the topic if needed: I changed the test to create it, without changing the error.

[BrandonY]

You cannot create a notification to publish to a given Cloud Pub/Sub topic until the following conditions are met:
1.) the topic exists.
2.) the GCS service account associated with the project that owns this bucket has permission to publish to that topic.

GCS enforces this constraint, which is what you're seeing. The error message is correct.

[tseaver]

@BrandonY thanks for the followup. Two points:

• The docs for enabling notification say:

If you use a [TOPIC_NAME] that doesn't exist in your project, one is created for you.

I'm guessing that the described behavior is a feature of the gsutil notification create subcommand, rather than the notifications/insert API.

• I see that error message even when I precreate the topic using the same service account as the one used to create the bucket, and which I'm using to attempt to invoke notifications/insert. I can successfully verify that the account has the 'pubsub.topics.publish` permission on that topic, e.g.:

(pdb)  pp self.publisher_client.test_iam_permissions(self.topic_path, ['pubsub.topics.publish'])
permissions: "pubsub.topics.publish"

[dhermes]

@tseaver If you're using gsutil to understand the behavior, you can actually log the requests made (as well as the payloads sent and received).

[tseaver]

@dhermes I'm not running gsutil: I'm reading docs.

[dhermes]

@tseaver OK, no worries.

[tseaver]

@BrandonY, @dhermes See this gist.

[tseaver]

Looking at the registering object changes doc vs. the notifications/insert doc, I have tried using the shorter form for the topic reference (projects/<project-id>/topics/<topid-id>), but still get the same 403 error (it even re-expands the path reference to the longer form).

[BrandonY]

Hi @tseaver,

GCS will not create a topic for you if one does not exist.

I believe you're looking at the instructions under the "gsutil" tab. It is true that the gsutil program will create a topic if you do not specify one. If you click the other tab header, JSON API, you should not see that text.

That is a bit confusing, though. I'll see about changing that bit of doc.

[tseaver]

@BrandonY thanks for the follow-up.

I had decided already that auto-creation of the topic was not an API feature. The system tests I am working on create the topic before creating the bucket: they then attempts to create the notification, but fail with the 403.

The Ruby and PHP guys implementing the feature followed up on my query. Maybe their phrasing makes the issue clearer?

[tseaver]

@BrandonY 7d9ee94 adds the binding to the topic's IAM policy, required to enable creating the notifications. Those system tests are now passing.

[tseaver]

@BrandonY re: round-trip tests. The concept docs say:

When you add a notification configuration, Cloud Storage may take up to 30 seconds to begin sending notifications associated with it.

That makes writing a non-flaky system test for the round-trip a good deal harder: how can we know when the notification is active?

[BrandonY]

Yes, the service account for the project must have permission to publish to the topic before the call to create the subscription will succeed.

Writing a system test that obeys this constraint will require waiting 30 seconds after you create a notification before verifying that changes are being received by Cloud Pub/Sub. In practice, you can probably get away with 10-15 seconds.