diff --git a/.changelog/33374.txt b/.changelog/33374.txt new file mode 100644 index 000000000000..57cf99a62716 --- /dev/null +++ b/.changelog/33374.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_glue_catalog_table: Fix removal of `metadata_location` and `table_type` `parameters` when updating Iceberg tables +``` \ No newline at end of file diff --git a/internal/service/glue/catalog_table.go b/internal/service/glue/catalog_table.go index cbaca75030b3..7933d91e4f04 100644 --- a/internal/service/glue/catalog_table.go +++ b/internal/service/glue/catalog_table.go @@ -15,11 +15,13 @@ import ( "github.com/aws/aws-sdk-go/service/glue" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "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/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" "github.com/hashicorp/terraform-provider-aws/internal/verify" ) @@ -422,21 +424,21 @@ func resourceCatalogTableRead(ctx context.Context, d *schema.ResourceData, meta catalogID, dbName, name, err := ReadTableID(d.Id()) if err != nil { - return sdkdiag.AppendErrorf(diags, "reading Glue Catalog Table (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } - out, err := FindTableByName(ctx, conn, catalogID, dbName, name) - if err != nil { - if tfawserr.ErrCodeEquals(err, glue.ErrCodeEntityNotFoundException) { - log.Printf("[WARN] Glue Catalog Table (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags - } + table, err := FindTableByName(ctx, conn, catalogID, dbName, name) + if !d.IsNewResource() && tfresource.NotFound(err) { + log.Printf("[WARN] Glue Catalog Table (%s) not found, removing from state", d.Id()) + d.SetId("") + return diags + } + + if err != nil { return sdkdiag.AppendErrorf(diags, "reading Glue Catalog Table (%s): %s", d.Id(), err) } - table := out.Table tableArn := arn.ARN{ Partition: meta.(*conns.AWSClient).Partition, Service: "glue", @@ -445,11 +447,10 @@ func resourceCatalogTableRead(ctx context.Context, d *schema.ResourceData, meta Resource: fmt.Sprintf("table/%s/%s", dbName, aws.StringValue(table.Name)), }.String() d.Set("arn", tableArn) - - d.Set("name", table.Name) d.Set("catalog_id", catalogID) d.Set("database_name", dbName) d.Set("description", table.Description) + d.Set("name", table.Name) d.Set("owner", table.Owner) d.Set("retention", table.Retention) @@ -477,18 +478,20 @@ func resourceCatalogTableRead(ctx context.Context, d *schema.ResourceData, meta d.Set("target_table", nil) } - partIndexInput := &glue.GetPartitionIndexesInput{ - CatalogId: out.Table.CatalogId, - TableName: out.Table.Name, - DatabaseName: out.Table.DatabaseName, + input := &glue.GetPartitionIndexesInput{ + CatalogId: aws.String(catalogID), + DatabaseName: aws.String(dbName), + TableName: aws.String(name), } - partOut, err := conn.GetPartitionIndexesWithContext(ctx, partIndexInput) + + output, err := conn.GetPartitionIndexesWithContext(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "getting Glue Partition Indexes: %s", err) + return sdkdiag.AppendErrorf(diags, "reading Glue Catalog Table (%s) partition indexes: %s", d.Id(), err) } - if partOut != nil && len(partOut.PartitionIndexDescriptorList) > 0 { - if err := d.Set("partition_index", flattenPartitionIndexes(partOut.PartitionIndexDescriptorList)); err != nil { + if output != nil && len(output.PartitionIndexDescriptorList) > 0 { + if err := d.Set("partition_index", flattenPartitionIndexes(output.PartitionIndexDescriptorList)); err != nil { return sdkdiag.AppendErrorf(diags, "setting partition_index: %s", err) } } @@ -500,18 +503,38 @@ func resourceCatalogTableUpdate(ctx context.Context, d *schema.ResourceData, met var diags diag.Diagnostics conn := meta.(*conns.AWSClient).GlueConn(ctx) - catalogID, dbName, _, err := ReadTableID(d.Id()) + catalogID, dbName, name, err := ReadTableID(d.Id()) if err != nil { - return sdkdiag.AppendErrorf(diags, "updating Glue Catalog Table (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } - updateTableInput := &glue.UpdateTableInput{ + input := &glue.UpdateTableInput{ CatalogId: aws.String(catalogID), DatabaseName: aws.String(dbName), TableInput: expandTableInput(d), } - if _, err := conn.UpdateTableWithContext(ctx, updateTableInput); err != nil { + // Add back any managed parameters. See flattenNonManagedParameters. + table, err := FindTableByName(ctx, conn, catalogID, dbName, name) + + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading Glue Catalog Table (%s): %s", d.Id(), err) + } + + if allParameters := aws.StringValueMap(table.Parameters); allParameters["table_type"] == "ICEBERG" { + for _, k := range []string{"table_type", "metadata_location"} { + if v := allParameters[k]; v != "" { + if input.TableInput.Parameters == nil { + input.TableInput.Parameters = make(map[string]*string) + } + input.TableInput.Parameters[k] = aws.String(v) + } + } + } + + _, err = conn.UpdateTableWithContext(ctx, input) + + if err != nil { return sdkdiag.AppendErrorf(diags, "updating Glue Catalog Table (%s): %s", d.Id(), err) } @@ -524,24 +547,54 @@ func resourceCatalogTableDelete(ctx context.Context, d *schema.ResourceData, met catalogID, dbName, name, err := ReadTableID(d.Id()) if err != nil { - return sdkdiag.AppendErrorf(diags, "deleting Glue Catalog Table (%s): %s", d.Id(), err) + return sdkdiag.AppendFromErr(diags, err) } - log.Printf("[DEBUG] Glue Catalog Table: %s:%s:%s", catalogID, dbName, name) + log.Printf("[DEBUG] Deleting Glue Catalog Table: %s", d.Id()) _, err = conn.DeleteTableWithContext(ctx, &glue.DeleteTableInput{ CatalogId: aws.String(catalogID), Name: aws.String(name), DatabaseName: aws.String(dbName), }) + + if tfawserr.ErrCodeEquals(err, glue.ErrCodeEntityNotFoundException) { + return diags + } + if err != nil { - if tfawserr.ErrCodeEquals(err, glue.ErrCodeEntityNotFoundException) { - return diags - } return sdkdiag.AppendErrorf(diags, "deleting Glue Catalog Table (%s): %s", d.Id(), err) } + return diags } +func FindTableByName(ctx context.Context, conn *glue.Glue, catalogID, dbName, name string) (*glue.TableData, error) { + input := &glue.GetTableInput{ + CatalogId: aws.String(catalogID), + DatabaseName: aws.String(dbName), + Name: aws.String(name), + } + + output, err := conn.GetTableWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, glue.ErrCodeEntityNotFoundException) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil || output.Table == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output.Table, nil +} + func expandTableInput(d *schema.ResourceData) *glue.TableInput { tableInput := &glue.TableInput{ Name: aws.String(d.Get("name").(string)), diff --git a/internal/service/glue/catalog_table_test.go b/internal/service/glue/catalog_table_test.go index 9c63fde1e5fd..f6daa12a8532 100644 --- a/internal/service/glue/catalog_table_test.go +++ b/internal/service/glue/catalog_table_test.go @@ -8,15 +8,14 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/glue" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/hashicorp/terraform-provider-aws/internal/acctest" "github.com/hashicorp/terraform-provider-aws/internal/conns" tfglue "github.com/hashicorp/terraform-provider-aws/internal/service/glue" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) func init() { @@ -694,7 +693,7 @@ func TestAccGlueCatalogTable_openTableFormat(t *testing.T) { CheckDestroy: testAccCheckTableDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccCatalogTableConfig_openTableFormat(rName), + Config: testAccCatalogTableConfig_openTableFormat(rName, "comment1"), Destroy: false, Check: resource.ComposeTestCheckFunc( testAccCheckCatalogTableExists(ctx, resourceName), @@ -710,6 +709,17 @@ func TestAccGlueCatalogTable_openTableFormat(t *testing.T) { ImportStateVerify: true, ImportStateVerifyIgnore: []string{"open_table_format_input"}, }, + { + Config: testAccCatalogTableConfig_openTableFormat(rName, "comment2"), + Destroy: false, + Check: resource.ComposeTestCheckFunc( + testAccCheckCatalogTableExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "open_table_format_input.#", "1"), + resource.TestCheckResourceAttr(resourceName, "open_table_format_input.0.iceberg_input.#", "1"), + resource.TestCheckResourceAttr(resourceName, "open_table_format_input.0.iceberg_input.0.metadata_operation", "CREATE"), + resource.TestCheckResourceAttr(resourceName, "open_table_format_input.0.iceberg_input.0.version", "2"), + ), + }, }, }) } @@ -1168,57 +1178,45 @@ func testAccCheckTableDestroy(ctx context.Context) resource.TestCheckFunc { continue } - catalogId, dbName, resourceName, err := tfglue.ReadTableID(rs.Primary.ID) + catalogID, dbName, name, err := tfglue.ReadTableID(rs.Primary.ID) if err != nil { return err } - if _, err := tfglue.FindTableByName(ctx, conn, catalogId, dbName, resourceName); err != nil { - //Verify the error is what we want - if tfawserr.ErrCodeEquals(err, glue.ErrCodeEntityNotFoundException) { - continue - } + _, err = tfglue.FindTableByName(ctx, conn, catalogID, dbName, name) + if tfresource.NotFound(err) { + continue + } + + if err != nil { return err } - return fmt.Errorf("still exists") + + return fmt.Errorf("Glue Catalog Table %s still exists", rs.Primary.ID) } + return nil } } -func testAccCheckCatalogTableExists(ctx context.Context, name string) resource.TestCheckFunc { +func testAccCheckCatalogTableExists(ctx context.Context, n string) resource.TestCheckFunc { return func(s *terraform.State) error { - rs, ok := s.RootModule().Resources[name] + rs, ok := s.RootModule().Resources[n] if !ok { - return fmt.Errorf("Not found: %s", name) - } - - if rs.Primary.ID == "" { - return fmt.Errorf("No ID is set") + return fmt.Errorf("Not found: %s", n) } - catalogId, dbName, resourceName, err := tfglue.ReadTableID(rs.Primary.ID) + catalogID, dbName, name, err := tfglue.ReadTableID(rs.Primary.ID) if err != nil { return err } conn := acctest.Provider.Meta().(*conns.AWSClient).GlueConn(ctx) - out, err := tfglue.FindTableByName(ctx, conn, catalogId, dbName, resourceName) - if err != nil { - return err - } - if out.Table == nil { - return fmt.Errorf("No Glue Table Found") - } - - if aws.StringValue(out.Table.Name) != resourceName { - return fmt.Errorf("Glue Table Mismatch - existing: %q, state: %q", - aws.StringValue(out.Table.Name), resourceName) - } + _, err = tfglue.FindTableByName(ctx, conn, catalogID, dbName, name) - return nil + return err } } @@ -1443,8 +1441,7 @@ resource "aws_glue_catalog_table" "test2" { `, rName) } -func testAccCatalogTableConfig_openTableFormat(rName string) string { - //var trimmed_name = strings.Trim(rName, "\"") +func testAccCatalogTableConfig_openTableFormat(rName, columnComment string) string { return fmt.Sprintf(` resource "aws_glue_catalog_database" "test" { name = %[1]q @@ -1473,15 +1470,15 @@ resource "aws_glue_catalog_table" "test" { columns { name = "my_column_1" type = "int" - comment = "my_column1_comment" + comment = %[2]q } columns { name = "my_column_2" type = "string" - comment = "my_column2_comment" + comment = %[2]q } } } -`, rName) +`, rName, columnComment) } diff --git a/internal/service/glue/find.go b/internal/service/glue/find.go index 5165d243278c..338af5ddca7a 100644 --- a/internal/service/glue/find.go +++ b/internal/service/glue/find.go @@ -112,22 +112,6 @@ func FindDataQualityRulesetByName(ctx context.Context, conn *glue.Glue, name str return output, nil } -// FindTableByName returns the Table corresponding to the specified name. -func FindTableByName(ctx context.Context, conn *glue.Glue, catalogID, dbName, name string) (*glue.GetTableOutput, error) { - input := &glue.GetTableInput{ - CatalogId: aws.String(catalogID), - DatabaseName: aws.String(dbName), - Name: aws.String(name), - } - - output, err := conn.GetTableWithContext(ctx, input) - if err != nil { - return nil, err - } - - return output, nil -} - // FindTriggerByName returns the Trigger corresponding to the specified name. func FindTriggerByName(ctx context.Context, conn *glue.Glue, name string) (*glue.GetTriggerOutput, error) { input := &glue.GetTriggerInput{