feat(alicloud): Add alicloud source plugin to sync bss and oss resources by lshmouse · Pull Request #6418 · cloudquery/cloudquery

lshmouse · 2023-01-06T09:14:46Z

Summary

Add the alicloud source plugin to sync bss and oss resources

github-actions · 2023-01-06T09:15:06Z

This PR has the following changes to source plugin(s) tables:

Table alicloud_bss_bill was added
Table alicloud_bss_bill_overview was added
Table alicloud_oss_bucket_stats was added
Table alicloud_oss_buckets was added

hermanschaaf

This is amazing, thank you @lshmouse! Really excited to be adding support for Alicloud. I left a few comments after an initial review. I'll also try and test it out a bit soon so I can give more thorough feedback. Let me know if you're willing to add some mock tests or if you'd like some help with that :)

hermanschaaf · 2023-01-06T13:17:45Z

+
+func ResolveUpdateDate() schema.ColumnResolver {
+	return func(_ context.Context, _ schema.ClientMeta, r *schema.Resource, c schema.Column) error {
+		return r.Set(c.Name, time.Now().AddDate(0, 0, -1).Format("2006-01-02"))


Why is this column resolver needed, and why does it add -1 days? Do you need information about exactly when a row was added? The _cq_sync_time column doesn't give exact row-level data, but it does tell you when a sync started, which is usually good enough.

Oh, after reading a bit more of the PR, I see that this column is part of the PK, probably to avoid overwriting rows from previous syncs. You can also achieve the same by leaving the PK empty, in which case it will default to _cq_sync_id. In overwrite-delete-stale mode CloudQuery will first sync all rows, then delete rows from previous syncs.

hermanschaaf · 2023-01-06T13:20:22Z

+	bucketName := codegen.ColumnDefinition{
+		Name:     "name",
+		Type:     schema.TypeString,
+		Resolver: `client.ResolveParentColumn("Name")`,


I think we should be able to use schema.ParentColumnResolver from the plugin-sdk library here (e.g. https://github.com/cloudquery/cloudquery/blob/ec1d132529824d16682a92a83ee326fcba939efc/plugins/source/aws/codegen/recipes/s3.go#LL61C2-L61C2), and remove the custom resolver from this plugin. I notice this pattern was copied from the Github plugin, we should probably fix it there too in a follow-up PR :)

hermanschaaf · 2023-01-06T13:31:20Z

+  version: "v1.1.12"
+  write_mode: "append" # CSV only supports 'append' mode
+  spec:
+    directory: './cq_csv_output'


This is a good way for developers to test the plugin against real infrastructure for now, but eventually we'll probably remove it here and move it to the docs.

For testing in CI we'll require mock tests for the resolvers. Here's an example from the Github plugin. I can also help you write those if you like, they are fairly mechanical once you've created the mocks :)

hermanschaaf · 2023-01-06T13:36:15Z

+	github.com/alibabacloud-go/tea v1.1.20
+	github.com/aliyun/alibaba-cloud-sdk-go v1.62.80
+	github.com/aliyun/aliyun-oss-go-sdk v2.2.6+incompatible
+	github.com/cloudquery/plugin-sdk v1.11.1


This plugin-sdk version is a little out-of-date, let's make sure to use the latest version (v1.21.0)

hermanschaaf · 2023-01-06T13:37:59Z

+}
+
+/*
+* https://help.aliyun.com/document_detail/100392.html


These links are great - we should also add them to the Description field of each table so they show up in the docs :)

hermanschaaf · 2023-01-06T13:42:56Z

+ */
+func fetchBssBill(_ context.Context, meta schema.ClientMeta, _ *schema.Resource, res chan<- any) error {
+	spec := meta.(*client.Client).Spec
+	cli, err := bssopenapi.NewClientWithAccessKey(spec.RegionID, spec.AccessKey, spec.SecretKey)


I don't know if it's possible in this case, but plugins generally initialize the client in client.go, and store it as a property on the client.Client struct, so we could then access it here like

c := meta.(*client.Client) cli := c.Client

This has the added benefit of erroring out earlier if there is an issue initializing the client, before calling all the resolvers.

hermanschaaf · 2023-01-06T13:45:49Z

+	RegionID    string `json:"region_id,omitempty"`
+	AccessKey   string `json:"access_key,omitempty"`
+	SecretKey   string `json:"secret_key,omitempty"`
+	BillHistory int    `json:"bill_history,omitempty"`


What does this option do? Looks like it either loads the billing history or only the latest bill? Should it be a boolean in that case? I think we'll probably soon have a better way of handling historical billing cycles (cloudquery/plugin-sdk#569), so maybe we can update it to use that when it's ready.

hermanschaaf · 2023-01-06T13:48:16Z

+			return err
+		}
+		if response.GetHttpStatus() != http.StatusOK {
+			return fmt.Errorf("httpcode %d", response.GetHttpStatus())


We may want to handle retryable errors (like 5xx and 429), either here or in a generalized function. We don't have a perfect pattern for this yet, but see the Slack plugin for an example: https://github.com/cloudquery/cloudquery/blob/main/plugins/source/slack/resources/services/users/users_fetch.go#L20 and https://github.com/cloudquery/cloudquery/blob/main/plugins/source/slack/client/rate_limiting.go#L13

hermanschaaf · 2023-01-06T13:49:47Z

@@ -0,0 +1,64 @@
+package models
+
+type BillOverview struct {


Mostly out of curiosity, what is the difference between these structs and the ones provided by the Alicloud SDK? Just wondering why (or whether) we need it

lshmouse · 2023-01-09T02:43:24Z

@hermanschaaf Thanks very much for your careful review. I will update the pull request later, thanks~

…e SDK

hermanschaaf · 2023-01-13T18:15:54Z

Hi @lshmouse 👋 Just wanted to update you: I spent a bit more time going through the code today, and started bringing it up-to-date with the latest plugin-sdk and best practices. I'll push the commits to your branch once they're ready and then we can merge, but there's still a bit more work to do which I'll pick up next week. My work-in-progress branch is here if you want to have a look or merge it into your branch sooner rather than later: https://github.com/cloudquery/cloudquery/tree/alicloud Hope you don't mind!

hermanschaaf

Thanks again @lshmouse for this great contribution!

🤖 I have created a release *beep* *boop* --- ## 1.0.0 (2023-01-18) ### Features * **alicloud:** Add alicloud source plugin to sync bss and oss resources ([#6418](#6418)) ([a8ee1c9](a8ee1c9)) * **alicloud:** Add support for Alicloud ECS instances ([#6901](#6901)) ([c592fbe](c592fbe)) * **alicloud:** Prepare CI environment, fix PKs and column types, add docs ([#6907](#6907)) ([3696d1e](3696d1e)) ### Bug Fixes * **deps:** Update module github.com/cloudquery/plugin-sdk to v1.27.0 ([#6856](#6856)) ([545799b](545799b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

lshmouse · 2023-01-20T04:19:58Z

@hermanschaaf I was blocked by something else in the past two weeks. Thanks very much for completing the patch~

lshmouse added 2 commits January 6, 2023 16:21

Add alicloud source plugin by coping from test source plugin

3487cb3

Add alicloud source plugin to sync bss and oss resources

52fe4e1

lshmouse requested review from a team and bbernays and removed request for a team January 6, 2023 09:14

yevgenypats changed the title ~~feat(alicloud): add alicloud source plugin to sync bss and oss resources~~ feat(alicloud): Add alicloud source plugin to sync bss and oss resources Jan 6, 2023

hermanschaaf requested review from hermanschaaf and yevgenypats and removed request for bbernays January 6, 2023 09:23

hermanschaaf suggested changes Jan 6, 2023

View reviewed changes

hermanschaaf added 6 commits January 13, 2023 15:31

Start adding multiplexers

0a02b36

Save

a5c9f67

Merge branch 'main' of github.com:cloudquery/cloudquery into alicloud

58a4bf1

Use service intefaces, transformers, multiplexers, refactor and updat…

722d9a6

…e SDK

Fix PKs

b525f35

Fix more PKs

c9898b0

hermanschaaf added 4 commits January 16, 2023 09:39

Update plugin-sdk

0cb9e56

Add mock tests

2e8bb51

Fixes

032ceef

Make gen

e2b8335

cq-bot added the postgresql label Jan 16, 2023

Update readme and config file

6380c7f

hermanschaaf added the alicloud label Jan 16, 2023

Update

94cd1bb

hermanschaaf requested a review from erezrokah as a code owner January 16, 2023 15:27

cq-bot removed the postgresql label Jan 16, 2023

hermanschaaf approved these changes Jan 16, 2023

View reviewed changes

hermanschaaf added the automerge Automatically merge once required checks pass label Jan 16, 2023

Merge branch 'main' into dev/shaohui/alicloud-source-plugin

00b16d3

vercel Bot deployed to Preview January 16, 2023 15:34 View deployment

kodiakhq Bot merged commit a8ee1c9 into cloudquery:main Jan 16, 2023

cq-bot mentioned this pull request Jan 17, 2023

chore(main): Release plugins-source-alicloud v1.0.0 #6911

Merged

hermanschaaf mentioned this pull request Jan 18, 2023

New Source Plugin: AliCloud #6416

Closed

Conversation

lshmouse commented Jan 6, 2023

Summary

Uh oh!

github-actions Bot commented Jan 6, 2023

This PR has the following changes to source plugin(s) tables:

Uh oh!

hermanschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lshmouse commented Jan 9, 2023

Uh oh!

hermanschaaf commented Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hermanschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

lshmouse commented Jan 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hermanschaaf commented Jan 13, 2023 •

edited

Loading