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

default tags: zero value, duplicate, and computed tags #30793

Merged
merged 28 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
547c902
Update CHANGELOG.md for #30828
Apr 20, 2023
339cd32
Update CHANGELOG.md for #30828
Apr 20, 2023
cc4cc7a
WIP
johnsonaj Apr 13, 2023
6bf7249
handle empty tags in default tags
johnsonaj Apr 14, 2023
931cc2b
chore: cleanup logging
johnsonaj Apr 14, 2023
c842758
chore: cleanup logging
johnsonaj Apr 18, 2023
e6ae0ca
chore: linter error
johnsonaj Apr 18, 2023
68fff1e
allow update from empty to known value
johnsonaj Apr 18, 2023
85c06f6
only add/remove necessary tags
johnsonaj Apr 18, 2023
97524bc
update default_tags without tag changes
johnsonaj Apr 18, 2023
b596610
set tags_all computed if tags are unknown
johnsonaj Apr 18, 2023
43cead2
WIP
johnsonaj Apr 19, 2023
dcc9446
wip: factor out final tags update
johnsonaj Apr 19, 2023
5fbd080
refactor tagsReadFun and tagsListFunc
johnsonaj Apr 19, 2023
0a09def
chore: cleanup
johnsonaj Apr 19, 2023
bbf4f45
chore: schemaResourcData refactor
johnsonaj Apr 19, 2023
52056d9
listFunc -> updateFun
johnsonaj Apr 19, 2023
61554fe
use constants for tags and tags_all
johnsonaj Apr 19, 2023
10ec30e
allow duplicate tags
johnsonaj Apr 20, 2023
64470f6
cleanup interfaces
johnsonaj Apr 20, 2023
868b028
allow duplicate tags
johnsonaj Apr 20, 2023
ce86e39
gofmt
johnsonaj Apr 20, 2023
7b54800
cleanup: rename fuctions
johnsonaj Apr 21, 2023
59fcef9
test: computed tag values
johnsonaj Apr 21, 2023
3d79521
test: zero value tags
johnsonaj Apr 21, 2023
025ed97
testfmt
johnsonaj Apr 21, 2023
394de93
fix rebase conflicts
johnsonaj Apr 21, 2023
2da2e99
lintfix
johnsonaj Apr 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 67 additions & 40 deletions internal/provider/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand All @@ -17,17 +18,29 @@ import (
"github.com/hashicorp/terraform-provider-aws/names"
)

// schemaResourceData is an interface that implements functions from schema.ResourceData
type schemaResourceData interface {
Get(key string) any
GetChange(key string) (any, any)
GetRawConfig() cty.Value
GetRawPlan() cty.Value
GetRawState() cty.Value
HasChange(key string) bool
Id() string
Set(string, any) error
}

// An interceptor is functionality invoked during the CRUD request lifecycle.
// If a Before interceptor returns Diagnostics indicating an error occurred then
// no further interceptors in the chain are run and neither is the schema's method.
// In other cases all interceptors in the chain are run.
type interceptor interface {
run(context.Context, *schema.ResourceData, any, when, why, diag.Diagnostics) (context.Context, diag.Diagnostics)
run(context.Context, schemaResourceData, any, when, why, diag.Diagnostics) (context.Context, diag.Diagnostics)
}

type interceptorFunc func(context.Context, *schema.ResourceData, any, when, why, diag.Diagnostics) (context.Context, diag.Diagnostics)
type interceptorFunc func(context.Context, schemaResourceData, any, when, why, diag.Diagnostics) (context.Context, diag.Diagnostics)

func (f interceptorFunc) run(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
func (f interceptorFunc) run(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return f(ctx, d, meta, when, why, diags)
}

Expand Down Expand Up @@ -178,12 +191,16 @@ func (r *wrappedResource) StateUpgrade(f schema.StateUpgradeFunc) schema.StateUp
}
}

type tagsCRUDFunc func(context.Context, schemaResourceData, conns.ServicePackage, *types.ServicePackageResourceTags, string, string, any, diag.Diagnostics) (context.Context, diag.Diagnostics)

// tagsInterceptor implements transparent tagging.
type tagsInterceptor struct {
tags *types.ServicePackageResourceTags
tags *types.ServicePackageResourceTags
updateFunc tagsCRUDFunc
readFunc tagsCRUDFunc
}

func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
func (r tagsInterceptor) run(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
if r.tags == nil {
return ctx, diags
}
Expand Down Expand Up @@ -228,44 +245,46 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
break
}

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

// 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 d.GetRawPlan().GetAttr("tags_all").IsWhollyKnown() {
if d.HasChange(names.AttrTagsAll) {
if identifierAttribute := r.tags.IdentifierAttribute; identifierAttribute != "" {
var identifier string
if identifierAttribute == "id" {
identifier = d.Id()
} else {
identifier = d.Get(identifierAttribute).(string)
}
o, n := d.GetChange(names.AttrTagsAll)

// 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
tflog.Warn(ctx, "failed updating tags for resource", map[string]interface{}{
r.tags.IdentifierAttribute: identifier,
"error": err.Error(),
})
if verify.ErrorISOUnsupported(meta.(*conns.AWSClient).Partition, err) {
// ISO partitions may not support tagging, giving error
tflog.Warn(ctx, "failed updating tags for resource", map[string]interface{}{
r.tags.IdentifierAttribute: identifier,
"error": err.Error(),
})

return ctx, diags
}
return ctx, diags
}

if err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "updating tags for %s %s (%s): %s", serviceName, resourceName, identifier, err)
if err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "updating tags for %s %s (%s): %s", serviceName, resourceName, identifier, err)
}
}
// TODO If the only change was to tags it would be nice to not call the resource's U handler.
}
// TODO If the only change was to tags it would be nice to not call the resource's U handler.
}
}
case After:
Expand Down Expand Up @@ -328,8 +347,8 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
// 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)

// 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 {
// The resource's configured tags can now include duplicate tags that have been configured on the provider.
if err := d.Set(names.AttrTags, tags.ResolveDuplicates(ctx, tagsInContext.DefaultConfig, d).Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTags, err)
}

Expand All @@ -338,6 +357,14 @@ func (r tagsInterceptor) run(ctx context.Context, d *schema.ResourceData, meta a
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTagsAll, err)
}
}
case Finally:
switch why {
case Update:
if !d.GetRawPlan().GetAttr(names.AttrTagsAll).IsWhollyKnown() {
ctx, diags = r.updateFunc(ctx, d, sp, r.tags, serviceName, resourceName, meta, diags)
ctx, diags = r.readFunc(ctx, d, sp, r.tags, serviceName, resourceName, meta, diags)
}
}
}

return ctx, diags
Expand Down
12 changes: 6 additions & 6 deletions internal/provider/intercept_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ func TestInterceptorsWhy(t *testing.T) {
interceptors = append(interceptors, interceptorItem{
when: Before,
why: Create,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
interceptors = append(interceptors, interceptorItem{
when: After,
why: Delete,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
interceptors = append(interceptors, interceptorItem{
when: Before,
why: Create,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
Expand All @@ -58,21 +58,21 @@ func TestInterceptedHandler(t *testing.T) {
interceptors = append(interceptors, interceptorItem{
when: Before,
why: Create,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
interceptors = append(interceptors, interceptorItem{
when: After,
why: Delete,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
interceptors = append(interceptors, interceptorItem{
when: Before,
why: Create,
interceptor: interceptorFunc(func(ctx context.Context, d *schema.ResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
interceptor: interceptorFunc(func(ctx context.Context, d schemaResourceData, meta any, when when, why why, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
return ctx, diags
}),
})
Expand Down
10 changes: 7 additions & 3 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,13 @@ func New(ctx context.Context) (*schema.Provider, error) {
}

interceptors = append(interceptors, interceptorItem{
when: Before | After,
why: Create | Read | Update,
interceptor: tagsInterceptor{tags: v.Tags},
when: Before | After | Finally,
why: Create | Read | Update,
interceptor: tagsInterceptor{
tags: v.Tags,
updateFunc: tagsUpdateFunc,
readFunc: tagsReadFunc,
},
})
}

Expand Down
155 changes: 155 additions & 0 deletions internal/provider/tags_interceptor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
package provider

import (
"context"

"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-log/tflog"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
"github.com/hashicorp/terraform-provider-aws/internal/types"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
"github.com/hashicorp/terraform-provider-aws/names"
)

func tagsUpdateFunc(ctx context.Context, d schemaResourceData, sp conns.ServicePackage, spt *types.ServicePackageResourceTags, serviceName, resourceName string, meta any, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
inContext, ok := conns.FromContext(ctx)
if !ok {
return ctx, diags
}

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

configTags := make(map[string]string)
if config := d.GetRawConfig(); !config.IsNull() && config.IsKnown() {
c := config.GetAttr(names.AttrTags)
if !c.IsNull() {
for k, v := range c.AsValueMap() {
configTags[k] = v.AsString()
}
}
}

stateTags := make(map[string]string)
if state := d.GetRawState(); !state.IsNull() && state.IsKnown() {
s := state.GetAttr(names.AttrTagsAll)
for k, v := range s.AsValueMap() {
stateTags[k] = v.AsString()
}
}

tagsAll := tftags.New(ctx, stateTags)
// if tags_all was computed because not wholly known
// Merge the resource's configured tags with any provider configured default_tags.
configAll := tagsInContext.DefaultConfig.MergeTags(tftags.New(ctx, configTags))
// Remove system tags.
configAll = configAll.IgnoreSystem(inContext.ServicePackageName)

toAdd := configAll.Difference(tagsAll)
toRemove := tagsAll.Difference(configAll)

var identifier string
if identifierAttribute := spt.IdentifierAttribute; identifierAttribute == "id" {
identifier = d.Id()
} else {
identifier = d.Get(identifierAttribute).(string)
}
// 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, toRemove, toAdd)
} else if v, ok := sp.(interface {
UpdateTags(context.Context, any, string, string, any, any) error
}); ok && spt.ResourceType != "" {
err = v.UpdateTags(ctx, meta, identifier, spt.ResourceType, toRemove, toAdd)
}

if verify.ErrorISOUnsupported(meta.(*conns.AWSClient).Partition, err) {
// ISO partitions may not support tagging, giving error
tflog.Warn(ctx, "failed updating tags for resource", map[string]interface{}{
spt.IdentifierAttribute: identifier,
"error": err.Error(),
})

return ctx, diags
}

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

return ctx, diags
}

func tagsReadFunc(ctx context.Context, d schemaResourceData, sp conns.ServicePackage, spt *types.ServicePackageResourceTags, serviceName, resourceName string, meta any, diags diag.Diagnostics) (context.Context, diag.Diagnostics) {
inContext, ok := conns.FromContext(ctx)
if !ok {
return ctx, diags
}

tagsInContext, ok := tftags.FromContext(ctx)
if !ok {
return ctx, diags
}
var identifier string
if identifierAttribute := spt.IdentifierAttribute; identifierAttribute == "id" {
identifier = d.Id()
} else {
identifier = d.Get(identifierAttribute).(string)
}

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 && spt.ResourceType != "" {
err = v.ListTags(ctx, meta, identifier, spt.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{}{
spt.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 can now include duplicate tags that have been configured on the provider.
if err := d.Set(names.AttrTags, toAdd.ResolveDuplicates(ctx, tagsInContext.DefaultConfig, d).Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTags, err)
}

// Computed tags_all do.
if err := d.Set(names.AttrTagsAll, toAdd.Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTagsAll, err)
}

return ctx, diags
}
Loading