Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f/instance d/instance: Get (Windows) password data #2219

Merged
merged 4 commits into from
Mar 13, 2018
Merged

f/instance d/instance: Get (Windows) password data #2219

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b01af5
r/instance d/instance: Get (Windows) password data.
Nov 8, 2017
fe12cd7
d/instance: Acceptance tests for getting password data.
Nov 8, 2017
9124635
PR feedback tweaks.
Mar 8, 2018
9d94d53
Fix instance import.
Mar 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion aws/data_source_aws_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ func dataSourceAwsInstance() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"get_password_data": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
"password_data": {
Type: schema.TypeString,
Computed: true,
},
"public_dns": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -266,7 +275,17 @@ func dataSourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
}

log.Printf("[DEBUG] aws_instance - Single Instance ID found: %s", *instance.InstanceId)
return instanceDescriptionAttributes(d, instance, conn)
if err := instanceDescriptionAttributes(d, instance, conn); err != nil {
return err
}

if d.Get("get_password_data").(bool) {
if err := readPasswordData(d, instance, conn); err != nil {
return err
}
}

return nil
}

// Populate instance attribute fields with the returned instance
Expand Down
100 changes: 100 additions & 0 deletions aws/data_source_aws_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,84 @@ func TestAccAWSInstanceDataSource_VPCSecurityGroups(t *testing.T) {
})
}

func TestAccAWSInstanceDataSource_getPasswordData_true(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reduce these down to two acceptance tests please? true -> false and false -> true should be sufficient

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccInstanceDataSourceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "true"),
resource.TestCheckResourceAttrSet("data.aws_instance.foo", "password_data"),
),
},
},
})
}

func TestAccAWSInstanceDataSource_getPasswordData_false(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccInstanceDataSourceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "false"),
resource.TestCheckNoResourceAttr("data.aws_instance.foo", "password_data"),
),
},
},
})
}

func TestAccAWSInstanceDataSource_getPasswordData_trueToFalse(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccInstanceDataSourceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "true"),
resource.TestCheckResourceAttrSet("data.aws_instance.foo", "password_data"),
),
},
{
Config: testAccInstanceDataSourceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "false"),
resource.TestCheckNoResourceAttr("data.aws_instance.foo", "password_data"),
),
},
},
})
}

func TestAccAWSInstanceDataSource_getPasswordData_falseToTrue(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccInstanceDataSourceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "false"),
resource.TestCheckNoResourceAttr("data.aws_instance.foo", "password_data"),
),
},
{
Config: testAccInstanceDataSourceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_instance.foo", "get_password_data", "true"),
resource.TestCheckResourceAttrSet("data.aws_instance.foo", "password_data"),
),
},
},
})
}

// Lookup based on InstanceID
const testAccInstanceDataSourceConfig = `
resource "aws_instance" "web" {
Expand Down Expand Up @@ -502,3 +580,25 @@ data "aws_instance" "foo" {
instance_id = "${aws_instance.foo_instance.id}"
}
`

func testAccInstanceDataSourceConfig_getPasswordData(val bool) string {
return fmt.Sprintf(`
resource "aws_key_pair" "foo" {
key_name = "tf-acc-winpasswordtest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key name needs to be randomized for parallel or broken test runs. key_name = "tf-acc-%d" (where %d can come from the top level test function, e.g. rInt := acctest.RandInt()

public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAQEAq6U3HQYC4g8WzU147gZZ7CKQH8TgYn3chZGRPxaGmHW1RUwsyEs0nmombmIhwxudhJ4ehjqXsDLoQpd6+c7BuLgTMvbv8LgE9LX53vnljFe1dsObsr/fYLvpU9LTlo8HgHAqO5ibNdrAUvV31ronzCZhms/Gyfdaue88Fd0/YnsZVGeOZPayRkdOHSpqme2CBrpa8myBeL1CWl0LkDG4+YCURjbaelfyZlIApLYKy3FcCan9XQFKaL32MJZwCgzfOvWIMtYcU8QtXMgnA3/I3gXk8YDUJv5P4lj0s/PJXuTM8DygVAUtebNwPuinS7wwonm5FXcWMuVGsVpG5K7FGQ== tf-acc-winpasswordtest"
}

resource "aws_instance" "foo" {
# us-west-2 (oregon) Microsoft Windows Server 2016 Core on Windows Server 2016 Base 10 - 2017.10.13
ami = "ami-7730f60f"
instance_type = "t2.medium"
key_name = "${aws_key_pair.foo.key_name}"
}

data "aws_instance" "foo" {
instance_id = "${aws_instance.foo.id}"

get_password_data = %t
}
`, val)
}
55 changes: 55 additions & 0 deletions aws/resource_aws_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,17 @@ func resourceAwsInstance() *schema.Resource {
Computed: true,
},

"get_password_data": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},

"password_data": {
Type: schema.TypeString,
Computed: true,
},

"subnet_id": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -759,6 +770,10 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {
}
}

if err := readPasswordData(d, instance, conn); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -1528,6 +1543,46 @@ func readSecurityGroups(d *schema.ResourceData, instance *ec2.Instance, conn *ec
return nil
}

func readPasswordData(d *schema.ResourceData, instance *ec2.Instance, conn *ec2.EC2) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try to reduce this function's signature to one action and leave the state management outside of this function, e.g.

func getAwsEc2InstancePasswordData(instanceID string, conn *ec2.EC2) (string, error)

if !d.Get("get_password_data").(bool) {
if err := d.Set("password_data", nil); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using d.Set() with *string, we do not need the additional err checking. 👍

return err
}

return nil
}

log.Printf("[INFO] Reading password data for instance %s", d.Id())

timeout := time.Now().Add(15 * time.Minute)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: we have the resource.Retry function we regularly use for these type of retryable operations:

var passwordData string

input := &ec2.GetPasswordDataInput{
  InstanceId: aws.String(instanceID),
}

err := resource.Retry(15 * time.Minute, func() *resource.RetryError {
  out, err := conn.GetPasswordData(input)

  if err != nil {
    return resource.NonRetryableError(err)
  }

  if resp.PasswordData == nil || *resp.PasswordData == "" {
    return resource.RetryableError(fmt.Errorf("Password data is blank for instance ID: %s", instanceID))
  }

  passwordData = strings.TrimSpace(*resp.PasswordData)
  return nil
})

if err != nil {
  return "", err
}

return passwordData, nil

for {
resp, err := conn.GetPasswordData(&ec2.GetPasswordDataInput{
InstanceId: instance.InstanceId,
})

if err != nil {
return err
}

if resp.PasswordData != nil && *resp.PasswordData != "" {
passwordData := strings.TrimSpace(*resp.PasswordData)
if err := d.Set("password_data", passwordData); err != nil {
return err
}

log.Printf("[INFO] Password data read for instance %s", d.Id())
return nil
}

if time.Now().After(timeout) {
return fmt.Errorf("Timeout exceeded waiting for password data to become available for instance %s", d.Id())
}

log.Printf("[TRACE] Password data is blank for instance %s, will retry in 5s...", d.Id())
time.Sleep(5 * time.Second)
}
}

type awsInstanceOpts struct {
BlockDeviceMappings []*ec2.BlockDeviceMapping
DisableAPITermination *bool
Expand Down
120 changes: 120 additions & 0 deletions aws/resource_aws_instance_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,108 @@ func TestAccAWSInstance_associatePublic_overridePrivate(t *testing.T) {
})
}

func TestAccAWSInstance_getPasswordData_true(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reduce this down to two acceptance tests please? false -> true and true -> false should be sufficient

var before ec2.Instance
resName := "aws_instance.foo"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &before),
resource.TestCheckResourceAttr(resName, "get_password_data", "true"),
resource.TestCheckResourceAttrSet(resName, "password_data"),
),
},
},
})
}

func TestAccAWSInstance_getPasswordData_false(t *testing.T) {
var before ec2.Instance
resName := "aws_instance.foo"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &before),
resource.TestCheckResourceAttr(resName, "get_password_data", "false"),
resource.TestCheckResourceAttr(resName, "password_data", ""),
),
},
},
})
}

func TestAccAWSInstance_getPasswordData_falseToTrue(t *testing.T) {
var before, after ec2.Instance
resName := "aws_instance.foo"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &before),
resource.TestCheckResourceAttr(resName, "get_password_data", "false"),
resource.TestCheckResourceAttr(resName, "password_data", ""),
),
},
{
Config: testAccInstanceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &after),
testAccCheckInstanceNotRecreated(t, &before, &after),
resource.TestCheckResourceAttr(resName, "get_password_data", "true"),
resource.TestCheckResourceAttrSet(resName, "password_data"),
),
},
},
})
}

func TestAccAWSInstance_getPasswordData_trueToFalse(t *testing.T) {
var before, after ec2.Instance
resName := "aws_instance.foo"

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_getPasswordData(true),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &before),
resource.TestCheckResourceAttr(resName, "get_password_data", "true"),
resource.TestCheckResourceAttrSet(resName, "password_data"),
),
},
{
Config: testAccInstanceConfig_getPasswordData(false),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resName, &after),
testAccCheckInstanceNotRecreated(t, &before, &after),
resource.TestCheckResourceAttr(resName, "get_password_data", "false"),
resource.TestCheckResourceAttr(resName, "password_data", ""),
),
},
},
})
}

func testAccCheckInstanceNotRecreated(t *testing.T,
before, after *ec2.Instance) resource.TestCheckFunc {
return func(s *terraform.State) error {
Expand Down Expand Up @@ -2719,3 +2821,21 @@ resource "aws_instance" "foo" {
}
}`, rInt, rInt)
}

func testAccInstanceConfig_getPasswordData(val bool) string {
return fmt.Sprintf(`
resource "aws_key_pair" "foo" {
key_name = "tf-acc-winpasswordtest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key name needs to be randomized for parallel or broken test runs. key_name = "tf-acc-%d" (where %d can come from the top level test function, e.g. rInt := acctest.RandInt()

public_key = "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAQEAq6U3HQYC4g8WzU147gZZ7CKQH8TgYn3chZGRPxaGmHW1RUwsyEs0nmombmIhwxudhJ4ehjqXsDLoQpd6+c7BuLgTMvbv8LgE9LX53vnljFe1dsObsr/fYLvpU9LTlo8HgHAqO5ibNdrAUvV31ronzCZhms/Gyfdaue88Fd0/YnsZVGeOZPayRkdOHSpqme2CBrpa8myBeL1CWl0LkDG4+YCURjbaelfyZlIApLYKy3FcCan9XQFKaL32MJZwCgzfOvWIMtYcU8QtXMgnA3/I3gXk8YDUJv5P4lj0s/PJXuTM8DygVAUtebNwPuinS7wwonm5FXcWMuVGsVpG5K7FGQ== tf-acc-winpasswordtest"
}

resource "aws_instance" "foo" {
# us-west-2 (oregon) Microsoft Windows Server 2016 Core on Windows Server 2016 Base 10 - 2017.10.13
ami = "ami-7730f60f"
instance_type = "t2.medium"
key_name = "${aws_key_pair.foo.key_name}"

get_password_data = %t
}
`, val)
}
4 changes: 4 additions & 0 deletions website/docs/d/instance.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ exactly match a pair on the desired Instance.
several valid keys, for a full reference, check out
[describe-instances in the AWS CLI reference][1].

* `get_password_data` - (Optional) If true, wait for password data to become available and retrieve it. The password data is exported to the `password_data` attribute.

~> **NOTE:** At least one of `filter`, `instance_tags`, or `instance_id` must be specified.

~> **NOTE:** If anything other than a single match is returned by the search,
Expand Down Expand Up @@ -75,6 +77,8 @@ interpolation.
* `key_name` - The key name of the Instance.
* `monitoring` - Whether detailed monitoring is enabled or disabled for the Instance (Boolean).
* `network_interface_id` - The ID of the network interface that was created with the Instance.
* `password_data` - Base-64 encoded encrypted password data for the instance.
This attribute is only exported if `get_password_data` is true.
* `placement_group` - The placement group of the Instance.
* `private_dns` - The private DNS name assigned to the Instance. Can only be
used inside the Amazon EC2, and only available if you've enabled DNS hostnames
Expand Down
4 changes: 4 additions & 0 deletions website/docs/r/instance.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ instance. Amazon defaults this to `stop` for EBS-backed instances and
instances. See [Shutdown Behavior](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/terminating-instances.html#Using_ChangingInstanceInitiatedShutdownBehavior) for more information.
* `instance_type` - (Required) The type of instance to start
* `key_name` - (Optional) The key name to use for the instance.
* `get_password_data` - (Optional) If true, wait for password data to become available and retrieve it. The password data is exported to the `password_data` attribute.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all mentions, we should specifically add Microsoft Windows in the documentation and potentially link to the API documentation: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_GetPasswordData.html

* `monitoring` - (Optional) If true, the launched EC2 instance will have detailed monitoring enabled. (Available since v0.6.0)
* `security_groups` - (Optional) A list of security group names to associate with.
If you are creating Instances in a VPC, use `vpc_security_group_ids` instead.
Expand Down Expand Up @@ -220,6 +221,9 @@ The following attributes are exported:
* `availability_zone` - The availability zone of the instance.
* `placement_group` - The placement group of the instance.
* `key_name` - The key name of the instance
* `password_data` - Base-64 encoded encrypted password data for the instance.
This attribute is only exported if `get_password_data` is true.
Note that this encrypted value will be stored in the state file, as with all exported attributes.
* `public_dns` - The public DNS name assigned to the instance. For EC2-VPC, this
is only available if you've enabled DNS hostnames for your VPC
* `public_ip` - The public IP address assigned to the instance, if applicable. **NOTE**: If you are using an [`aws_eip`](/docs/providers/aws/r/eip.html) with your instance, you should refer to the EIP's address directly and not use `public_ip`, as this field will change after the EIP is attached.
Expand Down