feat: Add publish command #1143

yevgenypats · 2023-08-13T17:21:20Z

As we are productionizing and rolling out our own registry we need to move from our format being documented in a yaml file inside goreleaser to being part of our code and part of the sdk.

Right now this just build the package without publishing as our backend is not ready but nailing down this will also help finalize the backend which Im working on as well.

The idea here is not to introduce a package.json file like in npm and then create yet another language inside json or yaml config but rather the other way around and use all the information we already have from the plugin author and build a package with a manifest.json so everything is configurable in the code.

we will have two types of packages in our registry:

native (for languages like Go/Rust/C)
docker (for runtime language like .net, Java, Python)

For native this will looks the following:

ls -l ./dist
(base) yevgenyp@yevgenys-mbp aws % ls -l dist 
total 1142360
-rw-r--r--  1 yevgenyp  staff        447 Aug 13 20:08 plugin.json
-rw-r--r--  1 yevgenyp  staff  145537822 Aug 13 20:08 plugin_darwin_amd64.zip
-rw-r--r--  1 yevgenyp  staff  145537822 Aug 13 20:08 plugin_darwin_arm64.zip
-rw-r--r--  1 yevgenyp  staff  145537820 Aug 13 20:07 plugin_linux_amd64.zip
-rw-r--r--  1 yevgenyp  staff  145537824 Aug 13 20:07 plugin_windows_amd64.zip
-rw-r--r--  1 yevgenyp  staff    2726434 Aug 13 20:07 tables.json
(base) yevgenyp@yevgenys-mbp aws %

manifest will look the following:

cat plugin.json
{
  "name": "aws",
  "version": "Development",
  "title": "aws",
  "short_description": "",
  "description": "",
  "categories": [],
  "protocols": [
    3
  ],
  "supported_targets": [
    {
      "os": "linux",
      "arch": "amd64"
    },
    {
      "os": "windows",
      "arch": "amd64"
    },
    {
      "os": "darwin",
      "arch": "amd64"
    },
    {
      "os": "darwin",
      "arch": "arm64"
    }
  ],
  "package_type": "native"
}

This will make sure the cloudquery package registry has all the information needed to publish it, display it on CloudQuery Hub and give it for client to download without running the plugins itself. Not running the plugin itself is important for the following reasons:

Security: not running untrusted code in our environment.
Developer experience: moving any issues closer to the developer and shortening the debugging/publishing loop.

Once it gets a first review I'll also add tests.

codecov · 2023-08-13T17:24:49Z

Codecov Report

Patch coverage: 47.61% and project coverage change: -0.03% ⚠️

Comparison is base (763c549) 48.71% compared to head (8f5c28b) 48.68%.

❗ Current head 8f5c28b differs from pull request most recent head b1f92c6. Consider uploading reports for the commit b1f92c6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
- Coverage   48.71%   48.68%   -0.03%     
==========================================
  Files          85       86       +1     
  Lines        7833     8000     +167     
==========================================
+ Hits         3816     3895      +79     
- Misses       3682     3755      +73     
- Partials      335      350      +15

Files Changed	Coverage Δ
plugin/options.go	`0.00% <0.00%> (ø)`
schema/column.go	`48.07% <0.00%> (-11.45%)`	⬇️
schema/table.go	`49.44% <ø> (ø)`
plugin/plugin.go	`34.78% <23.07%> (-2.72%)`	⬇️
serve/publish.go	`61.66% <61.66%> (ø)`
serve/plugin.go	`62.68% <100.00%> (+0.37%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2023-08-13T17:27:06Z

⏱️ Benchmark results

Glob-8 ns/op: 98.17

hermanschaaf · 2023-08-14T09:13:49Z

plugin/plugin_publish.go

+		ShortDescription: p.shortDescription,
+		Description:      p.description,
+		Categories:       p.categories,
+		Protocols:        []int{3},


Small thing but it would be nice if we can rather import the supported protocols as a constant from the relevant package

Agree. It was outdated, It's moved now to serve.

erezrokah

I think this PR has two features:

Adding a manifest file to describe a plugin
Adding a publish command for plugins to replace Go Releaser (depends on 1.)

Regarding the first feature, I think usually it's done the other way around, where you have a static manifest file, and the runtime reads properties from it (for example in Go we can embed a JSON file will all the properties).
Then during publishing we just copy the file.

Having a static file makes it easier to declare, modify (for example during release to inject a version) and validate (e.g. if we have a JSON schema of the manifest).

At the moment the plugin manifest properties are spread around the code, and you need to invoke publish to know what is the manifest.

Regarding 2. I think it's good to have plugins more "self contained", we should make sure we don't reimplement Go Releaser, for example at the moment we:

Set some env variables
Pack policies with plugins
Sign plugins with a checksum
Some plugins can only run publish in a specific environment https://github.com/cloudquery/cloudquery/blob/db142bc867e631f00cb06f21a4a0566210fc7fd0/.github/workflows/release_plugin_dest_duckdb.yml#L11

I would maybe split this PR into 2:

Create the designated manifest file format, embed it and read the properties from it
The publish command

With that being said since this is a new feature and doesn't breaking anything we can move forward and see how it work

erezrokah · 2023-08-14T09:41:23Z

serve/publish.go

+func (s *PluginServe) writeManifest(ctx context.Context, dir string) error {
+	manifest := Manifest{
+		Name:             s.plugin.Name(),
+		Version:          s.plugin.Version(),


I think we're missing the part of injecting the plugin version via Go Releaser here https://github.com/cloudquery/cloudquery/blob/db142bc867e631f00cb06f21a4a0566210fc7fd0/plugins/.goreleaser.yaml#L12

Not sure how this would work in this approach. We could configure release please maybe to update the version in the go code like we do with python, see https://github.com/cloudquery/cloudquery/pull/12869/files#diff-e10085e6cd58ab584a2a0fd98e5b4bcdda801327fe2f8e571ee049a3bceaf1e0L13

erezrokah · 2023-08-14T09:44:32Z

serve/publish.go

+			}); err != nil {
+				return err
+			}
+			if err := s.writeTablesJson(cmd.Context(), distPath); err != nil {


There's a bit of overlap here with the CLI tables command. Not sure what to do about it yet, just stating it.
Also this is for our internal usage so maybe better to have it separated.

I would create a schema.json for both the manifest and tables so we can validate those. We'll need to be aware that changing/adding things in the Column/Table structs can break the JSON

yevgenypats · 2023-08-14T10:34:54Z

I think this PR has two features:

Adding a manifest file to describe a plugin

Adding a publish command for plugins to replace Go Releaser (depends on 1.)

Regarding the first feature, I think usually it's done the other way around, where you have a static manifest file, and the runtime reads properties from it (for example in Go we can embed a JSON file will all the properties). Then during publishing we just copy the file.

Having a static file makes it easier to declare, modify (for example during release to inject a version) and validate (e.g. if we have a JSON schema of the manifest).

At the moment the plugin manifest properties are spread around the code, and you need to invoke publish to know what is the manifest.

Regarding 2. I think it's good to have plugins more "self contained", we should make sure we don't reimplement Go Releaser, for example at the moment we:

Set some env variables

Pack policies with plugins

Sign plugins with a checksum

Some plugins can only run publish in a specific environment https://github.com/cloudquery/cloudquery/blob/db142bc867e631f00cb06f21a4a0566210fc7fd0/.github/workflows/release_plugin_dest_duckdb.yml#L11

I would maybe split this PR into 2:

Create the designated manifest file format, embed it and read the properties from it

The publish command

With that being said since this is a new feature and doesn't breaking anything we can move forward and see how it work

Thanks for the review. Few notes on 1,2.

It's not always the otherway around - for example in Python you have setup.py and in Go you don't have such thing at all. It's only in node.js/npm. The only reason it's manifest.json is because a node package doesn't conform to any API. In our plugin side it is already conforming to an SDK in our control which can generate the manifest instead of a user having to look up our documentation of a json file. Also, there are things that only the SDK knows - for example which protocol version it supports so it will have to generate another manifest2.json instead of just having one. Having said that we do use the manifest file because the registry can't run any user defined code as it's a security issue.
goreleaser has different things and features it was build for - building any go application and publishing to variety of registries. we only need to build cloudquery plugin and publish them to cloudquery hub in one very specific way otherwise our package won't work.

erezrokah · 2023-08-14T11:55:06Z

It's not always the otherway around - for example in Python you have setup.py and in Go you don't have such thing at all. It's only in node.js/npm. The only reason it's manifest.json is because a node package doesn't conform to any API.

Good point, maybe for other SDKs it would make sense to use a static file instead of code to define the plugin. Chrome extensions also use a static file, VS Code extensions too, they (ab)use package.json.
Same with Terraform, Figma and ChatGPT plugins.

instead of a user having to look up our documentation of a json file

The user still needs to look up the docs to know which methods to implement. If we read the data from a file they would only need to create the file, instead of implementing the Title, Description, etc. methods (and that would be identical to plugins in different languages, like JS, Python, Java).

Also, there are things that only the SDK knows - for example which protocol version it supports so it will have to generate another manifest2.json instead of just having one

Plugins can declare which SDK version they use (see Terraform and Figma).

goreleaser has different things and features it was build for - building any go application and publishing to variety of registries. we only need to build cloudquery plugin and publish them to cloudquery hub in one very specific way otherwise our package won't work.

👍 Mostly pointing out that we would need the build command to support the current features we use. Also we would need to somehow handle the case it can only run in a specific environment gracefully.

hermanschaaf · 2023-08-14T13:10:55Z

Looks like the tests are failing on MacOS

🤖 I have created a release *beep* *boop* --- ## [4.5.0](v4.4.0...v4.5.0) (2023-08-14) ### Features * Add publish command ([#1143](#1143)) ([fdd44d5](fdd44d5)) ### Bug Fixes * **deps:** Update github.com/cloudquery/arrow/go/v13 digest to e9683e1 ([#1144](#1144)) ([763c549](763c549)) * Scalar timestamp parsing ([#1109](#1109)) ([c15b214](c15b214)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

feat: Add publish command

27e4104

yevgenypats requested review from erezrokah and hermanschaaf August 13, 2023 17:21

github-actions bot added the feat label Aug 13, 2023

fmt

1dcb132

hermanschaaf reviewed Aug 14, 2023

View reviewed changes

erezrokah approved these changes Aug 14, 2023

View reviewed changes

fmt

135e3d2

add tests

b38462f

yevgenypats requested a review from hermanschaaf August 14, 2023 12:58

yevgenypats added the automerge label Aug 14, 2023

Merge branch 'main' into feat/publish_command

93496f8

yevgenypats added 3 commits August 14, 2023 16:18

fix tests

8f5c28b

fix tests

1820223

fix tests on windows

b1f92c6

kodiakhq bot merged commit fdd44d5 into main Aug 14, 2023

kodiakhq bot deleted the feat/publish_command branch August 14, 2023 15:02

cq-bot mentioned this pull request Aug 14, 2023

chore(main): Release v4.5.0 #1140

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Add publish command #1143

feat: Add publish command #1143

Uh oh!

yevgenypats commented Aug 13, 2023

Uh oh!

codecov bot commented Aug 13, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Aug 13, 2023 •

edited

Loading

Uh oh!

hermanschaaf Aug 14, 2023

Uh oh!

yevgenypats Aug 14, 2023

Uh oh!

erezrokah left a comment

Uh oh!

erezrokah Aug 14, 2023

Uh oh!

erezrokah Aug 14, 2023

Uh oh!

yevgenypats commented Aug 14, 2023

Uh oh!

erezrokah commented Aug 14, 2023

Uh oh!

hermanschaaf commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat: Add publish command #1143

feat: Add publish command #1143

Uh oh!

Conversation

yevgenypats commented Aug 13, 2023

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Aug 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⏱️ Benchmark results

Uh oh!

hermanschaaf Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

yevgenypats Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

erezrokah left a comment

Choose a reason for hiding this comment

Uh oh!

erezrokah Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

erezrokah Aug 14, 2023

Choose a reason for hiding this comment

Uh oh!

yevgenypats commented Aug 14, 2023

Uh oh!

erezrokah commented Aug 14, 2023

Uh oh!

hermanschaaf commented Aug 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Aug 13, 2023 •

edited

Loading

github-actions bot commented Aug 13, 2023 •

edited

Loading