Skip to content

feat: Add resource_record_sets to GCP plugin #10917

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
kodiakhq merged 6 commits into from
May 26, 2023
Merged

feat: Add resource_record_sets to GCP plugin #10917

kodiakhq merged 6 commits into from
May 26, 2023

Conversation

@paulopontesm
Copy link
Contributor

@paulopontesm paulopontesm commented May 24, 2023

Summary

This PR adds the gcp_dns_resource_record_sets table to the GCP plugin.

These are the columns for the new table:

_cq_source_name,_cq_sync_time,_cq_id,_cq_parent_id,project_id,kind,name,routing_policy,rrdatas,signature_rrdatas,ttl,type

Tested this on my GCP project and seems to be working ok. Let me know if there's something I'm missing.

@github-actions
Copy link

github-actions bot commented May 24, 2023

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

  • Table gcp_dns_resource_record_sets was added

@hermanschaaf hermanschaaf changed the title feat: add resource_record_sets sync to gcp plugin feat: Add resource_record_sets to GCP plugin May 24, 2023
paulopontesm
paulopontesm commented May 24, 2023
View reviewed changes
plugins/source/gcp/resources/plugin/tables.go Outdated
containeranalysis.Occurrences(),
dns.Policies(),
dns.ManagedZones(),
dns.ResourceRecordSets(),
Copy link
Contributor Author

@paulopontesm paulopontesm May 24, 2023

Choose a reason for hiding this comment

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

I'm not 100% sure if I should add this here also, or if the relationship with the managed zones enought. Can you confirm, please?

Copy link
Member

@hermanschaaf hermanschaaf May 25, 2023

Choose a reason for hiding this comment

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

The relationship with managed zones is enough, it shouldn't be added here as well (otherwise I would expect the sync to fail because there is no associated parent for the resolver to use)

hermanschaaf
hermanschaaf reviewed May 25, 2023
View reviewed changes
plugins/source/gcp/resources/services/dns/resource_record_sets.go Outdated
"google.golang.org/api/dns/v1"
)

func ResourceRecordSets() *schema.Table {
Copy link
Member

@hermanschaaf hermanschaaf May 25, 2023

Choose a reason for hiding this comment

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

Suggested change
func ResourceRecordSets() *schema.Table {
func resourceRecordSets() *schema.Table {

You can make this private to indicate that it's a relational table and shouldn't be used directly

hermanschaaf
hermanschaaf reviewed May 25, 2023
View reviewed changes
plugins/source/gcp/resources/services/dns/resource_record_sets.go
CreationOptions: schema.ColumnCreationOptions{
PrimaryKey: true,
},
},
Copy link
Member

@hermanschaaf hermanschaaf May 25, 2023

Choose a reason for hiding this comment

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

I think we should to add a column here to indicate the managed zone that was used to resolve this resource, and then make it part of the PK, because it seems like that's how uniqueness is determined in GCP (though I am by no means an expert here). You can use a parent column resolver; something like this:

			{
				Name:     "managed_zone_name",
				Type:     schema.TypeString,
				Resolver: schema.ParentColumnResolver("name"),
				CreationOptions: schema.ColumnCreationOptions{
					PrimaryKey: true,
				},
			},

Let me know if this doesn't make sense!

Copy link
Contributor Author

@paulopontesm paulopontesm May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That makes sense. It will make some queries a lot easier 👍

hermanschaaf
hermanschaaf requested changes May 25, 2023
View reviewed changes
Copy link
Member

@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.

Couple of comments, but overall looks great!

@paulopontesm
Copy link
Contributor Author

paulopontesm commented May 26, 2023

@hermanschaaf I think I implemented all your suggestions 👍

Also rebased and bumped the schema to v3.

@paulopontesm paulopontesm requested a review from hermanschaaf May 26, 2023 13:03
hermanschaaf
hermanschaaf approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@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.

Looks good, thanks @paulopontesm (and thanks for rebasing onto v3!) 🚀

@hermanschaaf hermanschaaf added the automerge Automatically merge once required checks pass label May 26, 2023
@kodiakhq kodiakhq bot merged commit f750cd2 into cloudquery:main May 26, 2023
@cq-bot cq-bot mentioned this pull request May 26, 2023
Merged
kodiakhq bot pushed a commit that referenced this pull request Jun 6, 2023
@cq-bot
chore(main): Release plugins-source-gcp v9.1.0 (#11039)
63de173 
🤖 I have created a release *beep* *boop*
---


## [9.1.0](plugins-source-gcp-v9.0.0...plugins-source-gcp-v9.1.0) (2023-06-06)


### Features

* Add resource_record_sets to GCP plugin ([#10917](#10917)) ([f750cd2](f750cd2))


### Bug Fixes

* **deps:** Update github.com/apache/arrow/go/v13 digest to e07e22c ([#11151](#11151)) ([5083cf7](5083cf7))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 20b0de9 ([#11199](#11199)) ([dc3565d](dc3565d))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 88d5dc2 ([#11226](#11226)) ([9f306bc](9f306bc))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to a7aad4c ([#11184](#11184)) ([8a0822e](8a0822e))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to c67fb39 ([#11169](#11169)) ([dcb0f92](dcb0f92))
* **deps:** Update golang.org/x/exp digest to 2e198f4 ([#11155](#11155)) ([c46c62b](c46c62b))
* **deps:** Update google.golang.org/genproto digest to e85fd2c ([#11156](#11156)) ([dbe7e92](dbe7e92))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.0.9 ([#11240](#11240)) ([f92cd4b](f92cd4b))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.10.3 ([#11150](#11150)) ([dc00994](dc00994))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.10.4 ([#11244](#11244)) ([8fceef6](8fceef6))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.6.7 ([#11043](#11043)) ([3c6d885](3c6d885))
* **deps:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.7.0 ([#11113](#11113)) ([487bf87](487bf87))
* **gcp:** Update module github.com/cloudquery/plugin-sdk/v3 to v3.10.2 ([#11198](#11198)) ([07f4564](07f4564))

---
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

Reviewers

@hermanschaaf hermanschaaf hermanschaaf approved these changes

@yevgenypats yevgenypats Awaiting requested review from yevgenypats

@disq disq Awaiting requested review from disq

Assignees

No one assigned

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@paulopontesm @hermanschaaf @cq-bot