Skip to content

Commit

Permalink
handle empty tags in default tags
Browse files Browse the repository at this point in the history
  • Loading branch information
@johnsonaj
johnsonaj committed Apr 14, 2023
1 parent c6d0d78 commit b8497ed
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 67 deletions.
78 changes: 35 additions & 43 deletions internal/provider/intercept.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,17 +344,9 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
break
}
}
fallthrough
case Finally:
switch why {
case 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.IgnoreSystem(inContext.ServicePackageName)

tagsInContext.TagsIn = types.Some(tags)

if !d.GetRawPlan().GetAttr("tags_all").IsWhollyKnown() {

configTags := make(map[string]string)
Expand Down Expand Up @@ -421,47 +413,47 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
if err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "updating tags for %s %s (%s): %s", serviceName, resourceName, identifier, err)
}
//
//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
// tflog.Warn(ctx, "failed listing tags for resource", map[string]interface{}{
// r.tags.IdentifierAttribute: d.Id(),
// "error": err.Error(),
// })
// return ctx, diags
//}
//
//if inContext.ServicePackageName == names.DynamoDB && err != nil {
// // When a DynamoDB Table is `ARCHIVED`, ListTags returns `ResourceNotFoundException`.
// if tfresource.NotFound(err) || tfawserr.ErrMessageContains(err, "UnknownOperationException", "Tagging is not currently supported in DynamoDB Local.") {
// err = nil
// }
//}
//
//if err != nil {
// return ctx, sdkdiag.AppendErrorf(diags, "listing tags for %s %s (%s): %s", serviceName, resourceName, identifier, err)
//}

//// Remove any provider configured ignore_tags and system tags from those returned from the service API.
// tags = tagsInContext.TagsOut.UnwrapOrDefault().IgnoreSystem(inContext.ServicePackageName).IgnoreConfig(tagsInContext.IgnoreConfig)

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
tflog.Warn(ctx, "failed listing tags for resource", map[string]interface{}{
r.tags.IdentifierAttribute: d.Id(),
"error": err.Error(),
})
return ctx, diags
}

if inContext.ServicePackageName == names.DynamoDB && err != nil {
// When a DynamoDB Table is `ARCHIVED`, ListTags returns `ResourceNotFoundException`.
if tfresource.NotFound(err) || tfawserr.ErrMessageContains(err, "UnknownOperationException", "Tagging is not currently supported in DynamoDB Local.") {
err = nil
}
}

if err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "listing tags for %s %s (%s): %s", serviceName, resourceName, identifier, err)
}

// Remove any provider configured ignore_tags and system tags from those returned from the service API.
toAdd = tagsInContext.TagsOut.UnwrapOrDefault().IgnoreSystem(inContext.ServicePackageName).IgnoreConfig(tagsInContext.IgnoreConfig)

// The resource's configured tags do not include any provider configured default_tags.
if err := d.Set(names.AttrTags, tags.RemoveDefaultConfig(tagsInContext.DefaultConfig).Map()); err != nil {
if err := d.Set(names.AttrTags, toAdd.RemoveDefaultConfig(tagsInContext.DefaultConfig).Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTags, err)
}

// Computed tags_all do.
if err := d.Set(names.AttrTagsAll, tags.Map()); err != nil {
if err := d.Set(names.AttrTagsAll, toAdd.Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTagsAll, err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ func New(ctx context.Context) (*schema.Provider, error) {
}

interceptors = append(interceptors, interceptorItem{
when: Before | After,
when: Before | After | Finally,
why: Create | Read | Update,
interceptor: tagsInterceptor{tags: v.Tags},
})
Expand Down
42 changes: 19 additions & 23 deletions internal/verify/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,30 @@ func SetTagsDiff(ctx context.Context, diff *schema.ResourceDiff, meta interface{

// To ensure "tags_all" is correctly computed, we explicitly set the attribute diff
// when the merger of resource-level tags onto provider-level tags results in n > 0 tags,
// otherwise we mark the attribute as "Computed" only when their is a known diff (excluding an empty map)
// otherwise we mark the attribute as "Computed" only when there is a known diff (excluding an empty map)
// or a change for "tags_all".
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/18366
// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/19005

//if len(allTags) > 0 {
// if err := diff.SetNew("tags_all", allTags.Map()); err != nil {
// return fmt.Errorf("error setting new tags_all diff: %w", err)
// }
//} else if allTags.HasZeroValue() {
// if err := diff.SetNewComputed("tags_all"); err != nil {
// return fmt.Errorf("error setting tags_all to computed: %w", err)
// }
//} else if len(diff.Get("tags_all").(map[string]interface{})) > 0 {
// if err := diff.SetNewComputed("tags_all"); err != nil {
// return fmt.Errorf("error setting tags_all to computed: %w", err)
// }
//} else if diff.HasChange("tags_all") {
// if err := diff.SetNewComputed("tags_all"); err != nil {
// return fmt.Errorf("error setting tags_all to computed: %w", err)
// }
//}

if allTags.HasZeroValue() {
if err := diff.SetNewComputed("tags_all"); err != nil {
return fmt.Errorf("error setting tags_all to computed: %w", err)
if diff.HasChange("tags") {
_, n := diff.GetChange("tags")
newTags := tftags.New(ctx, n.(map[string]interface{}))

if newTags.HasZeroValue() {
if err := diff.SetNewComputed("tags_all"); err != nil {
return fmt.Errorf("error setting tags_all to computed: %w", err)
}
}
} else if tagsAll, ok := diff.Get("tags_all").(map[string]interface{}); ok {
ta := tftags.New(ctx, tagsAll)
if !ta.ContainsAll(allTags) {
if allTags.HasZeroValue() {
if err := diff.SetNewComputed("tags_all"); err != nil {
return fmt.Errorf("error setting tags_all to computed: %w", err)
}
}
}
} else if len(allTags) > 0 {
} else if len(allTags) > 0 && !allTags.HasZeroValue() {
if err := diff.SetNew("tags_all", allTags.Map()); err != nil {
return fmt.Errorf("error setting new tags_all diff: %w", err)
}
Expand Down

0 comments on commit b8497ed

Please sign in to comment.