Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contributions to Go Cloud #3449

Open
sandeepvinayak opened this issue Jul 2, 2024 · 12 comments
Open

Contributions to Go Cloud #3449

sandeepvinayak opened this issue Jul 2, 2024 · 12 comments

Comments

@sandeepvinayak
Copy link

Hey Folks,

We are actively using Go-Cloud as a multi-cloud SDK at our organization and have forked it for internal use. We have made significant contributions to our internal fork, such as onboarding new services like STS (security token service) and workflows (step functions in AWS). We have also added support for the Alibaba substrate in existing services such as pubsub and blob. Additionally, we have introduced a new way for openers, alongside the existing URL opener.

We would love to port these changes back to upstream. Could you please guide us on the next steps? Ideally, we would like to port these changes in stages to ensure diligent reviews. What do you think?

@eliben
Copy link
Member

eliben commented Jul 3, 2024

Thanks for reaching out!

For changes that apply across all cloud providers, you should be able to send PRs.

For changes that add new providers or apply only for specific providers, things are more complicated. Generally, the CDK will only accept contributions that are portable across all major cloud providers. We don't have the capacity to support many providers in our repository, so new providers are best kept out-of-tree or in forks like you currently have.

@sandeepvinayak
Copy link
Author

@eliben Thanks for your response and I wanted to clarify there are two categories of additions related to new providers:

  1. Addition of new providers for existing services such as blob, pubsub
  2. Addition of brand new services(or portable APIs) such as workflows, sts for aws + alibaba

Do you mean we don't want to have the new provider (incase this case, alibaba) at all in this repo?
How about the changes for onboarding new services (workflows), so far only aws is supported but can be backed by GCP for sure, we just don't have that atm.

@eliben
Copy link
Member

eliben commented Jul 3, 2024

@eliben Thanks for your response and I wanted to clarify there are two categories of additions related to new providers:

1. Addition of new providers for existing services such as blob, pubsub

2. Addition of brand new services(or portable APIs) such as workflows, sts for aws + alibaba

Do you mean we don't want to have the new provider (incase this case, alibaba) at all in this repo?

Correct. The potential number of providers for each service is very large (including Clouds, self-hosted services, OSS services etc.); we can't maintain all of them in the main repository of go-cloud, hence we only focus on the most major cloud providers.

How about the changes for onboarding new services (workflows), so far only aws is supported but can be backed by GCP for sure, we just don't have that atm.

We'll consider a new service if it can provide clean portable abstractions and be implemented for the major providers we support (GCP, AWS, Azure)

@sandeepvinayak
Copy link
Author

hence we only focus on the most major cloud providers.

alibaba is 4th major cloud cloud provider after GCP, definitely not close to top 3 ;-)
trying to pitch in if we need to re-consider that for onboarding alibaba provider. If we don't want to, it's totally fine and we can keep it in our fork.

We'll consider a new service if it can provide clean portable abstractions and be implemented for the major providers we support

We have the portable abstraction and AWS implementation so far (given we are not considering alibaba), should we consider bringing in that for now and we can open issues/jiras to onboard GCP and azure provider? We can potentially work on it but cannot guarantee the timeline to be immediate.

@eliben
Copy link
Member

eliben commented Jul 4, 2024

hence we only focus on the most major cloud providers.

alibaba is 4th major cloud cloud provider after GCP, definitely not close to top 3 ;-) trying to pitch in if we need to re-consider that for onboarding alibaba provider. If we don't want to, it's totally fine and we can keep it in our fork.

We'll consider a new service if it can provide clean portable abstractions and be implemented for the major providers we support

We have the portable abstraction and AWS implementation so far (given we are not considering alibaba), should we consider bringing in that for now and we can open issues/jiras to onboard GCP and azure provider? We can potentially work on it but cannot guarantee the timeline to be immediate.

We'll likely want to see all the major providers in place before accepting a new abstraction; it's also not easy to design a truly portable abstraction until differences between the implementations of different cloud providers are tackled head on.

The good news is that the portable abstractions we have are very stable by now, so maintaining a new provider out of tree should be easy - there's very low chance of merge conflicts. The same can be said for a completely new abstraction - there the chance of conflicts is even lower, so it sounds like keeping things in your own fork for now (and occasionally pulling from upstream) is the best approach for you.

@jkutner
Copy link

jkutner commented Jul 10, 2024

@eliben have you discussed or considered any kind of formal extension/plugin model that would allow us to maintain our own Alibaba provider/abstraction in a separate lib without forking this library? Would you be open to that?

Even without merge conflicts, a fork could be a significant maintenance burden, and makes it harder to manage dependency updates, vuln detection, etc. We'd like to find a way to be on the "mainline".

@eliben
Copy link
Member

eliben commented Jul 10, 2024

@jkutner generally speaking, go-cloud is already organized in a modular way (see https://gocloud.dev/concepts/structure/)

With a concrete example in mind, if you were to provide an Alibaba blob provider out-of-tree today, what prevents you from doing so?

@jkutner
Copy link

jkutner commented Jul 10, 2024

Sure, but that still requires a fork, merge back, and then possible API changes that need fixing and conflicts in dependencies, etc. Generally, it's just a less preferable way of using go-cloud than if we had a formal extension/plugin mechanism with semantic versioning of the API/contract etc. I'm just wondering that's ever been discussed, and if you'd be open to something like that.

We've done something similar in other projects with https://github.com/hashicorp/go-plugin. I'm not sure if that's the right pattern here, but that's the kind of thing I had in mind.

@eliben
Copy link
Member

eliben commented Jul 11, 2024

@jkutner I'm not sure I follow. go-cloud already providers a plugin mechanism that can be used to build out-of-tree providers. It's just a build-time mechanism, not a run-time mechanism (like hashicorp/go-plugin).

To be more concrete, here's a sample repository that contains an out-of-tree blob provider: https://github.com/eliben/gocloudsampleprovider

It's just a toy local-file based provider but it can be an Alibaba one or anything else. A user module would use it as follows: https://github.com/eliben/gocloudsampleprovider/blob/master/using-sample-provider/main.go

@sandeepvinayak
Copy link
Author

sandeepvinayak commented Jul 11, 2024

@jkutner I believe it's technically possible to have alibaba providers outside of this repo without forking. The framework expect that's the provider register itself during the runtime. Even if alibaba provider is located anywhere, as long as it registers itself, implements driver interfaces, the portable api will be able to locate the provider. And registration happens with dummy imports which calls the init function, for example: https://github.com/google/go-cloud/blob/master/blob/s3blob/s3blob.go#L95

But this can be a bit of maintenance overhead if want to maintain this provider like other providers with existing testing methods such as conformance tests. The tests are defined at driver layer and not really sure atm what complexities could be there. https://github.com/google/go-cloud/blob/master/pubsub/pubsub_test.go#L35, (It seems technically possible to utilize the same tests but just need to try it out to uncover unknowns)

In addition to the new providers, we have also onboarded new services such as workflows, sts and as per @eliben the requirement is to have azure, gcp implementations as well before we commit to upstream.

We can open the PR's for other improvements we did for new openers which lets user supply STS credentials, and seek the feedback. What do you think?

@jkutner
Copy link

jkutner commented Jul 15, 2024

Yea, I think having the out of tree provider will work. But not being able to contribute the new service (STS) requires that we keep a fork. That may be what we have to do for now.

@eliben In the near future we may be able to implement the other required providers (GCP and Azure). But I expect it will take a few months. In the meantime, we'd like to align on keeping things in sync as much as possible so that when it comes time to contribute the new service it goes smoothly.

Here's what I'm hoping we can get out of this conversation:

  • General acknowledge that there is interest in supporting an STS abstraction when the other providers are ready.
  • Accept a PR for improvements in the existing services (such as new openers and supplying creds) that minimize the divergence of our fork.
  • Add some kind of linkage to our new provider from https://gocloud.dev/. Would it be possible to add a "Partners" page, or even add some kind of "marketplace" for out of tree providers? My hope is that any one doing the same thing as us would consider contributing to our implementation before starting there own.

Let us know if there's any kind of formal proposal process you'd like us to follow for the last item. We're very eager to contribute our work back upstream, and stay aligned with project.

@eliben
Copy link
Member

eliben commented Jul 16, 2024

@jkutner while in general we're interested in new portable services, there are caveats that make it impossible to commit to things right now. First, it's unclear what the demand for this service is - looking back quite a few years of the project's existence there hasn't been much demand for this. Second, the devil is in the details when implementing a new portable service - it may turn out to be challenging to design a portable API. So I can't promise more than to evaluate it when it's actually ready for full review.

Yes, in general PRs for improvements in existing services are welcome.

And yes, we can mention established out-of-tree providers in documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants