Updated the Data Source Tags structure

[Ninir]

Issues & Context

Fixes #853
Fixes #1423

Tests

AWS Instance

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_ -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_basic
--- PASS: TestAccAWSInstanceDataSource_basic (171.58s)
=== RUN   TestAccAWSInstanceDataSource_tags
--- PASS: TestAccAWSInstanceDataSource_tags (172.30s)
=== RUN   TestAccAWSInstanceDataSource_AzUserData
--- PASS: TestAccAWSInstanceDataSource_AzUserData (171.20s)
=== RUN   TestAccAWSInstanceDataSource_gp2IopsDevice
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (150.83s)
=== RUN   TestAccAWSInstanceDataSource_blockDevices
--- PASS: TestAccAWSInstanceDataSource_blockDevices (157.89s)
=== RUN   TestAccAWSInstanceDataSource_rootInstanceStore
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (145.93s)
=== RUN   TestAccAWSInstanceDataSource_privateIP
--- PASS: TestAccAWSInstanceDataSource_privateIP (200.84s)
=== RUN   TestAccAWSInstanceDataSource_keyPair
--- PASS: TestAccAWSInstanceDataSource_keyPair (126.20s)
=== RUN   TestAccAWSInstanceDataSource_VPC
--- PASS: TestAccAWSInstanceDataSource_VPC (203.34s)
=== RUN   TestAccAWSInstanceDataSource_SecurityGroups
--- PASS: TestAccAWSInstanceDataSource_SecurityGroups (158.47s)
=== RUN   TestAccAWSInstanceDataSource_VPCSecurityGroups
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (222.70s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1881.28s

AWS AMI

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAmiDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAmiDataSource_ -timeout 120m
=== RUN   TestAccAWSAmiDataSource_natInstance
--- PASS: TestAccAWSAmiDataSource_natInstance (38.85s)
=== RUN   TestAccAWSAmiDataSource_windowsInstance
--- PASS: TestAccAWSAmiDataSource_windowsInstance (41.52s)
=== RUN   TestAccAWSAmiDataSource_instanceStore
--- PASS: TestAccAWSAmiDataSource_instanceStore (37.32s)
=== RUN   TestAccAWSAmiDataSource_owners
--- PASS: TestAccAWSAmiDataSource_owners (68.84s)
=== RUN   TestAccAWSAmiDataSource_ownersEmpty
--- PASS: TestAccAWSAmiDataSource_ownersEmpty (126.56s)
=== RUN   TestAccAWSAmiDataSource_localNameFilter
--- PASS: TestAccAWSAmiDataSource_localNameFilter (62.73s)
PASS
ok	github.com/terraform-providers/terraform-provider-aws/aws	376.069s

EBS Snapshot

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEbsSnapshotDataSource'                                             
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEbsSnapshotDataSource -timeout 120m
=== RUN   TestAccAWSEbsSnapshotDataSource_basic
--- PASS: TestAccAWSEbsSnapshotDataSource_basic (81.23s)
=== RUN   TestAccAWSEbsSnapshotDataSource_multipleFilters
--- PASS: TestAccAWSEbsSnapshotDataSource_multipleFilters (103.58s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	184.850s

EBS Volume

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEbsVolumeDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEbsVolumeDataSource_ -timeout 120m
=== RUN   TestAccAWSEbsVolumeDataSource_basic
--- PASS: TestAccAWSEbsVolumeDataSource_basic (58.11s)
=== RUN   TestAccAWSEbsVolumeDataSource_multipleFilters
--- PASS: TestAccAWSEbsVolumeDataSource_multipleFilters (54.35s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	112.494s

[Ninir]

Hey @apparentlymart

Since we talked about this in #1423, I would like to know if we really require a migration there (which can be done), or if, as it does not work at all right now, we don't need it?

Will continue to push updates for other data sources.

Thanks!

[Ninir]

This is not even used in this Data Source

[Ninir]

This is not even used in this Data Source

[Ninir]

@apparentlymart @radeksimko Letting you the final review :)
As it is not working at the moment, not sure this would need a migration, tell me if so.

Thanks! 👍

[radeksimko]

AFAICT data sources don't need migrations in general, because the data is always pulled from scratch as part of every plan/apply.

The reason we usually need a migration for a resource is to avoid no-op diffs (the exact same data, just differently structured) or avoid crashes caused by reading old state. Data sources practically never read old state or generally never compare state and produce diffs.

Also terraform apply -refresh=false is no-op for data sources, so even if it's referenced anywhere, you'll just get the old state, but the Read() method won't get executed.

[Ninir]

Thanks for the info there @radeksimko :)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!