Skip to content

Commit

Permalink
Merge pull request #33374 from hashicorp/r-aws_glue_catalog_table-par…
Browse files Browse the repository at this point in the history 
…ameters-Iceberg

r/aws_glue_catalog_table: Fix removal of Parameters when updating Iceberg table
  • Loading branch information
@ewbankkit
ewbankkit authored Sep 8, 2023
2 parents 730555b + 0c372c8 commit 41bb39a
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 80 deletions.
3 changes: 3 additions & 0 deletions .changelog/33374.txt
View file
Original file line number Diff line number Diff line change
@@ -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
```
109 changes: 81 additions & 28 deletions internal/service/glue/catalog_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
Expand All @@ -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)

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}

Expand All @@ -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)),
Expand Down
69 changes: 33 additions & 36 deletions internal/service/glue/catalog_table_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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),
Expand All @@ -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"),
),
},
},
})
}
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
16 changes: 0 additions & 16 deletions internal/service/glue/find.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 41bb39a

Please sign in to comment.