Add quicksight user resource #10401
Add quicksight user resource #10401
Conversation
ghost
added
size/XL
bflad
added
new-resource
There was a problem hiding this comment.
Hi @mjgpy3 👋 Thanks for submitting this and overall its off to a good start. Please see the below for an initial review and reach out if you have any questions. Your command above for running the acceptance testing looks right at first glance, so not sure why it would not be running the tests. To troubleshoot, you can try running what the make testacc command should be doing directly, e.g.
$ TF_ACC=1 go test ./aws -v -timeout 120m -parallel 20 -run='TestAccAWSQuickSightUser_'Also, if you have not seen the Contributing Guide, would highly recommend checking that out. There's a section on running acceptance testing available here: https://github.com/terraform-providers/terraform-provider-aws/blob/master/.github/CONTRIBUTING.md#running-an-acceptance-test
| }) | ||
| } | ||
|
|
||
| func TestAccAWSQuickSightUser_withRealEmail(t *testing.T) { |
There was a problem hiding this comment.
Nit: This test doesn't appear to be testing real emails, so maybe worth just noting which attribute we are testing instead
| func TestAccAWSQuickSightUser_withRealEmail(t *testing.T) { | |
| func TestAccAWSQuickSightUser_Email(t *testing.T) { |
There was a problem hiding this comment.
I renamed this to TestAccAWSQuickSightUser_withInvalidFormattedEmailStillWorks which is more what I was going for.
- 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": "someemail@pfake.com",
"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.
|
@bflad if you get a moment, please check out these updates (especially the last commit message). Thanks! |
|
Thanks @bflad! |
This makes things consistent with hashicorp#10401 and the AWS documentation.
This makes things consistent with #10401 and the AWS documentation.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
ghost
locked and limited conversation to collaborators
Community Note
Relates #8146, also #8233 (based heavily on the latter, thanks @kterada0509 for the clean PR)
Output from acceptance testing: