diff --git a/.changelog/18043.txt b/.changelog/18043.txt new file mode 100644 index 000000000000..17c43bb4bee9 --- /dev/null +++ b/.changelog/18043.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_instance: Allow updates to `user_data` and `user_data_base64` without forcing resource replacement +``` \ No newline at end of file diff --git a/internal/service/ec2/instance.go b/internal/service/ec2/instance.go index 1e3b4074388f..eda7d08eec8a 100644 --- a/internal/service/ec2/instance.go +++ b/internal/service/ec2/instance.go @@ -564,7 +564,6 @@ func ResourceInstance() *schema.Resource { "user_data": { Type: schema.TypeString, Optional: true, - ForceNew: true, Computed: true, ConflictsWith: []string{"user_data_base64"}, DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { @@ -589,7 +588,6 @@ func ResourceInstance() *schema.Resource { "user_data_base64": { Type: schema.TypeString, Optional: true, - ForceNew: true, Computed: true, ConflictsWith: []string{"user_data"}, ValidateFunc: func(v interface{}, name string) (warns []string, errs []error) { @@ -1423,73 +1421,57 @@ func resourceInstanceUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("instance_type") && !d.IsNewResource() { - log.Printf("[INFO] Stopping Instance %q for instance_type change", d.Id()) - _, err := conn.StopInstances(&ec2.StopInstancesInput{ - InstanceIds: []*string{aws.String(d.Id())}, - }) - if err != nil { - return fmt.Errorf("error stopping instance (%s): %s", d.Id(), err) - } - - if err := WaitForInstanceStopping(conn, d.Id(), InstanceStopTimeout); err != nil { - return err - } - - log.Printf("[INFO] Modifying instance type %s", d.Id()) - _, err = conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ - InstanceId: aws.String(d.Id()), - InstanceType: &ec2.AttributeValue{ - Value: aws.String(d.Get("instance_type").(string)), - }, - }) - if err != nil { - return err - } + if d.HasChanges("instance_type", "user_data", "user_data_base64") && !d.IsNewResource() { + // For each argument change, we start and stop the instance + // to account for behaviors occurring outside terraform. + // Only one attribute can be modified at a time, else we get + // "InvalidParameterCombination: Fields for multiple attribute types specified" + if d.HasChange("instance_type") { + log.Printf("[INFO] Modifying instance type %s", d.Id()) - log.Printf("[INFO] Starting Instance %q after instance_type change", d.Id()) + input := &ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(d.Id()), + InstanceType: &ec2.AttributeValue{ + Value: aws.String(d.Get("instance_type").(string)), + }, + } - input := &ec2.StartInstancesInput{ - InstanceIds: []*string{aws.String(d.Id())}, + if err := modifyAttributeWithInstanceStopStart(d, conn, input); err != nil { + return fmt.Errorf("error updating instance (%s) instance type: %w", d.Id(), err) + } } - // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16433 - err = resource.Retry(InstanceAttributePropagationTimeout, func() *resource.RetryError { - _, err := conn.StartInstances(input) - - if tfawserr.ErrMessageContains(err, ErrCodeInvalidParameterValue, "LaunchPlan instance type does not match attribute value") { - return resource.RetryableError(err) + if d.HasChange("user_data") { + log.Printf("[INFO] Modifying user data %s", d.Id()) + input := &ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(d.Id()), + UserData: &ec2.BlobAttributeValue{ + Value: []byte(d.Get("user_data").(string)), + }, } - if err != nil { - return resource.NonRetryableError(err) + if err := modifyAttributeWithInstanceStopStart(d, conn, input); err != nil { + return fmt.Errorf("error updating instance (%s) user data: %w", d.Id(), err) } - - return nil - }) - - if tfresource.TimedOut(err) { - _, err = conn.StartInstances(input) } - if err != nil { - return fmt.Errorf("error starting EC2 Instance (%s): %w", d.Id(), err) - } + if d.HasChange("user_data_base64") { + log.Printf("[INFO] Modifying user data base64 %s", d.Id()) + userData, err := base64.URLEncoding.DecodeString(d.Get("user_data_base64").(string)) + if err != nil { + return fmt.Errorf("error updating instance (%s) user data base64: %w", d.Id(), err) + } - stateConf := &resource.StateChangeConf{ - Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameStopped}, - Target: []string{ec2.InstanceStateNameRunning}, - Refresh: InstanceStateRefreshFunc(conn, d.Id(), []string{ec2.InstanceStateNameTerminated}), - Timeout: d.Timeout(schema.TimeoutUpdate), - Delay: 10 * time.Second, - MinTimeout: 3 * time.Second, - } + input := &ec2.ModifyInstanceAttributeInput{ + InstanceId: aws.String(d.Id()), + UserData: &ec2.BlobAttributeValue{ + Value: userData, + }, + } - _, err = stateConf.WaitForState() - if err != nil { - return fmt.Errorf( - "Error waiting for instance (%s) to become ready: %s", - d.Id(), err) + if err := modifyAttributeWithInstanceStopStart(d, conn, input); err != nil { + return fmt.Errorf("error updating instance (%s) user data base64: %w", d.Id(), err) + } } } @@ -1766,6 +1748,62 @@ func resourceInstanceDisableAPITermination(conn *ec2.EC2, id string, disableAPIT return nil } +// modifyAttributeWithInstanceStopStart modifies a specific attribute provided +// as input by first stopping the EC2 instance before the modification +// and then starting up the EC2 instance after modification. +// Reference: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Stop_Start.html +func modifyAttributeWithInstanceStopStart(d *schema.ResourceData, conn *ec2.EC2, input *ec2.ModifyInstanceAttributeInput) error { + log.Printf("[INFO] Stopping Instance %q for attribute change", d.Id()) + _, err := conn.StopInstances(&ec2.StopInstancesInput{ + InstanceIds: []*string{aws.String(d.Id())}, + }) + + if err != nil { + return fmt.Errorf("error stopping EC2 Instance (%s): %w", d.Id(), err) + } + + if err := WaitForInstanceStopping(conn, d.Id(), InstanceStopTimeout); err != nil { + return err + } + + if _, err := conn.ModifyInstanceAttribute(input); err != nil { + return err + } + + startInput := &ec2.StartInstancesInput{ + InstanceIds: []*string{aws.String(d.Id())}, + } + + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/16433 + err = resource.Retry(InstanceAttributePropagationTimeout, func() *resource.RetryError { + _, err := conn.StartInstances(startInput) + + if tfawserr.ErrMessageContains(err, ErrCodeInvalidParameterValue, "LaunchPlan instance type does not match attribute value") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.StartInstances(startInput) + } + + if err != nil { + return fmt.Errorf("error starting EC2 Instance (%s): %w", d.Id(), err) + } + + if err := WaitForInstanceRunning(conn, d.Id(), InstanceStartTimeout); err != nil { + return err + } + + return nil +} + // InstanceStateRefreshFunc returns a resource.StateRefreshFunc that is used to watch // an EC2 instance. func InstanceStateRefreshFunc(conn *ec2.EC2, instanceID string, failStates []string) resource.StateRefreshFunc { @@ -2781,6 +2819,27 @@ func terminateInstance(conn *ec2.EC2, id string, timeout time.Duration) error { return waitForInstanceDeletion(conn, id, timeout) } +func WaitForInstanceRunning(conn *ec2.EC2, id string, timeout time.Duration) error { + log.Printf("[DEBUG] Waiting for instance (%s) to be running", id) + + stateConf := &resource.StateChangeConf{ + Pending: []string{ec2.InstanceStateNamePending, ec2.InstanceStateNameStopped}, + Target: []string{ec2.InstanceStateNameRunning}, + Refresh: InstanceStateRefreshFunc(conn, id, []string{ec2.InstanceStateNameTerminated}), + Timeout: timeout, + Delay: 10 * time.Second, + MinTimeout: 3 * time.Second, + } + + _, err := stateConf.WaitForState() + if err != nil { + return fmt.Errorf( + "error waiting for instance (%s) to be running: %s", id, err) + } + + return nil +} + func WaitForInstanceStopping(conn *ec2.EC2, id string, timeout time.Duration) error { log.Printf("[DEBUG] Waiting for instance (%s) to become stopped", id) diff --git a/internal/service/ec2/instance_test.go b/internal/service/ec2/instance_test.go index f43ea72a178b..d68d8c50f1a7 100644 --- a/internal/service/ec2/instance_test.go +++ b/internal/service/ec2/instance_test.go @@ -1,6 +1,8 @@ package ec2_test import ( + "crypto/sha1" + "encoding/hex" "fmt" "reflect" "regexp" @@ -374,7 +376,7 @@ func TestAccEC2Instance_userDataBase64(t *testing.T) { CheckDestroy: testAccCheckInstanceDestroy, Steps: []resource.TestStep{ { - Config: testAccInstanceConfigWithUserDataBase64(rName), + Config: testAccInstanceConfigWithUserDataBase64(rName, "hello world"), Check: resource.ComposeTestCheckFunc( testAccCheckInstanceExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "user_data_base64", "aGVsbG8gd29ybGQ="), @@ -390,6 +392,41 @@ func TestAccEC2Instance_userDataBase64(t *testing.T) { }) } +func TestAccEC2Instance_userDataBase64_update(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfigWithUserDataBase64(rName, "hello world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "user_data_base64", "aGVsbG8gd29ybGQ="), + ), + }, + { + Config: testAccInstanceConfigWithUserDataBase64(rName, "new world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "user_data_base64", "bmV3IHdvcmxk"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user_data"}, + }, + }, + }) +} + func TestAccEC2Instance_gp2IopsDevice(t *testing.T) { var v ec2.Instance resourceName := "aws_instance.test" @@ -1609,6 +1646,85 @@ func TestAccEC2Instance_changeInstanceType(t *testing.T) { }) } +func TestAccEC2Instance_changeInstanceTypeAndUserData(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + hash := sha1.Sum([]byte("hello world")) + expectedUserData := hex.EncodeToString(hash[:]) + hash2 := sha1.Sum([]byte("new world")) + expectedUserDataUpdated := hex.EncodeToString(hash2[:]) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfigInstanceTypeAndUserData(rName, "t2.medium", "hello world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "instance_type", "t2.medium"), + resource.TestCheckResourceAttr(resourceName, "user_data", expectedUserData), + ), + }, + { + Config: testAccInstanceConfigInstanceTypeAndUserData(rName, "t2.large", "new world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "instance_type", "t2.large"), + resource.TestCheckResourceAttr(resourceName, "user_data", expectedUserDataUpdated), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user_data"}, + }, + }, + }) +} + +func TestAccEC2Instance_changeInstanceTypeAndUserDataBase64(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfigInstanceTypeAndUserDataBase64(rName, "t2.medium", "hello world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "instance_type", "t2.medium"), + resource.TestCheckResourceAttr(resourceName, "user_data_base64", "aGVsbG8gd29ybGQ="), + ), + }, + { + Config: testAccInstanceConfigInstanceTypeAndUserDataBase64(rName, "t2.large", "new world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "instance_type", "t2.large"), + resource.TestCheckResourceAttr(resourceName, "user_data_base64", "bmV3IHdvcmxk"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user_data"}, + }, + }, + }) +} + func TestAccEC2Instance_EBSRootDevice_basic(t *testing.T) { var instance ec2.Instance resourceName := "aws_instance.test" @@ -3407,6 +3523,66 @@ func TestAccEC2Instance_disappears(t *testing.T) { }) } +func TestAccEC2Instance_UserData(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfigWithUserData(rName, "hello world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user_data"}, + }, + }, + }) +} + +func TestAccEC2Instance_UserData_update(t *testing.T) { + var v ec2.Instance + resourceName := "aws_instance.test" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckInstanceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfigWithUserData(rName, "hello world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + ), + }, + { + Config: testAccInstanceConfigWithUserData(rName, "new world"), + Check: resource.ComposeTestCheckFunc( + testAccCheckInstanceExists(resourceName, &v), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"user_data"}, + }, + }, + }) +} + func TestAccEC2Instance_UserData_emptyStringToUnspecified(t *testing.T) { var v ec2.Instance resourceName := "aws_instance.test" @@ -4173,19 +4349,34 @@ resource "aws_instance" "test" { `, rName)) } -func testAccInstanceConfigWithUserDataBase64(rName string) string { +func testAccInstanceConfigWithUserData(rName, userData string) string { return acctest.ConfigCompose( acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), testAccInstanceVPCConfig(rName, false), - ` + fmt.Sprintf(` +resource "aws_instance" "test" { + ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + subnet_id = aws_subnet.test.id + + instance_type = "t2.small" + user_data = %[1]q +} +`, userData)) +} + +func testAccInstanceConfigWithUserDataBase64(rName, userData string) string { + return acctest.ConfigCompose( + acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), + testAccInstanceVPCConfig(rName, false), + fmt.Sprintf(` resource "aws_instance" "test" { ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id subnet_id = aws_subnet.test.id instance_type = "t2.small" - user_data_base64 = base64encode("hello world") + user_data_base64 = base64encode(%[1]q) } -`) +`, userData)) } func testAccInstanceConfigWithSmallInstanceType(rName string) string { @@ -4224,6 +4415,37 @@ resource "aws_instance" "test" { `) } +func testAccInstanceConfigInstanceTypeAndUserData(rName, instanceType, userData string) string { + return acctest.ConfigCompose( + acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), + testAccInstanceVPCConfig(rName, false), + fmt.Sprintf(` +resource "aws_instance" "test" { + ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + subnet_id = aws_subnet.test.id + + instance_type = %[1]q + user_data = %[2]q +} +`, instanceType, userData)) +} + +func testAccInstanceConfigInstanceTypeAndUserDataBase64(rName, instanceType, userData string) string { + return acctest.ConfigCompose( + acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), + testAccInstanceVPCConfig(rName, false), + fmt.Sprintf(` +resource "aws_instance" "test" { + ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id + subnet_id = aws_subnet.test.id + + instance_type = %[1]q + user_data_base64 = base64encode(%[2]q) + +} +`, instanceType, userData)) +} + func testAccInstanceGP2IopsDevice() string { return acctest.ConfigCompose(acctest.ConfigLatestAmazonLinuxHvmEbsAmi(), ` resource "aws_instance" "test" { diff --git a/internal/service/ec2/wait.go b/internal/service/ec2/wait.go index fe79d67d12eb..f5a0ba4a68d6 100644 --- a/internal/service/ec2/wait.go +++ b/internal/service/ec2/wait.go @@ -17,7 +17,8 @@ const ( // Maximum amount of time to wait for EC2 Instance attribute modifications to propagate InstanceAttributePropagationTimeout = 2 * time.Minute - InstanceStopTimeout = 10 * time.Minute + InstanceStartTimeout = 10 * time.Minute + InstanceStopTimeout = 10 * time.Minute // General timeout for EC2 resource creations to propagate PropagationTimeout = 2 * time.Minute