Merge pull request #27257 from hashicorp/b-appstream-panic

resource/aws_appstream_stack: Fix panic on read
hashicorp · Oct 20, 2022 · 5e03789 · 5e03789
2 parents 647880f + 551df4c
commit 5e03789
Show file tree

Hide file tree

Showing 4 changed files with 291 additions and 50 deletions.
diff --git a/.changelog/27257.txt b/.changelog/27257.txt
@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_appstream_stack: Fix panic with `application_settings`.
+```
+
+```release-note:enhancement
+resource/aws_appstream_stack: Add validation for `application_settings`.
+```
diff --git a/internal/service/appstream/stack.go b/internal/service/appstream/stack.go
@@ -3,14 +3,17 @@ package appstream
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/appstream"
 	"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"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"
@@ -19,6 +22,7 @@ import (
 	"github.com/hashicorp/terraform-provider-aws/internal/flex"
 	tftags "github.com/hashicorp/terraform-provider-aws/internal/tags"
 	"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
+	"github.com/hashicorp/terraform-provider-aws/internal/verify"
 )
 
 var (
@@ -66,11 +70,12 @@ func ResourceStack() *schema.Resource {
 					Schema: map[string]*schema.Schema{
 						"enabled": {
 							Type:     schema.TypeBool,
-							Optional: true,
+							Required: true,
 						},
 						"settings_group": {
-							Type:     schema.TypeString,
-							Optional: true,
+							Type:         schema.TypeString,
+							Optional:     true,
+							ValidateFunc: validation.StringLenBetween(0, 100),
 						},
 					},
 				},
@@ -182,6 +187,53 @@ func ResourceStack() *schema.Resource {
 			"tags":     tftags.TagsSchemaForceNew(),
 			"tags_all": tftags.TagsSchemaComputed(),
 		},
+
+		CustomizeDiff: customdiff.All(
+			verify.SetTagsDiff,
+			func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
+				if d.Id() == "" {
+					return nil
+				}
+
+				rawConfig := d.GetRawConfig()
+				configApplicationSettings := rawConfig.GetAttr("application_settings")
+				if configApplicationSettings.IsKnown() && !configApplicationSettings.IsNull() && configApplicationSettings.LengthInt() > 0 {
+					return nil
+				}
+
+				rawState := d.GetRawState()
+				stateApplicationSettings := rawState.GetAttr("application_settings")
+				if stateApplicationSettings.IsKnown() && !stateApplicationSettings.IsNull() && stateApplicationSettings.LengthInt() > 0 {
+					setting := stateApplicationSettings.Index(cty.NumberIntVal(0))
+					if setting.IsKnown() && !setting.IsNull() {
+						enabled := setting.GetAttr("enabled")
+						if enabled.IsKnown() && !enabled.IsNull() && enabled.True() {
+							// Trigger a diff
+							return d.SetNew("application_settings", []map[string]any{
+								{
+									"enabled":        false,
+									"settings_group": "",
+								},
+							})
+						}
+					}
+				}
+
+				return nil
+			},
+			func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
+				_, enabled := d.GetOk("application_settings.0.enabled")
+				v, sg := d.GetOk("application_settings.0.settings_group")
+				log.Print(v)
+
+				if enabled && !sg {
+					return errors.New("application_settings.settings_group must be set when application_settings.enabled is true")
+				} else if !enabled && sg {
+					return errors.New("application_settings.settings_group must not be set when application_settings.enabled is false")
+				}
+				return nil
+			},
+		),
 	}
 }
 
@@ -283,14 +335,14 @@ func resourceStackRead(ctx context.Context, d *schema.ResourceData, meta interfa
 			return diag.FromErr(fmt.Errorf("error setting `%s` for AppStream Stack (%s): %w", "access_endpoints", d.Id(), err))
 		}
 		if err = d.Set("application_settings", flattenApplicationSettings(v.ApplicationSettings)); err != nil {
-			return diag.FromErr(fmt.Errorf("error setting `%s` for AppStream Stack (%s): %w", "user_settings", d.Id(), err))
+			return diag.FromErr(fmt.Errorf("error setting `%s` for AppStream Stack (%s): %w", "application_settings", d.Id(), err))
 		}
 		d.Set("arn", v.Arn)
 		d.Set("created_time", aws.TimeValue(v.CreatedTime).Format(time.RFC3339))
 		d.Set("description", v.Description)
 		d.Set("display_name", v.DisplayName)
 		if err = d.Set("embed_host_domains", flex.FlattenStringList(v.EmbedHostDomains)); err != nil {
-			return diag.FromErr(fmt.Errorf("error setting `%s` for AppStream Stack (%s): %w", "user_settings", d.Id(), err))
+			return diag.FromErr(fmt.Errorf("error setting `%s` for AppStream Stack (%s): %w", "embed_host_domains", d.Id(), err))
 		}
 		d.Set("feedback_url", v.FeedbackURL)
 		d.Set("name", v.Name)
@@ -334,7 +386,7 @@ func resourceStackUpdate(ctx context.Context, d *schema.ResourceData, meta inter
 	}
 
 	if d.HasChange("application_settings") {
-		input.ApplicationSettings = expandApplicationSettings(d.Get("application_settings").(*schema.Set).List())
+		input.ApplicationSettings = expandApplicationSettings(d.Get("application_settings").([]interface{}))
 	}
 
 	if d.HasChange("description") {
@@ -473,15 +525,17 @@ func flattenAccessEndpoints(apiObjects []*appstream.AccessEndpoint) []map[string
 	return tfList
 }
 
-func expandApplicationSetting(tfMap map[string]interface{}) *appstream.ApplicationSettings {
-	if tfMap == nil {
-		return nil
+func expandApplicationSettings(tfList []interface{}) *appstream.ApplicationSettings {
+	if len(tfList) == 0 {
+		return &appstream.ApplicationSettings{
+			Enabled: aws.Bool(false),
+		}
 	}
 
-	apiObject := &appstream.ApplicationSettings{}
+	tfMap := tfList[0].(map[string]interface{})
 
-	if v, ok := tfMap["enabled"]; ok {
-		apiObject.Enabled = aws.Bool(v.(bool))
+	apiObject := &appstream.ApplicationSettings{
+		Enabled: aws.Bool(tfMap["enabled"].(bool)),
 	}
 	if v, ok := tfMap["settings_group"]; ok {
 		apiObject.SettingsGroup = aws.String(v.(string))
@@ -490,36 +544,15 @@ func expandApplicationSetting(tfMap map[string]interface{}) *appstream.Applicati
 	return apiObject
 }
 
-func expandApplicationSettings(tfList []interface{}) *appstream.ApplicationSettings {
-	if len(tfList) == 0 {
-		return nil
-	}
-
-	var apiObject *appstream.ApplicationSettings
-
-	for _, tfMapRaw := range tfList {
-		tfMap, ok := tfMapRaw.(map[string]interface{})
-
-		if !ok {
-			continue
-		}
-
-		apiObject = expandApplicationSetting(tfMap)
-	}
-
-	return apiObject
-}
-
 func flattenApplicationSetting(apiObject *appstream.ApplicationSettingsResponse) map[string]interface{} {
 	if apiObject == nil {
 		return nil
 	}
 
-	tfMap := map[string]interface{}{}
-	tfMap["enabled"] = aws.BoolValue(apiObject.Enabled)
-	tfMap["settings_group"] = aws.StringValue(apiObject.SettingsGroup)
-
-	return tfMap
+	return map[string]interface{}{
+		"enabled":        aws.BoolValue(apiObject.Enabled),
+		"settings_group": aws.StringValue(apiObject.SettingsGroup),
+	}
 }
 
 func flattenApplicationSettings(apiObject *appstream.ApplicationSettingsResponse) []interface{} {