r/aws_glue_catalog_table: Fix removal of Parameters when updating Iceberg table

.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
+```

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)),

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)
 }

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{