Implement Alibaba cloud SLS project

crossplane-contrib / provider-alibaba

Crossplane provider for Alibaba Cloud

Apache License 2.0

50 stars 18 forks source link

Implement Alibaba cloud SLS project #58

Closed zzxwill closed 3 years ago

zzxwill commented 3 years ago

Description of your changes

Support provision Alibaba Cloud SLS project, create examples directory for official examples and update makefile

Fixes #

I have:

[x] Read and followed Crossplane's contribution process.
[x] Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

wonderflow commented 3 years ago

@negz Hi, Nic, we're going to implement more cloud resources here, do you have any suggestion about the priority of cloud resources needed?

negz commented 3 years ago

Hi, Nic, we're going to implement more cloud resources here

That's great news!

do you have any suggestion about the priority of cloud resources needed?

Why don't we start an issue on this repository to gather input from users - Slack would be a great place to ask the community for input on this.

negz commented 3 years ago

Thanks for working on this @zzxwill! Looks like a great start, but I would prefer if this managed resource was more in line with typical Crossplane conventions. If you strongly disagree with any we can discuss that and perhaps capture how provider-alibaba will diverge and why.

wonderflow commented 3 years ago

HI @negz Can we merge it first and fix the rest in following PRs. We need it to be merged and show in demos.

negz commented 3 years ago

HI @negz Can we merge it first and fix the rest in following PRs. We need it to be merged and show in demos.

No, sorry. I recommend building the provider from your branch if you need to use it for demos. I'm happy to merge once the remaining comments are addressed.

zzxwill commented 3 years ago

This is looking good @zzxwill, thank you. The main thing that still needs addressing is using meta.GetExternalName. Done. Per your suggestion, I believe SLS project name spec.froProvider.name should be deleted and the managed resource name should be regarded as the SLS project name.

I also wonder if we should agree on the pattern proposed in #63 before we merge this PR, but if it's urgent I could be convinced to approve while we figure out #63. I will add more some examples in #63 after I talked with my colleagues which will integrated provider-base base logics.

zzxwill commented 3 years ago

@negz And I am so sorry for the late of addressing your newly two comments:) I was so busy with some other stuffs.

negz commented 3 years ago

No problem @zzxwill! We've been very busy too. :) Thanks for addressing my comments about using GetExternalName and pkg/errors. It seems like there are now only two things we need to sort out to merge this:

The latest commit is failing our linter; I think one of the files needs a gofmt.
Figuring out what we are going to do about https://github.com/crossplane/provider-alibaba/issues/63.

I prefer to not introduce the BaseObserve pattern in this PR until after we have a chance to discuss the context and approach in https://github.com/crossplane/provider-alibaba/issues/63. How do you feel about updating this PR to use a 'normal' ExternalClient implementation for now, until we discuss further and agree?

zzxwill commented 3 years ago

@negz Your suggestion is helpful for decomposing the problems. Addressed all of the two. Thanks help review it:)

zzxwill commented 3 years ago

Why don't we start an issue on this repository to gather input from users - Slack would be a great place to ask the community for input on this.

Create the issue #67, and I will talk to some potential users and contributors offline first.