Skip to content

Commit

Permalink
Address review comments for aws_quicksight_user by
Browse files Browse the repository at this point in the history
 - Using go-sdk constants
 - Running, fixing and cleaning tests

Here's my current test output
``` bash
AWS_DEFAULT_REGION=us-east-1 AWS_PROFILE=default TF_ACC=1 go test ./aws -v -timeout 120m -parallel 20 -run='TestAccAWSQuickSightUser_'
```

> go: finding github.com/terraform-providers/terraform-provider-tls
> v2.1.1+incompatible
> go: finding github.com/terraform-providers/terraform-provider-tls
> v2.1.1+incompatible
> === RUN   TestAccAWSQuickSightUser_basic
> === PAUSE TestAccAWSQuickSightUser_basic
> === RUN   TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks
> === PAUSE TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks
> === RUN   TestAccAWSQuickSightUser_disappears
> === PAUSE TestAccAWSQuickSightUser_disappears
> === CONT  TestAccAWSQuickSightUser_basic
> === CONT  TestAccAWSQuickSightUser_disappears
> === CONT  TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks
> --- PASS: TestAccAWSQuickSightUser_disappears (14.19s)
> --- PASS: TestAccAWSQuickSightUser_basic (25.44s)
> --- PASS: TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks
> (26.08s)
> PASS
> ok  	github.com/terraform-providers/terraform-provider-aws/aws
> 26.109s

Note that the tests exposed that the `describe-user` QuickSight API does
not return the schema that is documented.

For example, when  I run it via the CLI I get

``` json
{
    "RequestId": "some-guid-looking-thing",
    "Status": 200,
    "User": {
        "Arn":
"arn:aws:quicksight:us-west-2:01234567890:user/default/some_user",
        "PrincipalId": "federated/iam/some_user",
        "Email": "[email protected]",
        "Active": true,
        "Role": "ADMIN",
        "UserName": "some_user"
    }
}
```

Notice that `IdentityType` is missing though documented!

This means that I had to remove it from attributes and remove the
importer :( since we can't reconcile that value.

If there's a way around this please let me know.

Also note that I couldn't use `acctest.RandomWithPrefix` because the
resultant username was not valid in QuickSight.
  • Loading branch information
mjgpy3 committed Oct 10, 2019
1 parent 3c20fce commit fd07518
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 58 deletions.
4 changes: 0 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ NOTES:

* provider: The underlying Terraform codebase dependency for the provider SDK and acceptance testing framework has been migrated from `github.com/hashicorp/terraform` to `github.com/hashicorp/terraform-plugin-sdk`. They are functionality equivalent and this should only impact codebase development to switch imports. For more information see the [Terraform Plugin SDK page in the Extending Terraform documentation](https://www.terraform.io/docs/extend/plugin-sdk.html). [GH-10367]

FEATURES:

* **New Resource:** `aws_quicksight_users`

BUG FIXES:

* resource/aws_s3_bucket: Prevent infinite deletion recursion with `force_destroy` argument and object keys with empty "directory" prefixes present since version 2.29.0 [GH-10388]
Expand Down
17 changes: 6 additions & 11 deletions aws/resource_aws_quicksight_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ func resourceAwsQuickSightUser() *schema.Resource {
Update: resourceAwsQuickSightUserUpdate,
Delete: resourceAwsQuickSightUserDelete,

Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"arn": {
Type: schema.TypeString,
Expand Down Expand Up @@ -53,8 +49,8 @@ func resourceAwsQuickSightUser() *schema.Resource {
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
"IAM",
"QUICKSIGHT",
quicksight.IdentityTypeIam,
quicksight.IdentityTypeQuicksight,
}, false),
},

Expand Down Expand Up @@ -85,9 +81,9 @@ func resourceAwsQuickSightUser() *schema.Resource {
Required: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{
"READER",
"AUTHOR",
"ADMIN",
quicksight.UserRoleReader,
quicksight.UserRoleAuthor,
quicksight.UserRoleAdmin,
}, false),
},
},
Expand Down Expand Up @@ -127,7 +123,7 @@ func resourceAwsQuickSightUserCreate(d *schema.ResourceData, meta interface{}) e

resp, err := conn.RegisterUser(createOpts)
if err != nil {
return fmt.Errorf("Error registering user in QuickSight Group: %s", err)
return fmt.Errorf("Error registering QuickSight user: %s", err)
}

d.SetId(fmt.Sprintf("%s/%s/%s", awsAccountID, namespace, aws.StringValue(resp.User.UserName)))
Expand Down Expand Up @@ -162,7 +158,6 @@ func resourceAwsQuickSightUserRead(d *schema.ResourceData, meta interface{}) err
d.Set("arn", resp.User.Arn)
d.Set("aws_account_id", awsAccountID)
d.Set("email", resp.User.Email)
d.Set("identity_type", resp.User.IdentityType)
d.Set("namespace", namespace)
d.Set("user_role", resp.User.Role)
d.Set("user_name", resp.User.UserName)
Expand Down
58 changes: 21 additions & 37 deletions aws/resource_aws_quicksight_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ import (

func TestAccAWSQuickSightUser_basic(t *testing.T) {
var user quicksight.User
resourceName := "aws_quicksight_user.default"
rName1 := acctest.RandomWithPrefix("tf-acc-test")
rName2 := acctest.RandomWithPrefix("tf-acc-test")
rName1 := "tfacctest" + acctest.RandString(10)
resourceName1 := "aws_quicksight_user." + rName1
rName2 := "tfacctest" + acctest.RandString(10)
resourceName2 := "aws_quicksight_user." + rName2

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -28,32 +29,27 @@ func TestAccAWSQuickSightUser_basic(t *testing.T) {
{
Config: testAccAWSQuickSightUserConfig(rName1),
Check: resource.ComposeTestCheckFunc(
testAccCheckQuickSightUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "user_name", rName1),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "quicksight", fmt.Sprintf("user/default/%s", rName1)),
testAccCheckQuickSightUserExists(resourceName1, &user),
resource.TestCheckResourceAttr(resourceName1, "user_name", rName1),
testAccCheckResourceAttrRegionalARN(resourceName1, "arn", "quicksight", fmt.Sprintf("user/default/%s", rName1)),
),
},
{
Config: testAccAWSQuickSightUserConfig(rName2),
Check: resource.ComposeTestCheckFunc(
testAccCheckQuickSightUserExists(resourceName, &user),
resource.TestCheckResourceAttr(resourceName, "user_name", rName2),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "quicksight", fmt.Sprintf("user/default/%s", rName2)),
testAccCheckQuickSightUserExists(resourceName2, &user),
resource.TestCheckResourceAttr(resourceName2, "user_name", rName2),
testAccCheckResourceAttrRegionalARN(resourceName2, "arn", "quicksight", fmt.Sprintf("user/default/%s", rName2)),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSQuickSightUser_withRealEmail(t *testing.T) {
func TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks(t *testing.T) {
var user quicksight.User
resourceName := "aws_quicksight_user.default"
rName := acctest.RandomWithPrefix("tf-acc-test")
rName := "tfacctest" + acctest.RandString(10)
resourceName := "aws_quicksight_user." + rName

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand All @@ -74,19 +70,14 @@ func TestAccAWSQuickSightUser_withRealEmail(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "email", "nottarealemailbutworks2"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSQuickSightUser_disappears(t *testing.T) {
var user quicksight.User
resourceName := "aws_quicksight_user.default"
rName := acctest.RandomWithPrefix("tf-acc-test")
rName := "tfacctest" + acctest.RandString(10)
resourceName := "aws_quicksight_user." + rName

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Expand Down Expand Up @@ -197,27 +188,20 @@ func testAccCheckQuickSightUserDisappears(v *quicksight.User) resource.TestCheck
}
}

func testAccAWSQuickSightUserConfig(rName string) string {
return fmt.Sprintf(`
resource "aws_quicksight_user" "default" {
user_name = %[1]q
email = "[email protected]"
identity_type = "IAM"
user_role = "READER"
}
`, rName)
}

func testAccAWSQuickSightUserConfigWithEmail(rName, email string) string {
return fmt.Sprintf(`
data "aws_caller_identity" "current" {}
resource "aws_quicksight_user" "default" {
resource "aws_quicksight_user" %[1]q {
aws_account_id = "${data.aws_caller_identity.current.account_id}"
user_name = %[1]q
email = %[2]q
identity_type = "IAM"
identity_type = "QUICKSIGHT"
user_role = "READER"
}
`, rName, email)
}

func testAccAWSQuickSightUserConfig(rName string) string {
return testAccAWSQuickSightUserConfigWithEmail(rName, "[email protected]")
}
8 changes: 2 additions & 6 deletions website/docs/r/quicksight_user.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,10 @@ The following arguments are supported:

## Attributes Reference

All above attributes except for `session_name` are exported as well as:
All above attributes except for `session_name` and `identity_type` are exported as well as:

* `arn` - Amazon Resource Name (ARN) of the user

## Import

QuickSight User can be imported using the AWS account ID, namespace and user name separated by `/`.

```
$ terraform import aws_quicksight_user.example 123456789123/default/a-reader-user
```
Importing is currently not supported on this resource.

0 comments on commit fd07518

Please sign in to comment.