Skip to content

feat(gandi): Gandi Source Plugin#4940

Merged
kodiakhq[bot] merged 16 commits intocloudquery:mainfrom
disq:feat/gandi_plugin
Nov 25, 2022
Merged

feat(gandi): Gandi Source Plugin#4940
kodiakhq[bot] merged 16 commits intocloudquery:mainfrom
disq:feat/gandi_plugin

Conversation

@disq
Copy link
Copy Markdown
Member

@disq disq commented Nov 22, 2022

@disq disq requested review from a team, cqgaurav, hermanschaaf and yevgenypats and removed request for a team November 22, 2022 19:38
@disq disq requested a review from erezrokah as a code owner November 23, 2022 08:31
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

Nice, LGTM for a first version 👍 I left some non-blocking comments.

I think to release this we'll need some additional updates to the GitHub workflows, but I'm happy for that to be done in a separate PR (probably better that way too).

mainTemplate := r.Template + ".go.tpl"
tpl, err := template.New(mainTemplate).ParseFS(templatesFS, "templates/"+mainTemplate)
if err != nil {
log.Fatal(fmt.Errorf("failed to parse gandi templates: %w", err))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not blocking this PR, but is there some value in sharing this method (by parameterizing the name of the plugin) between all plugins ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@cqgaurav How we do things change between plugins especially with complex SDKs. It would mean lots of "flags" to understand and configure per plugin... Maybe a very limited set can be "copied" and configured per plugin (ie. this one was copied from the Cloudflare codegen and slightly modified). Eventually parts of it can be extracted as utilities and put into sdk the codegen package which provides main codegen functionality.

@disq disq force-pushed the feat/gandi_plugin branch from 4f09e8e to eba6b51 Compare November 24, 2022 10:42
@disq disq requested a review from hermanschaaf November 24, 2022 15:28
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

Now looks even better 👍 Just left a few comments for things to double-check

for _, r := range resources {
r.Infer()
}
if err := recipes.SetParentChildRelationships(resources); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unless we have resolver/mock templates that handle parent & child relations in this plugin (do we?), we can probably remove this call and the associated function.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's used to properly generate child table names, completely optional, but gandi doesn't have 100% of resources covered (or the sdk has 100% implemented) so more resources will pop up and this could help us down the line (we'll eventually have more then a few child tables)

|_cq_sync_time|Timestamp|
|_cq_id|UUID|
|_cq_parent_id|UUID|
|sharing_id|String|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should sharing_id maybe be part of the PK here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a "global" table with values like:

sharing_id      |
name            | cert_std_3_0_0
name_label      |
href            | https://api.gandi.net/v5/certificate/packages/cert_std_3_0_0
max_domains     | 3
type            |
type_label      |
wildcard        | f

it would be the same rows repeated if sharing_id was a key. The sharing_id column is provided because if the user sets it in the config it's passed down to the client automatically so filtering is probably done on the server side.

|_cq_sync_time|Timestamp|
|_cq_id|UUID|
|_cq_parent_id|UUID|
|sharing_id|String|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should sharing_id be part of the PK here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think certificates have clashing ids to warrant for an additional PK.

Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

I didn't review the code but I think it is good for first version and @hermanschaaf already kindly reviewed. I think you can add to this PR https://github.com/cloudquery/cloudquery/blob/main/contributing/adding_a_new_plugin_to_cq_monorepo.md and then we can merge

@cq-bot cq-bot removed k8s labels Nov 25, 2022
@disq disq force-pushed the feat/gandi_plugin branch from a2c586c to 69c7b4e Compare November 25, 2022 15:09
@disq disq requested a review from yevgenypats November 25, 2022 15:09
@disq
Copy link
Copy Markdown
Member Author

disq commented Nov 25, 2022

@yevgenypats done

@yevgenypats yevgenypats added the automerge Automatically merge once required checks pass label Nov 25, 2022
@kodiakhq kodiakhq bot merged commit 5819f6c into cloudquery:main Nov 25, 2022
@disq disq deleted the feat/gandi_plugin branch November 25, 2022 17:28
kodiakhq bot pushed a commit that referenced this pull request Nov 30, 2022
🤖 I have created a release *beep* *boop*
---


## 1.0.0 (2022-11-30)


### Features

* **gandi:** Gandi Source Plugin ([#4940](#4940)) ([5819f6c](5819f6c))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk to v1.10.0 ([#5153](#5153)) ([ea1f77e](ea1f77e))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants