From c84a86f3e2dcaf3e89d3fb53a3b8f99c0cb5f5e3 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:23:35 -0400 Subject: [PATCH 01/15] provider: Add resource_lf_tags resource --- internal/provider/provider.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/provider/provider.go b/internal/provider/provider.go index cccc08412c9..8d6903f6072 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -1630,6 +1630,7 @@ func Provider() *schema.Provider { "aws_lakeformation_lf_tag": lakeformation.ResourceLFTag(), "aws_lakeformation_permissions": lakeformation.ResourcePermissions(), "aws_lakeformation_resource": lakeformation.ResourceResource(), + "aws_lakeformation_resource_lf_tags": lakeformation.ResourceResourceLFTags(), "aws_lambda_alias": lambda.ResourceAlias(), "aws_lambda_code_signing_config": lambda.ResourceCodeSigningConfig(), From 5d6ed3ad328d9d0722735136b25ae88efe29fc91 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:24:13 -0400 Subject: [PATCH 02/15] names: Add setting error action --- names/errors.go | 1 + 1 file changed, 1 insertion(+) diff --git a/names/errors.go b/names/errors.go index 420b33539d7..737151ac919 100644 --- a/names/errors.go +++ b/names/errors.go @@ -13,6 +13,7 @@ const ( ErrActionDeleting = "deleting" ErrActionUpdating = "updating" ErrActionCreating = "creating" + ErrActionSetting = "setting" ErrActionCheckingExistence = "checking existence" ErrActionCheckingDestroyed = "checking destroyed" ) From ccbd2fa01f14f762d85f4e6227230fa80139669a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:24:56 -0400 Subject: [PATCH 03/15] lf/lf_tag/test: Minor cleanup --- internal/service/lakeformation/lf_tag_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/lakeformation/lf_tag_test.go b/internal/service/lakeformation/lf_tag_test.go index 8d93a2dca95..76bac122a08 100644 --- a/internal/service/lakeformation/lf_tag_test.go +++ b/internal/service/lakeformation/lf_tag_test.go @@ -213,7 +213,7 @@ resource "aws_lakeformation_data_lake_settings" "test" { resource "aws_lakeformation_lf_tag" "test" { key = %[1]q - values = [%s] + values = [%[2]s] # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } From ff925198c7976b3de4421942e78f141e1f1ad182 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:25:22 -0400 Subject: [PATCH 04/15] test/lakeformation: Add new resource tests --- internal/service/lakeformation/lakeformation_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/service/lakeformation/lakeformation_test.go b/internal/service/lakeformation/lakeformation_test.go index aa690a1fe20..8cd8afbe2e3 100644 --- a/internal/service/lakeformation/lakeformation_test.go +++ b/internal/service/lakeformation/lakeformation_test.go @@ -54,6 +54,13 @@ func TestAccLakeFormation_serial(t *testing.T) { "disappears": testAccLFTag_disappears, "values": testAccLFTag_values, }, + "ResourceLFTags": { + "basic": testAccResourceLFTags_basic, + "database": testAccResourceLFTags_database, + "databaseMultiple": testAccResourceLFTags_databaseMultiple, + "table": testAccResourceLFTags_table, + "tableWithColumns": testAccResourceLFTags_tableWithColumns, + }, } for group, m := range testCases { From fdddae6486b81bd99865b860cc16a682dbf75aae Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:25:41 -0400 Subject: [PATCH 05/15] docs/lakeformation: Add new resource docs --- ...keformation_resource_lf_tags.html.markdown | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 website/docs/r/lakeformation_resource_lf_tags.html.markdown diff --git a/website/docs/r/lakeformation_resource_lf_tags.html.markdown b/website/docs/r/lakeformation_resource_lf_tags.html.markdown new file mode 100644 index 00000000000..c81145fed63 --- /dev/null +++ b/website/docs/r/lakeformation_resource_lf_tags.html.markdown @@ -0,0 +1,123 @@ +--- +subcategory: "Lake Formation" +layout: "aws" +page_title: "AWS: aws_lakeformation_resource_lf_tags" +description: |- + Manages an attachment between one or more LF-tags and an existing Lake Formation resource. +--- + +# Resource: aws_lakeformation_resource_lf_tags + +Manages an attachment between one or more existing LF-tags and an existing Lake Formation resource. + +## Example Usage + +### Database Example + +```terraform +resource "aws_lakeformation_lf_tag" "example" { + key = "copse" + values = ["luffield"] +} + +resource "aws_lakeformation_resource_lf_tags" "example" { + catalog_id = data.aws_caller_identity.current.account_id + + database { + name = aws_glue_catalog_database.example.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.example.key + values = aws_lakeformation_lf_tag.example.values + } +} +``` + +### Multiple Tags Example + +```terraform +resource "aws_lakeformation_lf_tag" "example" { + key = "copse" + values = ["luffield"] +} + +resource "aws_lakeformation_lf_tag" "example2" { + key = "stowe" + values = ["brooklands"] +} + +resource "aws_lakeformation_resource_lf_tags" "example" { + catalog_id = data.aws_caller_identity.current.account_id + + database { + name = aws_glue_catalog_database.example.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.example.key + values = aws_lakeformation_lf_tag.example.values + } + + lf_tag { + key = aws_lakeformation_lf_tag.example2.key + values = aws_lakeformation_lf_tag.example2.values + } +} +``` + +## Argument Reference + +The following arguments are required: + +* `lf_tag` – (Required) Set of LF-tags to attach to the resource. + +One of the following is required: + +* `database` - (Optional) Configuration block for a database resource. See below. +* `table` - (Optional) Configuration block for a table resource. See below. +* `table_with_columns` - (Optional) Configuration block for a table with columns resource. See below. + +The following arguments are optional: + +* `catalog_id` – (Optional) Identifier for the Data Catalog. By default, the account ID. The Data Catalog is the persistent metadata store. It contains database definitions, table definitions, and other control information to manage your Lake Formation environment. + +### database + +The following argument is required: + +* `name` – (Required) Name of the database resource. Unique to the Data Catalog. + +The following argument is optional: + +* `catalog_id` - (Optional) Identifier for the Data Catalog. By default, it is the account ID of the caller. + +### table + +The following argument is required: + +* `database_name` – (Required) Name of the database for the table. Unique to a Data Catalog. +* `name` - (Required, at least one of `name` or `wildcard`) Name of the table. +* `wildcard` - (Required, at least one of `name` or `wildcard`) Whether to use a wildcard representing every table under a database. Defaults to `false`. + +The following arguments are optional: + +* `catalog_id` - (Optional) Identifier for the Data Catalog. By default, it is the account ID of the caller. + +### table_with_columns + +The following arguments are required: + +* `column_names` - (Required, at least one of `column_names` or `wildcard`) Set of column names for the table. +* `database_name` – (Required) Name of the database for the table with columns resource. Unique to the Data Catalog. +* `name` – (Required) Name of the table resource. +* `wildcard` - (Required, at least one of `column_names` or `wildcard`) Whether to use a column wildcard. If `excluded_column_names` is included, `wildcard` must be set to `true` to avoid Terraform reporting a difference. + +The following arguments are optional: + +* `catalog_id` - (Optional) Identifier for the Data Catalog. By default, it is the account ID of the caller. +* `excluded_column_names` - (Optional) Set of column names for the table to exclude. If `excluded_column_names` is included, `wildcard` must be set to `true` to avoid Terraform reporting a difference. + +## Attributes Reference + +No additional attributes are exported. From 3fd6b8465f58c0e07cf7dd59f80e446caee253af Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:26:15 -0400 Subject: [PATCH 06/15] lakeformation: Add new resource_lf_tags resource --- .../service/lakeformation/resource_lf_tags.go | 524 ++++++++++++++++++ 1 file changed, 524 insertions(+) create mode 100644 internal/service/lakeformation/resource_lf_tags.go diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go new file mode 100644 index 00000000000..90af981754f --- /dev/null +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -0,0 +1,524 @@ +package lakeformation + +import ( + "bytes" + "context" + "fmt" + "log" + "reflect" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/lakeformation" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/create" + "github.com/hashicorp/terraform-provider-aws/internal/flex" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/internal/verify" + "github.com/hashicorp/terraform-provider-aws/names" +) + +const ( + resourceLFTags = "Resource LF Tags" +) + +func ResourceResourceLFTags() *schema.Resource { + return &schema.Resource{ + CreateContext: resourceResourceLFTagsCreate, + ReadContext: resourceResourceLFTagsRead, + DeleteContext: resourceResourceLFTagsDelete, + + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(20 * time.Minute), + Delete: schema.DefaultTimeout(20 * time.Minute), + }, + + Schema: map[string]*schema.Schema{ + "catalog_id": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + Optional: true, + ValidateFunc: verify.ValidAccountID, + }, + "database": { + Type: schema.TypeList, + Computed: true, + ForceNew: true, + MaxItems: 1, + Optional: true, + ExactlyOneOf: []string{ + "database", + "table", + "table_with_columns", + }, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "catalog_id": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + Optional: true, + ValidateFunc: verify.ValidAccountID, + }, + "name": { + Type: schema.TypeString, + ForceNew: true, + Required: true, + }, + }, + }, + }, + "lf_tag": { + Type: schema.TypeSet, + Required: true, + ForceNew: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "catalog_id": { + Type: schema.TypeString, + ForceNew: true, + Optional: true, + Computed: true, + }, + "key": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringLenBetween(1, 128), + }, + "values": { + Type: schema.TypeSet, + Required: true, + MinItems: 1, + MaxItems: 15, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validateLFTagValues(), + }, + Set: schema.HashString, + }, + }, + }, + Set: lfTagsHash, + }, + "table": { + Type: schema.TypeList, + Computed: true, + ForceNew: true, + MaxItems: 1, + Optional: true, + ExactlyOneOf: []string{ + "database", + "table", + "table_with_columns", + }, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "catalog_id": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + Optional: true, + ValidateFunc: verify.ValidAccountID, + }, + "database_name": { + Type: schema.TypeString, + ForceNew: true, + Required: true, + }, + "name": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + Optional: true, + AtLeastOneOf: []string{ + "table.0.name", + "table.0.wildcard", + }, + }, + "wildcard": { + Type: schema.TypeBool, + Default: false, + ForceNew: true, + Optional: true, + AtLeastOneOf: []string{ + "table.0.name", + "table.0.wildcard", + }, + }, + }, + }, + }, + "table_with_columns": { + Type: schema.TypeList, + Computed: true, + ForceNew: true, + MaxItems: 1, + Optional: true, + ExactlyOneOf: []string{ + "database", + "table", + "table_with_columns", + }, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "catalog_id": { + Type: schema.TypeString, + Computed: true, + ForceNew: true, + Optional: true, + ValidateFunc: verify.ValidAccountID, + }, + "column_names": { + Type: schema.TypeSet, + ForceNew: true, + Optional: true, + Set: schema.HashString, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + AtLeastOneOf: []string{ + "table_with_columns.0.column_names", + "table_with_columns.0.wildcard", + }, + }, + "database_name": { + Type: schema.TypeString, + ForceNew: true, + Required: true, + }, + "excluded_column_names": { + Type: schema.TypeSet, + ForceNew: true, + Optional: true, + Set: schema.HashString, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + }, + "name": { + Type: schema.TypeString, + ForceNew: true, + Required: true, + }, + "wildcard": { + Type: schema.TypeBool, + Default: false, + ForceNew: true, + Optional: true, + AtLeastOneOf: []string{ + "table_with_columns.0.column_names", + "table_with_columns.0.wildcard", + }, + }, + }, + }, + }, + }, + } +} + +func resourceResourceLFTagsCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).LakeFormationConn + + input := &lakeformation.AddLFTagsToResourceInput{ + Resource: &lakeformation.Resource{}, + } + + if v, ok := d.GetOk("catalog_id"); ok { + input.CatalogId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("database"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Database = ExpandDatabaseResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("lf_tag"); ok && v.(*schema.Set).Len() > 0 { + input.LFTags = expandLFTagPairs(v.(*schema.Set).List()) + } + + if v, ok := d.GetOk("table"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Table = ExpandTableResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("table_with_columns"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.TableWithColumns = expandTableColumnsResource(v.([]interface{})[0].(map[string]interface{})) + } + + var output *lakeformation.AddLFTagsToResourceOutput + err := resource.Retry(IAMPropagationTimeout, func() *resource.RetryError { + var err error + output, err = conn.AddLFTagsToResource(input) + if err != nil { + if tfawserr.ErrCodeEquals(err, lakeformation.ErrCodeConcurrentModificationException) { + return resource.RetryableError(err) + } + if tfawserr.ErrMessageContains(err, "AccessDeniedException", "is not authorized") { + return resource.RetryableError(err) + } + + return resource.NonRetryableError(fmt.Errorf("error creating Lake Formation LF Tag Resource: %w", err)) + } + return nil + }) + + if tfresource.TimedOut(err) { + output, err = conn.AddLFTagsToResource(input) + } + + if err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionCreating, resourceLFTags, input.String(), err) + } + + if output != nil && len(output.Failures) > 0 { + var failures *multierror.Error + + for _, v := range output.Failures { + if v.LFTag == nil || v.Error == nil { + continue + } + + origErr := fmt.Errorf("catalog id:%s, tag key:%s, values:%+v", aws.StringValue(v.LFTag.CatalogId), aws.StringValue(v.LFTag.TagKey), aws.StringValueSlice(v.LFTag.TagValues)) + err := awserr.New(aws.StringValue(v.Error.ErrorCode), aws.StringValue(v.Error.ErrorMessage), origErr) + failures = multierror.Append(failures, err) + } + + if failures.Len() > 0 { + return names.DiagError(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", failures.ErrorOrNil()) + } + } + + d.SetId(fmt.Sprintf("%d", create.StringHashcode(input.String()))) + + return resourceResourceLFTagsRead(ctx, d, meta) +} + +func resourceResourceLFTagsRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).LakeFormationConn + + input := &lakeformation.GetResourceLFTagsInput{ + Resource: &lakeformation.Resource{}, + ShowAssignedLFTags: aws.Bool(true), + } + + if v, ok := d.GetOk("catalog_id"); ok { + input.CatalogId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("database"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Database = ExpandDatabaseResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("table"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Table = ExpandTableResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("table_with_columns"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.TableWithColumns = expandTableColumnsResource(v.([]interface{})[0].(map[string]interface{})) + } + + output, err := conn.GetResourceLFTags(input) + + if err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionReading, resourceLFTags, d.Id(), err) + } + + if len(output.LFTagOnDatabase) > 0 { + if err := d.Set("lf_tag", flattenLFTagPairs(output.LFTagOnDatabase)); err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionSetting, resourceLFTags, d.Id(), err) + } + } + + if len(output.LFTagsOnColumns) > 0 { + for _, v := range output.LFTagsOnColumns { + if aws.StringValue(v.Name) != d.Get("table_with_columns.0.name").(string) { + continue + } + + if err := d.Set("lf_tag", flattenLFTagPairs(v.LFTags)); err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionSetting, resourceLFTags, d.Id(), err) + } + } + } + + if len(output.LFTagsOnTable) > 0 { + if err := d.Set("lf_tag", flattenLFTagPairs(output.LFTagsOnTable)); err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionSetting, resourceLFTags, d.Id(), err) + } + } + + return nil +} + +func resourceResourceLFTagsDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).LakeFormationConn + + input := &lakeformation.RemoveLFTagsFromResourceInput{ + Resource: &lakeformation.Resource{}, + } + + if v, ok := d.GetOk("catalog_id"); ok { + input.CatalogId = aws.String(v.(string)) + } + + if v, ok := d.GetOk("database"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Database = ExpandDatabaseResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("lf_tag"); ok && v.(*schema.Set).Len() > 0 { + input.LFTags = expandLFTagPairs(v.(*schema.Set).List()) + } + + if v, ok := d.GetOk("table"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.Table = ExpandTableResource(v.([]interface{})[0].(map[string]interface{})) + } + + if v, ok := d.GetOk("table_with_columns"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil { + input.Resource.TableWithColumns = expandTableColumnsResource(v.([]interface{})[0].(map[string]interface{})) + } + + if input.Resource == nil || reflect.DeepEqual(input.Resource, &lakeformation.Resource{}) { + // if resource is empty, don't delete = it won't delete anything since this is the predicate + log.Printf("[WARN] No Lake Formation Resource LF Tags to remove") + return nil + } + + err := resource.Retry(d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { + var err error + _, err = conn.RemoveLFTagsFromResource(input) + if err != nil { + if tfawserr.ErrCodeEquals(err, lakeformation.ErrCodeConcurrentModificationException) { + return resource.RetryableError(err) + } + if tfawserr.ErrMessageContains(err, "AccessDeniedException", "is not authorized") { + return resource.RetryableError(err) + } + + return resource.NonRetryableError(fmt.Errorf("unable to revoke Lake Formation Permissions: %w", err)) + } + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.RemoveLFTagsFromResource(input) + } + + if err != nil { + return names.DiagError(names.LakeFormation, names.ErrActionDeleting, resourceLFTags, d.Id(), err) + } + + return nil +} + +func lfTagsHash(v interface{}) int { + m, ok := v.(map[string]interface{}) + + if !ok { + return 0 + } + + var buf bytes.Buffer + buf.WriteString(m["key"].(string)) + buf.WriteString(fmt.Sprintf("%+v", m["values"].(*schema.Set))) + buf.WriteString(m["catalog_id"].(string)) + + return create.StringHashcode(buf.String()) +} + +func expandLFTagPair(tfMap map[string]interface{}) *lakeformation.LFTagPair { + if tfMap == nil { + return nil + } + + apiObject := &lakeformation.LFTagPair{} + + if v, ok := tfMap["catalog_id"].(string); ok && v != "" { + apiObject.CatalogId = aws.String(v) + } + + if v, ok := tfMap["key"].(string); ok && v != "" { + apiObject.TagKey = aws.String(v) + } + + if v, ok := tfMap["values"].(*schema.Set); ok && v != nil { + apiObject.TagValues = flex.ExpandStringSet(v) + } + + return apiObject +} + +func expandLFTagPairs(tfList []interface{}) []*lakeformation.LFTagPair { + if len(tfList) == 0 { + return nil + } + + var apiObjects []*lakeformation.LFTagPair + + for _, tfMapRaw := range tfList { + tfMap, ok := tfMapRaw.(map[string]interface{}) + + if !ok { + continue + } + + apiObject := expandLFTagPair(tfMap) + + if apiObject == nil { + continue + } + + apiObjects = append(apiObjects, apiObject) + } + + return apiObjects +} + +func flattenLFTagPair(apiObject *lakeformation.LFTagPair) map[string]interface{} { + if apiObject == nil { + return nil + } + + tfMap := map[string]interface{}{} + + if v := apiObject.CatalogId; v != nil { + tfMap["catalog_id"] = aws.StringValue(v) + } + + if v := apiObject.TagKey; v != nil { + tfMap["key"] = aws.StringValue(v) + } + + if v := apiObject.TagValues; v != nil && len(v) > 0 { + tfMap["values"] = flex.FlattenStringList(apiObject.TagValues) + } + + return tfMap +} + +func flattenLFTagPairs(apiObjects []*lakeformation.LFTagPair) []interface{} { + if len(apiObjects) == 0 { + return nil + } + + var tfList []interface{} + + for _, apiObject := range apiObjects { + if apiObject == nil { + continue + } + + tfList = append(tfList, flattenLFTagPair(apiObject)) + } + + return tfList +} From b562d1f38e8266431a1549b2d212f5aef75a1b3a Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:26:36 -0400 Subject: [PATCH 07/15] tests/lakeformation: Add tests for new resource_lf_tags resource --- .../lakeformation/resource_lf_tags_test.go | 651 ++++++++++++++++++ 1 file changed, 651 insertions(+) create mode 100644 internal/service/lakeformation/resource_lf_tags_test.go diff --git a/internal/service/lakeformation/resource_lf_tags_test.go b/internal/service/lakeformation/resource_lf_tags_test.go new file mode 100644 index 00000000000..8697870f205 --- /dev/null +++ b/internal/service/lakeformation/resource_lf_tags_test.go @@ -0,0 +1,651 @@ +package lakeformation_test + +import ( + "fmt" + "strconv" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go/service/lakeformation" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-provider-aws/internal/acctest" + "github.com/hashicorp/terraform-provider-aws/internal/conns" +) + +func testAccResourceLFTags_basic(t *testing.T) { + resourceName := "aws_lakeformation_resource_lf_tags.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) }, + ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckDatabaseLFTagsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceLFTagsConfig_basic(rName, []string{"copse"}), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "copse", + }), + acctest.CheckResourceAttrAccountID(resourceName, "catalog_id"), + ), + }, + }, + }) +} + +func testAccResourceLFTags_database(t *testing.T) { + resourceName := "aws_lakeformation_resource_lf_tags.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) }, + ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckDatabaseLFTagsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceLFTagsConfig_database(rName, []string{"copse"}), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "copse", + }), + ), + }, + { + Config: testAccResourceLFTagsConfig_database(rName, []string{"luffield"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "luffield", + }), + ), + }, + }, + }) +} + +func testAccResourceLFTags_databaseMultiple(t *testing.T) { + resourceName := "aws_lakeformation_resource_lf_tags.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) }, + ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckDatabaseLFTagsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"copse"}, []string{"luffield"}), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "copse", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": fmt.Sprintf("%s-2", rName), + "values.0": "luffield", + }), + ), + }, + { + Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"luffield"}, []string{"copse"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "luffield", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": fmt.Sprintf("%s-2", rName), + "values.0": "copse", + }), + ), + }, + }, + }) +} + +func testAccResourceLFTags_table(t *testing.T) { + resourceName := "aws_lakeformation_resource_lf_tags.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) }, + ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckDatabaseLFTagsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceLFTagsConfig_table(rName, []string{"copse"}), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "copse", + }), + ), + }, + { + Config: testAccResourceLFTagsConfig_table(rName, []string{"luffield"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "luffield", + }), + ), + }, + }, + }) +} + +func testAccResourceLFTags_tableWithColumns(t *testing.T) { + resourceName := "aws_lakeformation_resource_lf_tags.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckPartitionHasService(lakeformation.EndpointsID, t) }, + ErrorCheck: acctest.ErrorCheck(t, lakeformation.EndpointsID), + ProviderFactories: acctest.ProviderFactories, + CheckDestroy: testAccCheckDatabaseLFTagsDestroy, + Steps: []resource.TestStep{ + { + Config: testAccResourceLFTagsConfig_tableWithColumns(rName, []string{"copse"}), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "copse", + }), + ), + }, + { + Config: testAccResourceLFTagsConfig_tableWithColumns(rName, []string{"luffield"}), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseLFTagsExists(resourceName), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": rName, + "values.0": "luffield", + }), + ), + }, + }, + }) +} + +func testAccCheckDatabaseLFTagsDestroy(s *terraform.State) error { + conn := acctest.Provider.Meta().(*conns.AWSClient).LakeFormationConn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_lakeformation_resource_lf_tags" { + continue + } + + input := &lakeformation.GetResourceLFTagsInput{ + Resource: &lakeformation.Resource{}, + ShowAssignedLFTags: aws.Bool(true), + } + + if v, ok := rs.Primary.Attributes["catalog_id"]; ok { + input.CatalogId = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["database.0.name"]; ok { + input.Resource.Database = &lakeformation.DatabaseResource{ + Name: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["database.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.Database.CatalogId = aws.String(v) + } + } + + if v, ok := rs.Primary.Attributes["table.0.database_name"]; ok { + input.Resource.Table = &lakeformation.TableResource{ + DatabaseName: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["table.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.Table.CatalogId = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table.0.name"]; ok { + input.Resource.Table.Name = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table.0.wildcard"]; ok && v == "true" { + input.Resource.Table.TableWildcard = &lakeformation.TableWildcard{} + } + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.database_name"]; ok { + input.Resource.TableWithColumns = &lakeformation.TableWithColumnsResource{ + DatabaseName: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.name"]; ok { + input.Resource.TableWithColumns.Name = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.TableWithColumns.CatalogId = aws.String(v) + } + + if n, err := strconv.Atoi(rs.Primary.Attributes["table_with_columns.0.column_names.#"]); err == nil && n > 0 { + var cols []string + for i := 0; i < n; i++ { + cols = append(cols, rs.Primary.Attributes[fmt.Sprintf("table_with_columns.0.column_names.%d", i)]) + } + input.Resource.TableWithColumns.ColumnNames = aws.StringSlice(cols) + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.wildcard"]; ok && v == "true" { + input.Resource.TableWithColumns.ColumnWildcard = &lakeformation.ColumnWildcard{} + } + + if n, err := strconv.Atoi(rs.Primary.Attributes["table_with_columns.0.excluded_column_names.#"]); err == nil && n > 0 { + var cols []string + for i := 0; i < n; i++ { + cols = append(cols, rs.Primary.Attributes[fmt.Sprintf("table_with_columns.0.excluded_column_names.%d", i)]) + } + input.Resource.TableWithColumns.ColumnWildcard = &lakeformation.ColumnWildcard{ + ExcludedColumnNames: aws.StringSlice(cols), + } + } + } + + if _, err := conn.GetResourceLFTags(input); err != nil { + if tfawserr.ErrCodeEquals(err, lakeformation.ErrCodeEntityNotFoundException) { + continue + } + + if tfawserr.ErrMessageContains(err, lakeformation.ErrCodeInvalidInputException, "not found") { + continue + } + + // If the lake formation admin has been revoked, there will be access denied instead of entity not found + if tfawserr.ErrCodeEquals(err, lakeformation.ErrCodeAccessDeniedException) { + continue + } + return err + } + return fmt.Errorf("Lake Formation Resource LF Tag (%s) still exists", rs.Primary.ID) + } + + return nil +} + +func testAccCheckDatabaseLFTagsExists(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[resourceName] + + if !ok { + return fmt.Errorf("acceptance test: resource not found: %s", resourceName) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("no ID is set") + } + + input := &lakeformation.GetResourceLFTagsInput{ + Resource: &lakeformation.Resource{}, + ShowAssignedLFTags: aws.Bool(true), + } + + if v, ok := rs.Primary.Attributes["catalog_id"]; ok { + input.CatalogId = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["database.0.name"]; ok { + input.Resource.Database = &lakeformation.DatabaseResource{ + Name: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["database.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.Database.CatalogId = aws.String(v) + } + } + + if v, ok := rs.Primary.Attributes["table.0.database_name"]; ok { + input.Resource.Table = &lakeformation.TableResource{ + DatabaseName: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["table.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.Table.CatalogId = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table.0.name"]; ok { + input.Resource.Table.Name = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table.0.wildcard"]; ok && v == "true" { + input.Resource.Table.TableWildcard = &lakeformation.TableWildcard{} + } + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.database_name"]; ok { + input.Resource.TableWithColumns = &lakeformation.TableWithColumnsResource{ + DatabaseName: aws.String(v), + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.name"]; ok { + input.Resource.TableWithColumns.Name = aws.String(v) + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.catalog_id"]; ok && len(v) > 1 { + input.Resource.TableWithColumns.CatalogId = aws.String(v) + } + + if n, err := strconv.Atoi(rs.Primary.Attributes["table_with_columns.0.column_names.#"]); err == nil && n > 0 { + var cols []string + for i := 0; i < n; i++ { + cols = append(cols, rs.Primary.Attributes[fmt.Sprintf("table_with_columns.0.column_names.%d", i)]) + } + input.Resource.TableWithColumns.ColumnNames = aws.StringSlice(cols) + } + + if v, ok := rs.Primary.Attributes["table_with_columns.0.wildcard"]; ok && v == "true" { + input.Resource.TableWithColumns.ColumnWildcard = &lakeformation.ColumnWildcard{} + } + + if n, err := strconv.Atoi(rs.Primary.Attributes["table_with_columns.0.excluded_column_names.#"]); err == nil && n > 0 { + var cols []string + for i := 0; i < n; i++ { + cols = append(cols, rs.Primary.Attributes[fmt.Sprintf("table_with_columns.0.excluded_column_names.%d", i)]) + } + input.Resource.TableWithColumns.ColumnWildcard = &lakeformation.ColumnWildcard{ + ExcludedColumnNames: aws.StringSlice(cols), + } + } + } + + conn := acctest.Provider.Meta().(*conns.AWSClient).LakeFormationConn + _, err := conn.GetResourceLFTags(input) + + if err != nil { + return err + } + + return nil + } +} + +func testAccResourceLFTagsConfig_basic(rName string, values []string) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_lakeformation_data_lake_settings" "test" { + admins = [data.aws_iam_session_context.current.issuer_arn] +} + +resource "aws_glue_catalog_database" "test" { + name = %[1]q +} + +resource "aws_lakeformation_lf_tag" "test" { + key = %[1]q + values = [%[2]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_resource_lf_tags" "test" { + catalog_id = data.aws_caller_identity.current.account_id + + database { + name = aws_glue_catalog_database.test.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.test.key + values = aws_lakeformation_lf_tag.test.values + } + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +} + +func testAccResourceLFTagsConfig_database(rName string, values []string) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_lakeformation_data_lake_settings" "test" { + admins = [data.aws_iam_session_context.current.issuer_arn] +} + +resource "aws_glue_catalog_database" "test" { + name = %[1]q +} + +resource "aws_lakeformation_lf_tag" "test" { + key = %[1]q + values = [%[2]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_resource_lf_tags" "test" { + database { + name = aws_glue_catalog_database.test.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.test.key + values = aws_lakeformation_lf_tag.test.values + } + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +} + +func testAccResourceLFTagsConfig_databaseMultiple(rName string, values, values2 []string) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_lakeformation_data_lake_settings" "test" { + admins = [data.aws_iam_session_context.current.issuer_arn] +} + +resource "aws_glue_catalog_database" "test" { + name = %[1]q +} + +resource "aws_lakeformation_lf_tag" "test" { + key = %[1]q + values = [%[2]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_lf_tag" "test2" { + key = "%[1]s-2" + values = [%[3]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_resource_lf_tags" "test" { + database { + name = aws_glue_catalog_database.test.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.test.key + values = aws_lakeformation_lf_tag.test.values + } + + lf_tag { + key = aws_lakeformation_lf_tag.test2.key + values = aws_lakeformation_lf_tag.test2.values + } + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`))) +} + +func testAccResourceLFTagsConfig_table(rName string, values []string) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_lakeformation_data_lake_settings" "test" { + admins = [data.aws_iam_session_context.current.issuer_arn] +} + +resource "aws_glue_catalog_database" "test" { + name = %[1]q +} + +resource "aws_glue_catalog_table" "test" { + name = %[1]q + database_name = aws_glue_catalog_database.test.name + + storage_descriptor { + columns { + name = "event" + type = "string" + } + + columns { + name = "timestamp" + type = "date" + } + + columns { + name = "value" + type = "double" + } + } +} + +resource "aws_lakeformation_lf_tag" "test" { + key = %[1]q + values = [%[2]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_resource_lf_tags" "test" { + table { + database_name = aws_glue_catalog_table.test.database_name + name = aws_glue_catalog_table.test.name + } + + lf_tag { + key = aws_lakeformation_lf_tag.test.key + values = aws_lakeformation_lf_tag.test.values + } + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +} + +func testAccResourceLFTagsConfig_tableWithColumns(rName string, values []string) string { + return fmt.Sprintf(` +data "aws_caller_identity" "current" {} + +data "aws_iam_session_context" "current" { + arn = data.aws_caller_identity.current.arn +} + +resource "aws_lakeformation_data_lake_settings" "test" { + admins = [data.aws_iam_session_context.current.issuer_arn] +} + +resource "aws_glue_catalog_database" "test" { + name = %[1]q +} + +resource "aws_glue_catalog_table" "test" { + name = %[1]q + database_name = aws_glue_catalog_database.test.name + + storage_descriptor { + columns { + name = "event" + type = "string" + } + + columns { + name = "timestamp" + type = "date" + } + + columns { + name = "transactionamount" + type = "double" + } + } +} + +resource "aws_lakeformation_lf_tag" "test" { + key = %[1]q + values = [%[2]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + +resource "aws_lakeformation_resource_lf_tags" "test" { + table_with_columns { + database_name = aws_glue_catalog_table.test.database_name + name = aws_glue_catalog_table.test.name + column_names = ["event", "timestamp"] + } + + lf_tag { + key = aws_lakeformation_lf_tag.test.key + values = aws_lakeformation_lf_tag.test.values + } + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +} From 68add08a8ab3517c24d6a62cb980ffb65feee371 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:28:20 -0400 Subject: [PATCH 08/15] Add changelog --- .changelog/25565.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/25565.txt diff --git a/.changelog/25565.txt b/.changelog/25565.txt new file mode 100644 index 00000000000..60fe4bfd2d2 --- /dev/null +++ b/.changelog/25565.txt @@ -0,0 +1,3 @@ +```release-note:new-resource +aws_lakeformation_resource_lf_tags +``` \ No newline at end of file From c4e2026a5b908d9bfb64ed032c0d1e975ebfe6e0 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 16:33:18 -0400 Subject: [PATCH 09/15] lakeformation: Linted windows --- website/docs/r/lakeformation_resource_lf_tags.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/lakeformation_resource_lf_tags.html.markdown b/website/docs/r/lakeformation_resource_lf_tags.html.markdown index c81145fed63..75f54c04980 100644 --- a/website/docs/r/lakeformation_resource_lf_tags.html.markdown +++ b/website/docs/r/lakeformation_resource_lf_tags.html.markdown @@ -62,7 +62,7 @@ resource "aws_lakeformation_resource_lf_tags" "example" { lf_tag { key = aws_lakeformation_lf_tag.example2.key values = aws_lakeformation_lf_tag.example2.values - } + } } ``` From ebdd2a9b0879dea55bb6a0dee108adefe202861d Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 17:47:18 -0400 Subject: [PATCH 10/15] lakeformation/res_lf_tags: Switch to one value --- .../service/lakeformation/resource_lf_tags.go | 24 ++-- .../lakeformation/resource_lf_tags_test.go | 127 ++++++++++-------- ...keformation_resource_lf_tags.html.markdown | 39 ++++-- 3 files changed, 108 insertions(+), 82 deletions(-) diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go index 90af981754f..e8a447d29b1 100644 --- a/internal/service/lakeformation/resource_lf_tags.go +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -19,7 +19,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" - "github.com/hashicorp/terraform-provider-aws/internal/flex" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" "github.com/hashicorp/terraform-provider-aws/names" @@ -94,16 +93,11 @@ func ResourceResourceLFTags() *schema.Resource { ForceNew: true, ValidateFunc: validation.StringLenBetween(1, 128), }, - "values": { - Type: schema.TypeSet, - Required: true, - MinItems: 1, - MaxItems: 15, - Elem: &schema.Schema{ - Type: schema.TypeString, - ValidateFunc: validateLFTagValues(), - }, - Set: schema.HashString, + "value": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validateLFTagValues(), }, }, }, @@ -429,7 +423,7 @@ func lfTagsHash(v interface{}) int { var buf bytes.Buffer buf.WriteString(m["key"].(string)) - buf.WriteString(fmt.Sprintf("%+v", m["values"].(*schema.Set))) + buf.WriteString(m["value"].(string)) buf.WriteString(m["catalog_id"].(string)) return create.StringHashcode(buf.String()) @@ -450,8 +444,8 @@ func expandLFTagPair(tfMap map[string]interface{}) *lakeformation.LFTagPair { apiObject.TagKey = aws.String(v) } - if v, ok := tfMap["values"].(*schema.Set); ok && v != nil { - apiObject.TagValues = flex.ExpandStringSet(v) + if v, ok := tfMap["value"].(string); ok && v != "" { + apiObject.TagValues = aws.StringSlice([]string{v}) } return apiObject @@ -499,7 +493,7 @@ func flattenLFTagPair(apiObject *lakeformation.LFTagPair) map[string]interface{} } if v := apiObject.TagValues; v != nil && len(v) > 0 { - tfMap["values"] = flex.FlattenStringList(apiObject.TagValues) + tfMap["value"] = aws.StringValue(apiObject.TagValues[0]) } return tfMap diff --git a/internal/service/lakeformation/resource_lf_tags_test.go b/internal/service/lakeformation/resource_lf_tags_test.go index 8697870f205..8186e5685e0 100644 --- a/internal/service/lakeformation/resource_lf_tags_test.go +++ b/internal/service/lakeformation/resource_lf_tags_test.go @@ -27,13 +27,13 @@ func testAccResourceLFTags_basic(t *testing.T) { CheckDestroy: testAccCheckDatabaseLFTagsDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceLFTagsConfig_basic(rName, []string{"copse"}), + Config: testAccResourceLFTagsConfig_basic(rName, []string{"copse"}, "copse"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "copse", + "key": rName, + "value": "copse", }), acctest.CheckResourceAttrAccountID(resourceName, "catalog_id"), ), @@ -53,23 +53,23 @@ func testAccResourceLFTags_database(t *testing.T) { CheckDestroy: testAccCheckDatabaseLFTagsDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceLFTagsConfig_database(rName, []string{"copse"}), + Config: testAccResourceLFTagsConfig_database(rName, []string{"copse"}, "copse"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "copse", + "key": rName, + "value": "copse", }), ), }, { - Config: testAccResourceLFTagsConfig_database(rName, []string{"luffield"}), + Config: testAccResourceLFTagsConfig_database(rName, []string{"luffield"}, "luffield"), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "luffield", + "key": rName, + "value": "luffield", }), ), }, @@ -88,31 +88,31 @@ func testAccResourceLFTags_databaseMultiple(t *testing.T) { CheckDestroy: testAccCheckDatabaseLFTagsDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"copse"}, []string{"luffield"}), + Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "woodcote", "theloop"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "copse", + "key": rName, + "value": "woodcote", }), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": fmt.Sprintf("%s-2", rName), - "values.0": "luffield", + "key": fmt.Sprintf("%s-2", rName), + "value": "theloop", }), ), }, { - Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"luffield"}, []string{"copse"}), + Config: testAccResourceLFTagsConfig_databaseMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "stowe", "becketts"), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "luffield", + "key": rName, + "value": "stowe", }), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": fmt.Sprintf("%s-2", rName), - "values.0": "copse", + "key": fmt.Sprintf("%s-2", rName), + "value": "becketts", }), ), }, @@ -131,23 +131,23 @@ func testAccResourceLFTags_table(t *testing.T) { CheckDestroy: testAccCheckDatabaseLFTagsDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceLFTagsConfig_table(rName, []string{"copse"}), + Config: testAccResourceLFTagsConfig_table(rName, []string{"copse", "abbey", "farm"}, "abbey"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "copse", + "key": rName, + "value": "abbey", }), ), }, { - Config: testAccResourceLFTagsConfig_table(rName, []string{"luffield"}), + Config: testAccResourceLFTagsConfig_table(rName, []string{"copse", "abbey", "farm"}, "farm"), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "luffield", + "key": rName, + "value": "farm", }), ), }, @@ -166,23 +166,31 @@ func testAccResourceLFTags_tableWithColumns(t *testing.T) { CheckDestroy: testAccCheckDatabaseLFTagsDestroy, Steps: []resource.TestStep{ { - Config: testAccResourceLFTagsConfig_tableWithColumns(rName, []string{"copse"}), + Config: testAccResourceLFTagsConfig_tableWithColumnsMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "luffield", "vale"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "copse", + "key": rName, + "value": "luffield", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": fmt.Sprintf("%s-2", rName), + "value": "vale", }), ), }, { - Config: testAccResourceLFTagsConfig_tableWithColumns(rName, []string{"luffield"}), + Config: testAccResourceLFTagsConfig_tableWithColumnsMultiple(rName, []string{"abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"}, []string{"farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"}, "copse", "aintree"), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseLFTagsExists(resourceName), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ - "key": rName, - "values.0": "luffield", + "key": rName, + "value": "copse", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "lf_tag.*", map[string]string{ + "key": fmt.Sprintf("%s-2", rName), + "value": "aintree", }), ), }, @@ -388,7 +396,7 @@ func testAccCheckDatabaseLFTagsExists(resourceName string) resource.TestCheckFun } } -func testAccResourceLFTagsConfig_basic(rName string, values []string) string { +func testAccResourceLFTagsConfig_basic(rName string, values []string, value string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} @@ -420,17 +428,17 @@ resource "aws_lakeformation_resource_lf_tags" "test" { } lf_tag { - key = aws_lakeformation_lf_tag.test.key - values = aws_lakeformation_lf_tag.test.values + key = aws_lakeformation_lf_tag.test.key + value = %[3]q } # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } -`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), value) } -func testAccResourceLFTagsConfig_database(rName string, values []string) string { +func testAccResourceLFTagsConfig_database(rName string, values []string, value string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} @@ -460,17 +468,17 @@ resource "aws_lakeformation_resource_lf_tags" "test" { } lf_tag { - key = aws_lakeformation_lf_tag.test.key - values = aws_lakeformation_lf_tag.test.values + key = aws_lakeformation_lf_tag.test.key + value = %[3]q } # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } -`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), value) } -func testAccResourceLFTagsConfig_databaseMultiple(rName string, values, values2 []string) string { +func testAccResourceLFTagsConfig_databaseMultiple(rName string, values1, values2 []string, value1, value2 string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} @@ -508,22 +516,22 @@ resource "aws_lakeformation_resource_lf_tags" "test" { } lf_tag { - key = aws_lakeformation_lf_tag.test.key - values = aws_lakeformation_lf_tag.test.values + key = aws_lakeformation_lf_tag.test.key + value = %[4]q } lf_tag { - key = aws_lakeformation_lf_tag.test2.key - values = aws_lakeformation_lf_tag.test2.values + key = aws_lakeformation_lf_tag.test2.key + value = %[5]q } # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } -`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`))) +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values1, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`)), value1, value2) } -func testAccResourceLFTagsConfig_table(rName string, values []string) string { +func testAccResourceLFTagsConfig_table(rName string, values []string, value string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} @@ -576,17 +584,17 @@ resource "aws_lakeformation_resource_lf_tags" "test" { } lf_tag { - key = aws_lakeformation_lf_tag.test.key - values = aws_lakeformation_lf_tag.test.values + key = aws_lakeformation_lf_tag.test.key + value = %[3]q } # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } -`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`)), value) } -func testAccResourceLFTagsConfig_tableWithColumns(rName string, values []string) string { +func testAccResourceLFTagsConfig_tableWithColumnsMultiple(rName string, values1, values2 []string, value1 string, value2 string) string { return fmt.Sprintf(` data "aws_caller_identity" "current" {} @@ -632,6 +640,14 @@ resource "aws_lakeformation_lf_tag" "test" { depends_on = [aws_lakeformation_data_lake_settings.test] } +resource "aws_lakeformation_lf_tag" "test2" { + key = "%[1]s-2" + values = [%[3]s] + + # for consistency, ensure that admins are setup before testing + depends_on = [aws_lakeformation_data_lake_settings.test] +} + resource "aws_lakeformation_resource_lf_tags" "test" { table_with_columns { database_name = aws_glue_catalog_table.test.database_name @@ -640,12 +656,17 @@ resource "aws_lakeformation_resource_lf_tags" "test" { } lf_tag { - key = aws_lakeformation_lf_tag.test.key - values = aws_lakeformation_lf_tag.test.values + key = aws_lakeformation_lf_tag.test.key + value = %[4]q + } + + lf_tag { + key = aws_lakeformation_lf_tag.test2.key + value = %[5]q } # for consistency, ensure that admins are setup before testing depends_on = [aws_lakeformation_data_lake_settings.test] } -`, rName, fmt.Sprintf(`"%s"`, strings.Join(values, `", "`))) +`, rName, fmt.Sprintf(`"%s"`, strings.Join(values1, `", "`)), fmt.Sprintf(`"%s"`, strings.Join(values2, `", "`)), value1, value2) } diff --git a/website/docs/r/lakeformation_resource_lf_tags.html.markdown b/website/docs/r/lakeformation_resource_lf_tags.html.markdown index 75f54c04980..d0fe9672d1c 100644 --- a/website/docs/r/lakeformation_resource_lf_tags.html.markdown +++ b/website/docs/r/lakeformation_resource_lf_tags.html.markdown @@ -16,8 +16,8 @@ Manages an attachment between one or more existing LF-tags and an existing Lake ```terraform resource "aws_lakeformation_lf_tag" "example" { - key = "copse" - values = ["luffield"] + key = "right" + values = ["abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"] } resource "aws_lakeformation_resource_lf_tags" "example" { @@ -28,8 +28,8 @@ resource "aws_lakeformation_resource_lf_tags" "example" { } lf_tag { - key = aws_lakeformation_lf_tag.example.key - values = aws_lakeformation_lf_tag.example.values + key = aws_lakeformation_lf_tag.example.key + value = "stowe" } } ``` @@ -38,13 +38,13 @@ resource "aws_lakeformation_resource_lf_tags" "example" { ```terraform resource "aws_lakeformation_lf_tag" "example" { - key = "copse" - values = ["luffield"] + key = "right" + values = ["abbey", "village", "luffield", "woodcote", "copse", "chapel", "stowe", "club"] } resource "aws_lakeformation_lf_tag" "example2" { - key = "stowe" - values = ["brooklands"] + key = "left" + values = ["farm", "theloop", "aintree", "brooklands", "maggotts", "becketts", "vale"] } resource "aws_lakeformation_resource_lf_tags" "example" { @@ -55,13 +55,13 @@ resource "aws_lakeformation_resource_lf_tags" "example" { } lf_tag { - key = aws_lakeformation_lf_tag.example.key - values = aws_lakeformation_lf_tag.example.values + key = "right" + value = "luffield" } lf_tag { - key = aws_lakeformation_lf_tag.example2.key - values = aws_lakeformation_lf_tag.example2.values + key = "left" + value = "aintree" } } ``` @@ -70,9 +70,9 @@ resource "aws_lakeformation_resource_lf_tags" "example" { The following arguments are required: -* `lf_tag` – (Required) Set of LF-tags to attach to the resource. +* `lf_tag` – (Required) Set of LF-tags to attach to the resource. See below. -One of the following is required: +Exactly one of the following is required: * `database` - (Optional) Configuration block for a database resource. See below. * `table` - (Optional) Configuration block for a table resource. See below. @@ -82,6 +82,17 @@ The following arguments are optional: * `catalog_id` – (Optional) Identifier for the Data Catalog. By default, the account ID. The Data Catalog is the persistent metadata store. It contains database definitions, table definitions, and other control information to manage your Lake Formation environment. +### lf_tag + +The following arguments are required: + +* `key` – (Required) Key name for an existing LF-tag. +* `value` - (Required) Value from the possible values for the LF-tag. + +The following argument is optional: + +* `catalog_id` - (Optional) Identifier for the Data Catalog. By default, it is the account ID of the caller. + ### database The following argument is required: From cab3233b6c1562ed58e5ab02e12f8e11cdaf9982 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 17:52:48 -0400 Subject: [PATCH 11/15] ci/lint: Fixes --- .github/workflows/terraform_provider.yml | 2 +- internal/service/lakeformation/resource_lf_tags.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/terraform_provider.yml b/.github/workflows/terraform_provider.yml index 552be8576fc..8a7b0b227db 100644 --- a/.github/workflows/terraform_provider.yml +++ b/.github/workflows/terraform_provider.yml @@ -293,7 +293,7 @@ jobs: key: ${{ runner.os }}-go-pkg-mod-${{ hashFiles('go.sum') }} - run: cd tools && go install github.com/golangci/golangci-lint/cmd/golangci-lint shell: bash - - run: golangci-lint run ./internal/... + - run: golangci-lint run ./internal/... --timeout 75m shell: bash - run: cd providerlint && golangci-lint run ./... shell: bash diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go index e8a447d29b1..786e6794088 100644 --- a/internal/service/lakeformation/resource_lf_tags.go +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -492,7 +492,7 @@ func flattenLFTagPair(apiObject *lakeformation.LFTagPair) map[string]interface{} tfMap["key"] = aws.StringValue(v) } - if v := apiObject.TagValues; v != nil && len(v) > 0 { + if v := apiObject.TagValues; len(v) > 0 { tfMap["value"] = aws.StringValue(apiObject.TagValues[0]) } From 06bb5660e663f9da04cd9ed66493314823cb3bf9 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Fri, 24 Jun 2022 17:54:09 -0400 Subject: [PATCH 12/15] Linteresting --- website/docs/r/lakeformation_resource_lf_tags.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/lakeformation_resource_lf_tags.html.markdown b/website/docs/r/lakeformation_resource_lf_tags.html.markdown index d0fe9672d1c..2d6f14365b3 100644 --- a/website/docs/r/lakeformation_resource_lf_tags.html.markdown +++ b/website/docs/r/lakeformation_resource_lf_tags.html.markdown @@ -87,7 +87,7 @@ The following arguments are optional: The following arguments are required: * `key` – (Required) Key name for an existing LF-tag. -* `value` - (Required) Value from the possible values for the LF-tag. +* `value` - (Required) Value from the possible values for the LF-tag. The following argument is optional: From 9a6c5fd08ba4eb627d3395104538c3435e02d136 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 27 Jun 2022 16:07:25 -0400 Subject: [PATCH 13/15] lakeformation: Review fixes --- internal/service/lakeformation/resource_lf_tags.go | 2 +- website/docs/r/lakeformation_resource_lf_tags.html.markdown | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go index 786e6794088..8052c163c09 100644 --- a/internal/service/lakeformation/resource_lf_tags.go +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -261,7 +261,7 @@ func resourceResourceLFTagsCreate(ctx context.Context, d *schema.ResourceData, m return resource.RetryableError(err) } - return resource.NonRetryableError(fmt.Errorf("error creating Lake Formation LF Tag Resource: %w", err)) + return resource.NonRetryableError(err) } return nil }) diff --git a/website/docs/r/lakeformation_resource_lf_tags.html.markdown b/website/docs/r/lakeformation_resource_lf_tags.html.markdown index 2d6f14365b3..432426e005c 100644 --- a/website/docs/r/lakeformation_resource_lf_tags.html.markdown +++ b/website/docs/r/lakeformation_resource_lf_tags.html.markdown @@ -21,8 +21,6 @@ resource "aws_lakeformation_lf_tag" "example" { } resource "aws_lakeformation_resource_lf_tags" "example" { - catalog_id = data.aws_caller_identity.current.account_id - database { name = aws_glue_catalog_database.example.name } @@ -48,8 +46,6 @@ resource "aws_lakeformation_lf_tag" "example2" { } resource "aws_lakeformation_resource_lf_tags" "example" { - catalog_id = data.aws_caller_identity.current.account_id - database { name = aws_glue_catalog_database.example.name } From ec66549c16cd7a91e5f3eefcf3df820b1791a443 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Mon, 27 Jun 2022 19:14:04 -0400 Subject: [PATCH 14/15] lakeformation/resource_lf_tags: Rework failures as warning --- .../service/lakeformation/resource_lf_tags.go | 28 ++++++---- names/errors.go | 53 ++++++++++--------- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go index 8052c163c09..7fc5b696e3a 100644 --- a/internal/service/lakeformation/resource_lf_tags.go +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -3,6 +3,7 @@ package lakeformation import ( "bytes" "context" + "errors" "fmt" "log" "reflect" @@ -12,7 +13,6 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/lakeformation" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -274,27 +274,37 @@ func resourceResourceLFTagsCreate(ctx context.Context, d *schema.ResourceData, m return names.DiagError(names.LakeFormation, names.ErrActionCreating, resourceLFTags, input.String(), err) } - if output != nil && len(output.Failures) > 0 { - var failures *multierror.Error + diags := diag.Diagnostics{} + if output != nil && len(output.Failures) > 0 { for _, v := range output.Failures { if v.LFTag == nil || v.Error == nil { continue } - origErr := fmt.Errorf("catalog id:%s, tag key:%s, values:%+v", aws.StringValue(v.LFTag.CatalogId), aws.StringValue(v.LFTag.TagKey), aws.StringValueSlice(v.LFTag.TagValues)) - err := awserr.New(aws.StringValue(v.Error.ErrorCode), aws.StringValue(v.Error.ErrorMessage), origErr) - failures = multierror.Append(failures, err) + diags = names.AddWarning( + diags, + names.LakeFormation, + names.ErrActionCreating, + resourceLFTags, + fmt.Sprintf("catalog id:%s, tag key:%s, values:%+v", aws.StringValue(v.LFTag.CatalogId), aws.StringValue(v.LFTag.TagKey), aws.StringValueSlice(v.LFTag.TagValues)), + awserr.New(aws.StringValue(v.Error.ErrorCode), aws.StringValue(v.Error.ErrorMessage), nil), + ) } - if failures.Len() > 0 { - return names.DiagError(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", failures.ErrorOrNil()) + if len(diags) == len(input.LFTags) { + return append(diags, + diag.Diagnostic{ + Severity: diag.Error, + Summary: names.ProblemStandardMessage(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", errors.New(fmt.Sprintf("attempted to add %d tags, %d failures", len(input.LFTags), len(diags)))), + }, + ) } } d.SetId(fmt.Sprintf("%d", create.StringHashcode(input.String()))) - return resourceResourceLFTagsRead(ctx, d, meta) + return append(resourceResourceLFTagsRead(ctx, d, meta), diags...) } func resourceResourceLFTagsRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { diff --git a/names/errors.go b/names/errors.go index 737151ac919..04e1452036a 100644 --- a/names/errors.go +++ b/names/errors.go @@ -18,46 +18,51 @@ const ( ErrActionCheckingDestroyed = "checking destroyed" ) -func Error(service, action, resource, id string, gotError error) error { +// ProblemStandardMessage is a standardized message for reporting errors and warnings +func ProblemStandardMessage(service, action, resource, id string, gotError error) string { hf, err := FullHumanFriendly(service) if err != nil { - return fmt.Errorf("finding human-friendly name for service (%s) while creating error (%s, %s, %s, %s): %w", service, action, resource, id, gotError, err) + return fmt.Sprintf("finding human-friendly name for service (%s) while creating error (%s, %s, %s, %s): %s", service, action, resource, id, gotError, err) } if gotError == nil { - return fmt.Errorf("%s %s %s (%s)", action, hf, resource, id) + return fmt.Sprintf("%s %s %s (%s)", action, hf, resource, id) } - return fmt.Errorf("%s %s %s (%s): %w", action, hf, resource, id, gotError) + return fmt.Sprintf("%s %s %s (%s): %s", action, hf, resource, id, gotError) } -func DiagError(service, action, resource, id string, gotError error) diag.Diagnostics { - hf, err := FullHumanFriendly(service) - - if err != nil { - return diag.Errorf("finding human-friendly name for service (%s) while creating error (%s, %s, %s, %s): %s", service, action, resource, id, gotError, err) - } +// Error returns an errors.Error with a standardized error message +func Error(service, action, resource, id string, gotError error) error { + return errors.New(ProblemStandardMessage(service, action, resource, id, gotError)) +} - if gotError == nil { - return diag.Errorf("%s %s %s (%s)", action, hf, resource, id) +// DiagError returns a 1-length diag.Diagnostics with a diag.Error-level diag.Diagnostic +// with a standardized error message +func DiagError(service, action, resource, id string, gotError error) diag.Diagnostics { + return diag.Diagnostics{ + diag.Diagnostic{ + Severity: diag.Error, + Summary: ProblemStandardMessage(service, action, resource, id, gotError), + }, } +} - return diag.Errorf("%s %s %s (%s): %s", action, hf, resource, id, gotError) +// AddWarning returns diag.Diagnostics with an additional diag.Diagnostic containing +// a warning using a standardized problem message +func AddWarning(diags diag.Diagnostics, service, action, resource, id string, gotError error) diag.Diagnostics { + return append(diags, + diag.Diagnostic{ + Severity: diag.Warning, + Summary: ProblemStandardMessage(service, action, resource, id, gotError), + }, + ) } +// WarnLog logs to the default logger a standardized problem message func WarnLog(service, action, resource, id string, gotError error) { - hf, err := FullHumanFriendly(service) - - if err != nil { - log.Printf("[ERROR] finding human-friendly name for service (%s) while logging warn (%s, %s, %s, %s): %s", service, action, resource, id, gotError, err) - } - - if gotError == nil { - log.Printf("[WARN] %s %s %s (%s)", action, hf, resource, id) - } - - log.Printf("[WARN] %s %s %s (%s): %s", action, hf, resource, id, gotError) + log.Printf("[WARN] %s", ProblemStandardMessage(service, action, resource, id, gotError)) } func LogNotFoundRemoveState(service, action, resource, id string) { From 8ba4b3dae5f6949e24676ed38ed236751748c81e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Tue, 28 Jun 2022 10:54:14 -0400 Subject: [PATCH 15/15] lakeformation/resource_lf_tags: Simplify --- internal/service/lakeformation/resource_lf_tags.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/service/lakeformation/resource_lf_tags.go b/internal/service/lakeformation/resource_lf_tags.go index 7fc5b696e3a..7876db8d574 100644 --- a/internal/service/lakeformation/resource_lf_tags.go +++ b/internal/service/lakeformation/resource_lf_tags.go @@ -3,7 +3,6 @@ package lakeformation import ( "bytes" "context" - "errors" "fmt" "log" "reflect" @@ -296,7 +295,7 @@ func resourceResourceLFTagsCreate(ctx context.Context, d *schema.ResourceData, m return append(diags, diag.Diagnostic{ Severity: diag.Error, - Summary: names.ProblemStandardMessage(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", errors.New(fmt.Sprintf("attempted to add %d tags, %d failures", len(input.LFTags), len(diags)))), + Summary: names.ProblemStandardMessage(names.LakeFormation, names.ErrActionCreating, resourceLFTags, "", fmt.Errorf("attempted to add %d tags, %d failures", len(input.LFTags), len(diags))), }, ) }