asyncapi / bindings

AsyncAPI bindings specifications
Apache License 2.0
73 stars 75 forks source link

Google Cloud Pub/Sub Server Binding #227

Open justin-richert opened 12 months ago

justin-richert commented 12 months ago

Reason/Context

I'll make sure and state upfront that I'm relatively new to asyncapi and don't know everything there is to know about Google Pub/Sub either. My statement of facts below should certainly be read through that lens, and it should be understood that I'm more than happy to close this if there is a clear way to accomplish what I'm aiming for without alteration of the present binding definitions.

I noticed while doing some research and trying to apply asyncapi to our existing event ecosystem that it would be difficult to use separate Google Cloud projects in different code environments, which is exactly what we already do (and I would think this would be a relatively common pattern). My hope with this request / proposal is that we could define a server binding for googlepubsub that would allow project specification at that level. That would also clean up the specification of topic as well as schemaSettings in the body of the channel binding definition. I'll illustrate the problem here with an abbreviated spec, and then towards the bottom I'll illustrate what I hope to implement or have implemented by someone else.

...
servers:
  global:  # no use in defining a staging and production server, because they'd be identical
    url: pubsub.googleapis.com
    description: Google Cloud Pub/Sub global endpoint

...
channels:
  my-staging-topic:
    bindings:
      googlepubsub:
        topic: projects/my-gcp-staging-project/topics/my-topic
  my-production-topic:  # Fully duplicated outside of the projectId
    bindings:
      googlepubsub:
        topic: projects/my-gcp-production-project/topics/my-topic
...

The issue here is that channels should be shared amongst environments, but the project name is embedded in the topic string and is therefore inflexible to that case. It could be that using variables here is legitimate and we could enumerate an embedded project variable instead, though I might still submit that the proposed way is cleaner and doesn't require linking an enum definition under every channel. Additionally, the use of variables is not documented as being a valid solution here, so I doubt it's intended as of this time anyway.

Description

This will ideally be a fully backwards compatible change. We would need to define a server binding for the googlepubsub binding set which, according to the README at least, is currently not defined. As of this proposal, the server binding need only have one key (though it could almost certainly have others) -- projectId. This would improve the above example definition that illustrates my issue in the following way:

...
servers:
  staging:
    url: pubsub.googleapis.com
    bindings:
      googlepubsub:
        projectId: my-gcp-staging-project
  production:
    url: pubsub.googleapis.com  # or could use a region specific endpoint as an additional variance here
    bindings:
      googlepubsub:
        projectId: my-gcp-production-project
...
channels:
  my-topic:
    bindings:
      googlepubsub:
        # a known implied prefix of `projects/{projectId}/topics` would be in front of this now
        topic: my-topic  # also much cleaner for more nested topic names with forward slashes in them
...

As far as backwards compatibility is concerned, I think it would be reasonable to support still specifying the project in the topic path (as well as schemaSettings). Any code generation tools should be able to reasonably check for topics that start with projects/ under this binding and know not to prepend the implied prefix built by using the projectId. Then, while there's some small challenge in documenting both cases as options cleanly, a user adopting this new version need not be concerned with the additional spec properties until they require their use.

I would be more than happy to do the pull request to provide this functionality, but per your contributing guidelines, I wanted to make sure this was an agreed upon approach to what I'm trying to do. Thanks so much for reading and for your consideration!

github-actions[bot] commented 12 months ago

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

derberg commented 11 months ago

@whitlockjc please have a look 🙏🏼

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

whitlockjc commented 7 months ago

Taking a peek, sorry for the delay.

dwij2812 commented 4 months ago

@whitlockjc any thoughts on this, as we are in the same situation and are looking to vary the projectid's by environments? Any workarounds would be helpful as well before the changes to bindings can be considered.

whitlockjc commented 3 months ago

I apologize, it's been a pressing time for me lately and I dropped the ball on this. I think I understand the issue and I would like @derberg to chime in. The reason I want his opinion is because I want to make sure our solution is idiomatic AsyncAPI and that we solve real problems instead of tailoring a solution to a specific need. Initial thoughts are that the proposal makes sense and I would be 👍 on it. But I can easily see that each environment being a separate API (different projects, different request/response schemas, ...) and in that case, they should each have their own AsyncAPI document (rendering this change unnecessary).