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.

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

[WIP]Transfer Server Support VPC Endpoint Type #11751

Closed
wants to merge 3 commits into from

Conversation

DrFaust92
Copy link
Collaborator

@DrFaust92 DrFaust92 commented Jan 24, 2020

Community Note

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

Closes #11724
Closes #11569
Closes #11593

Release note for CHANGELOG:

resource_aws_transfer_server: support` VPC` endpoint type

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'

...

@DrFaust92 DrFaust92 requested a review from a team January 24, 2020 18:05
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/transfer Issues and PRs that pertain to the transfer service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. labels Jan 24, 2020
@DrFaust92
Copy link
Collaborator Author

Having issues with this as describing the transfer server returns an empty list of eip association ids.
Tried putting a sleep of a few minutes before the read, didnt help. seems like a bug on AWS side.

@bazimov
Copy link

bazimov commented Jan 31, 2020

One thing I noticed, probably you know already. I tried to launch sftp from console. With this feature AWS launched VPC endpoint automatically, I did not expect that, I had to attach security group to that vpc endpoint afterwards.

@ChristianPe
Copy link

Im stuck with the same problem as the previous poster. After creating a sftp server with endpoint style "vpc" there is no way to change/attach security groups with terraform, since one can not change the network interfaces nor get the endpoint resource to attach new security groups. Or do I get something wrong here?

@marcoreni
Copy link
Contributor

@DrFaust92 according to the doc you can use EndpointDetails/AddressAllocationIds only during UpdateServer calls. Maybe this is the reason why you're obtaining an empty list during the describe?

@bazimov @ChristianPe since a vpc endpoint is internally created by AWS when using VPC Endpoint Type, maybe we can create a vpce manually in Terraform and set that inside the SFTP Server resource during creation, so that AWS doesn't create one by himself. With this we can add SGs directly to the vpce as usual. (NOTE: I haven't tried this, I'm just making assumptions).

@DrFaust92
Copy link
Collaborator Author

DrFaust92 commented Feb 13, 2020

@marcoreni, i think that's the reason as well, albeit its a stupid one IMO for AWS to do this.

for the second comment, that wont work as you cant specify an endpoint when the type is VPC

@mbelang
Copy link

mbelang commented Mar 30, 2020

What is the progress on this?

@DrFaust92
Copy link
Collaborator Author

just to update all that are expecting this, the AWS API for transfer server doesn't return the IPs allocated to it and causes a perpetual diff. i dont want to mix another api all to EC2 services for this.
ill try to think of a workaround in the next couple of days for this and update again.

@sshearn
Copy link
Contributor

sshearn commented May 4, 2020

@DrFaust92 I'm not sure what issues you're seeing exactly but I have it fully working here #12599. Including support for the VPCE SG.

@DrFaust92
Copy link
Collaborator Author

@sshearn, your PR calls 2 different service APIs. it's not a common workaround in the provider (i can't really find an an example, possibly its the same with Lambda ENIs)
i noted it's not a solution i want to work with in this PR (unless mainteners approve).

@bflad, thoughts?

@DrFaust92
Copy link
Collaborator Author

DrFaust92 commented May 4, 2020

@sshearn ill take a look again

edit, you seem to have removed your comment(?)

but i still cant read these resources:

Reading Transfer Server {
  Arn: "arn:aws:transfer:us-west-2:476956259333:server/s-dc18316d31fc45f08",
  EndpointDetails: {
    AddressAllocationIds: [],
    SubnetIds: ["subnet-0806ed7cfa276d37d","subnet-001b721b0c188b2fc"],
    VpcEndpointId: "vpce-0f3c57bbe15f1a011",
    VpcId: "vpc-0fcc896ac2289a595"
  },
  EndpointType: "VPC",
  HostKeyFingerprint: "SHA256:Xjfcza/Mzn/LucKHH9oDXi+jPdxztcgoyZYBsm+Ov9U=",
  IdentityProviderType: "SERVICE_MANAGED",
  Protocols: ["SFTP"],
  ServerId: "s-dc18316d31fc45f08",
  State: "ONLINE",
  Tags: [],
  UserCount: 0
}

AddressAllocationIds is empty

@DrFaust92
Copy link
Collaborator Author

A thought, adding an additional update call in the read func to get the IPs. I would really hate to do it but I'll give it a go

@DrFaust92
Copy link
Collaborator Author

Closing in favor of #12599

@DrFaust92 DrFaust92 closed this Jun 8, 2020
@DrFaust92 DrFaust92 deleted the transfer-server-vpc branch June 23, 2020 11:25
@ghost
Copy link

ghost commented Jul 9, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 9, 2020
@breathingdust breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/transfer Issues and PRs that pertain to the transfer service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
7 participants