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

tags: duplicate and computed tags on framework #30973

Merged
merged 4 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 17 additions & 9 deletions internal/framework/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,15 @@ func (r *ResourceWithConfigure) SetTagsAll(ctx context.Context, request resource
}

if !planTags.IsUnknown() {
resourceTags := tftags.New(ctx, planTags)
if !mapHasUnknownElements(planTags) {
resourceTags := tftags.New(ctx, planTags)

if defaultTagsConfig.TagsEqual(resourceTags) {
response.Diagnostics.AddError(
`"tags" are identical to those in the "default_tags" configuration block of the provider`,
"please de-duplicate and try again")
}

allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig)
allTags := defaultTagsConfig.MergeTags(resourceTags).IgnoreConfig(ignoreTagsConfig)

response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root("tags_all"), flex.FlattenFrameworkStringValueMapLegacy(ctx, allTags.Map()))...)
response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root("tags_all"), flex.FlattenFrameworkStringValueMapLegacy(ctx, allTags.Map()))...)
} else {
response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root("tags_all"), tftags.Unknown)...)
}
} else {
response.Diagnostics.Append(response.Plan.SetAttribute(ctx, path.Root("tags_all"), tftags.Unknown)...)
}
Expand Down Expand Up @@ -210,3 +208,13 @@ func (w *WithTimeouts) DeleteTimeout(ctx context.Context, timeouts timeouts.Valu

return timeout
}

func mapHasUnknownElements(m types.Map) bool {
for _, v := range m.Elements() {
if v.IsUnknown() {
return true
}
}

return false
}
2 changes: 1 addition & 1 deletion internal/provider/fwprovider/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ func (r tagsInterceptor) read(ctx context.Context, request resource.ReadRequest,
stateTags := tftags.Null
// Remove any provider configured ignore_tags and system tags from those returned from the service API.
// The resource's configured tags do not include any provider configured default_tags.
if v := apiTags.IgnoreSystem(inContext.ServicePackageName).IgnoreConfig(tagsInContext.IgnoreConfig).RemoveDefaultConfig(tagsInContext.DefaultConfig).Map(); len(v) > 0 {
if v := apiTags.IgnoreSystem(inContext.ServicePackageName).IgnoreConfig(tagsInContext.IgnoreConfig).ResolveDuplicatesFramework(ctx, tagsInContext.DefaultConfig, tagsInContext.IgnoreConfig, response, diags).Map(); len(v) > 0 {
stateTags = flex.FlattenFrameworkStringValueMapLegacy(ctx, v)
}
diags.Append(response.State.SetAttribute(ctx, path.Root(names.AttrTags), &stateTags)...)
Expand Down
2 changes: 1 addition & 1 deletion internal/provider/intercept.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ func (r tagsInterceptor) run(ctx context.Context, d schemaResourceData, meta any
tags := 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, tags.ResolveDuplicates(ctx, tagsInContext.DefaultConfig, d).Map()); err != nil {
if err := d.Set(names.AttrTags, tags.ResolveDuplicates(ctx, tagsInContext.DefaultConfig, tagsInContext.IgnoreConfig, d).Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTags, err)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/provider/tags_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func tagsReadFunc(ctx context.Context, d schemaResourceData, sp conns.ServicePac
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 {
if err := d.Set(names.AttrTags, toAdd.ResolveDuplicates(ctx, tagsInContext.DefaultConfig, tagsInContext.IgnoreConfig, d).Map()); err != nil {
return ctx, sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrTags, err)
}

Expand Down
54 changes: 52 additions & 2 deletions internal/service/ec2/vpc_security_group_ingress_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,30 @@ func TestAccVPCSecurityGroupIngressRule_tags(t *testing.T) {
})
}

func TestAccVPCSecurityGroupIngressRule_tags_computed(t *testing.T) {
ctx := acctest.Context(t)
var v ec2.SecurityGroupRule
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_vpc_security_group_ingress_rule.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckSecurityGroupIngressRuleDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccVPCSecurityGroupIngressRuleConfig_computed(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckSecurityGroupIngressRuleExists(ctx, resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttrSet(resourceName, "tags.eip"),
),
},
},
})
}

func TestAccVPCSecurityGroupIngressRule_DefaultTags_providerOnly(t *testing.T) {
ctx := acctest.Context(t)
var v ec2.SecurityGroupRule
Expand Down Expand Up @@ -461,6 +485,8 @@ func TestAccVPCSecurityGroupIngressRule_DefaultTagsProviderAndResource_overlappi
func TestAccVPCSecurityGroupIngressRule_DefaultTagsProviderAndResource_duplicateTag(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
var v ec2.SecurityGroupRule
resourceName := "aws_vpc_security_group_ingress_rule.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
Expand All @@ -473,8 +499,11 @@ func TestAccVPCSecurityGroupIngressRule_DefaultTagsProviderAndResource_duplicate
acctest.ConfigDefaultTags_Tags1("overlapkey", "overlapvalue"),
testAccVPCSecurityGroupIngressRuleConfig_tags1(rName, "overlapkey", "overlapvalue"),
),
PlanOnly: true,
ExpectError: regexp.MustCompile(`"tags" are identical to those in the "default_tags" configuration block`),
Check: resource.ComposeTestCheckFunc(
testAccCheckSecurityGroupIngressRuleExists(ctx, resourceName, &v),
resource.TestCheckResourceAttr(resourceName, "tags.%", "1"),
resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"),
),
},
},
})
Expand Down Expand Up @@ -1084,6 +1113,27 @@ resource "aws_vpc_security_group_ingress_rule" "test" {
`, tagKey1, tagValue1))
}

func testAccVPCSecurityGroupIngressRuleConfig_computed(rName string) string {
return acctest.ConfigCompose(testAccVPCSecurityGroupRuleConfig_base(rName), `
resource "aws_eip" "test" {
vpc = true
}

resource "aws_vpc_security_group_ingress_rule" "test" {
security_group_id = aws_security_group.test.id

cidr_ipv4 = "10.0.0.0/8"
from_port = 80
ip_protocol = "tcp"
to_port = 8080

tags = {
eip = aws_eip.test.public_ip
}
}
`)
}

func testAccVPCSecurityGroupIngressRuleConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
return acctest.ConfigCompose(testAccVPCSecurityGroupRuleConfig_base(rName), fmt.Sprintf(`
resource "aws_vpc_security_group_ingress_rule" "test" {
Expand Down
46 changes: 44 additions & 2 deletions internal/tags/key_value_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ import (
"reflect"
"regexp"
"sort"
"strconv"
"strings"

"github.com/hashicorp/go-cty/cty"
fwdiag "github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-provider-aws/internal/create"
"github.com/hashicorp/terraform-provider-aws/internal/flex"
Expand Down Expand Up @@ -745,7 +749,7 @@ type schemaResourceData interface {
GetRawState() cty.Value
}

func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *DefaultConfig, d schemaResourceData) KeyValueTags {
func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *DefaultConfig, ignoreConfig *IgnoreConfig, d schemaResourceData) KeyValueTags {
// remove default config.
t := tags.RemoveDefaultConfig(defaultConfig)

Expand Down Expand Up @@ -803,7 +807,45 @@ func (tags KeyValueTags) ResolveDuplicates(ctx context.Context, defaultConfig *D
}
}

return New(ctx, result)
return New(ctx, result).IgnoreConfig(ignoreConfig)
}

func (tags KeyValueTags) ResolveDuplicatesFramework(ctx context.Context, defaultConfig *DefaultConfig, ignoreConfig *IgnoreConfig, resp *resource.ReadResponse, diags fwdiag.Diagnostics) KeyValueTags {
// remove default config.
t := tags.RemoveDefaultConfig(defaultConfig)

var tagsAll types.Map
diags.Append(resp.State.GetAttribute(ctx, path.Root("tags"), &tagsAll)...)

if diags.HasError() {
return KeyValueTags{}
}

result := make(map[string]string)
for k, v := range t {
result[k] = v.ValueString()
}

for k, v := range tagsAll.Elements() {
if _, ok := result[k]; !ok {
if defaultConfig != nil {
s, err := strconv.Unquote(v.String()) // TODO rework to use Framework Map.Equals() value

if err != nil {
diags.AddError(
"unable to normalize string",
"unable to normalize string default value",
)
}

if val, ok := defaultConfig.Tags[k]; ok && val.ValueString() == s {
result[k] = s
}
}
}
}

return New(ctx, result).IgnoreConfig(ignoreConfig)
}

// ToSnakeCase converts a string to snake case.
Expand Down