From fda82079e6e12b43fb2e3fd609467d618ba5d728 Mon Sep 17 00:00:00 2001 From: jumpei-miyata Date: Tue, 26 Oct 2021 17:58:48 +0900 Subject: [PATCH 01/19] r/aws_athena_database: remove ForceNew from bucket field --- internal/service/athena/database.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 4827d58866a..455120b20c4 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -31,7 +31,6 @@ func ResourceDatabase() *schema.Resource { "bucket": { Type: schema.TypeString, Required: true, - ForceNew: true, }, "force_destroy": { Type: schema.TypeBool, From 07d494e1ca4ec46152dbb36b3eabb51b012f06ea Mon Sep 17 00:00:00 2001 From: jumpei-miyata Date: Tue, 26 Oct 2021 18:31:34 +0900 Subject: [PATCH 02/19] Add changelog entry --- .changelog/21491.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/21491.txt diff --git a/.changelog/21491.txt b/.changelog/21491.txt new file mode 100644 index 00000000000..112ea1c70a4 --- /dev/null +++ b/.changelog/21491.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_athena_database: Remove ForceNew from bucket field. +``` From 02adf61f5e2888a507fb313a4fe189e7694e6d68 Mon Sep 17 00:00:00 2001 From: drexler Date: Wed, 1 Dec 2021 21:54:24 -0500 Subject: [PATCH 03/19] feat: support comment property for athena databases --- internal/service/athena/database.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 4827d58866a..9845bd9a265 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -33,6 +33,10 @@ func ResourceDatabase() *schema.Resource { Required: true, ForceNew: true, }, + "comment": { + Type: schema.TypeString, + Optional: true, + }, "force_destroy": { Type: schema.TypeBool, Optional: true, @@ -93,8 +97,15 @@ func expandAthenaResultConfiguration(bucket string, encryptionConfigurationList func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn + var databaseDescription string + if v, ok := d.GetOk("comment"); ok { + databaseDescription = strings.Replace(v.(string), "'", "\\'", -1) + } else { + databaseDescription = "" + } + input := &athena.StartQueryExecutionInput{ - QueryString: aws.String(fmt.Sprintf("create database `%s`;", d.Get("name").(string))), + QueryString: aws.String(fmt.Sprintf("create database `%[1]s` comment '%[2]s';", d.Get("name").(string), databaseDescription)), ResultConfiguration: expandAthenaResultConfiguration(d.Get("bucket").(string), d.Get("encryption_configuration").([]interface{})), } From f6c364d6014d965ab238e7ee1bb21525cb8f47bf Mon Sep 17 00:00:00 2001 From: drexler Date: Wed, 1 Dec 2021 21:54:52 -0500 Subject: [PATCH 04/19] test: add cover test for comment property --- internal/service/athena/database_test.go | 72 ++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/internal/service/athena/database_test.go b/internal/service/athena/database_test.go index 3d96f064e3a..943807129f4 100644 --- a/internal/service/athena/database_test.go +++ b/internal/service/athena/database_test.go @@ -35,6 +35,46 @@ func TestAccAthenaDatabase_basic(t *testing.T) { }) } +func TestAccAthenaDatabase_description(t *testing.T) { + rInt := sdkacctest.RandInt() + dbName := sdkacctest.RandString(8) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseCommentConfig(rInt, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists("aws_athena_database.hoge"), + resource.TestCheckResourceAttr("aws_athena_database.hoge", "name", dbName), + ), + }, + }, + }) +} + +func TestAccAthenaDatabase_unescaped_description(t *testing.T) { + rInt := sdkacctest.RandInt() + dbName := sdkacctest.RandString(8) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseUnescapedCommentConfig(rInt, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists("aws_athena_database.hoge"), + resource.TestCheckResourceAttr("aws_athena_database.hoge", "name", dbName), + ), + }, + }, + }) +} + func TestAccAthenaDatabase_encryption(t *testing.T) { rInt := sdkacctest.RandInt() dbName := sdkacctest.RandString(8) @@ -354,6 +394,38 @@ resource "aws_athena_database" "hoge" { `, randInt, dbName, forceDestroy) } +func testAccAthenaDatabaseCommentConfig(randInt int, dbName string, forceDestroy bool) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "hoge" { + bucket = "tf-test-athena-db-%[1]d" + force_destroy = true +} + +resource "aws_athena_database" "hoge" { + name = "%[2]s" + bucket = aws_s3_bucket.hoge.bucket + comment = "athena is a goddess" + force_destroy = %[3]t +} +`, randInt, dbName, forceDestroy) +} + +func testAccAthenaDatabaseUnescapedCommentConfig(randInt int, dbName string, forceDestroy bool) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "hoge" { + bucket = "tf-test-athena-db-%[1]d" + force_destroy = true +} + +resource "aws_athena_database" "hoge" { + name = "%[2]s" + bucket = aws_s3_bucket.hoge.bucket + comment = "athena's a goddess" + force_destroy = %[3]t +} +`, randInt, dbName, forceDestroy) +} + func testAccAthenaDatabaseWithKMSConfig(randInt int, dbName string, forceDestroy bool) string { return fmt.Sprintf(` resource "aws_kms_key" "hoge" { From 09afa0eb15ba92ee2359638c8a511f6d16b374c6 Mon Sep 17 00:00:00 2001 From: drexler Date: Wed, 1 Dec 2021 21:55:21 -0500 Subject: [PATCH 05/19] docs: add documentation for comment property --- website/docs/r/athena_database.html.markdown | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/r/athena_database.html.markdown b/website/docs/r/athena_database.html.markdown index 53ee0b6d92b..917f834a136 100644 --- a/website/docs/r/athena_database.html.markdown +++ b/website/docs/r/athena_database.html.markdown @@ -29,6 +29,7 @@ The following arguments are supported: * `name` - (Required) Name of the database to create. * `bucket` - (Required) Name of s3 bucket to save the results of the query execution. +* `comment` - (Optional) Description of the database. * `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. An `encryption_configuration` block is documented below. * `force_destroy` - (Optional, Default: false) A boolean that indicates all tables should be deleted from the database so that the database can be destroyed without error. The tables are *not* recoverable. From e63d7c7a5752d28c0c64d0b0d6cbacfcd4489111 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 18 Mar 2022 11:10:38 +0200 Subject: [PATCH 06/19] add args and refactor --- internal/service/athena/database.go | 156 ++++++++++++------- internal/service/athena/database_test.go | 188 +++++++++++++---------- 2 files changed, 205 insertions(+), 139 deletions(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 4827d58866a..448459e3f51 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -2,12 +2,14 @@ package athena import ( "fmt" + "log" "regexp" "strings" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/athena" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "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" @@ -22,72 +24,116 @@ func ResourceDatabase() *schema.Resource { Delete: resourceDatabaseDelete, Schema: map[string]*schema.Schema{ - "name": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - ValidateFunc: validation.StringMatch(regexp.MustCompile("^[_a-z0-9]+$"), "must be lowercase letters, numbers, or underscore ('_')"), - }, - "bucket": { - Type: schema.TypeString, - Required: true, - ForceNew: true, - }, - "force_destroy": { - Type: schema.TypeBool, + "acl_configuration": { + Type: schema.TypeList, Optional: true, - Default: false, + ForceNew: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "s3_acl_option": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(athena.S3AclOption_Values(), false), + ForceNew: true, + }, + }, + }, }, "encryption_configuration": { Type: schema.TypeList, Optional: true, + ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "kms_key": { Type: schema.TypeString, Optional: true, + ForceNew: true, }, "encryption_option": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice([]string{ - athena.EncryptionOptionCseKms, - athena.EncryptionOptionSseKms, - athena.EncryptionOptionSseS3, - }, false), + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice(athena.EncryptionOption_Values(), false), + ForceNew: true, }, }, }, }, + "expected_bucket_owner": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "bucket": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, + "force_destroy": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.StringMatch(regexp.MustCompile("^[_a-z0-9]+$"), "must be lowercase letters, numbers, or underscore ('_')"), + }, }, } } -func expandAthenaResultConfiguration(bucket string, encryptionConfigurationList []interface{}) *athena.ResultConfiguration { - resultConfig := athena.ResultConfiguration{ - OutputLocation: aws.String("s3://" + bucket), +func expandAthenaResultConfiguration(d *schema.ResourceData) *athena.ResultConfiguration { + + resultConfig := &athena.ResultConfiguration{ + OutputLocation: aws.String("s3://" + d.Get("bucket").(string)), + EncryptionConfiguration: expandAthenaResultConfigurationEncryptionConfig(d.Get("encryption_configuration").([]interface{})), + } + + if v, ok := d.GetOk("expected_bucket_owner"); ok && v.(string) != "" { + resultConfig.ExpectedBucketOwner = aws.String(v.(string)) } - if len(encryptionConfigurationList) <= 0 { - return &resultConfig + if v, ok := d.GetOk("acl_configuration"); ok && len(v.([]interface{})) > 0 { + resultConfig.AclConfiguration = expandAthenaResultConfigurationAclConfig(v.([]interface{})) } - data := encryptionConfigurationList[0].(map[string]interface{}) - keyType := data["encryption_option"].(string) - keyID := data["kms_key"].(string) + return resultConfig +} - encryptionConfig := athena.EncryptionConfiguration{ - EncryptionOption: aws.String(keyType), +func expandAthenaResultConfigurationEncryptionConfig(config []interface{}) *athena.EncryptionConfiguration { + if len(config) <= 0 { + return nil } - if len(keyID) > 0 { - encryptionConfig.KmsKey = aws.String(keyID) + data := config[0].(map[string]interface{}) + + encryptionConfig := &athena.EncryptionConfiguration{ + EncryptionOption: aws.String(data["encryption_option"].(string)), } - resultConfig.EncryptionConfiguration = &encryptionConfig + if v, ok := data["kms_key"].(string); ok && v != "" { + encryptionConfig.KmsKey = aws.String(v) + } - return &resultConfig + return encryptionConfig +} + +func expandAthenaResultConfigurationAclConfig(config []interface{}) *athena.AclConfiguration { + if len(config) <= 0 { + return nil + } + + data := config[0].(map[string]interface{}) + + encryptionConfig := &athena.AclConfiguration{ + S3AclOption: aws.String(data["s3_acl_option"].(string)), + } + + return encryptionConfig } func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { @@ -95,7 +141,7 @@ func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { input := &athena.StartQueryExecutionInput{ QueryString: aws.String(fmt.Sprintf("create database `%s`;", d.Get("name").(string))), - ResultConfiguration: expandAthenaResultConfiguration(d.Get("bucket").(string), d.Get("encryption_configuration").([]interface{})), + ResultConfiguration: expandAthenaResultConfiguration(d), } resp, err := conn.StartQueryExecution(input) @@ -103,7 +149,7 @@ func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { return err } - if err := executeAndExpectNoRowsWhenCreate(*resp.QueryExecutionId, conn); err != nil { + if err := executeAndExpectNoRows(*resp.QueryExecutionId, "create", conn); err != nil { return err } d.SetId(d.Get("name").(string)) @@ -114,13 +160,25 @@ func resourceDatabaseRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn input := &athena.GetDatabaseInput{ - DatabaseName: aws.String(d.Get("name").(string)), + DatabaseName: aws.String(d.Id()), CatalogName: aws.String("AwsDataCatalog"), } - _, err := conn.GetDatabase(input) + res, err := conn.GetDatabase(input) + + if tfawserr.ErrMessageContains(err, athena.ErrCodeMetadataException, "not found") && !d.IsNewResource() { + log.Printf("[WARN] Athena Database (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { - return err + return fmt.Errorf("error reading Athena Database (%s): %w", d.Id(), err) } + + db := res.Database + + d.Set("name", db.Name) + return nil } @@ -141,7 +199,7 @@ func resourceDatabaseDelete(d *schema.ResourceData, meta interface{}) error { input := &athena.StartQueryExecutionInput{ QueryString: aws.String(queryString), - ResultConfiguration: expandAthenaResultConfiguration(d.Get("bucket").(string), d.Get("encryption_configuration").([]interface{})), + ResultConfiguration: expandAthenaResultConfiguration(d), } resp, err := conn.StartQueryExecution(input) @@ -149,30 +207,20 @@ func resourceDatabaseDelete(d *schema.ResourceData, meta interface{}) error { return err } - if err := executeAndExpectNoRowsWhenDrop(*resp.QueryExecutionId, conn); err != nil { + if err := executeAndExpectNoRows(*resp.QueryExecutionId, "delete", conn); err != nil { return err } - return nil -} -func executeAndExpectNoRowsWhenCreate(qeid string, conn *athena.Athena) error { - rs, err := QueryExecutionResult(qeid, conn) - if err != nil { - return err - } - if len(rs.Rows) != 0 { - return fmt.Errorf("Athena create database, unexpected query result: %s", flattenAthenaResultSet(rs)) - } return nil } -func executeAndExpectNoRowsWhenDrop(qeid string, conn *athena.Athena) error { +func executeAndExpectNoRows(qeid, action string, conn *athena.Athena) error { rs, err := QueryExecutionResult(qeid, conn) if err != nil { return err } if len(rs.Rows) != 0 { - return fmt.Errorf("Athena drop database, unexpected query result: %s", flattenAthenaResultSet(rs)) + return fmt.Errorf("Athena %s database, unexpected query result: %s", action, flattenAthenaResultSet(rs)) } return nil } diff --git a/internal/service/athena/database_test.go b/internal/service/athena/database_test.go index 3c4646d046a..0deae0746e9 100644 --- a/internal/service/athena/database_test.go +++ b/internal/service/athena/database_test.go @@ -7,7 +7,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/athena" - "github.com/aws/aws-sdk-go/service/s3" 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" @@ -19,6 +18,7 @@ import ( func TestAccAthenaDatabase_basic(t *testing.T) { rInt := sdkacctest.RandInt() dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -28,7 +28,34 @@ func TestAccAthenaDatabase_basic(t *testing.T) { { Config: testAccAthenaDatabaseConfig(rInt, dbName, false), Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", dbName), + resource.TestCheckResourceAttrPair(resourceName, "bucket", "aws_s3_bucket.test", "bucket"), + resource.TestCheckResourceAttr(resourceName, "acl_configuration.#", "0"), + resource.TestCheckResourceAttr(resourceName, "encryption_configuration.#", "0"), + ), + }, + }, + }) +} + +func TestAccAthenaDatabase_acl(t *testing.T) { + rInt := sdkacctest.RandInt() + dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseAclConfig(rInt, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", dbName), + resource.TestCheckResourceAttr(resourceName, "acl_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "acl_configuration.0.s3_acl_option", "BUCKET_OWNER_FULL_CONTROL"), ), }, }, @@ -38,6 +65,8 @@ func TestAccAthenaDatabase_basic(t *testing.T) { func TestAccAthenaDatabase_encryption(t *testing.T) { rInt := sdkacctest.RandInt() dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -47,8 +76,10 @@ func TestAccAthenaDatabase_encryption(t *testing.T) { { Config: testAccAthenaDatabaseWithKMSConfig(rInt, dbName, false), Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), - resource.TestCheckResourceAttr("aws_athena_database.hoge", "encryption_configuration.0.encryption_option", "SSE_KMS"), + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "encryption_configuration.#", "1"), + resource.TestCheckResourceAttr(resourceName, "encryption_configuration.0.encryption_option", "SSE_KMS"), + resource.TestCheckResourceAttrPair(resourceName, "encryption_configuration.0.kms_key", "aws_kms_key.test", "arn"), ), }, }, @@ -58,6 +89,8 @@ func TestAccAthenaDatabase_encryption(t *testing.T) { func TestAccAthenaDatabase_nameStartsWithUnderscore(t *testing.T) { rInt := sdkacctest.RandInt() dbName := "_" + sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -67,8 +100,8 @@ func TestAccAthenaDatabase_nameStartsWithUnderscore(t *testing.T) { { Config: testAccAthenaDatabaseConfig(rInt, dbName, false), Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), - resource.TestCheckResourceAttr("aws_athena_database.hoge", "name", dbName), + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", dbName), ), }, }, @@ -104,7 +137,7 @@ func TestAccAthenaDatabase_destroyFailsIfTablesExist(t *testing.T) { { Config: testAccAthenaDatabaseConfig(rInt, dbName, false), Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), + testAccCheckDatabaseExists("aws_athena_database.test"), testAccDatabaseCreateTables(dbName), testAccCheckDatabaseDropFails(dbName), testAccDatabaseDestroyTables(dbName), @@ -126,7 +159,7 @@ func TestAccAthenaDatabase_forceDestroyAlwaysSucceeds(t *testing.T) { { Config: testAccAthenaDatabaseConfig(rInt, dbName, true), Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), + testAccCheckDatabaseExists("aws_athena_database.test"), testAccDatabaseCreateTables(dbName), ), }, @@ -134,88 +167,54 @@ func TestAccAthenaDatabase_forceDestroyAlwaysSucceeds(t *testing.T) { }) } -// StartQueryExecution requires OutputLocation but terraform destroy deleted S3 bucket as well. -// So temporary S3 bucket as OutputLocation is created to confirm whether the database is actually deleted. +func TestAccAthenaDatabase_disppears(t *testing.T) { + rInt := sdkacctest.RandInt() + dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists(resourceName), + acctest.CheckResourceDisappears(acctest.Provider, tfathena.ResourceDatabase(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func testAccCheckDatabaseDestroy(s *terraform.State) error { - athenaconn := acctest.Provider.Meta().(*conns.AWSClient).AthenaConn - s3conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn + conn := acctest.Provider.Meta().(*conns.AWSClient).AthenaConn for _, rs := range s.RootModule().Resources { if rs.Type != "aws_athena_database" { continue } - rInt := sdkacctest.RandInt() - bucketName := fmt.Sprintf("tf-test-athena-db-%d", rInt) - _, err := s3conn.CreateBucket(&s3.CreateBucketInput{ - Bucket: aws.String(bucketName), - }) - if err != nil { - return err - } - - input := &athena.StartQueryExecutionInput{ - QueryString: aws.String("show databases;"), - ResultConfiguration: &athena.ResultConfiguration{ - OutputLocation: aws.String("s3://" + bucketName), - }, + input := &athena.ListDatabasesInput{ + CatalogName: aws.String("AwsDataCatalog"), } - resp, err := athenaconn.StartQueryExecution(input) + res, err := conn.ListDatabases(input) if err != nil { return err } - ers, err := tfathena.QueryExecutionResult(*resp.QueryExecutionId, athenaconn) - if err != nil { - return err - } - found := false - dbName := rs.Primary.Attributes["name"] - for _, row := range ers.Rows { - for _, datum := range row.Data { - if *datum.VarCharValue == dbName { - found = true - } + var database *athena.Database + for _, db := range res.DatabaseList { + if aws.StringValue(db.Name) == rs.Primary.ID { + database = db + break } } - if found { - return fmt.Errorf("[DELETE ERROR] Athena failed to drop database: %s", dbName) - } - loresp, err := s3conn.ListObjectsV2( - &s3.ListObjectsV2Input{ - Bucket: aws.String(bucketName), - }, - ) - if err != nil { - return fmt.Errorf("[DELETE ERROR] S3 Bucket list Objects err: %s", err) - } - - objectsToDelete := make([]*s3.ObjectIdentifier, 0) - - if len(loresp.Contents) != 0 { - for _, v := range loresp.Contents { - objectsToDelete = append(objectsToDelete, &s3.ObjectIdentifier{ - Key: v.Key, - }) - } - } - - _, err = s3conn.DeleteObjects(&s3.DeleteObjectsInput{ - Bucket: aws.String(bucketName), - Delete: &s3.Delete{ - Objects: objectsToDelete, - }, - }) - if err != nil { - return fmt.Errorf("[DELETE ERROR] S3 Bucket delete Objects err: %s", err) - } - - _, err = s3conn.DeleteBucket(&s3.DeleteBucketInput{ - Bucket: aws.String(bucketName), - }) - if err != nil { - return fmt.Errorf("[DELETE ERROR] S3 Bucket delete Bucket err: %s", err) + if database != nil { + return fmt.Errorf("Athena database (%s) still exists", rs.Primary.ID) } } @@ -341,52 +340,71 @@ func testAccAthenaDatabaseFindBucketName(s *terraform.State, dbName string) (buc func testAccAthenaDatabaseConfig(randInt int, dbName string, forceDestroy bool) string { return fmt.Sprintf(` -resource "aws_s3_bucket" "hoge" { +resource "aws_s3_bucket" "test" { + bucket = "tf-test-athena-db-%[1]d" + force_destroy = true +} + +resource "aws_athena_database" "test" { + name = "%[2]s" + bucket = aws_s3_bucket.test.bucket + force_destroy = %[3]t +} +`, randInt, dbName, forceDestroy) +} + +func testAccAthenaDatabaseAclConfig(randInt int, dbName string, forceDestroy bool) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { bucket = "tf-test-athena-db-%[1]d" force_destroy = true } -resource "aws_athena_database" "hoge" { +resource "aws_athena_database" "test" { name = "%[2]s" - bucket = aws_s3_bucket.hoge.bucket + bucket = aws_s3_bucket.test.bucket force_destroy = %[3]t + + acl_configuration { + s3_acl_option = "BUCKET_OWNER_FULL_CONTROL" + } } `, randInt, dbName, forceDestroy) } func testAccAthenaDatabaseWithKMSConfig(randInt int, dbName string, forceDestroy bool) string { return fmt.Sprintf(` -resource "aws_kms_key" "hoge" { +resource "aws_kms_key" "test" { deletion_window_in_days = 10 } -resource "aws_s3_bucket" "hoge" { +resource "aws_s3_bucket" "test" { bucket = "tf-test-athena-db-%[1]d" force_destroy = true } resource "aws_s3_bucket_server_side_encryption_configuration" "test" { - bucket = aws_s3_bucket.hoge.id + bucket = aws_s3_bucket.test.id rule { apply_server_side_encryption_by_default { - kms_master_key_id = aws_kms_key.hoge.arn + kms_master_key_id = aws_kms_key.test.arn sse_algorithm = "aws:kms" } } } -resource "aws_athena_database" "hoge" { +resource "aws_athena_database" "test" { # Must have bucket SSE enabled first depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] name = "%[2]s" - bucket = aws_s3_bucket.hoge.bucket + bucket = aws_s3_bucket.test.bucket force_destroy = %[3]t encryption_configuration { encryption_option = "SSE_KMS" - kms_key = aws_kms_key.hoge.arn + kms_key = aws_kms_key.test.arn } } `, randInt, dbName, forceDestroy) From d837b153dd08bb225126ad74f6d2bbd8c6077cd9 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 18 Mar 2022 11:19:55 +0200 Subject: [PATCH 07/19] docs --- website/docs/r/athena_database.html.markdown | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/website/docs/r/athena_database.html.markdown b/website/docs/r/athena_database.html.markdown index 53ee0b6d92b..adadd83dd35 100644 --- a/website/docs/r/athena_database.html.markdown +++ b/website/docs/r/athena_database.html.markdown @@ -13,13 +13,13 @@ Provides an Athena database. ## Example Usage ```terraform -resource "aws_s3_bucket" "hoge" { - bucket = "hoge" +resource "aws_s3_bucket" "example" { + bucket = "example" } -resource "aws_athena_database" "hoge" { +resource "aws_athena_database" "example" { name = "database_name" - bucket = aws_s3_bucket.hoge.bucket + bucket = aws_s3_bucket.example.bucket } ``` @@ -29,14 +29,20 @@ The following arguments are supported: * `name` - (Required) Name of the database to create. * `bucket` - (Required) Name of s3 bucket to save the results of the query execution. -* `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. An `encryption_configuration` block is documented below. +* `expected_bucket_owner` - (Optional) The AWS account ID that you expect to be the owner of the Amazon S3 bucket. +* `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. See [Encryption Configuration](#encryption-configuration) Below. +* `acl_configuration` - (Optional) Indicates that an Amazon S3 canned ACL should be set to control ownership of stored query results. See [ACL Configuration](#acl-configuration) Below. * `force_destroy` - (Optional, Default: false) A boolean that indicates all tables should be deleted from the database so that the database can be destroyed without error. The tables are *not* recoverable. -An `encryption_configuration` block supports the following arguments: +### Encryption Configuration * `encryption_option` - (Required) The type of key; one of `SSE_S3`, `SSE_KMS`, `CSE_KMS` * `kms_key` - (Optional) The KMS key ARN or ID; required for key types `SSE_KMS` and `CSE_KMS`. +### ACL Configuration + +* `s3_acl_option` - (Required) The Amazon S3 canned ACL that Athena should specify when storing query results. Valid value is `BUCKET_OWNER_FULL_CONTROL`. + ~> **NOTE:** When Athena queries are executed, result files may be created in the specified bucket. Consider using `force_destroy` on the bucket too in order to avoid any problems when destroying the bucket. ## Attributes Reference From a87a3ef5eee98156bc6f3cdc62c16b3e1bd7aa8f Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 18 Mar 2022 11:22:30 +0200 Subject: [PATCH 08/19] changelog --- .changelog/23745.txt | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changelog/23745.txt diff --git a/.changelog/23745.txt b/.changelog/23745.txt new file mode 100644 index 00000000000..0c83cf82776 --- /dev/null +++ b/.changelog/23745.txt @@ -0,0 +1,7 @@ +```release-note:enhancement +resource/aws_athena_database: Add `acl_configuration` and `expected_bucket_owner` args. +``` + +```release-note:enhancement +resource/aws_athena_database: Remove from state on read if deleted outside of terraform. +``` \ No newline at end of file From 472de456e245f4b911838cebd3b1d48223288faf Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 18 Mar 2022 11:23:18 +0200 Subject: [PATCH 09/19] fmt --- internal/service/athena/database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 448459e3f51..564a12ea440 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -93,7 +93,7 @@ func expandAthenaResultConfiguration(d *schema.ResourceData) *athena.ResultConfi EncryptionConfiguration: expandAthenaResultConfigurationEncryptionConfig(d.Get("encryption_configuration").([]interface{})), } - if v, ok := d.GetOk("expected_bucket_owner"); ok && v.(string) != "" { + if v, ok := d.GetOk("expected_bucket_owner"); ok { resultConfig.ExpectedBucketOwner = aws.String(v.(string)) } From a5bfcd233bb16e8d9535a6e941801aaee9cdf36a Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Fri, 18 Mar 2022 12:00:58 +0200 Subject: [PATCH 10/19] sweeper --- internal/service/athena/database.go | 4 +- internal/service/athena/sweep.go | 78 +++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 internal/service/athena/sweep.go diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 564a12ea440..4d65f3d0a55 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -189,9 +189,7 @@ func resourceDatabaseUpdate(d *schema.ResourceData, meta interface{}) error { func resourceDatabaseDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn - name := d.Get("name").(string) - - queryString := fmt.Sprintf("drop database `%s`", name) + queryString := fmt.Sprintf("drop database `%s`", d.Id()) if d.Get("force_destroy").(bool) { queryString += " cascade" } diff --git a/internal/service/athena/sweep.go b/internal/service/athena/sweep.go new file mode 100644 index 00000000000..fd250b80307 --- /dev/null +++ b/internal/service/athena/sweep.go @@ -0,0 +1,78 @@ +//go:build sweep +// +build sweep + +package athena + +import ( + "fmt" + "log" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/athena" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/sweep" +) + +func init() { + resource.AddTestSweepers("aws_athena_database", &resource.Sweeper{ + Name: "aws_athena_database", + F: sweepDatabases, + }) +} + +func sweepDatabases(region string) error { + client, err := sweep.SharedRegionalSweepClient(region) + if err != nil { + return fmt.Errorf("error getting client: %s", err) + } + conn := client.(*conns.AWSClient).AthenaConn + input := &athena.ListDatabasesInput{ + CatalogName: aws.String("AwsDataCatalog"), + } + var errs *multierror.Error + + sweepResources := make([]*sweep.SweepResource, 0) + for { + output, err := conn.ListDatabases(input) + + for _, v := range output.DatabaseList { + name := aws.StringValue(v.Name) + if name == "default" { + continue + } + r := ResourceDatabase() + d := r.Data(nil) + d.SetId(name) + + if err != nil { + err := fmt.Errorf("error listing Athena Databases (%s): %w", name, err) + log.Printf("[ERROR] %s", err) + errs = multierror.Append(errs, err) + continue + } + + sweepResources = append(sweepResources, sweep.NewSweepResource(r, d, client)) + } + + if aws.StringValue(output.NextToken) == "" { + break + } + + input.NextToken = output.NextToken + } + + if sweep.SkipSweepError(err) { + log.Printf("[WARN] Skipping Athena Database sweep for %s: %s", region, err) + return nil + } + + err = sweep.SweepOrchestrator(sweepResources) + + if err != nil { + errs = multierror.Append(errs, fmt.Errorf("error sweeping Athena Databases (%s): %w", region, err)) + } + + return errs.ErrorOrNil() +} From 6d9c41bd3024cf1ab65ae40d1c9fef8c157d6b0e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:49:06 -0400 Subject: [PATCH 11/19] r/aws_athena_database: Alphabetize attributes. --- internal/service/athena/database.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 4d65f3d0a55..8ef0506c89a 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -40,6 +40,11 @@ func ResourceDatabase() *schema.Resource { }, }, }, + "bucket": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, "encryption_configuration": { Type: schema.TypeList, Optional: true, @@ -47,17 +52,17 @@ func ResourceDatabase() *schema.Resource { MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ - "kms_key": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - }, "encryption_option": { Type: schema.TypeString, Required: true, ValidateFunc: validation.StringInSlice(athena.EncryptionOption_Values(), false), ForceNew: true, }, + "kms_key": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, }, }, }, @@ -66,11 +71,6 @@ func ResourceDatabase() *schema.Resource { Optional: true, ForceNew: true, }, - "bucket": { - Type: schema.TypeString, - Optional: true, - ForceNew: true, - }, "force_destroy": { Type: schema.TypeBool, Optional: true, From ed2a57f05b42c13ff687770ca3912c2cc99ece8b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:50:52 -0400 Subject: [PATCH 12/19] r/aws_athena_database: Move CRUD methods up. --- internal/service/athena/database.go | 100 ++++++++++++++-------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 8ef0506c89a..ffe42d04fb5 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -86,56 +86,6 @@ func ResourceDatabase() *schema.Resource { } } -func expandAthenaResultConfiguration(d *schema.ResourceData) *athena.ResultConfiguration { - - resultConfig := &athena.ResultConfiguration{ - OutputLocation: aws.String("s3://" + d.Get("bucket").(string)), - EncryptionConfiguration: expandAthenaResultConfigurationEncryptionConfig(d.Get("encryption_configuration").([]interface{})), - } - - if v, ok := d.GetOk("expected_bucket_owner"); ok { - resultConfig.ExpectedBucketOwner = aws.String(v.(string)) - } - - if v, ok := d.GetOk("acl_configuration"); ok && len(v.([]interface{})) > 0 { - resultConfig.AclConfiguration = expandAthenaResultConfigurationAclConfig(v.([]interface{})) - } - - return resultConfig -} - -func expandAthenaResultConfigurationEncryptionConfig(config []interface{}) *athena.EncryptionConfiguration { - if len(config) <= 0 { - return nil - } - - data := config[0].(map[string]interface{}) - - encryptionConfig := &athena.EncryptionConfiguration{ - EncryptionOption: aws.String(data["encryption_option"].(string)), - } - - if v, ok := data["kms_key"].(string); ok && v != "" { - encryptionConfig.KmsKey = aws.String(v) - } - - return encryptionConfig -} - -func expandAthenaResultConfigurationAclConfig(config []interface{}) *athena.AclConfiguration { - if len(config) <= 0 { - return nil - } - - data := config[0].(map[string]interface{}) - - encryptionConfig := &athena.AclConfiguration{ - S3AclOption: aws.String(data["s3_acl_option"].(string)), - } - - return encryptionConfig -} - func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn @@ -212,6 +162,56 @@ func resourceDatabaseDelete(d *schema.ResourceData, meta interface{}) error { return nil } +func expandAthenaResultConfiguration(d *schema.ResourceData) *athena.ResultConfiguration { + + resultConfig := &athena.ResultConfiguration{ + OutputLocation: aws.String("s3://" + d.Get("bucket").(string)), + EncryptionConfiguration: expandAthenaResultConfigurationEncryptionConfig(d.Get("encryption_configuration").([]interface{})), + } + + if v, ok := d.GetOk("expected_bucket_owner"); ok { + resultConfig.ExpectedBucketOwner = aws.String(v.(string)) + } + + if v, ok := d.GetOk("acl_configuration"); ok && len(v.([]interface{})) > 0 { + resultConfig.AclConfiguration = expandAthenaResultConfigurationAclConfig(v.([]interface{})) + } + + return resultConfig +} + +func expandAthenaResultConfigurationEncryptionConfig(config []interface{}) *athena.EncryptionConfiguration { + if len(config) <= 0 { + return nil + } + + data := config[0].(map[string]interface{}) + + encryptionConfig := &athena.EncryptionConfiguration{ + EncryptionOption: aws.String(data["encryption_option"].(string)), + } + + if v, ok := data["kms_key"].(string); ok && v != "" { + encryptionConfig.KmsKey = aws.String(v) + } + + return encryptionConfig +} + +func expandAthenaResultConfigurationAclConfig(config []interface{}) *athena.AclConfiguration { + if len(config) <= 0 { + return nil + } + + data := config[0].(map[string]interface{}) + + encryptionConfig := &athena.AclConfiguration{ + S3AclOption: aws.String(data["s3_acl_option"].(string)), + } + + return encryptionConfig +} + func executeAndExpectNoRows(qeid, action string, conn *athena.Athena) error { rs, err := QueryExecutionResult(qeid, conn) if err != nil { From 5576d07673efc772469d632a539949cd2573786b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:54:42 -0400 Subject: [PATCH 13/19] Revert "Add changelog entry" This reverts commit 07d494e1ca4ec46152dbb36b3eabb51b012f06ea. --- .changelog/21491.txt | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .changelog/21491.txt diff --git a/.changelog/21491.txt b/.changelog/21491.txt deleted file mode 100644 index 112ea1c70a4..00000000000 --- a/.changelog/21491.txt +++ /dev/null @@ -1,3 +0,0 @@ -```release-note:bug -resource/aws_athena_database: Remove ForceNew from bucket field. -``` From 45ea71f6c870ee599d9f790ece3e44a8fe58e66a Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:54:52 -0400 Subject: [PATCH 14/19] Revert "r/aws_athena_database: remove ForceNew from bucket field" This reverts commit fda82079e6e12b43fb2e3fd609467d618ba5d728. --- internal/service/athena/database.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 455120b20c4..4827d58866a 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -31,6 +31,7 @@ func ResourceDatabase() *schema.Resource { "bucket": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "force_destroy": { Type: schema.TypeBool, From 4932c85c08e67925225621d913d528224d231f08 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:56:55 -0400 Subject: [PATCH 15/19] Revert "docs: add documentation for comment property" This reverts commit 09afa0eb15ba92ee2359638c8a511f6d16b374c6. --- website/docs/r/athena_database.html.markdown | 1 - 1 file changed, 1 deletion(-) diff --git a/website/docs/r/athena_database.html.markdown b/website/docs/r/athena_database.html.markdown index 917f834a136..53ee0b6d92b 100644 --- a/website/docs/r/athena_database.html.markdown +++ b/website/docs/r/athena_database.html.markdown @@ -29,7 +29,6 @@ The following arguments are supported: * `name` - (Required) Name of the database to create. * `bucket` - (Required) Name of s3 bucket to save the results of the query execution. -* `comment` - (Optional) Description of the database. * `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. An `encryption_configuration` block is documented below. * `force_destroy` - (Optional, Default: false) A boolean that indicates all tables should be deleted from the database so that the database can be destroyed without error. The tables are *not* recoverable. From 43a1082630ff0fd30a9b9b5f6181e7a67f10b013 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:57:06 -0400 Subject: [PATCH 16/19] Revert "test: add cover test for comment property" This reverts commit f6c364d6014d965ab238e7ee1bb21525cb8f47bf. --- internal/service/athena/database_test.go | 72 ------------------------ 1 file changed, 72 deletions(-) diff --git a/internal/service/athena/database_test.go b/internal/service/athena/database_test.go index 943807129f4..3d96f064e3a 100644 --- a/internal/service/athena/database_test.go +++ b/internal/service/athena/database_test.go @@ -35,46 +35,6 @@ func TestAccAthenaDatabase_basic(t *testing.T) { }) } -func TestAccAthenaDatabase_description(t *testing.T) { - rInt := sdkacctest.RandInt() - dbName := sdkacctest.RandString(8) - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckDatabaseDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAthenaDatabaseCommentConfig(rInt, dbName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), - resource.TestCheckResourceAttr("aws_athena_database.hoge", "name", dbName), - ), - }, - }, - }) -} - -func TestAccAthenaDatabase_unescaped_description(t *testing.T) { - rInt := sdkacctest.RandInt() - dbName := sdkacctest.RandString(8) - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckDatabaseDestroy, - Steps: []resource.TestStep{ - { - Config: testAccAthenaDatabaseUnescapedCommentConfig(rInt, dbName, false), - Check: resource.ComposeTestCheckFunc( - testAccCheckDatabaseExists("aws_athena_database.hoge"), - resource.TestCheckResourceAttr("aws_athena_database.hoge", "name", dbName), - ), - }, - }, - }) -} - func TestAccAthenaDatabase_encryption(t *testing.T) { rInt := sdkacctest.RandInt() dbName := sdkacctest.RandString(8) @@ -394,38 +354,6 @@ resource "aws_athena_database" "hoge" { `, randInt, dbName, forceDestroy) } -func testAccAthenaDatabaseCommentConfig(randInt int, dbName string, forceDestroy bool) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "hoge" { - bucket = "tf-test-athena-db-%[1]d" - force_destroy = true -} - -resource "aws_athena_database" "hoge" { - name = "%[2]s" - bucket = aws_s3_bucket.hoge.bucket - comment = "athena is a goddess" - force_destroy = %[3]t -} -`, randInt, dbName, forceDestroy) -} - -func testAccAthenaDatabaseUnescapedCommentConfig(randInt int, dbName string, forceDestroy bool) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "hoge" { - bucket = "tf-test-athena-db-%[1]d" - force_destroy = true -} - -resource "aws_athena_database" "hoge" { - name = "%[2]s" - bucket = aws_s3_bucket.hoge.bucket - comment = "athena's a goddess" - force_destroy = %[3]t -} -`, randInt, dbName, forceDestroy) -} - func testAccAthenaDatabaseWithKMSConfig(randInt int, dbName string, forceDestroy bool) string { return fmt.Sprintf(` resource "aws_kms_key" "hoge" { From b3dd5f61bf64d53301d8c5b57296468d7dbee18c Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 08:57:25 -0400 Subject: [PATCH 17/19] Revert "feat: support comment property for athena databases" This reverts commit 02adf61f5e2888a507fb313a4fe189e7694e6d68. --- internal/service/athena/database.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index 9845bd9a265..4827d58866a 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -33,10 +33,6 @@ func ResourceDatabase() *schema.Resource { Required: true, ForceNew: true, }, - "comment": { - Type: schema.TypeString, - Optional: true, - }, "force_destroy": { Type: schema.TypeBool, Optional: true, @@ -97,15 +93,8 @@ func expandAthenaResultConfiguration(bucket string, encryptionConfigurationList func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn - var databaseDescription string - if v, ok := d.GetOk("comment"); ok { - databaseDescription = strings.Replace(v.(string), "'", "\\'", -1) - } else { - databaseDescription = "" - } - input := &athena.StartQueryExecutionInput{ - QueryString: aws.String(fmt.Sprintf("create database `%[1]s` comment '%[2]s';", d.Get("name").(string), databaseDescription)), + QueryString: aws.String(fmt.Sprintf("create database `%s`;", d.Get("name").(string))), ResultConfiguration: expandAthenaResultConfiguration(d.Get("bucket").(string), d.Get("encryption_configuration").([]interface{})), } From 83df268c75c8e2afb6d9c8aebdbb906748cfc626 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 09:04:57 -0400 Subject: [PATCH 18/19] r/aws_athena_database: Remove ForceNew from 'bucket'. --- .changelog/23745.txt | 8 ++++++-- internal/service/athena/database.go | 7 +------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.changelog/23745.txt b/.changelog/23745.txt index 0c83cf82776..7e74e9e4a75 100644 --- a/.changelog/23745.txt +++ b/.changelog/23745.txt @@ -1,7 +1,11 @@ ```release-note:enhancement -resource/aws_athena_database: Add `acl_configuration` and `expected_bucket_owner` args. +resource/aws_athena_database: Add `acl_configuration` and `expected_bucket_owner` arguments +``` + +```release-note:bug +resource/aws_athena_database: Remove from state on resource Read if deleted outside of Terraform ``` ```release-note:enhancement -resource/aws_athena_database: Remove from state on read if deleted outside of terraform. +resource/aws_athena_database: Do not recreate the resource if `bucket` changes ``` \ No newline at end of file diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index ffe42d04fb5..f05c90421e3 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -20,7 +20,7 @@ func ResourceDatabase() *schema.Resource { return &schema.Resource{ Create: resourceDatabaseCreate, Read: resourceDatabaseRead, - Update: resourceDatabaseUpdate, + Update: schema.Noop, Delete: resourceDatabaseDelete, Schema: map[string]*schema.Schema{ @@ -43,7 +43,6 @@ func ResourceDatabase() *schema.Resource { "bucket": { Type: schema.TypeString, Optional: true, - ForceNew: true, }, "encryption_configuration": { Type: schema.TypeList, @@ -132,10 +131,6 @@ func resourceDatabaseRead(d *schema.ResourceData, meta interface{}) error { return nil } -func resourceDatabaseUpdate(d *schema.ResourceData, meta interface{}) error { - return resourceDatabaseRead(d, meta) -} - func resourceDatabaseDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn From 9fd4b4d5297023d1cc401af5dab38c253df3ecda Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Fri, 18 Mar 2022 09:23:13 -0400 Subject: [PATCH 19/19] r/aws_athena_database: Add 'comment' argument. --- .changelog/23745.txt | 4 + internal/service/athena/database.go | 23 ++- internal/service/athena/database_test.go | 139 +++++++++++++++---- website/docs/r/athena_database.html.markdown | 17 +-- 4 files changed, 144 insertions(+), 39 deletions(-) diff --git a/.changelog/23745.txt b/.changelog/23745.txt index 7e74e9e4a75..9993b30dc29 100644 --- a/.changelog/23745.txt +++ b/.changelog/23745.txt @@ -8,4 +8,8 @@ resource/aws_athena_database: Remove from state on resource Read if deleted outs ```release-note:enhancement resource/aws_athena_database: Do not recreate the resource if `bucket` changes +``` + +```release-note:enhancement +resource/aws_athena_database: Add `comment` argument to support database descriptions ``` \ No newline at end of file diff --git a/internal/service/athena/database.go b/internal/service/athena/database.go index f05c90421e3..079a03ce2ae 100644 --- a/internal/service/athena/database.go +++ b/internal/service/athena/database.go @@ -44,6 +44,11 @@ func ResourceDatabase() *schema.Resource { Type: schema.TypeString, Optional: true, }, + "comment": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + }, "encryption_configuration": { Type: schema.TypeList, Optional: true, @@ -88,20 +93,32 @@ func ResourceDatabase() *schema.Resource { func resourceDatabaseCreate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).AthenaConn + name := d.Get("name").(string) + var queryString string + + if v, ok := d.GetOk("comment"); ok { + queryString = fmt.Sprintf("create database `%[1]s` comment '%[2]s';", name, strings.Replace(v.(string), "'", "\\'", -1)) + } else { + queryString = fmt.Sprintf("create database `%[1]s`;", name) + } + input := &athena.StartQueryExecutionInput{ - QueryString: aws.String(fmt.Sprintf("create database `%s`;", d.Get("name").(string))), + QueryString: aws.String(queryString), ResultConfiguration: expandAthenaResultConfiguration(d), } resp, err := conn.StartQueryExecution(input) + if err != nil { - return err + return fmt.Errorf("error starting Athena Database (%s) query execution: %w", name, err) } if err := executeAndExpectNoRows(*resp.QueryExecutionId, "create", conn); err != nil { return err } - d.SetId(d.Get("name").(string)) + + d.SetId(name) + return resourceDatabaseRead(d, meta) } diff --git a/internal/service/athena/database_test.go b/internal/service/athena/database_test.go index 0deae0746e9..639534f7372 100644 --- a/internal/service/athena/database_test.go +++ b/internal/service/athena/database_test.go @@ -16,9 +16,10 @@ import ( ) func TestAccAthenaDatabase_basic(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -26,7 +27,7 @@ func TestAccAthenaDatabase_basic(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", dbName), @@ -40,9 +41,10 @@ func TestAccAthenaDatabase_basic(t *testing.T) { } func TestAccAthenaDatabase_acl(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) resourceName := "aws_athena_database.test" + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -50,7 +52,7 @@ func TestAccAthenaDatabase_acl(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseAclConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseAclConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", dbName), @@ -63,7 +65,7 @@ func TestAccAthenaDatabase_acl(t *testing.T) { } func TestAccAthenaDatabase_encryption(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) resourceName := "aws_athena_database.test" @@ -74,7 +76,7 @@ func TestAccAthenaDatabase_encryption(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseWithKMSConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseWithKMSConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists(resourceName), resource.TestCheckResourceAttr(resourceName, "encryption_configuration.#", "1"), @@ -87,7 +89,7 @@ func TestAccAthenaDatabase_encryption(t *testing.T) { } func TestAccAthenaDatabase_nameStartsWithUnderscore(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := "_" + sdkacctest.RandString(8) resourceName := "aws_athena_database.test" @@ -98,7 +100,7 @@ func TestAccAthenaDatabase_nameStartsWithUnderscore(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", dbName), @@ -109,8 +111,9 @@ func TestAccAthenaDatabase_nameStartsWithUnderscore(t *testing.T) { } func TestAccAthenaDatabase_nameCantHaveUppercase(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := "A" + sdkacctest.RandString(8) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -118,7 +121,7 @@ func TestAccAthenaDatabase_nameCantHaveUppercase(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseConfig(rName, dbName, false), ExpectError: regexp.MustCompile(`must be lowercase letters, numbers, or underscore \('_'\)`), }, }, @@ -126,8 +129,9 @@ func TestAccAthenaDatabase_nameCantHaveUppercase(t *testing.T) { } func TestAccAthenaDatabase_destroyFailsIfTablesExist(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -135,7 +139,7 @@ func TestAccAthenaDatabase_destroyFailsIfTablesExist(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists("aws_athena_database.test"), testAccDatabaseCreateTables(dbName), @@ -148,8 +152,9 @@ func TestAccAthenaDatabase_destroyFailsIfTablesExist(t *testing.T) { } func TestAccAthenaDatabase_forceDestroyAlwaysSucceeds(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), @@ -157,7 +162,7 @@ func TestAccAthenaDatabase_forceDestroyAlwaysSucceeds(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, true), + Config: testAccAthenaDatabaseConfig(rName, dbName, true), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists("aws_athena_database.test"), testAccDatabaseCreateTables(dbName), @@ -167,9 +172,54 @@ func TestAccAthenaDatabase_forceDestroyAlwaysSucceeds(t *testing.T) { }) } +func TestAccAthenaDatabase_description(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseCommentConfig(rName, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", dbName), + ), + }, + }, + }) +} + +func TestAccAthenaDatabase_unescaped_description(t *testing.T) { + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, athena.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckDatabaseDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAthenaDatabaseUnescapedCommentConfig(rName, dbName, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckDatabaseExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", dbName), + ), + }, + }, + }) +} + func TestAccAthenaDatabase_disppears(t *testing.T) { - rInt := sdkacctest.RandInt() + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) dbName := sdkacctest.RandString(8) + resourceName := "aws_athena_database.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(t) }, @@ -178,7 +228,7 @@ func TestAccAthenaDatabase_disppears(t *testing.T) { CheckDestroy: testAccCheckDatabaseDestroy, Steps: []resource.TestStep{ { - Config: testAccAthenaDatabaseConfig(rInt, dbName, false), + Config: testAccAthenaDatabaseConfig(rName, dbName, false), Check: resource.ComposeTestCheckFunc( testAccCheckDatabaseExists(resourceName), acctest.CheckResourceDisappears(acctest.Provider, tfathena.ResourceDatabase(), resourceName), @@ -338,30 +388,30 @@ func testAccAthenaDatabaseFindBucketName(s *terraform.State, dbName string) (buc return bucket, err } -func testAccAthenaDatabaseConfig(randInt int, dbName string, forceDestroy bool) string { +func testAccAthenaDatabaseConfig(rName string, dbName string, forceDestroy bool) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { - bucket = "tf-test-athena-db-%[1]d" + bucket = %[1]q force_destroy = true } resource "aws_athena_database" "test" { - name = "%[2]s" + name = %[2]q bucket = aws_s3_bucket.test.bucket force_destroy = %[3]t } -`, randInt, dbName, forceDestroy) +`, rName, dbName, forceDestroy) } -func testAccAthenaDatabaseAclConfig(randInt int, dbName string, forceDestroy bool) string { +func testAccAthenaDatabaseAclConfig(rName string, dbName string, forceDestroy bool) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { - bucket = "tf-test-athena-db-%[1]d" + bucket = %[1]q force_destroy = true } resource "aws_athena_database" "test" { - name = "%[2]s" + name = %[2]q bucket = aws_s3_bucket.test.bucket force_destroy = %[3]t @@ -369,17 +419,18 @@ resource "aws_athena_database" "test" { s3_acl_option = "BUCKET_OWNER_FULL_CONTROL" } } -`, randInt, dbName, forceDestroy) +`, rName, dbName, forceDestroy) } -func testAccAthenaDatabaseWithKMSConfig(randInt int, dbName string, forceDestroy bool) string { +func testAccAthenaDatabaseWithKMSConfig(rName string, dbName string, forceDestroy bool) string { return fmt.Sprintf(` resource "aws_kms_key" "test" { deletion_window_in_days = 10 + description = %[1]q } resource "aws_s3_bucket" "test" { - bucket = "tf-test-athena-db-%[1]d" + bucket = %[1]q force_destroy = true } @@ -398,7 +449,7 @@ resource "aws_athena_database" "test" { # Must have bucket SSE enabled first depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] - name = "%[2]s" + name = %[2]q bucket = aws_s3_bucket.test.bucket force_destroy = %[3]t @@ -407,5 +458,37 @@ resource "aws_athena_database" "test" { kms_key = aws_kms_key.test.arn } } -`, randInt, dbName, forceDestroy) +`, rName, dbName, forceDestroy) +} + +func testAccAthenaDatabaseCommentConfig(rName string, dbName string, forceDestroy bool) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + force_destroy = true +} + +resource "aws_athena_database" "test" { + name = %[2]q + bucket = aws_s3_bucket.test.bucket + comment = "athena is a goddess" + force_destroy = %[3]t +} +`, rName, dbName, forceDestroy) +} + +func testAccAthenaDatabaseUnescapedCommentConfig(rName string, dbName string, forceDestroy bool) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + force_destroy = true +} + +resource "aws_athena_database" "test" { + name = %[2]q + bucket = aws_s3_bucket.test.bucket + comment = "athena's a goddess" + force_destroy = %[3]t +} +`, rName, dbName, forceDestroy) } diff --git a/website/docs/r/athena_database.html.markdown b/website/docs/r/athena_database.html.markdown index adadd83dd35..0585ef7066f 100644 --- a/website/docs/r/athena_database.html.markdown +++ b/website/docs/r/athena_database.html.markdown @@ -27,24 +27,25 @@ resource "aws_athena_database" "example" { The following arguments are supported: +* `bucket` - (Required) Name of S3 bucket to save the results of the query execution. * `name` - (Required) Name of the database to create. -* `bucket` - (Required) Name of s3 bucket to save the results of the query execution. +* `acl_configuration` - (Optional) Indicates that an Amazon S3 canned ACL should be set to control ownership of stored query results. See [ACL Configuration](#acl-configuration) below. +* `comment` - (Optional) Description of the database. +* `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. See [Encryption Configuration](#encryption-configuration) below. * `expected_bucket_owner` - (Optional) The AWS account ID that you expect to be the owner of the Amazon S3 bucket. -* `encryption_configuration` - (Optional) The encryption key block AWS Athena uses to decrypt the data in S3, such as an AWS Key Management Service (AWS KMS) key. See [Encryption Configuration](#encryption-configuration) Below. -* `acl_configuration` - (Optional) Indicates that an Amazon S3 canned ACL should be set to control ownership of stored query results. See [ACL Configuration](#acl-configuration) Below. * `force_destroy` - (Optional, Default: false) A boolean that indicates all tables should be deleted from the database so that the database can be destroyed without error. The tables are *not* recoverable. -### Encryption Configuration - -* `encryption_option` - (Required) The type of key; one of `SSE_S3`, `SSE_KMS`, `CSE_KMS` -* `kms_key` - (Optional) The KMS key ARN or ID; required for key types `SSE_KMS` and `CSE_KMS`. - ### ACL Configuration * `s3_acl_option` - (Required) The Amazon S3 canned ACL that Athena should specify when storing query results. Valid value is `BUCKET_OWNER_FULL_CONTROL`. ~> **NOTE:** When Athena queries are executed, result files may be created in the specified bucket. Consider using `force_destroy` on the bucket too in order to avoid any problems when destroying the bucket. +### Encryption Configuration + +* `encryption_option` - (Required) The type of key; one of `SSE_S3`, `SSE_KMS`, `CSE_KMS` +* `kms_key` - (Optional) The KMS key ARN or ID; required for key types `SSE_KMS` and `CSE_KMS`. + ## Attributes Reference In addition to all arguments above, the following attributes are exported: