Skip to content
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.

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

Tech debt: Reduce tags boilerplate code - Phase 2 #30280

Merged
merged 89 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
89 commits
Select commit Hold shift + click to select a range
2cd9027
fix(resource_ssm_document): handle dynamic field refresh when content…
jBouyoud Dec 20, 2022
471cf08
Add 'ServicePackageResourceTags.ResourceType'.
ewbankkit Mar 24, 2023
7e8eede
r/aws_ssm_document: Alphabetize attributes.
ewbankkit Mar 24, 2023
c1b86c4
Simplify 'Option.Unwrap...'.
ewbankkit Mar 27, 2023
258b3bf
'InContext.TagsIn' is 'Option[KeyValueTags]'.
ewbankkit Mar 27, 2023
cf6058c
Add 'Option.MustUnwrap'.
ewbankkit Mar 27, 2023
0250103
Revert "'InContext.TagsIn' is 'Option[KeyValueTags]'."
ewbankkit Mar 27, 2023
702361a
Initialize 'InContext.TagsOut'.
ewbankkit Mar 27, 2023
4e23235
Revert "Revert "'InContext.TagsIn' is 'Option[KeyValueTags]'.""
ewbankkit Mar 27, 2023
3b528a6
Tweak generated 'GetTagsIn'.
ewbankkit Mar 27, 2023
a35e5c4
Run 'make gen'.
ewbankkit Mar 27, 2023
2f2464a
Add 'slices.Chunks'.
ewbankkit Mar 27, 2023
06fb130
r/aws_ssm_document: Tidy up resource Create and Delete.
ewbankkit Mar 27, 2023
748e08f
Merge branch 'main' into f-reduce-tags-boilerplate-phase2
ewbankkit Mar 27, 2023
a475cbc
r/aws_ssm_document: Tidy up resource Read.
ewbankkit Mar 27, 2023
f86015f
r/aws_ssm_document: Tidy up resource Update.
ewbankkit Mar 27, 2023
bd09390
r/aws_ssm_document: Tidy up acceptance tests
ewbankkit Mar 27, 2023
973f8c6
d/aws_ssm_document: Tidy up.
ewbankkit Mar 27, 2023
ba9115e
r/aws_ssm_document: Simplify.
ewbankkit Mar 27, 2023
869ab83
Merge remote-tracking branch 'jBouyoud/fix-ssm-doc-refresh' into f-re…
ewbankkit Mar 27, 2023
cf9660c
Add CHANGELOG entry.
ewbankkit Mar 27, 2023
dfd2ff0
Handle 'ListTags' and 'UpdateTags' methods with extra, resource type,…
ewbankkit Mar 27, 2023
98719fd
r/aws_ssm_document: Remove tagging boilerplate.
ewbankkit Mar 27, 2023
fe83e9f
r/aws_ssm_maintenance_window: Alphabetize attributes.
ewbankkit Mar 27, 2023
2a7c147
r/aws_ssm_maintenance_window: Tidy up resource Create.
ewbankkit Mar 27, 2023
1674f8d
r/aws_ssm_maintenance_window: Tidy up resource Update and Delete.
ewbankkit Mar 27, 2023
1172e41
r/aws_ssm_maintenance_window: Tidy up resource Read.
ewbankkit Mar 27, 2023
ed82d0f
r/aws_ssm_maintenance_window: Tidy up acceptance tests.
ewbankkit Mar 27, 2023
992d7fd
r/aws_ssm_maintenance_window: Remove tagging boilerplate.
ewbankkit Mar 27, 2023
5cbd94e
Fix terrafmt error.
ewbankkit Mar 27, 2023
e72e09c
Fix golangci-lint 'ineffassign'.
ewbankkit Mar 28, 2023
0fc25cf
r/aws_ssm_activation: Alphabetize attributes.
ewbankkit Mar 28, 2023
1b573ca
r/aws_ssm_document: Better error handling in Delete.
ewbankkit Mar 28, 2023
779ce39
r/aws_ssm_activation: Tidy up resource Create and Delete.
ewbankkit Mar 28, 2023
5904122
'FindMaintenanceWindowsByID' -> 'FindMaintenanceWindowByID'.
ewbankkit Mar 28, 2023
8709146
r/aws_ssm_activation: Tidy up resource Read.
ewbankkit Mar 28, 2023
d7fe84d
r/aws_ssm_activation: Tidy up acceptance tests.
ewbankkit Mar 28, 2023
9148d8e
Add CHANGELOG entry (#27952).
ewbankkit Mar 28, 2023
46c6340
Fix terrafmt error.
ewbankkit Mar 28, 2023
02c1438
'identifierAttribute' is optional for '@Tags' annotation.
ewbankkit Mar 28, 2023
a134ce4
'identifierAttribute' is optional for '@Tags' annotation.
ewbankkit Mar 28, 2023
e3e3f02
r/aws_ssm_activation: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
ca2dd72
r/aws_ssm_parameter: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
3fa24da
r/aws_ssm_patch_baseline: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
7e2964e
r/aws_ssoadmin_permission_set: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
7286042
r/aws_route53_health_check: Alphabetize attributes.
ewbankkit Mar 28, 2023
b70329d
r/aws_route53_health_check: Tidy up resource Create.
ewbankkit Mar 28, 2023
451f4c4
r/aws_route53_health_check: Tidy up resource Update and Delete.
ewbankkit Mar 28, 2023
01adfc9
r/aws_route53_health_check: Tidy up resource Read.
ewbankkit Mar 28, 2023
24d76df
r/aws_route53_health_check: Tidy up acceptance tests.
ewbankkit Mar 28, 2023
34dbdb5
r/aws_route53_health_check: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
6397719
r/aws_route53_zone: Tidy up.
ewbankkit Mar 28, 2023
beb3e4a
r/aws_route53_zone: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
b3723bf
r/aws_inspector_assessment_template: Alphabetize attributes.
ewbankkit Mar 28, 2023
40487f9
r/aws_inspector_assessment_template: Tidy up.
ewbankkit Mar 28, 2023
2874063
r/aws_inspector_assessment_template: Remove tagging boilerplate.
ewbankkit Mar 28, 2023
edefe1e
Add 'v := v'.
ewbankkit Mar 29, 2023
9bc06d5
r/aws_cloudwatch_log_group: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
cc2d10f
r/aws_cloudwatch_log_destination: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
8f0eae9
r/aws_iam_instance_profile: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
1e1b137
r/aws_iam_openid_connect_provider: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
b9949ab
r/aws_iam_policy: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
c917455
r/aws_iam_role: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
fea79b1
r/aws_iam_saml_provider: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
8c4ccdb
r/aws_iam_server_certificate: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
746b008
r/aws_iam_service_linked_role: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
de6bce1
r/aws_iam_user: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
5f0a04a
r/aws_iam_virtual_mfa_device: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
bc73505
r/aws_servicecatalog_portfolio: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
0801eae
r/aws_servicecatalog_product: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
b26f3dd
Fix golangci-lint 'unparam'.
ewbankkit Mar 29, 2023
3cff2c2
Set 'InContext.TagsIn' for Update.
ewbankkit Mar 29, 2023
38c4d7d
r/aws_servicecatalog_provisioned_product: Remove tagging boilerplate.
ewbankkit Mar 29, 2023
c318241
IAM: Tidy up custom tagging functions.
ewbankkit Mar 30, 2023
93c8abc
Service Catalog: Tidy up custom tagging functions.
ewbankkit Mar 30, 2023
1954e79
S3: Tidy up custom tagging functions.
ewbankkit Mar 30, 2023
6ee0032
Move S3 error codes.
ewbankkit Mar 30, 2023
aaa63d8
Make some S3 error codes package private.
ewbankkit Mar 30, 2023
e553f63
r/aws_s3_bucket: Alphabetize attributes.
ewbankkit Mar 30, 2023
9a3edfb
r/aws_s3_bucket: Use 'create.Name' and 'create.NamePrefixFromName'.
ewbankkit Mar 30, 2023
dfab531
r/aws_s3_bucket: Order CRUD handlers.
ewbankkit Mar 30, 2023
285ca26
r/aws_s3_bucket: Use 'FindBucket'.
ewbankkit Mar 30, 2023
3fe67f7
r/aws_s3_bucket: Remove tagging boilerplate.
ewbankkit Mar 30, 2023
05c06ba
S3: Use 'FindObjectByThreePartKey'.
ewbankkit Mar 30, 2023
96a73d2
r/aws_s3_bucket_object: Remove tagging boilerplate.
ewbankkit Mar 30, 2023
c3d7c03
r/aws_s3_object: Remove tagging boilerplate.
ewbankkit Mar 30, 2023
cb81e0f
r/aws_s3_object_copy: Remove tagging boilerplate.
ewbankkit Mar 30, 2023
989dcef
Merge branch 'main' into f-reduce-tags-boilerplate-phase2
ewbankkit Mar 30, 2023
1821565
Fix typo.
ewbankkit Mar 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/28489.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ssm_document: Correctly set `default_version`, `document_version`, `hash`, `latest_version` and `parameter` as Computed when `content` changes
```
3 changes: 3 additions & 0 deletions .changelog/30280.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ssm_activation: Fix IAM eventual consistency errors on resource Create
```
11 changes: 8 additions & 3 deletions internal/generate/servicepackages/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ func main() {
type ResourceDatum struct {
FactoryName string
Name string // Friendly name (without service name), e.g. "Topic", not "SNS Topic"
TransparentTagging bool
TagsIdentifierAttribute string
TagsResourceType string
}

type ServiceDatum struct {
Expand Down Expand Up @@ -222,15 +224,18 @@ func (v *visitor) processFuncDecl(funcDecl *ast.FuncDecl) {
if m := annotation.FindStringSubmatch(line); len(m) > 0 && m[1] == "Tags" {
args := common.ParseArgs(m[3])

d.TransparentTagging = true

if attr, ok := args.Keyword["identifierAttribute"]; ok {
if d.TagsIdentifierAttribute != "" {
v.err = multierror.Append(v.err, fmt.Errorf("multiple Tags annotations: %s", fmt.Sprintf("%s.%s", v.packageName, v.functionName)))
}

d.TagsIdentifierAttribute = attr
} else {
v.err = multierror.Append(v.err, fmt.Errorf("no Tags(identifierAttribute): %s", fmt.Sprintf("%s.%s", v.packageName, v.functionName)))
continue
}

if attr, ok := args.Keyword["resourceType"]; ok {
d.TagsResourceType = attr
}
}
}
Expand Down
28 changes: 24 additions & 4 deletions internal/generate/servicepackages/spd.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@ func (p *servicePackage) FrameworkDataSources(ctx context.Context) []*types.Serv
{{- if ne .Name "" }}
Name: "{{ .Name }}",
{{- end }}
{{- if ne .TagsIdentifierAttribute "" }}
{{- if .TransparentTagging }}
Tags: &types.ServicePackageResourceTags {
{{- if ne .TagsIdentifierAttribute "" }}
IdentifierAttribute: "{{ .TagsIdentifierAttribute }}",
{{- end }}
{{- if ne .TagsResourceType "" }}
ResourceType: "{{ .TagsResourceType }}",
{{- end }}
},
{{- end }}
},
Expand All @@ -39,9 +44,14 @@ func (p *servicePackage) FrameworkResources(ctx context.Context) []*types.Servic
{{- if ne .Name "" }}
Name: "{{ .Name }}",
{{- end }}
{{- if ne .TagsIdentifierAttribute "" }}
{{- if .TransparentTagging }}
Tags: &types.ServicePackageResourceTags {
{{- if ne .TagsIdentifierAttribute "" }}
IdentifierAttribute: "{{ .TagsIdentifierAttribute }}",
{{- end }}
{{- if ne .TagsResourceType "" }}
ResourceType: "{{ .TagsResourceType }}",
{{- end }}
},
{{- end }}
},
Expand All @@ -58,9 +68,14 @@ func (p *servicePackage) SDKDataSources(ctx context.Context) []*types.ServicePac
{{- if ne $value.Name "" }}
Name: "{{ $value.Name }}",
{{- end }}
{{- if ne $value.TagsIdentifierAttribute "" }}
{{- if $value.TransparentTagging }}
Tags: &types.ServicePackageResourceTags {
{{- if ne $value.TagsIdentifierAttribute "" }}
IdentifierAttribute: "{{ $value.TagsIdentifierAttribute }}",
{{- end }}
{{- if ne .TagsResourceType "" }}
ResourceType: "{{ .TagsResourceType }}",
{{- end }}
},
{{- end }}
},
Expand All @@ -77,9 +92,14 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka
{{- if ne $value.Name "" }}
Name: "{{ $value.Name }}",
{{- end }}
{{- if ne $value.TagsIdentifierAttribute "" }}
{{- if $value.TransparentTagging }}
Tags: &types.ServicePackageResourceTags {
{{- if ne $value.TagsIdentifierAttribute "" }}
IdentifierAttribute: "{{ $value.TagsIdentifierAttribute }}",
{{- end }}
{{- if ne .TagsResourceType "" }}
ResourceType: "{{ .TagsResourceType }}",
{{- end }}
},
{{- end }}
},
Expand Down
80 changes: 58 additions & 22 deletions internal/provider/fwprovider/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,18 +363,27 @@ func (r tagsInterceptor) read(ctx context.Context, request resource.ReadRequest,

// If the R handler didn't set tags, try and read them from the service API.
if tagsInContext.TagsOut.IsNone() {
// If the service package has a generic resource list tags methods, call it.
if v, ok := sp.(interface {
ListTags(context.Context, any, string) error
}); ok {
if identifierAttribute := r.tags.IdentifierAttribute; identifierAttribute != "" {
var identifier string
diags.Append(request.State.GetAttribute(ctx, path.Root(r.tags.IdentifierAttribute), &identifier)...)

diags.Append(request.State.GetAttribute(ctx, path.Root(identifierAttribute), &identifier)...)

if diags.HasError() {
return ctx, diags
}

err := v.ListTags(ctx, meta, identifier) // Sets tags in Context
// If the service package has a generic resource list tags methods, call it.
var err error

if v, ok := sp.(interface {
ListTags(context.Context, any, string) error
}); ok {
err = v.ListTags(ctx, meta, identifier) // Sets tags in Context
} else if v, ok := sp.(interface {
ListTags(context.Context, any, string, string) error
}); ok && r.tags.ResourceType != "" {
err = v.ListTags(ctx, meta, identifier, r.tags.ResourceType) // Sets tags in Context
}

if err != nil {
diags.AddError(fmt.Sprintf("listing tags for %s %s (%s)", serviceName, resourceName, identifier), err.Error())
Expand Down Expand Up @@ -440,36 +449,63 @@ func (r tagsInterceptor) update(ctx context.Context, request resource.UpdateRequ
resourceName = "<thing>"
}

tagsInContext, ok := tftags.FromContext(ctx)
if !ok {
return ctx, diags
}

switch when {
case Before:
// If the service package has a generic resource update tags methods, call it.
if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, any, any) error
}); ok {
var oldTagsAll, newTagsAll fwtypes.Map
var planTags fwtypes.Map
diags.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTags), &planTags)...)

diags.Append(request.State.GetAttribute(ctx, path.Root(names.AttrTagsAll), &oldTagsAll)...)
if diags.HasError() {
return ctx, diags
}

if diags.HasError() {
return ctx, diags
}
// Merge the resource's configured tags with any provider configured default_tags.
tags := tagsInContext.DefaultConfig.MergeTags(tftags.New(ctx, planTags))
// Remove system tags.
tags = tags.IgnoreAWS()

diags.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTagsAll), &newTagsAll)...)
tagsInContext.TagsIn = types.Some(tags)

if diags.HasError() {
return ctx, diags
}
var oldTagsAll, newTagsAll fwtypes.Map

diags.Append(request.State.GetAttribute(ctx, path.Root(names.AttrTagsAll), &oldTagsAll)...)

if diags.HasError() {
return ctx, diags
}

diags.Append(request.Plan.GetAttribute(ctx, path.Root(names.AttrTagsAll), &newTagsAll)...)

if diags.HasError() {
return ctx, diags
}

if !newTagsAll.Equal(oldTagsAll) {
if !newTagsAll.Equal(oldTagsAll) {
if identifierAttribute := r.tags.IdentifierAttribute; identifierAttribute != "" {
var identifier string

diags.Append(request.Plan.GetAttribute(ctx, path.Root(r.tags.IdentifierAttribute), &identifier)...)
diags.Append(request.Plan.GetAttribute(ctx, path.Root(identifierAttribute), &identifier)...)

if diags.HasError() {
return ctx, diags
}

err := v.UpdateTags(ctx, meta, identifier, oldTagsAll, newTagsAll)
// If the service package has a generic resource update tags methods, call it.
var err error

if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, any, any) error
}); ok {
err = v.UpdateTags(ctx, meta, identifier, oldTagsAll, newTagsAll)
} else if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, string, any, any) error
}); ok && r.tags.ResourceType != "" {
err = v.UpdateTags(ctx, meta, identifier, r.tags.ResourceType, oldTagsAll, newTagsAll)
}

if verify.ErrorISOUnsupported(meta.Partition, err) {
// ISO partitions may not support tagging, giving error
Expand Down
2 changes: 2 additions & 0 deletions internal/provider/fwprovider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ func (p *fwprovider) DataSources(ctx context.Context) []func() datasource.DataSo
servicePackageName := sp.ServicePackageName()

for _, v := range sp.FrameworkDataSources(ctx) {
v := v
inner, err := v.Factory(ctx)

if err != nil {
Expand Down Expand Up @@ -347,6 +348,7 @@ func (p *fwprovider) Resources(ctx context.Context) []func() resource.Resource {
servicePackageName := sp.ServicePackageName()

for _, v := range sp.FrameworkResources(ctx) {
v := v
inner, err := v.Factory(ctx)

if err != nil {
Expand Down
56 changes: 38 additions & 18 deletions internal/provider/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,27 +218,40 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
switch when {
case Before:
switch why {
case Create:
case Create, Update:
// Merge the resource's configured tags with any provider configured default_tags.
tags := tagsInContext.DefaultConfig.MergeTags(tftags.New(ctx, d.Get(names.AttrTags).(map[string]interface{})))
// Remove system tags.
tags = tags.IgnoreAWS()

tagsInContext.TagsIn = types.Some(tags)
case Update:
// If the service package has a generic resource update tags methods, call it.
if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, any, any) error
}); ok {
if d.HasChange(names.AttrTagsAll) {

if why == Create {
break
}

if d.HasChange(names.AttrTagsAll) {
if identifierAttribute := r.tags.IdentifierAttribute; identifierAttribute != "" {
var identifier string
if key := r.tags.IdentifierAttribute; key == "id" {
if identifierAttribute == "id" {
identifier = d.Id()
} else {
identifier = d.Get(key).(string)
identifier = d.Get(identifierAttribute).(string)
}
o, n := d.GetChange(names.AttrTagsAll)
err := v.UpdateTags(ctx, meta, identifier, o, n)

// If the service package has a generic resource update tags methods, call it.
var err error

if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, any, any) error
}); ok {
err = v.UpdateTags(ctx, meta, identifier, o, n)
} else if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, string, any, any) error
}); ok && r.tags.ResourceType != "" {
err = v.UpdateTags(ctx, meta, identifier, r.tags.ResourceType, o, n)
}

if verify.ErrorISOUnsupported(meta.(*conns.AWSClient).Partition, err) {
// ISO partitions may not support tagging, giving error
Expand Down Expand Up @@ -271,19 +284,26 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
case Create, Update:
// If the R handler didn't set tags, try and read them from the service API.
if tagsInContext.TagsOut.IsNone() {
// If the service package has a generic resource list tags methods, call it.
if v, ok := sp.(interface {
ListTags(context.Context, any, string) error
}); ok {
if identifierAttribute := r.tags.IdentifierAttribute; identifierAttribute != "" {
var identifier string

if key := r.tags.IdentifierAttribute; key == "id" {
if identifierAttribute == "id" {
identifier = d.Id()
} else {
identifier = d.Get(key).(string)
identifier = d.Get(identifierAttribute).(string)
}

err := v.ListTags(ctx, meta, identifier) // Sets tags in Context
// If the service package has a generic resource list tags methods, call it.
var err error

if v, ok := sp.(interface {
ListTags(context.Context, any, string) error
}); ok {
err = v.ListTags(ctx, meta, identifier) // Sets tags in Context
} else if v, ok := sp.(interface {
ListTags(context.Context, any, string, string) error
}); ok && r.tags.ResourceType != "" {
err = v.ListTags(ctx, meta, identifier, r.tags.ResourceType) // Sets tags in Context
}

if verify.ErrorISOUnsupported(meta.(*conns.AWSClient).Partition, err) {
// ISO partitions may not support tagging, giving error
Expand Down
2 changes: 2 additions & 0 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ func New(ctx context.Context) (*schema.Provider, error) {
servicePackageMap[servicePackageName] = sp

for _, v := range sp.SDKDataSources(ctx) {
v := v
typeName := v.TypeName

if _, ok := provider.DataSourcesMap[typeName]; ok {
Expand Down Expand Up @@ -299,6 +300,7 @@ func New(ctx context.Context) (*schema.Provider, error) {
}

for _, v := range sp.SDKResources(ctx) {
v := v
typeName := v.TypeName

if _, ok := provider.ResourcesMap[typeName]; ok {
Expand Down