From 9f9e2a3dd693ee0064a859a3d3fd9344c5e72519 Mon Sep 17 00:00:00 2001 From: Stephen Date: Tue, 31 Mar 2020 17:16:13 -0400 Subject: [PATCH 01/10] Add Support for Transfer Server VPC Endpoint Type and Transfer Server User Home Directory Type / Mapping --- aws/resource_aws_transfer_server.go | 236 +++++++++++++++++-- aws/resource_aws_transfer_server_test.go | 114 +++++++++ aws/resource_aws_transfer_user.go | 97 +++++++- aws/resource_aws_transfer_user_test.go | 100 ++++++++ website/docs/r/transfer_server.html.markdown | 5 +- website/docs/r/transfer_user.html.markdown | 7 + 6 files changed, 541 insertions(+), 18 deletions(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index 36a5e87b90ff..31e864cce081 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -7,6 +7,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/transfer" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -41,6 +42,7 @@ func resourceAwsTransferServer() *schema.Resource { Default: transfer.EndpointTypePublic, ValidateFunc: validation.StringInSlice([]string{ transfer.EndpointTypePublic, + transfer.EndpointTypeVpc, transfer.EndpointTypeVpcEndpoint, }, false), }, @@ -52,8 +54,9 @@ func resourceAwsTransferServer() *schema.Resource { Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "vpc_endpoint_id": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"endpoint_details.0.address_allocation_ids", "endpoint_details.0.subnet_ids", "endpoint_details.0.vpc_id"}, ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { value := v.(string) validNamePattern := "^vpce-[0-9a-f]{17}$" @@ -64,6 +67,32 @@ func resourceAwsTransferServer() *schema.Resource { } return }, + DiffSuppressFunc: func(k, o, n string, d *schema.ResourceData) bool { + if n == "" && d.Get("endpoint_type").(string) == transfer.EndpointTypeVpc { + return true + } + return false + }, + }, + "address_allocation_ids": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + ConflictsWith: []string{"endpoint_details.0.vpc_endpoint_id"}, + }, + "subnet_ids": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + ConflictsWith: []string{"endpoint_details.0.vpc_endpoint_id"}, + }, + "vpc_id": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.NoZeroValues, + ConflictsWith: []string{"endpoint_details.0.vpc_endpoint_id"}, }, }, }, @@ -121,6 +150,7 @@ func resourceAwsTransferServer() *schema.Resource { } func resourceAwsTransferServerCreate(d *schema.ResourceData, meta interface{}) error { + updateAfterCreate := false conn := meta.(*AWSClient).transferconn tags := keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().TransferTags() createOpts := &transfer.CreateServerInput{} @@ -156,6 +186,11 @@ func resourceAwsTransferServerCreate(d *schema.ResourceData, meta interface{}) e if attr, ok := d.GetOk("endpoint_details"); ok { createOpts.EndpointDetails = expandTransferServerEndpointDetails(attr.([]interface{})) + + if createOpts.EndpointDetails.AddressAllocationIds != nil { + createOpts.EndpointDetails.AddressAllocationIds = nil + updateAfterCreate = true + } } if attr, ok := d.GetOk("host_key"); ok { @@ -171,6 +206,17 @@ func resourceAwsTransferServerCreate(d *schema.ResourceData, meta interface{}) e d.SetId(*resp.ServerId) + if updateAfterCreate { + updateOpts := &transfer.UpdateServerInput{ + ServerId: aws.String(d.Id()), + EndpointDetails: expandTransferServerEndpointDetails(d.Get("endpoint_details").([]interface{})), + } + + if err := doTransferServerUpdate(d, updateOpts, conn, meta.(*AWSClient).ec2conn, true); err != nil { + return err + } + } + return resourceAwsTransferServerRead(d, meta) } @@ -218,6 +264,7 @@ func resourceAwsTransferServerRead(d *schema.ResourceData, meta interface{}) err func resourceAwsTransferServerUpdate(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).transferconn updateFlag := false + stopFlag := false updateOpts := &transfer.UpdateServerInput{ ServerId: aws.String(d.Id()), } @@ -252,6 +299,10 @@ func resourceAwsTransferServerUpdate(d *schema.ResourceData, meta interface{}) e if attr, ok := d.GetOk("endpoint_details"); ok { updateOpts.EndpointDetails = expandTransferServerEndpointDetails(attr.([]interface{})) } + + if d.HasChange("endpoint_details.0.address_allocation_ids") { + stopFlag = true + } } if d.HasChange("host_key") { @@ -262,14 +313,8 @@ func resourceAwsTransferServerUpdate(d *schema.ResourceData, meta interface{}) e } if updateFlag { - _, err := conn.UpdateServer(updateOpts) - if err != nil { - if isAWSErr(err, transfer.ErrCodeResourceNotFoundException, "") { - log.Printf("[WARN] Transfer Server (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - return fmt.Errorf("error updating Transfer Server (%s): %s", d.Id(), err) + if err := doTransferServerUpdate(d, updateOpts, conn, meta.(*AWSClient).ec2conn, stopFlag); err != nil { + return err } } @@ -391,9 +436,25 @@ func expandTransferServerEndpointDetails(l []interface{}) *transfer.EndpointDeta } e := l[0].(map[string]interface{}) - return &transfer.EndpointDetails{ - VpcEndpointId: aws.String(e["vpc_endpoint_id"].(string)), + out := &transfer.EndpointDetails{} + + if v, ok := e["vpc_endpoint_id"]; ok && v != "" { + out.VpcEndpointId = aws.String(v.(string)) + } + + if v, ok := e["address_allocation_ids"]; ok { + out.AddressAllocationIds = expandStringSet(v.(*schema.Set)) + } + + if v, ok := e["subnet_ids"]; ok { + out.SubnetIds = expandStringSet(v.(*schema.Set)) + } + + if v, ok := e["vpc_id"]; ok { + out.VpcId = aws.String(v.(string)) } + + return out } func flattenTransferServerEndpointDetails(endpointDetails *transfer.EndpointDetails) []interface{} { @@ -401,9 +462,156 @@ func flattenTransferServerEndpointDetails(endpointDetails *transfer.EndpointDeta return []interface{}{} } - e := map[string]interface{}{ - "vpc_endpoint_id": aws.StringValue(endpointDetails.VpcEndpointId), + e := make(map[string]interface{}) + if endpointDetails.VpcEndpointId != nil { + e["vpc_endpoint_id"] = *endpointDetails.VpcEndpointId + } + if endpointDetails.AddressAllocationIds != nil { + e["address_allocation_ids"] = endpointDetails.AddressAllocationIds + } + if endpointDetails.SubnetIds != nil { + e["subnet_ids"] = endpointDetails.SubnetIds + } + if endpointDetails.VpcId != nil { + e["vpc_id"] = *endpointDetails.VpcId } return []interface{}{e} } + +func doTransferServerUpdate(d *schema.ResourceData, updateOpts *transfer.UpdateServerInput, transferConn *transfer.Transfer, ec2Conn *ec2.EC2, stopFlag bool) error { + if stopFlag { + if err := waitForTransferServerVPCEndpointState(transferConn, ec2Conn, d.Id(), d.Timeout(schema.TimeoutCreate)); err != nil { + return fmt.Errorf("error waiting for Transfer Server VPC Endpoint (%s) to start: %s", d.Id(), err) + } + + if err := stopAndWaitForTransferServer(d.Id(), transferConn, d.Timeout(schema.TimeoutCreate)); err != nil { + return err + } + } + + _, err := transferConn.UpdateServer(updateOpts) + if err != nil { + if isAWSErr(err, transfer.ErrCodeResourceNotFoundException, "") { + log.Printf("[WARN] Transfer Server (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + return fmt.Errorf("error updating Transfer Server (%s): %s", d.Id(), err) + } + + if stopFlag { + if err := startAndWaitForTransferServer(d.Id(), transferConn, d.Timeout(schema.TimeoutCreate)); err != nil { + return err + } + } + + return nil +} + +func stopAndWaitForTransferServer(serverId string, conn *transfer.Transfer, timeout time.Duration) error { + stopReq := &transfer.StopServerInput{ + ServerId: aws.String(serverId), + } + if _, err := conn.StopServer(stopReq); err != nil { + return fmt.Errorf("error stopping Transfer Server (%s): %s", serverId, err) + } + + stateChangeConf := &resource.StateChangeConf{ + Pending: []string{transfer.StateStarting, transfer.StateOnline, transfer.StateStopping}, + Target: []string{transfer.StateOffline}, + Refresh: refreshTransferServerStatus(conn, serverId), + Timeout: timeout, + Delay: 10 * time.Second, + } + + if _, err := stateChangeConf.WaitForState(); err != nil { + return fmt.Errorf("error waiting for Transfer Server (%s) to stop: %s", serverId, err) + } + + return nil +} + +func startAndWaitForTransferServer(serverId string, conn *transfer.Transfer, timeout time.Duration) error { + stopReq := &transfer.StartServerInput{ + ServerId: aws.String(serverId), + } + + if _, err := conn.StartServer(stopReq); err != nil { + return fmt.Errorf("error starting Transfer Server (%s): %s", serverId, err) + } + + stateChangeConf := &resource.StateChangeConf{ + Pending: []string{transfer.StateStarting, transfer.StateOffline, transfer.StateStopping}, + Target: []string{transfer.StateOnline}, + Refresh: refreshTransferServerStatus(conn, serverId), + Timeout: timeout, + Delay: 10 * time.Second, + } + + if _, err := stateChangeConf.WaitForState(); err != nil { + return fmt.Errorf("error waiting for Transfer Server (%s) to start: %s", serverId, err) + } + + return nil +} + +func refreshTransferServerStatus(conn *transfer.Transfer, serverId string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + server, err := describeTransferServer(conn, serverId) + + if server == nil { + return 42, "destroyed", nil + } + + return server, aws.StringValue(server.State), err + } +} + +func describeTransferServer(conn *transfer.Transfer, serverId string) (*transfer.DescribedServer, error) { + params := &transfer.DescribeServerInput{ + ServerId: aws.String(serverId), + } + + resp, err := conn.DescribeServer(params) + + return resp.Server, err +} + +func waitForTransferServerVPCEndpointState(transferConn *transfer.Transfer, ec2Conn *ec2.EC2, serverId string, timeout time.Duration) error { + server, err := describeTransferServer(transferConn, serverId) + + stateChangeConf := &resource.StateChangeConf{ + Pending: []string{ec2.VpcStatePending}, + Target: []string{ec2.VpcStateAvailable}, + Refresh: refreshTransferServerVPCEndpointStatus(ec2Conn, server.EndpointDetails.VpcEndpointId), + Timeout: timeout, + Delay: 10 * time.Second, + } + + _, err = stateChangeConf.WaitForState() + + return err +} + +func refreshTransferServerVPCEndpointStatus(conn *ec2.EC2, vpceId *string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + server, err := describeTransferServerVPCEndpoint(conn, vpceId) + + if server == nil { + return 42, "destroyed", nil + } + + return server, aws.StringValue(server.State), err + } +} + +func describeTransferServerVPCEndpoint(conn *ec2.EC2, vpceId *string) (*ec2.VpcEndpoint, error) { + params := &ec2.DescribeVpcEndpointsInput{ + VpcEndpointIds: []*string{vpceId}, + } + + resp, err := conn.DescribeVpcEndpoints(params) + + return resp.VpcEndpoints[0], err +} diff --git a/aws/resource_aws_transfer_server_test.go b/aws/resource_aws_transfer_server_test.go index bfa5eb18c4e8..3ddb8b3db2c6 100644 --- a/aws/resource_aws_transfer_server_test.go +++ b/aws/resource_aws_transfer_server_test.go @@ -111,6 +111,47 @@ func TestAccAWSTransferServer_basic(t *testing.T) { }) } +func TestAccAWSTransferServer_Vpc(t *testing.T) { + var conf transfer.DescribedServer + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, + IDRefreshName: "aws_transfer_server.test", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSTransferServerDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSTransferServerConfig_Vpc, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSTransferServerExists("aws_transfer_server.test", &conf), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "endpoint_type", "VPC"), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "endpoint_details.0.subnet_ids.#", "1"), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + ), + }, + { + ResourceName: "aws_transfer_server.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + { + Config: testAccAWSTransferServerConfig_VpcUpdate, + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSTransferServerExists("aws_transfer_server.test", &conf), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "endpoint_type", "VPC"), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + ), + }, + }, + }) +} + func TestAccAWSTransferServer_apigateway(t *testing.T) { var conf transfer.DescribedServer rName := acctest.RandString(5) @@ -655,6 +696,79 @@ resource "aws_transfer_server" "default" { } ` +const testAccAWSTransferServerConfig_VpcDefault = ` +data "aws_region" "current" {} + +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = "terraform-testacc-vpc" + } +} + +resource "aws_internet_gateway" "test" { + vpc_id = "${aws_vpc.test.id}" + + tags = { + Name = "terraform-testacc-igw" + } +} + +resource "aws_subnet" "test" { + vpc_id = "${aws_vpc.test.id}" + cidr_block = "10.0.0.0/24" + map_public_ip_on_launch = true + + depends_on = ["aws_internet_gateway.test"] +} + +resource "aws_default_route_table" "test" { + default_route_table_id = "${aws_vpc.test.default_route_table_id}" + + route { + cidr_block = "0.0.0.0/0" + gateway_id = "${aws_internet_gateway.test.id}" + } + + tags = { + Name = "terraform-testacc-subnet" + } +} + +resource "aws_eip" "testa" { + vpc = true +} + +resource "aws_eip" "testb" { + vpc = true +} +` + +const testAccAWSTransferServerConfig_Vpc = testAccAWSTransferServerConfig_VpcDefault + ` + +resource "aws_transfer_server" "test" { + endpoint_type = "VPC" + endpoint_details { + vpc_id = "${aws_vpc.test.id}" + address_allocation_ids = ["${aws_eip.testa.id}"] + subnet_ids = ["${aws_subnet.test.id}"] + } +} +` + +const testAccAWSTransferServerConfig_VpcUpdate = testAccAWSTransferServerConfig_VpcDefault + ` + +resource "aws_transfer_server" "test" { + endpoint_type = "VPC" + endpoint_details { + vpc_id = "${aws_vpc.test.id}" + address_allocation_ids = ["${aws_eip.testb.id}"] + subnet_ids = ["${aws_subnet.test.id}"] + } +} +` + func testAccAWSTransferServerConfig_hostKey(hostKey string) string { return fmt.Sprintf(` resource "aws_transfer_server" "default" { diff --git a/aws/resource_aws_transfer_user.go b/aws/resource_aws_transfer_user.go index 4747829b4e13..75b2e2ead75a 100644 --- a/aws/resource_aws_transfer_user.go +++ b/aws/resource_aws_transfer_user.go @@ -32,14 +32,46 @@ func resourceAwsTransferUser() *schema.Resource { }, "home_directory": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringLenBetween(0, 1024), + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"home_directory_mappings"}, + ValidateFunc: validation.StringLenBetween(0, 1024), + }, + + "home_directory_type": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringInSlice([]string{ + transfer.HomeDirectoryTypeLogical, + transfer.HomeDirectoryTypePath, + }, false), + }, + + "home_directory_mappings": { + Type: schema.TypeList, + Optional: true, + MaxItems: 50, + ConflictsWith: []string{"home_directory", "policy"}, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "entry": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringLenBetween(1, 1024), + }, + "target": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringLenBetween(1, 1024), + }, + }, + }, }, "policy": { Type: schema.TypeString, Optional: true, + ConflictsWith: []string{"home_directory_mappings"}, ValidateFunc: validateIAMPolicyJson, DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs, }, @@ -84,6 +116,14 @@ func resourceAwsTransferUserCreate(d *schema.ResourceData, meta interface{}) err createOpts.HomeDirectory = aws.String(attr.(string)) } + if attr, ok := d.GetOk("home_directory_type"); ok { + createOpts.HomeDirectoryType = aws.String(attr.(string)) + } + + if attr, ok := d.GetOk("home_directory_mappings"); ok { + createOpts.HomeDirectoryMappings = expandTransferServerHomeDirectoryMappings(attr.([]interface{})) + } + if attr, ok := d.GetOk("policy"); ok { createOpts.Policy = aws.String(attr.(string)) } @@ -132,6 +172,8 @@ func resourceAwsTransferUserRead(d *schema.ResourceData, meta interface{}) error d.Set("user_name", resp.User.UserName) d.Set("arn", resp.User.Arn) d.Set("home_directory", resp.User.HomeDirectory) + d.Set("home_directory_type", resp.User.HomeDirectoryType) + d.Set("home_directory_mappings", flattenTransferServerUserHomeDirectoryMappings(resp.User.HomeDirectoryMappings)) d.Set("policy", resp.User.Policy) d.Set("role", resp.User.Role) @@ -159,6 +201,14 @@ func resourceAwsTransferUserUpdate(d *schema.ResourceData, meta interface{}) err updateFlag = true } + if d.HasChange("home_directory_type") { + updateOpts.HomeDirectoryType = aws.String(d.Get("home_directory_type").(string)) + } + + if d.HasChange("home_directory_mappings") { + updateOpts.HomeDirectoryMappings = expandTransferServerHomeDirectoryMappings(d.Get("home_directory_mappings").([]interface{})) + } + if d.HasChange("policy") { updateOpts.Policy = aws.String(d.Get("policy").(string)) updateFlag = true @@ -259,3 +309,44 @@ func waitForTransferUserDeletion(conn *transfer.Transfer, serverID, userName str } return nil } + +func expandTransferServerHomeDirectoryMappings(m []interface{}) []*transfer.HomeDirectoryMapEntry { + ms := make([]*transfer.HomeDirectoryMapEntry, 0) + + for _, v := range m { + mv := v.(map[string]interface{}) + e := &transfer.HomeDirectoryMapEntry{} + + if v, ok := mv["entry"].(string); ok && v != "" { + e.Entry = aws.String(v) + } + + if v, ok := mv["target"].(string); ok && v != "" { + e.Target = aws.String(v) + } + + ms = append(ms, e) + } + + return ms +} + +func flattenTransferServerUserHomeDirectoryMappings(m []*transfer.HomeDirectoryMapEntry) []map[string]interface{} { + ms := make([]map[string]interface{}, 0) + + for _, e := range m { + if e.Entry != nil && e.Target != nil { + m := make(map[string]interface{}) + m["entry"] = *e.Entry + m["target"] = *e.Target + + ms = append(ms, m) + } + } + + if len(ms) > 0 { + return ms + } + + return nil +} diff --git a/aws/resource_aws_transfer_user_test.go b/aws/resource_aws_transfer_user_test.go index 4cb4d35ad976..8cfb9294f2a2 100644 --- a/aws/resource_aws_transfer_user_test.go +++ b/aws/resource_aws_transfer_user_test.go @@ -43,6 +43,38 @@ func TestAccAWSTransferUser_basic(t *testing.T) { }) } +func TestAccAWSTransferUser_restricted(t *testing.T) { + var conf transfer.DescribedUser + rName := acctest.RandString(10) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, + IDRefreshName: "aws_transfer_user.foo", + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSTransferUserDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSTransferUserConfig_restricted(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), + testAccMatchResourceAttrRegionalARN("aws_transfer_user.foo", "arn", "transfer", regexp.MustCompile(`user/.+`)), + resource.TestCheckResourceAttrPair( + "aws_transfer_user.foo", "server_id", "aws_transfer_server.foo", "id"), + resource.TestCheckResourceAttrPair( + "aws_transfer_user.foo", "role", "aws_iam_role.foo", "arn"), + resource.TestCheckResourceAttr( + "aws_transfer_user.foo", "home_directory_type", "LOGICAL"), + ), + }, + { + ResourceName: "aws_transfer_user.foo", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSTransferUser_modifyWithOptions(t *testing.T) { var conf transfer.DescribedUser rName := acctest.RandString(10) @@ -291,6 +323,74 @@ resource "aws_transfer_user" "foo" { `, rName, rName) } +func testAccAWSTransferUserConfig_restricted(rName string) string { + return fmt.Sprintf(` +resource "aws_transfer_server" "foo" { + identity_provider_type = "SERVICE_MANAGED" + + tags = { + NAME = "tf-acc-test-transfer-server" + } +} + +resource "aws_iam_role" "foo" { + name = "tf-test-transfer-user-iam-role-%s" + + assume_role_policy = < Date: Tue, 31 Mar 2020 17:31:59 -0400 Subject: [PATCH 02/10] Update --- aws/resource_aws_transfer_server.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index 31e864cce081..5c78c0a83f24 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -581,6 +581,10 @@ func describeTransferServer(conn *transfer.Transfer, serverId string) (*transfer func waitForTransferServerVPCEndpointState(transferConn *transfer.Transfer, ec2Conn *ec2.EC2, serverId string, timeout time.Duration) error { server, err := describeTransferServer(transferConn, serverId) + if err != nil { + return err + } + stateChangeConf := &resource.StateChangeConf{ Pending: []string{ec2.VpcStatePending}, Target: []string{ec2.VpcStateAvailable}, From 7e05a0644c1f6bd645bfb44c8e0ee2ab3bef04e2 Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 1 Apr 2020 15:01:18 -0400 Subject: [PATCH 03/10] Add VPC Endpoint Security Group Support --- aws/resource_aws_transfer_server.go | 76 ++++++++++++++------ aws/resource_aws_transfer_server_test.go | 17 +++++ website/docs/r/transfer_server.html.markdown | 1 + 3 files changed, 73 insertions(+), 21 deletions(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index 5c78c0a83f24..572b7077da23 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -98,6 +98,14 @@ func resourceAwsTransferServer() *schema.Resource { }, }, + "vpce_security_group_ids": { + Type: schema.TypeSet, + Optional: true, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: schema.HashString, + }, + "host_key": { Type: schema.TypeString, Optional: true, @@ -255,6 +263,15 @@ func resourceAwsTransferServerRead(d *schema.ResourceData, meta interface{}) err d.Set("logging_role", resp.Server.LoggingRole) d.Set("host_key_fingerprint", resp.Server.HostKeyFingerprint) + if resp.Server.EndpointDetails.VpcEndpointId != nil { + out, err := describeTransferServerVPCEndpoint(meta.(*AWSClient).ec2conn, resp.Server.EndpointDetails.VpcEndpointId) + if err != nil { + return err + } + + d.Set("vpce_security_group_ids", flattenVpcEndpointSecurityGroupIds(out.Groups)) + } + if err := d.Set("tags", keyvaluetags.TransferKeyValueTags(resp.Server.Tags).IgnoreAws().Map()); err != nil { return fmt.Errorf("Error setting tags: %s", err) } @@ -506,6 +523,10 @@ func doTransferServerUpdate(d *schema.ResourceData, updateOpts *transfer.UpdateS } } + if err := updateTransferServerVPCEndpointSecurityGroup(d, transferConn, ec2Conn); err != nil { + return err + } + return nil } @@ -585,29 +606,11 @@ func waitForTransferServerVPCEndpointState(transferConn *transfer.Transfer, ec2C return err } - stateChangeConf := &resource.StateChangeConf{ - Pending: []string{ec2.VpcStatePending}, - Target: []string{ec2.VpcStateAvailable}, - Refresh: refreshTransferServerVPCEndpointStatus(ec2Conn, server.EndpointDetails.VpcEndpointId), - Timeout: timeout, - Delay: 10 * time.Second, + if err := vpcEndpointWaitUntilAvailable(ec2Conn, *server.EndpointDetails.VpcEndpointId, timeout); err != nil { + return err } - _, err = stateChangeConf.WaitForState() - - return err -} - -func refreshTransferServerVPCEndpointStatus(conn *ec2.EC2, vpceId *string) resource.StateRefreshFunc { - return func() (interface{}, string, error) { - server, err := describeTransferServerVPCEndpoint(conn, vpceId) - - if server == nil { - return 42, "destroyed", nil - } - - return server, aws.StringValue(server.State), err - } + return nil } func describeTransferServerVPCEndpoint(conn *ec2.EC2, vpceId *string) (*ec2.VpcEndpoint, error) { @@ -619,3 +622,34 @@ func describeTransferServerVPCEndpoint(conn *ec2.EC2, vpceId *string) (*ec2.VpcE return resp.VpcEndpoints[0], err } + +func updateTransferServerVPCEndpointSecurityGroup(d *schema.ResourceData, transferConn *transfer.Transfer, ec2conn *ec2.EC2) error { + server, err := describeTransferServer(transferConn, d.Id()) + + if err != nil { + return fmt.Errorf("error describing Transfer Server VPC Endpoint: %s", err) + } + + req := &ec2.ModifyVpcEndpointInput{ + VpcEndpointId: aws.String(*server.EndpointDetails.VpcEndpointId), + } + + if d.IsNewResource() { + out, err := describeTransferServerVPCEndpoint(ec2conn, server.EndpointDetails.VpcEndpointId) + if err != nil { + return err + } + + req.RemoveSecurityGroupIds = append(req.RemoveSecurityGroupIds, out.Groups[0].GroupId) + + setVpcEndpointCreateList(d, "vpce_security_group_ids", &req.AddSecurityGroupIds) + } else { + setVpcEndpointUpdateLists(d, "vpce_security_group_ids", &req.AddSecurityGroupIds, &req.RemoveSecurityGroupIds) + } + + if _, err := ec2conn.ModifyVpcEndpoint(req); err != nil { + return fmt.Errorf("error updating VPC Endpoint: %s", err) + } + + return nil +} diff --git a/aws/resource_aws_transfer_server_test.go b/aws/resource_aws_transfer_server_test.go index 3ddb8b3db2c6..22a2a9d305d5 100644 --- a/aws/resource_aws_transfer_server_test.go +++ b/aws/resource_aws_transfer_server_test.go @@ -130,6 +130,8 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { "aws_transfer_server.test", "endpoint_details.0.subnet_ids.#", "1"), resource.TestCheckResourceAttr( "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "vpce_security_group_ids.#", "1"), ), }, { @@ -146,6 +148,8 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { "aws_transfer_server.test", "endpoint_type", "VPC"), resource.TestCheckResourceAttr( "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + resource.TestCheckResourceAttr( + "aws_transfer_server.test", "vpce_security_group_ids.#", "0"), ), }, }, @@ -736,6 +740,15 @@ resource "aws_default_route_table" "test" { } } +resource "aws_security_group" "test" { + name = "terraform-testacc-security-group" + vpc_id = data.aws_vpc.test.id + + tags = { + Name = "terraform-testacc-security-group" + } +} + resource "aws_eip" "testa" { vpc = true } @@ -754,6 +767,10 @@ resource "aws_transfer_server" "test" { address_allocation_ids = ["${aws_eip.testa.id}"] subnet_ids = ["${aws_subnet.test.id}"] } + + vpce_security_group_ids = [ + aws_security_group.test.id + ] } ` diff --git a/website/docs/r/transfer_server.html.markdown b/website/docs/r/transfer_server.html.markdown index c2f0d437b6b1..55a341a7f11a 100644 --- a/website/docs/r/transfer_server.html.markdown +++ b/website/docs/r/transfer_server.html.markdown @@ -76,6 +76,7 @@ The following arguments are supported: * `logging_role` - (Optional) Amazon Resource Name (ARN) of an IAM role that allows the service to write your SFTP users’ activity to your Amazon CloudWatch logs for monitoring and auditing purposes. * `force_destroy` - (Optional) A boolean that indicates all users associated with the server should be deleted so that the Server can be destroyed without error. The default value is `false`. * `tags` - (Optional) A mapping of tags to assign to the resource. +* `vpce_security_group_ids` (Optional) A List of security group IDs to be applied to the automatically created VPC endpoint. **endpoint_details** requires the following: From f74d2712668dc8c0971408f0d932eead794292df Mon Sep 17 00:00:00 2001 From: Stephen Date: Fri, 1 May 2020 16:09:05 -0400 Subject: [PATCH 04/10] fmt --- aws/resource_aws_transfer_server.go | 1 - 1 file changed, 1 deletion(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index c5b833a3966f..4a8831ab6a87 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -264,7 +264,6 @@ func resourceAwsTransferServerRead(d *schema.ResourceData, meta interface{}) err d.Set("logging_role", resp.Server.LoggingRole) d.Set("host_key_fingerprint", resp.Server.HostKeyFingerprint) - if resp.Server.EndpointDetails.VpcEndpointId != nil { out, err := describeTransferServerVPCEndpoint(meta.(*AWSClient).ec2conn, resp.Server.EndpointDetails.VpcEndpointId) if err != nil { From 44fd8325931c1b496a9d5f4ce66997b38ba9a66d Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 13:38:24 -0400 Subject: [PATCH 05/10] #12599 Updates from comments --- aws/resource_aws_transfer_server.go | 11 ----- aws/resource_aws_transfer_user.go | 31 ------------- aws/resource_aws_transfer_user_test.go | 63 ++++++++++++++------------ 3 files changed, 33 insertions(+), 72 deletions(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index c7f89ba2c78c..902206b6ac1b 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -3,7 +3,6 @@ package aws import ( "fmt" "log" - "regexp" "time" "github.com/aws/aws-sdk-go/aws" @@ -57,16 +56,6 @@ func resourceAwsTransferServer() *schema.Resource { Type: schema.TypeString, Optional: true, ConflictsWith: []string{"endpoint_details.0.address_allocation_ids", "endpoint_details.0.subnet_ids", "endpoint_details.0.vpc_id"}, - ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) { - value := v.(string) - validNamePattern := "^vpce-[0-9a-f]{17}$" - validName, nameMatchErr := regexp.MatchString(validNamePattern, value) - if !validName || nameMatchErr != nil { - errors = append(errors, fmt.Errorf( - "%q must match regex '%v'", k, validNamePattern)) - } - return - }, DiffSuppressFunc: func(k, o, n string, d *schema.ResourceData) bool { if n == "" && d.Get("endpoint_type").(string) == transfer.EndpointTypeVpc { return true diff --git a/aws/resource_aws_transfer_user.go b/aws/resource_aws_transfer_user.go index bfb9d41e2be5..6f6144e24aed 100644 --- a/aws/resource_aws_transfer_user.go +++ b/aws/resource_aws_transfer_user.go @@ -38,36 +38,6 @@ func resourceAwsTransferUser() *schema.Resource { ValidateFunc: validation.StringLenBetween(0, 1024), }, - "home_directory_type": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringInSlice([]string{ - transfer.HomeDirectoryTypeLogical, - transfer.HomeDirectoryTypePath, - }, false), - }, - - "home_directory_mappings": { - Type: schema.TypeList, - Optional: true, - MaxItems: 50, - ConflictsWith: []string{"home_directory", "policy"}, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "entry": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringLenBetween(1, 1024), - }, - "target": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringLenBetween(1, 1024), - }, - }, - }, - }, - "home_directory_mappings": { Type: schema.TypeList, Optional: true, @@ -97,7 +67,6 @@ func resourceAwsTransferUser() *schema.Resource { "policy": { Type: schema.TypeString, Optional: true, - ConflictsWith: []string{"home_directory_mappings"}, ValidateFunc: validateIAMPolicyJson, DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs, }, diff --git a/aws/resource_aws_transfer_user_test.go b/aws/resource_aws_transfer_user_test.go index c91d3bf16a02..33b099dc9ff9 100644 --- a/aws/resource_aws_transfer_user_test.go +++ b/aws/resource_aws_transfer_user_test.go @@ -15,27 +15,28 @@ import ( func TestAccAWSTransferUser_basic(t *testing.T) { var conf transfer.DescribedUser + resourceName := "aws_transfer_user.foo" rName := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_user.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferUserDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferUserConfig_basic(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), - testAccMatchResourceAttrRegionalARN("aws_transfer_user.foo", "arn", "transfer", regexp.MustCompile(`user/.+`)), + testAccCheckAWSTransferUserExists(resourceName, &conf), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "transfer", regexp.MustCompile(`user/.+`)), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "server_id", "aws_transfer_server.foo", "id"), + resourceName, "server_id", "aws_transfer_server.foo", "id"), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "role", "aws_iam_role.foo", "arn"), + resourceName, "role", "aws_iam_role.foo", "arn"), ), }, { - ResourceName: "aws_transfer_user.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -45,29 +46,30 @@ func TestAccAWSTransferUser_basic(t *testing.T) { func TestAccAWSTransferUser_restricted(t *testing.T) { var conf transfer.DescribedUser + resourceName := "aws_transfer_user.foo" rName := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_user.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferUserDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferUserConfig_restricted(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), - testAccMatchResourceAttrRegionalARN("aws_transfer_user.foo", "arn", "transfer", regexp.MustCompile(`user/.+`)), + testAccCheckAWSTransferUserExists(resourceName, &conf), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "transfer", regexp.MustCompile(`user/.+`)), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "server_id", "aws_transfer_server.foo", "id"), + resourceName, "server_id", "aws_transfer_server.foo", "id"), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "role", "aws_iam_role.foo", "arn"), + resourceName, "role", "aws_iam_role.foo", "arn"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "home_directory_type", "LOGICAL"), + resourceName, "home_directory_type", "LOGICAL"), ), }, { - ResourceName: "aws_transfer_user.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, }, @@ -77,57 +79,58 @@ func TestAccAWSTransferUser_restricted(t *testing.T) { func TestAccAWSTransferUser_modifyWithOptions(t *testing.T) { var conf transfer.DescribedUser + resourceName := "aws_transfer_user.foo" rName := acctest.RandString(10) rName2 := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_user.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferUserDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferUserConfig_options(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), + testAccCheckAWSTransferUserExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "home_directory", "/home/tftestuser"), + resourceName, "home_directory", "/home/tftestuser"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.%", "3"), + resourceName, "tags.%", "3"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.NAME", "tftestuser"), + resourceName, "tags.NAME", "tftestuser"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.ENV", "test"), + resourceName, "tags.ENV", "test"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.ADMIN", "test"), + resourceName, "tags.ADMIN", "test"), ), }, { Config: testAccAWSTransferUserConfig_modify(rName2), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), + testAccCheckAWSTransferUserExists(resourceName, &conf), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "role", "aws_iam_role.foo", "arn"), + resourceName, "role", "aws_iam_role.foo", "arn"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "home_directory", "/test"), + resourceName, "home_directory", "/test"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.%", "2"), + resourceName, "tags.%", "2"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.NAME", "tf-test-user"), + resourceName, "tags.NAME", "tf-test-user"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "tags.TEST", "test2"), + resourceName, "tags.TEST", "test2"), ), }, { Config: testAccAWSTransferUserConfig_forceNew(rName2), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferUserExists("aws_transfer_user.foo", &conf), + testAccCheckAWSTransferUserExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "user_name", "tftestuser2"), + resourceName, "user_name", "tftestuser2"), resource.TestCheckResourceAttrPair( - "aws_transfer_user.foo", "role", "aws_iam_role.foo", "arn"), + resourceName, "role", "aws_iam_role.foo", "arn"), resource.TestCheckResourceAttr( - "aws_transfer_user.foo", "home_directory", "/home/tftestuser2"), + resourceName, "home_directory", "/home/tftestuser2"), ), }, }, From 0da1afdcae742e906fbbfdd40daae852f1c56d03 Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 13:49:28 -0400 Subject: [PATCH 06/10] #12599 Updates from comments --- aws/resource_aws_transfer_server.go | 8 +-- aws/resource_aws_transfer_server_test.go | 74 +++++++++++++----------- aws/resource_aws_transfer_user.go | 7 +-- 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index 902206b6ac1b..fc5398547e80 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -471,16 +471,16 @@ func flattenTransferServerEndpointDetails(endpointDetails *transfer.EndpointDeta e := make(map[string]interface{}) if endpointDetails.VpcEndpointId != nil { - e["vpc_endpoint_id"] = *endpointDetails.VpcEndpointId + e["vpc_endpoint_id"] = aws.StringValue(endpointDetails.VpcEndpointId) } if endpointDetails.AddressAllocationIds != nil { - e["address_allocation_ids"] = endpointDetails.AddressAllocationIds + e["address_allocation_ids"] = flattenStringSet(endpointDetails.AddressAllocationIds) } if endpointDetails.SubnetIds != nil { - e["subnet_ids"] = endpointDetails.SubnetIds + e["subnet_ids"] = flattenStringSet(endpointDetails.SubnetIds) } if endpointDetails.VpcId != nil { - e["vpc_id"] = *endpointDetails.VpcId + e["vpc_id"] = aws.StringValue(endpointDetails.VpcId) } return []interface{}{e} diff --git a/aws/resource_aws_transfer_server_test.go b/aws/resource_aws_transfer_server_test.go index 655888fffef1..fa339d3c716c 100644 --- a/aws/resource_aws_transfer_server_test.go +++ b/aws/resource_aws_transfer_server_test.go @@ -67,28 +67,29 @@ func testSweepTransferServers(region string) error { func TestAccAWSTransferServer_basic(t *testing.T) { var conf transfer.DescribedServer + resourceName := "aws_transfer_server.foo" rName := acctest.RandString(5) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_server.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferServerDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferServerConfig_basic, Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.foo", &conf), - testAccMatchResourceAttrRegionalARN("aws_transfer_server.foo", "arn", "transfer", regexp.MustCompile(`server/.+`)), + testAccCheckAWSTransferServerExists(resourceName, &conf), + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "transfer", regexp.MustCompile(`server/.+`)), resource.TestMatchResourceAttr( - "aws_transfer_server.foo", "endpoint", regexp.MustCompile(fmt.Sprintf("^s-[a-z0-9]+.server.transfer.%s.amazonaws.com$", testAccGetRegion()))), + resourceName, "endpoint", regexp.MustCompile(fmt.Sprintf("^s-[a-z0-9]+.server.transfer.%s.amazonaws.com$", testAccGetRegion()))), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "identity_provider_type", "SERVICE_MANAGED"), - resource.TestCheckResourceAttr("aws_transfer_server.foo", "tags.%", "0"), + resourceName, "identity_provider_type", "SERVICE_MANAGED"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), }, { - ResourceName: "aws_transfer_server.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"force_destroy"}, @@ -96,15 +97,15 @@ func TestAccAWSTransferServer_basic(t *testing.T) { { Config: testAccAWSTransferServerConfig_basicUpdate(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.foo", &conf), + testAccCheckAWSTransferServerExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.%", "2"), + resourceName, "tags.%", "2"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.NAME", "tf-acc-test-transfer-server"), + resourceName, "tags.NAME", "tf-acc-test-transfer-server"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.ENV", "test"), + resourceName, "tags.ENV", "test"), resource.TestCheckResourceAttrPair( - "aws_transfer_server.foo", "logging_role", "aws_iam_role.foo", "arn"), + resourceName, "logging_role", "aws_iam_role.foo", "arn"), ), }, }, @@ -113,29 +114,30 @@ func TestAccAWSTransferServer_basic(t *testing.T) { func TestAccAWSTransferServer_Vpc(t *testing.T) { var conf transfer.DescribedServer + resourceName := "aws_transfer_server.test" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_server.test", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferServerDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferServerConfig_Vpc, Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.test", &conf), + testAccCheckAWSTransferServerExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "endpoint_type", "VPC"), + resourceName, "endpoint_type", "VPC"), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "endpoint_details.0.subnet_ids.#", "1"), + resourceName, "endpoint_details.0.subnet_ids.#", "1"), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + resourceName, "endpoint_details.0.address_allocation_ids.#", "1"), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "vpce_security_group_ids.#", "1"), + resourceName, "vpce_security_group_ids.#", "1"), ), }, { - ResourceName: "aws_transfer_server.test", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"force_destroy"}, @@ -143,13 +145,13 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { { Config: testAccAWSTransferServerConfig_VpcUpdate, Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.test", &conf), + testAccCheckAWSTransferServerExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "endpoint_type", "VPC"), + resourceName, "endpoint_type", "VPC"), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "endpoint_details.0.address_allocation_ids.#", "1"), + resourceName, "endpoint_details.0.address_allocation_ids.#", "1"), resource.TestCheckResourceAttr( - "aws_transfer_server.test", "vpce_security_group_ids.#", "0"), + resourceName, "vpce_security_group_ids.#", "0"), ), }, }, @@ -158,28 +160,29 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { func TestAccAWSTransferServer_apigateway(t *testing.T) { var conf transfer.DescribedServer + resourceName := "aws_transfer_server.foo" rName := acctest.RandString(5) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_server.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferServerDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferServerConfig_apigateway(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.foo", &conf), + testAccCheckAWSTransferServerExists(resourceName, &conf), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "identity_provider_type", "API_GATEWAY"), + resourceName, "identity_provider_type", "API_GATEWAY"), resource.TestCheckResourceAttrSet( - "aws_transfer_server.foo", "invocation_role"), + resourceName, "invocation_role"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.%", "2"), + resourceName, "tags.%", "2"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.NAME", "tf-acc-test-transfer-server"), + resourceName, "tags.NAME", "tf-acc-test-transfer-server"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "tags.TYPE", "apigateway"), + resourceName, "tags.TYPE", "apigateway"), ), }, }, @@ -209,29 +212,30 @@ func TestAccAWSTransferServer_disappears(t *testing.T) { func TestAccAWSTransferServer_forcedestroy(t *testing.T) { var conf transfer.DescribedServer var roleConf iam.GetRoleOutput + resourceName := "aws_transfer_server.foo" rName := acctest.RandString(5) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSTransfer(t) }, - IDRefreshName: "aws_transfer_server.foo", + IDRefreshName: resourceName, Providers: testAccProviders, CheckDestroy: testAccCheckAWSTransferServerDestroy, Steps: []resource.TestStep{ { Config: testAccAWSTransferServerConfig_forcedestroy(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSTransferServerExists("aws_transfer_server.foo", &conf), + testAccCheckAWSTransferServerExists(resourceName, &conf), testAccCheckAWSRoleExists("aws_iam_role.foo", &roleConf), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "identity_provider_type", "SERVICE_MANAGED"), + resourceName, "identity_provider_type", "SERVICE_MANAGED"), resource.TestCheckResourceAttr( - "aws_transfer_server.foo", "force_destroy", "true"), + resourceName, "force_destroy", "true"), testAccCheckAWSTransferCreateUser(&conf, &roleConf, rName), testAccCheckAWSTransferCreateSshKey(&conf, rName), ), }, { - ResourceName: "aws_transfer_server.foo", + ResourceName: resourceName, ImportState: true, ImportStateVerify: true, ImportStateVerifyIgnore: []string{"force_destroy", "host_key"}, diff --git a/aws/resource_aws_transfer_user.go b/aws/resource_aws_transfer_user.go index 6f6144e24aed..4b365ae7a22d 100644 --- a/aws/resource_aws_transfer_user.go +++ b/aws/resource_aws_transfer_user.go @@ -32,10 +32,9 @@ func resourceAwsTransferUser() *schema.Resource { }, "home_directory": { - Type: schema.TypeString, - Optional: true, - ConflictsWith: []string{"home_directory_mappings"}, - ValidateFunc: validation.StringLenBetween(0, 1024), + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringLenBetween(0, 1024), }, "home_directory_mappings": { From 45bfafff482e809c92bb33d633a72df97121662c Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 14:55:22 -0400 Subject: [PATCH 07/10] #12599 Updates from comments --- website/docs/r/transfer_user.html.markdown | 3 --- 1 file changed, 3 deletions(-) diff --git a/website/docs/r/transfer_user.html.markdown b/website/docs/r/transfer_user.html.markdown index 665c28407ce8..a6dfc63f1185 100644 --- a/website/docs/r/transfer_user.html.markdown +++ b/website/docs/r/transfer_user.html.markdown @@ -77,12 +77,9 @@ The following arguments are supported: * `home_directory_mappings` - (Optional) Logical directory mappings that specify what S3 paths and keys should be visible to your user and how you want to make them visible. documented below. * `home_directory_type` - (Optional) The type of landing directory (folder) you mapped for your users' home directory. Valid values are `PATH` and `LOGICAL`. * `policy` - (Optional) An IAM JSON policy document that scopes down user access to portions of their Amazon S3 bucket. IAM variables you can use inside this policy include `${Transfer:UserName}`, `${Transfer:HomeDirectory}`, and `${Transfer:HomeBucket}`. Since the IAM variable syntax matches Terraform's interpolation syntax, they must be escaped inside Terraform configuration strings (`$${Transfer:UserName}`). These are evaluated on-the-fly when navigating the bucket. -* `home_directory_type` (Optional) The type of landing directory (folder) you want your users' home directory to be when they log into the SFTP server. If you set it to PATH, the user will see the absolute Amazon S3 bucket paths as is in their SFTP clients. If you set it `LOGICAL`, you will need to provide mappings in the `home_directory_mappings` for how you want to make S3 paths visible to your user. Allowed Values: `LOGICAL` | `PATH`. -* `home_directory_mappings` (Optional) Logical directory mappings that specify what S3 paths and keys should be visible to your user and how you want to make them visible. You will need to specify the `entry` and `target` pair, where `entry` shows how the path is made visible and `target` is the actual S3 path. If you only specify a `target`, it will be displayed as is. You will need to also make sure that your AWS IAM Role provides access to paths in `target`. In most cases, you can use this value instead of the scope down policy to lock your user down to the designated home directory ("chroot"). To do this, you can set `entry` to `/` and set `target` to the `home_directory` parameter value. * `role` - (Requirement) Amazon Resource Name (ARN) of an IAM role that allows the service to controls your user’s access to your Amazon S3 bucket. * `tags` - (Optional) A map of tags to assign to the resource. - Home Directory Mappings (`home_directory_mappings`) support the following: * `entry` - (Requirement) Represents an entry and a target. From 4349dd7cfaef65bea2fc3b0d3dba4c1712494752 Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 18:50:45 -0400 Subject: [PATCH 08/10] #12599 Updates from comments --- aws/resource_aws_transfer_server.go | 69 +---------------------------- 1 file changed, 1 insertion(+), 68 deletions(-) diff --git a/aws/resource_aws_transfer_server.go b/aws/resource_aws_transfer_server.go index fc5398547e80..8048a92af621 100644 --- a/aws/resource_aws_transfer_server.go +++ b/aws/resource_aws_transfer_server.go @@ -56,12 +56,7 @@ func resourceAwsTransferServer() *schema.Resource { Type: schema.TypeString, Optional: true, ConflictsWith: []string{"endpoint_details.0.address_allocation_ids", "endpoint_details.0.subnet_ids", "endpoint_details.0.vpc_id"}, - DiffSuppressFunc: func(k, o, n string, d *schema.ResourceData) bool { - if n == "" && d.Get("endpoint_type").(string) == transfer.EndpointTypeVpc { - return true - } - return false - }, + Computed: true, }, "address_allocation_ids": { Type: schema.TypeSet, @@ -87,14 +82,6 @@ func resourceAwsTransferServer() *schema.Resource { }, }, - "vpce_security_group_ids": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - Elem: &schema.Schema{Type: schema.TypeString}, - Set: schema.HashString, - }, - "host_key": { Type: schema.TypeString, Optional: true, @@ -253,15 +240,6 @@ func resourceAwsTransferServerRead(d *schema.ResourceData, meta interface{}) err d.Set("logging_role", resp.Server.LoggingRole) d.Set("host_key_fingerprint", resp.Server.HostKeyFingerprint) - if resp.Server.EndpointDetails.VpcEndpointId != nil { - out, err := describeTransferServerVPCEndpoint(meta.(*AWSClient).ec2conn, resp.Server.EndpointDetails.VpcEndpointId) - if err != nil { - return err - } - - d.Set("vpce_security_group_ids", flattenVpcEndpointSecurityGroupIds(out.Groups)) - } - if err := d.Set("tags", keyvaluetags.TransferKeyValueTags(resp.Server.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { return fmt.Errorf("Error setting tags: %s", err) } @@ -513,10 +491,6 @@ func doTransferServerUpdate(d *schema.ResourceData, updateOpts *transfer.UpdateS } } - if err := updateTransferServerVPCEndpointSecurityGroup(d, transferConn, ec2Conn); err != nil { - return err - } - return nil } @@ -602,44 +576,3 @@ func waitForTransferServerVPCEndpointState(transferConn *transfer.Transfer, ec2C return nil } - -func describeTransferServerVPCEndpoint(conn *ec2.EC2, vpceId *string) (*ec2.VpcEndpoint, error) { - params := &ec2.DescribeVpcEndpointsInput{ - VpcEndpointIds: []*string{vpceId}, - } - - resp, err := conn.DescribeVpcEndpoints(params) - - return resp.VpcEndpoints[0], err -} - -func updateTransferServerVPCEndpointSecurityGroup(d *schema.ResourceData, transferConn *transfer.Transfer, ec2conn *ec2.EC2) error { - server, err := describeTransferServer(transferConn, d.Id()) - - if err != nil { - return fmt.Errorf("error describing Transfer Server VPC Endpoint: %s", err) - } - - req := &ec2.ModifyVpcEndpointInput{ - VpcEndpointId: aws.String(*server.EndpointDetails.VpcEndpointId), - } - - if d.IsNewResource() { - out, err := describeTransferServerVPCEndpoint(ec2conn, server.EndpointDetails.VpcEndpointId) - if err != nil { - return err - } - - req.RemoveSecurityGroupIds = append(req.RemoveSecurityGroupIds, out.Groups[0].GroupId) - - setVpcEndpointCreateList(d, "vpce_security_group_ids", &req.AddSecurityGroupIds) - } else { - setVpcEndpointUpdateLists(d, "vpce_security_group_ids", &req.AddSecurityGroupIds, &req.RemoveSecurityGroupIds) - } - - if _, err := ec2conn.ModifyVpcEndpoint(req); err != nil { - return fmt.Errorf("error updating VPC Endpoint: %s", err) - } - - return nil -} From f0f467a490c0ee313f65b0355398665362a9b77b Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 18:51:48 -0400 Subject: [PATCH 09/10] #12599 Updates from comments --- website/docs/r/transfer_server.html.markdown | 1 - 1 file changed, 1 deletion(-) diff --git a/website/docs/r/transfer_server.html.markdown b/website/docs/r/transfer_server.html.markdown index adb1991249e4..20ae3ed91575 100644 --- a/website/docs/r/transfer_server.html.markdown +++ b/website/docs/r/transfer_server.html.markdown @@ -76,7 +76,6 @@ The following arguments are supported: * `logging_role` - (Optional) Amazon Resource Name (ARN) of an IAM role that allows the service to write your SFTP users’ activity to your Amazon CloudWatch logs for monitoring and auditing purposes. * `force_destroy` - (Optional) A boolean that indicates all users associated with the server should be deleted so that the Server can be destroyed without error. The default value is `false`. * `tags` - (Optional) A map of tags to assign to the resource. -* `vpce_security_group_ids` (Optional) A List of security group IDs to be applied to the automatically created VPC endpoint. **endpoint_details** requires the following: From e459a280563f5711f03e6f9786e82d000d718b13 Mon Sep 17 00:00:00 2001 From: Stephen Date: Wed, 23 Sep 2020 18:52:50 -0400 Subject: [PATCH 10/10] #12599 Updates from comments --- aws/resource_aws_transfer_server_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/aws/resource_aws_transfer_server_test.go b/aws/resource_aws_transfer_server_test.go index fa339d3c716c..d8d011a8bfa8 100644 --- a/aws/resource_aws_transfer_server_test.go +++ b/aws/resource_aws_transfer_server_test.go @@ -132,8 +132,6 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { resourceName, "endpoint_details.0.subnet_ids.#", "1"), resource.TestCheckResourceAttr( resourceName, "endpoint_details.0.address_allocation_ids.#", "1"), - resource.TestCheckResourceAttr( - resourceName, "vpce_security_group_ids.#", "1"), ), }, { @@ -150,8 +148,6 @@ func TestAccAWSTransferServer_Vpc(t *testing.T) { resourceName, "endpoint_type", "VPC"), resource.TestCheckResourceAttr( resourceName, "endpoint_details.0.address_allocation_ids.#", "1"), - resource.TestCheckResourceAttr( - resourceName, "vpce_security_group_ids.#", "0"), ), }, }, @@ -769,10 +765,6 @@ resource "aws_transfer_server" "test" { address_allocation_ids = ["${aws_eip.testa.id}"] subnet_ids = ["${aws_subnet.test.id}"] } - - vpce_security_group_ids = [ - aws_security_group.test.id - ] } `