New resource for AppStream User and Stack User Association #21485

coderGo93 · 2021-10-25T22:40:41Z

Added a new resource, doc and tests for AppStream User called aws_appstream_user
Added a new resource, doc and tests for AppStream Stack User Association called aws_appstream_stack_user_association

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #6508

Output from acceptance testing:

AppStream User and Stack User Association

$  make testacc TESTARGS='-run=TestAccAppStreamUser' PKG_NAME=internal/service/appstream       
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/appstream/... -v -count 1 -parallel 20 -run=TestAccAppStreamUser -timeout 180m
=== RUN   TestAccAppStreamUserStackAssociation_basic
=== PAUSE TestAccAppStreamUserStackAssociation_basic
=== RUN   TestAccAppStreamUserStackAssociation_disappears
=== PAUSE TestAccAppStreamUserStackAssociation_disappears
=== RUN   TestAccAppStreamUserStackAssociation_complete
=== PAUSE TestAccAppStreamUserStackAssociation_complete
=== RUN   TestAccAppStreamUser_basic
=== PAUSE TestAccAppStreamUser_basic
=== RUN   TestAccAppStreamUser_disappears
=== PAUSE TestAccAppStreamUser_disappears
=== RUN   TestAccAppStreamUser_complete
=== PAUSE TestAccAppStreamUser_complete
=== CONT  TestAccAppStreamUserStackAssociation_basic
=== CONT  TestAccAppStreamUser_disappears
=== CONT  TestAccAppStreamUser_complete
=== CONT  TestAccAppStreamUserStackAssociation_complete
=== CONT  TestAccAppStreamUser_basic
=== CONT  TestAccAppStreamUserStackAssociation_disappears
--- PASS: TestAccAppStreamUser_basic (32.67s)
--- PASS: TestAccAppStreamUser_disappears (35.67s)
--- PASS: TestAccAppStreamUserStackAssociation_disappears (37.50s)
--- PASS: TestAccAppStreamUserStackAssociation_basic (41.20s)
--- PASS: TestAccAppStreamUser_complete (64.23s)
--- PASS: TestAccAppStreamUserStackAssociation_complete (67.03s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/appstream  68.864s

gdavison

Thanks for the PR, @coderGo93. I have a number of suggested changes.

internal/service/appstream/stack_user_association.go

internal/service/appstream/user.go

gdavison · 2021-11-08T22:10:23Z

internal/service/appstream/user.go

+		_, err = conn.CreateUserWithContext(ctx, input)
+	}
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("error creating Appstream User (%s): %w", d.Id(), err))


Error and log messages should use the name that AWS uses, "AppStream"

Suggested change

return diag.FromErr(fmt.Errorf("error creating Appstream User (%s): %w", d.Id(), err))

return diag.FromErr(fmt.Errorf("error creating AppStream User (%s): %w", d.Id(), err))

internal/service/appstream/user.go

gdavison · 2021-11-08T23:38:06Z

internal/service/appstream/stack_user_association.go

+	err = resource.RetryContext(ctx, fleetOperationTimeout, func() *resource.RetryError {
+		output, err = conn.BatchAssociateUserStackWithContext(ctx, &appstream.BatchAssociateUserStackInput{
+			UserStackAssociations: []*appstream.UserStackAssociation{input},
+		})
+		if err != nil {
+			if tfawserr.ErrCodeEquals(err, appstream.ErrCodeResourceNotFoundException) {
+				return resource.RetryableError(err)
+			}
+
+			return resource.NonRetryableError(err)
+		}
+
+		return nil
+	})
+
+	if tfresource.TimedOut(err) {
+		output, err = conn.BatchAssociateUserStackWithContext(ctx, &appstream.BatchAssociateUserStackInput{
+			UserStackAssociations: []*appstream.UserStackAssociation{input},
+		})
+	}


Does this need a Retry loop? The documentation for BatchAssociateUserStack seems to indicate that the function is synchronous.

internal/service/appstream/stack_user_association.go

gdavison · 2021-11-08T23:40:20Z

internal/service/appstream/stack_user_association_test.go

+			{
+				Config: testAccStackUserAssociationConfig(rName, authType, rEmail),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckStackUserAssociationExists(resourceName),


This should check all parameter values

gdavison · 2021-11-08T23:42:12Z

internal/service/appstream/stack_user_association_test.go

+	resourceName := "aws_appstream_stack_user_association.test"
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+	authType := "USERPOOL"
+	rEmail := acctest.RandomEmailAddress("hashicorp.com")


This should use a random domain name generated by acctest.RandomDomainName(), to avoid being delivered to an actual email address.

Suggested change

rEmail := acctest.RandomEmailAddress("hashicorp.com")

domain := acctest.RandomDomainName()

rEmail := acctest.RandomEmailAddress(domain)

internal/service/appstream/user.go

gdavison

Thanks for the updates, @coderGo93. There are a few things to change from the initial review plus some I've added to this review.

I've also added a commit to show what I meant about returning resource.NotFoundError from the FindUserByUserNameAndAuthType() function in #21485 (comment)

internal/service/appstream/user.go

gdavison · 2021-11-16T22:50:27Z

internal/service/appstream/user.go

+	// Enabling/disabling workflow
+	if v, ok := d.GetOk("enabled"); ok {
+		if v.(bool) {
+			input := &appstream.EnableUserInput{
+				AuthenticationType: aws.String(authType),
+				UserName:           aws.String(userName),
+			}
+
+			_, err = conn.EnableUserWithContext(ctx, input)
+			if err != nil {
+				return diag.FromErr(fmt.Errorf("error enabling AppStream User (%s): %w", d.Id(), err))
+			}
+		} else {
+			input := &appstream.DisableUserInput{
+				AuthenticationType: aws.String(authType),
+				UserName:           aws.String(userName),
+			}
+
+			_, err = conn.DisableUserWithContext(ctx, input)
+			if err != nil {
+				return diag.FromErr(fmt.Errorf("error disabling AppStream User (%s): %w", d.Id(), err))
+			}
+		}
+	}


It looks like users are enabled by default when they're created, so we only need to change this when enabled is false

gdavison · 2021-11-16T22:54:35Z

internal/service/appstream/wait.go

+// waitUserAvailable waits for a user be available
+func waitUserAvailable(ctx context.Context, conn *appstream.AppStream, username, authType string) (*appstream.User, error) {
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{"NotFound"},


When the Refresh function returns nil for the resource value, the status isn't checked, so the Pending list can be removed.

gdavison · 2021-11-16T22:55:19Z

internal/service/appstream/wait.go

+func waitUserAvailable(ctx context.Context, conn *appstream.AppStream, username, authType string) (*appstream.User, error) {
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{"NotFound"},
+		Target:  []string{"AVAILABLE"},


This should be a constant shared with the status function

gdavison · 2021-11-16T22:56:51Z

internal/service/appstream/user_test.go

+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.PreCheck(t)
+			acctest.PreCheckHasIAMRole(t, "AmazonAppStreamServiceAccess")


Creating AppStream User resources doesn't require this IAM role, so the check can be removed

gdavison · 2021-11-16T23:14:12Z

internal/service/appstream/stack_user_association.go

+
+		for _, err := range output.Errors {
+			errs = multierror.Append(errs, fmt.Errorf("%s: %s", aws.StringValue(err.ErrorCode), aws.StringValue(err.ErrorMessage)))
+		}


This should return errs, otherwise the errors collected here are ignored

gdavison · 2021-11-16T23:16:08Z

internal/service/appstream/stack_user_association.go

+	}
+
+	if len(resp.UserStackAssociations) == 0 {
+		log.Printf("[WARN] Appstream User (%s) not found, removing from state", d.Id())


Suggested change

log.Printf("[WARN] Appstream User (%s) not found, removing from state", d.Id())

log.Printf("[WARN] AppStream User Stack Association (%s) not found, removing from state", d.Id())

gdavison · 2021-11-16T23:16:17Z

internal/service/appstream/stack_user_association.go

+		})
+
+	if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, appstream.ErrCodeResourceNotFoundException) {
+		log.Printf("[WARN] Appstream Stack User Association (%s) not found, removing from state", d.Id())


Suggested change

log.Printf("[WARN] Appstream Stack User Association (%s) not found, removing from state", d.Id())

log.Printf("[WARN] AppStream User Stack Association (%s) not found, removing from state", d.Id())

gdavison · 2021-11-16T23:28:19Z

internal/service/appstream/user.go

+	if v, ok := d.GetOk("message_action"); ok {
+		input.MessageAction = aws.String(v.(string))
+	}


The message_action parameter should be restored. In #21485 (comment), I was suggesting removing RESEND as a valid option and ignoring changes to the value instead of making it ForceNew. There should still be the options of sending the welcome email or setting SUPPRESS to not send the email.
Another option would be to use a TypeBool send_welcome_email (or send_email_notification to match aws_appstream_user_stack_association)

Apologies, I misunderstood then, thanks for the clarification.

gdavison · 2021-11-16T23:33:21Z

internal/service/appstream/stack_user_association.go

+				Type:     schema.TypeString,
+				Required: true,
+				ForceNew: true,
+			},


The parameter send_email_notification should be restored here, so that the email notification can be controlled. In #21485 (comment), I meant that it should not be ForceNew and that changes should be ignored, since it only has an effect at creation time

…thentication Type are adjacent

gdavison

Thanks, @coderGo93. I've made a few updates to tweak the error messages and enhance the tests

Acceptance test results in Commercial partition

--- PASS: TestAccAppStreamUser_disappears (34.66s)
--- PASS: TestAccAppStreamUser_basic (45.50s)
--- PASS: TestAccAppStreamUserStackAssociation_disappears (49.88s)
--- PASS: TestAccAppStreamUserStackAssociation_basic (66.23s)
--- PASS: TestAccAppStreamUserStackAssociation_complete (78.13s)
--- PASS: TestAccAppStreamUser_complete (89.22s)

Acceptance test results in GovCloud (AppStream User pools are not supported in GovCloud)

--- SKIP: TestAccAppStreamUser_complete (15.94s)
--- SKIP: TestAccAppStreamUser_disappears (15.96s)
--- SKIP: TestAccAppStreamUser_basic (15.99s)
--- SKIP: TestAccAppStreamUserStackAssociation_basic (20.52s)
--- SKIP: TestAccAppStreamUserStackAssociation_complete (20.64s)
--- SKIP: TestAccAppStreamUserStackAssociation_disappears (20.79s)

github-actions · 2021-12-01T18:15:37Z

This functionality has been released in v3.67.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-06-07T02:32:42Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 26, 2021

gdavison mentioned this pull request Nov 8, 2021

Feature Request: AppStream support #6508

Closed

8 tasks

gdavison self-assigned this Nov 8, 2021

gdavison requested changes Nov 8, 2021

View reviewed changes

github-actions bot added the generators Relates to code generators. label Nov 9, 2021

coderGo93 requested a review from gdavison November 9, 2021 23:59

gdavison requested changes Nov 16, 2021

View reviewed changes

gdavison mentioned this pull request Nov 16, 2021

New resource for AppStream Stack Fleet Association #21484

Merged

Edgar Lopez and others added 12 commits November 17, 2021 15:57

feat: added resource and docs for appstream user

97eba2f

feat: added resource and docs for appstream stack user association

b04cd2a

fixes typo

f2153b4

fixes typo

d4e86d4

added changelog

0f5c08c

fixes github linter

02f8d79

refactor: updated with the suggestions of gdavison

dd0cdf7

updated doc

fb6211b

updated doc

e1f40db

fixes linter

345127f

Returns resource.NotFoundError from FindUserByUserNameAndAuthType()

969b46c

refactor

40e7c23

coderGo93 force-pushed the appstream-user branch from c65d4f5 to 40e7c23 Compare November 17, 2021 23:51

coderGo93 added 2 commits November 17, 2021 17:22

refactor: updated the way for enabled

05f8b76

refactor

0e679c6

coderGo93 requested a review from gdavison November 18, 2021 00:59

gdavison added 6 commits November 22, 2021 16:24

Error message tweaks

7feebdd

Uses random domain for acceptance tests

276fa7b

Uses finder function for AppStream User

b0e4952

Reorders test steps since step 1 and 2 were the same

66edfcd

Re-orders generated User Stack Association ID so that Username and Au…

9e7b10a

…thentication Type are adjacent

Skips User tests in unsupported regions and partitions

f130d56

gdavison approved these changes Nov 23, 2021

View reviewed changes

gdavison merged commit d088921 into hashicorp:main Nov 23, 2021

github-actions bot added this to the v3.67.0 milestone Nov 23, 2021

github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New resource for AppStream User and Stack User Association #21485

New resource for AppStream User and Stack User Association #21485

coderGo93 commented Oct 25, 2021 •

edited

Loading

gdavison left a comment

gdavison Nov 8, 2021

gdavison Nov 8, 2021

gdavison Nov 8, 2021

gdavison Nov 8, 2021

gdavison left a comment

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

gdavison Nov 16, 2021

coderGo93 Nov 17, 2021 •

edited

Loading

gdavison Nov 16, 2021

gdavison left a comment

github-actions bot commented Dec 1, 2021

github-actions bot commented Jun 7, 2022

	return diag.FromErr(fmt.Errorf("error creating Appstream User (%s): %w", d.Id(), err))
	return diag.FromErr(fmt.Errorf("error creating AppStream User (%s): %w", d.Id(), err))

	rEmail := acctest.RandomEmailAddress("hashicorp.com")
	domain := acctest.RandomDomainName()
	rEmail := acctest.RandomEmailAddress(domain)

	log.Printf("[WARN] Appstream User (%s) not found, removing from state", d.Id())
	log.Printf("[WARN] AppStream User Stack Association (%s) not found, removing from state", d.Id())

	log.Printf("[WARN] Appstream Stack User Association (%s) not found, removing from state", d.Id())
	log.Printf("[WARN] AppStream User Stack Association (%s) not found, removing from state", d.Id())

New resource for AppStream User and Stack User Association #21485

New resource for AppStream User and Stack User Association #21485

Conversation

coderGo93 commented Oct 25, 2021 • edited Loading

Community Note

gdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdavison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderGo93 Nov 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gdavison left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 1, 2021

github-actions bot commented Jun 7, 2022

coderGo93 commented Oct 25, 2021 •

edited

Loading

coderGo93 Nov 17, 2021 •

edited

Loading