fix(aws)!: Unified tag structure by disq · Pull Request #3330 · cloudquery/cloudquery

disq · 2022-10-31T12:21:46Z

All tags (except one) should be JSON objects (and not JSON arrays) now.
The exception is aws_autoscaling_groups, which tags is defined as []TagDescription, which is defined as:

// Describes a tag for an Auto Scaling group.
type TagDescription struct {

	// The tag key.
	Key *string

	// Determines whether the tag is added to new instances as they are launched in the
	// group.
	PropagateAtLaunch *bool

	// The name of the group.
	ResourceId *string

	// The type of resource. The only supported value is auto-scaling-group.
	ResourceType *string

	// The tag value.
	Value *string

	noSmithyDocumentSerde
}

disq · 2022-10-31T12:24:40Z

plugins/source/aws/codegen/recipes/iam.go

 						Name:     "user_tags",
 						Type:     schema.TypeJSON,
-						Resolver: `client.ResolveTags`,
+						Resolver: `client.ResolveTags`, // FIXME this is wrong, add path


This needs to be client.ResolveTagField("User.Tags") or something like that but I'm not sure about the syntax... (parent struct also has Tags and it's handled by the auto resolver)

Tested: User tags are not exposed in the User struct from ListVirtualMFADevices, so removing this column.

plugins/source/aws/codegen/recipes/base.go

erezrokah

Nice touch with the custom resolver.

I think this should be a fix! as it's a breaking change (even without the column rename)

plugins/source/aws/codegen/recipes/base.go

plugins/source/aws/docs/tables/aws_ec2_network_interfaces.md

disq · 2022-10-31T13:11:31Z

I think this should be a fix! as it's a breaking change (even without the column rename)

JSON is JSON though... and we weren't intending these to be arrays.

erezrokah · 2022-10-31T13:14:58Z

JSON is JSON though... and we weren't intending these to be arrays.

Yeah, but the schema of the JSON is different correct? i.e. if someone has a query on it, that query will break?

disq · 2022-10-31T13:19:30Z

JSON is JSON though... and we weren't intending these to be arrays.

Yeah, but the schema of the JSON is different correct? i.e. if someone has a query on it, that query will break?

yes but it will be better in the end

erezrokah · 2022-10-31T13:26:25Z

yes but it will be better in the end

Agree it's better but it's still breaking, and with a major version bump it's clear to users that they need to update their queries

plugins/source/aws/codegen/recipes/base.go

Barneyjm · 2022-10-31T13:31:53Z

Agree it's better but it's still breaking, and with a major version bump it's clear to users that they need to update their queries

FWIW the columns in some of these tables pre-V1 are type(Object) but in V1 are type(Array). This change would make it so that we don't have breaking changes

erezrokah · 2022-10-31T13:58:34Z

FWIW the columns in some of these tables pre-V1 are type(Object) but in V1 are type(Array). This change would make it so that we don't have breaking changes

Thanks for the added context @Barneyjm. Agree this is not breaking compared to v0, but it can be considered breaking compared to the latest version of the latest AWS plugin.

We can make this clear with a note in the changelog, WDYT?

bbernays · 2022-10-31T14:34:08Z

This doesn't help resources that have to make specific call to get tags like Neptune/DocDB/RDS:

cloudquery/plugins/source/aws/codegen/recipes/neptune.go

Lines 26 to 29 in 776d849

    
           	Name:     "tags", 
        
           	Type:     schema.TypeJSON, 
        
           	Resolver: `resolveNeptuneClusterParameterGroupTags`, 
        
           },

🤖 I have created a release *beep* *boop* --- ## [4.0.0](plugins-source-aws-v3.8.0...plugins-source-aws-v4.0.0) (2022-11-02) ### ⚠ BREAKING CHANGES * **aws:** Unified tag structure (#3330) ### Features * Add eventbridge resources ([#3160](#3160)) ([67d3a35](67d3a35)) * **aws:** Fraud Detector support ([#3076](#3076)) ([f0e309a](f0e309a)) * Migrate cli, plugins and destinations to new type system ([#3323](#3323)) ([f265a94](f265a94)) * Update AWS Services (new fields) ([#3324](#3324)) ([0b65803](0b65803)) ### Bug Fixes * Add id to aws_cloudfront_cache_policies, region to aws_ec2_transit_gateways ([#3444](#3444)) ([41362e2](41362e2)) * **aws:** Unified tag structure ([#3330](#3330)) ([c9c1e4c](c9c1e4c)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/credentials to v1.12.23 ([#3378](#3378)) ([c33bf73](c33bf73)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/feature/s3/manager to v1.11.37 ([#3379](#3379)) ([3f1a71d](3f1a71d)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/acm to v1.15.2 ([#3380](#3380)) ([b4329ee](b4329ee)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/apigateway to v1.15.22 ([#3381](#3381)) ([5816480](5816480)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/apigatewayv2 to v1.12.20 ([#3385](#3385)) ([2dd39a6](2dd39a6)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/applicationautoscaling to v1.15.20 ([#3386](#3386)) ([0702717](0702717)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/appsync to v1.15.12 ([#3387](#3387)) ([6bb936a](6bb936a)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/athena to v1.18.12 ([#3388](#3388)) ([e95f748](e95f748)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/autoscaling to v1.23.18 ([#3389](#3389)) ([5af977f](5af977f)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/backup to v1.17.11 ([#3390](#3390)) ([1ecaab7](1ecaab7)) * **deps:** Update module github.com/aws/aws-sdk-go-v2/service/cloudfront to v1.20.7 ([#3391](#3391)) ([46465d1](46465d1)) * **deps:** Update plugin-sdk for aws to v0.13.17 ([#3399](#3399)) ([f2cd266](f2cd266)) * **deps:** Update plugin-sdk for aws to v0.13.18 ([#3409](#3409)) ([92fa576](92fa576)) * Filtering for DocDB + Neptune ([#3271](#3271)) ([8080c6e](8080c6e)) * Update endpoints ([#3368](#3368)) ([b78c6b1](b78c6b1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

disq requested review from hermanschaaf and yevgenypats as code owners October 31, 2022 12:21

cq-bot added the aws label Oct 31, 2022

disq commented Oct 31, 2022

View reviewed changes

candiduslynx suggested changes Oct 31, 2022

View reviewed changes

plugins/source/aws/codegen/recipes/base.go Show resolved Hide resolved

plugins/source/aws/codegen/recipes/base.go Outdated Show resolved Hide resolved

erezrokah approved these changes Oct 31, 2022

View reviewed changes

plugins/source/aws/codegen/recipes/base.go Show resolved Hide resolved

plugins/source/aws/docs/tables/aws_ec2_network_interfaces.md Outdated Show resolved Hide resolved

disq changed the title ~~fix(aws): Unified tag structure~~ fix!(aws): Unified tag structure Oct 31, 2022

disq changed the title ~~fix!(aws): Unified tag structure~~ fix(aws)!: Unified tag structure Oct 31, 2022

hermanschaaf reviewed Oct 31, 2022

View reviewed changes

plugins/source/aws/codegen/recipes/base.go Outdated Show resolved Hide resolved

hermanschaaf reviewed Oct 31, 2022

View reviewed changes

plugins/source/aws/codegen/recipes/base.go Show resolved Hide resolved

disq requested a review from candiduslynx October 31, 2022 15:30

disq added 6 commits November 1, 2022 13:31

fix(aws): Unified tag structure

f4e2684

[controversial] Auto-tag treatment

de11bb0

Remove always-null UserTags field

5631013

CR: Use types for comparison

e069b61

revert "tag_set" change, use sdk default resolver generator

bd5dc21

shield/xray: Clean up (replace improper TagsIntoMap use with TagsToMap)

fbc4cd4

disq force-pushed the fix/aws-unify-tags branch from 455b358 to fbc4cd4 Compare November 1, 2022 13:33

hermanschaaf approved these changes Nov 1, 2022

View reviewed changes

candiduslynx approved these changes Nov 1, 2022

View reviewed changes

yevgenypats approved these changes Nov 1, 2022

View reviewed changes

disq added automerge Automatically merge once required checks pass priority merge labels Nov 1, 2022

Merge branch 'main' into fix/aws-unify-tags

0a93442

vercel bot deployed to Preview – cloudquery-web November 1, 2022 14:44 View deployment

kodiakhq bot merged commit c9c1e4c into cloudquery:main Nov 1, 2022

cq-bot mentioned this pull request Nov 1, 2022

chore(main): Release plugins-source-aws v4.0.0 #3369

Merged

disq deleted the fix/aws-unify-tags branch November 1, 2022 14:55

Conversation

disq commented Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

disq Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

disq Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erezrokah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

disq commented Oct 31, 2022

Uh oh!

erezrokah commented Oct 31, 2022

Uh oh!

disq commented Oct 31, 2022

Uh oh!

erezrokah commented Oct 31, 2022

Uh oh!

Uh oh!

Uh oh!

Barneyjm commented Oct 31, 2022

Uh oh!

erezrokah commented Oct 31, 2022

Uh oh!

bbernays commented Oct 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

disq commented Oct 31, 2022 •

edited

Loading

disq Oct 31, 2022 •

edited

Loading