Skip to content
New issue

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

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

Already on GitHub? # to your account

Support SLS logstore #77

Merged
merged 10 commits into from
Jun 24, 2021
Merged

Conversation

zzxwill
Copy link
Collaborator

@zzxwill zzxwill commented Jun 22, 2021

Description of your changes

Implemented SLS logstore.

Fixes #67

I have:

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

How has this code been tested

Apply examples/sls/store.yaml

@zzxwill zzxwill force-pushed the sls-logstore branch 3 times, most recently from a715c4e to 71a2359 Compare June 22, 2021 12:44
apis/sls/v1alpha1/store_types.go Outdated Show resolved Hide resolved
region string
)

switch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is being replicated in a number of controllers, might make sense to move it to a common package, such as client. crossplane-runtime also supports a common credentials extraction mechanism now that will allow you to fetch credentials from a variety of sources. Example here: https://github.com/crossplane/provider-aws/blob/3243f6c4cfa8c860ceee705c9eca72ea68398545/pkg/clients/aws.go#L105

Looks like there might need to be a crossplane-runtime update here first though so might make sense to tackle in a follow-up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this has already been implemented in #64, apologies for the delayed review there! I believe it needs a rebase but I will prioritize getting eyes on that!

pkg/controller/sls/store_controller.go Outdated Show resolved Hide resolved
examples/sls/store.yaml Outdated Show resolved Hide resolved
Comment on lines 51 to 54
// SLS project name
// +kubebuilder:validation:MinLength:=3
// +kubebuilder:validation:MaxLength:=63
ProjectName string `json:"projectName"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you will want cross resource referencing implemented for this field to that folks are able to point to their Project resource in the cluster to populate the ProjectName. Let me know if you have any questions around implementation :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. That's exactly what I need. I noticed a design doc on cross resource referencing https://github.com/crossplane/crossplane/blob/master/design/one-pager-cross-resource-referencing.md. A more straight example would be much appreciated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hasheddan I thought it over. Resource SLS Store can also be independent of SLS Project. If the project doesn't exist, a ProjectNotExist error can occur https://www.alibabacloud.com/help/zh/doc-detail/29015.htm which is acceptable.

zzxwill and others added 7 commits June 23, 2021 10:05
Implemented SLS logstore.

To fix crossplane-contrib#67

Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Co-authored-by: Daniel Mangum <31777345+hasheddan@users.noreply.github.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Co-authored-by: Daniel Mangum <31777345+hasheddan@users.noreply.github.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Co-authored-by: Daniel Mangum <31777345+hasheddan@users.noreply.github.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Signed-off-by: Zheng Xi Zhou <zzxwill@gmail.com>
Copy link
Contributor

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hasheddan Thanks dan, it's worked now, and let's refine in following PR. @zzxwill is also alibaba cloud staff and our community maintainer.

@wonderflow wonderflow merged commit 1f227e8 into crossplane-contrib:master Jun 24, 2021
@zzxwill zzxwill deleted the sls-logstore branch June 24, 2021 09:07
@zzxwill zzxwill mentioned this pull request Jul 20, 2021
6 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Which cloud resource would you expect to be supported by provider alibaba?
3 participants