Skip to content

feat!: Migrate Azure plugin to v2#1754

Merged
erezrokah merged 113 commits intocloudquery:mainfrom
erezrokah:chore/codegen_azure_v3
Sep 18, 2022
Merged

feat!: Migrate Azure plugin to v2#1754
erezrokah merged 113 commits intocloudquery:mainfrom
erezrokah:chore/codegen_azure_v3

Conversation

@erezrokah
Copy link
Copy Markdown
Member

Summary

Uses codegen via go templates instead of mock-gen


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Test locally on your own infrastructure
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@@ -1,60 +0,0 @@
//go:generate mockgen -destination=./mocks/ad_applications.go -package=mocks . ADApplicationsClient
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.

Wasn't used

@@ -1,4 +1,4 @@
//go:generate mockgen -destination=./mocks/authorization.go -package=mocks . RoleAssignmentsClient,RoleDefinitionsClient
//go:generate mockgen -destination=./mocks/authorization.go -package=mocks . AuthorizationRoleAssignmentsClient,AuthorizationRoleDefinitionsClient
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.

Since all mocks are in the same package mocks we prefix clients with the service name to avoid collisions

@@ -1,16 +1,18 @@
//go:generate mockgen -destination=./mocks/containerregistry.go -package=mocks . ContainerRegistriesClient,ContainerReplicationsClient
//go:generate mockgen -destination=./mocks/containerregistry.go -package=mocks . ContainerRegistriesClient,ContainerReplicationsClient,ContainerManagedClustersClient
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.

Instead of containerregistry.go and containerservice.go I put both in the same container.go file

@@ -1,25 +0,0 @@
//go:generate mockgen -destination=./mocks/containerservice.go -package=mocks . ManagedClustersClient
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.

Moved to container.go file


type EventHubClient interface {
type EventHubClient struct {
Namespaces EventHubNamespacesClient
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.

Some services didn't have the proper nesting so made code generation hard due to inconsistencies.
It was easier to fix the services than to create more templates


type FrontDoorClient interface {
type FrontDoorClient struct {
Doors FrontDoorDoorsClient
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.

The naming convention sometimes creates redundancy but I kept it to simplify code generation

// Source: github.com/cloudquery/cloudquery/plugins/source/azure/client/services (interfaces: ADApplicationsClient)

// Package mocks is a generated GoMock package.
package mocks
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.

Wasn't used

@@ -1,51 +0,0 @@
// Code generated by MockGen. DO NOT EDIT.
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.

Wasn't used

@@ -1,51 +0,0 @@
// Code generated by MockGen. DO NOT EDIT.
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.

Wasn't used

@@ -1,51 +0,0 @@
// Code generated by MockGen. DO NOT EDIT.
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.

Wasn't used

LogProfiles: logProfiles,
ActivityLogs: activityLogs,
DiagnosticSettings: diagnosticSettings,
Resources: resourcesClient,
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.

Moved resourcesClient from plugins/source/azure/client/services/resources.go as it's only used to get DiagnosticSettings.
I added a resources table under monitor package and made DiagnosticSettings a relation (more below)


type RedisClient interface {
type RedisClient struct {
ResourceTypes RedisResourceTypesClient
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.

As others created the proper nesting

ls.Authorizer = auth
return ResourcesClient{
Groups: groups,
Resources: client,
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.

Resources moved to plugins/source/azure/client/services/monitor.go as it's not used to get anything under the resources package


type SearchClient struct {
Service SearchServiceClient
Services SearchServicesClient
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.

As in other places this change is to keep naming conventions consistent between services


//go:generate mockgen -destination=./mocks/sql_server.go -package=mocks . SQLServerClient,SQLFirewallClient,SQLServerAdminClient,SQLServerBlobAuditingPolicies,SQLServerDevOpsAuditSettingsClient,SQLServerVulnerabilityAssessmentsClient,EncryptionProtectorsClient,SQLVirtualNetworkRulesClient,ServerSecurityAlertPoliciesClient
type SQLServerClient interface {
//go:generate mockgen -destination=./mocks/sql.go -package=mocks . SQLServersClient,SQLFirewallsClient,SQLServerAdminsClient,SQLServerBlobAuditingPoliciesClient,SQLServerDevOpsAuditSettingsClient,SQLServerVulnerabilityAssessmentsClient,SQLEncryptionProtectorsClient,SQLVirtualNetworkRulesClient,ServerSecurityAlertPoliciesClient,SQLDatabasesClient,SQLDatabaseBlobAuditingPoliciesClient,SQLDatabaseThreatDetectionPoliciesClient,SQLDatabaseVulnerabilityAssessmentsClient,SQLDatabaseVulnerabilityAssessmentScansClient,SQLTransparentDataEncryptionsClient,SQLBackupLongTermRetentionPoliciesClient,SQLManagedInstancesClient,SQLManagedInstanceVulnerabilityAssessmentsClient,SQLManagedInstanceEncryptionProtectorsClient,SQLManagedDatabasesClient,SQLManagedDatabaseVulnerabilityAssessmentsClient,SQLManagedDatabaseVulnerabilityAssessmentScansClient
Copy link
Copy Markdown
Member Author

@erezrokah erezrokah Sep 6, 2022

Choose a reason for hiding this comment

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

Moved all the go:generate stuff to the same place.

The sql package has a lot of custom code so I didn't simplify it yet

SubscriptionID: subscriptionId,
Subscriptions: s,
Tenants: t,
Locations: s,
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.

account/locations.go (see here) used the SubscriptionsClient which was a bit confusing.

I moved it under subscriptions/locations.go and added a separate SubscriptionsLocationsClient to make code generation easier and keep everything in a similar structure

return WebClient{
Apps: apps,
Apps: apps,
SiteAuthSettings: apps,
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.

Again I created a new WebSiteAuthSettingsClient to keep things consistent instead of having auth settings use the apps client (see here)

if len(os.Args) > 1 {
filter = os.Args[1]
}
pattern := regexp.MustCompile("(?i)" + filter)
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.

Found it useful to be able to generate only a part of the resources

providerResources := make(map[string]providerResource)
providerPackages := make(map[string]bool)

for _, r := range codegen.AllResources() {
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 tried to keep this file as simple as possible and leave all resource creating logic under recipes/base.go

Destination string
}

type Resource struct {
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 what is passes to the template engine

imports []string
}

type resourceDefinition struct {
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 the resource configuration. Has some duplication with Resource but I thought it would be easier to separate the 2 as Resource is a transformed version of resourceDefinition


func needsSubscriptionId(table *codegen.TableDefinition) bool {
for _, column := range table.Columns {
if column.Name == "subscription_id" {
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.

Some resources have subscription_id column returned from the API so no need to resolve it

{
azureStruct: &compute.Disk{},
listFunction: "List",
mockListResult: "DiskList",
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.

Some APIs have inconsistent naming so we need to override them in the tests

{
azureStruct: &hsmKeyValue.ManagedHsm{},
listFunction: "ListBySubscription",
listFunctionArgs: []string{"&maxResults"},
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.

Some list functions need additional arguments

listHandler: valueHandler,
subServiceOverride: "Accounts",
mockListResult: "DatabaseAccountsListResult",
relations: []string{"MongoDBDatabases(),SQLDatabases()"},
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.

MongoDBDatabases and SQLDatabases are generated below

listFunction: "ListMongoDBDatabases",
listHandler: valueHandler,
subServiceOverride: "MongoDBDatabases",
listFunctionArgsInit: []string{`account := parent.Item.(documentdb.DatabaseAccountGetResults)
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.

We get the account id and name from the parent so we can list the relation

{
azureStruct: &mariadb.Server{},
listFunction: "List",
listHandler: valueHandler,
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.

Some APIs return a paginated list and some a non paginated list. valueHandler is a handler for the non paginated list

}`},
mockListResult: "EventDataCollection",
mockListFunctionArgs: []string{"regexMatcher{filterRe}", `""`},
mockListFunctionArgsInit: []string{"filterRe := regexp.MustCompile(`eventTimestamp ge '\\d{4}-\\d\\d-\\d\\dT\\d\\d:\\d\\d:\\d\\d(\\.\\d+)Z' and eventTimestamp le '\\d{4}-\\d\\d-\\d\\dT\\d\\d:\\d\\d:\\d\\d(\\.\\d+)Z'`)"},
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 bit ugly since we want to match a timestamp in the tests

azureStruct: &analytics.DataLakeAnalyticsAccount{},
listFunction: "List",
listFunctionArgs: []string{`""`, `nil`, `nil`, `""`, `""`, `nil`},
getFunction: "Get",
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.

Example of a list/details pattern

Type: schema.TypeBool,
Resolver: "resolveEnabled",
}},
helpers: []string{`func resolveEnabled(ctx context.Context, meta schema.ClientMeta, resource *schema.Resource, c schema.Column) error {
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 bit tricky as there are multiple types of settings, only 2 has the Enabled field so we need a custom resolved that checks for the type

@erezrokah
Copy link
Copy Markdown
Member Author

2. Im worried about resolvers auto generation as it makes it harder to change and contribute in some places (Im ok with mock tests where it's vanila generation). I don't think we need to change anything now and it will be interesting to compare the developer experience as we go in azure where we generate everything vs all other plugins where we generate only the tables. So just something to keep and eye on.

Yes I agree and I think another iteration that makes sense for Azure (and also other plugins), is when we have inconsistent APIs and this makes auto generation hard, instead of having a _fetch with custom code, move the custom code under the services package and wrap it with an easy to generate API, for example:
https://github.com/erezrokah/cloudquery/blob/1171096bd4e56850cffb8119a326e96c1a04def6/plugins/source/azure/client/services/storage.go#L57
https://github.com/erezrokah/cloudquery/blob/1171096bd4e56850cffb8119a326e96c1a04def6/plugins/source/azure/client/services/web.go#L52
That should make code gen easier see https://github.com/erezrokah/cloudquery/blob/1171096bd4e56850cffb8119a326e96c1a04def6/plugins/source/azure/resources/services/web/publishing_profiles.go#L52 vs

func fetchWebAppPublishingProfiles(ctx context.Context, meta schema.ClientMeta, parent *schema.Resource, res chan<- interface{}) error {
, or https://github.com/erezrokah/cloudquery/blob/1171096bd4e56850cffb8119a326e96c1a04def6/plugins/source/azure/resources/services/storage/accounts.go#L234 vs
storageClient := meta.(*client.Client).Services().Storage

@erezrokah erezrokah force-pushed the chore/codegen_azure_v3 branch from 1171096 to 1b44033 Compare September 18, 2022 06:47
@erezrokah
Copy link
Copy Markdown
Member Author

do_not_run_extensions_on_overprovisioned_v_ms
ResourceURI in multiple places (needs to be referred as "ResourceURI" in SQL)
azure_compute_virtual_machine_scale_sets.host is missing, no way to get to the vm id? but azure_compute_virtual_machine_extensions contains compute_virtual_machine_id and no reference to scale set.

Thanks @disq, I'm going to do a fast follow PR with these fixed as this PR is already huge

@erezrokah erezrokah merged commit ee9bef2 into cloudquery:main Sep 18, 2022
kodiakhq bot pushed a commit that referenced this pull request Sep 21, 2022
🤖 I have created a release *beep* *boop*
---


## [0.14.0-pre.0](plugins/source/azure-v0.13.4-pre.0...plugins/source/azure/v0.14.0-pre.0) (2022-09-21)


### ⚠ BREAKING CHANGES

* Migrate Azure plugin to v2 (#1754)
* Fix Azure credential chain (#1283)

### Features

* Add website, docs and blog to our main repo ([#1159](#1159)) ([dd69948](dd69948))
* Added azure cdn profiles ([#1460](#1460)) ([cc154c5](cc154c5))
* Migrate Azure plugin to v2 ([#1754](#1754)) ([ee9bef2](ee9bef2))


### Bug Fixes

* **deps:** Update golang.org/x/sync digest to 7fc1605 ([#1652](#1652)) ([daafae1](daafae1))
* **deps:** Update module github.com/Azure/azure-sdk-for-go/sdk/azcore to v1.1.2 ([#1664](#1664)) ([5390e13](5390e13))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.10 ([#1474](#1474)) ([b142e13](b142e13))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.11 ([#1491](#1491)) ([5140bef](5140bef))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.12 ([#1503](#1503)) ([a740719](a740719))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.9 ([#1286](#1286)) ([67ac422](67ac422))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.3 ([#1858](#1858)) ([9e3ace7](9e3ace7))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.4 ([#1862](#1862)) ([5d141cf](5d141cf))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.1 ([#1865](#1865)) ([474bb70](474bb70))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.2 ([#1872](#1872)) ([49ed26d](49ed26d))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.3 ([#1886](#1886)) ([7435d59](7435d59))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.4 ([#1889](#1889)) ([63a5362](63a5362))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.9 ([#1891](#1891)) ([3469f20](3469f20))
* Fix Azure credential chain ([#1283](#1283)) ([c2aadf7](c2aadf7))
* Generate Azure date.time as Timestamps ([#1885](#1885)) ([92d41a1](92d41a1))
* Regenerate Azure resources ([#1875](#1875)) ([7411a27](7411a27))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
yevgenypats pushed a commit that referenced this pull request Sep 26, 2022
🤖 I have created a release *beep* *boop*
---


##
[1.0.0-pre.0](plugins-source-azure-v0.14.2-pre.0...plugins-source-azure-v1.0.0-pre.0)
(2022-09-26)


### ⚠ BREAKING CHANGES

* Migrate Azure plugin to v2 (#1754)
* Fix Azure credential chain (#1283)

### Features

* Add website, docs and blog to our main repo
([#1159](#1159))
([dd69948](dd69948))
* Added azure cdn profiles
([#1460](#1460))
([cc154c5](cc154c5))
* Migrate Azure plugin to v2
([#1754](#1754))
([ee9bef2](ee9bef2))


### Bug Fixes

* Add missing `azure_keyvault_secrets` tables
([#1937](#1937))
([491aa66](491aa66))
* **deps:** Update golang.org/x/sync digest to 7fc1605
([#1652](#1652))
([daafae1](daafae1))
* **deps:** Update module github.com/Azure/azure-sdk-for-go/sdk/azcore
to v1.1.2
([#1664](#1664))
([5390e13](5390e13))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.10
([#1474](#1474))
([b142e13](b142e13))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.11
([#1491](#1491))
([5140bef](5140bef))
* **deps:** Update module github.com/cloudquery/cq-gen to v0.0.12
([#1503](#1503))
([a740719](a740719))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.3
([#1858](#1858))
([9e3ace7](9e3ace7))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.4
([#1862](#1862))
([5d141cf](5d141cf))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.1
([#1865](#1865))
([474bb70](474bb70))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.2
([#1872](#1872))
([49ed26d](49ed26d))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.3
([#1886](#1886))
([7435d59](7435d59))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.4
([#1889](#1889))
([63a5362](63a5362))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.9
([#1891](#1891))
([3469f20](3469f20))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.0
([#1997](#1997))
([4fa40da](4fa40da))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.1
([#2024](#2024))
([8f88de4](8f88de4))
* **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.2
([#2044](#2044))
([9b69b46](9b69b46))
* Fix Azure credential chain
([#1283](#1283))
([c2aadf7](c2aadf7))
* Generate Azure date.time as Timestamps
([#1885](#1885))
([92d41a1](92d41a1))
* Regenerate Azure resources
([#1875](#1875))
([7411a27](7411a27))
* Update Azure codegen
([#1936](#1936))
([4b739db](4b739db))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
@erezrokah erezrokah deleted the chore/codegen_azure_v3 branch December 15, 2023 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants