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

resource_aws_instance: allow private instance in public subnet. #1200

Closed
wants to merge 1 commit into from

Conversation

alejandroEsc
Copy link

We allow the proper use of the associate_public_ip_addresses flag which means the ability to create a private instance when defining a public subnet id.

To summarize:
Currently the key associate_public_ip_addresses can be in three states: true, false, nil (not defined). The behavior of the provider, because of how GetOk behaves sets the value to nil when false was the value in the template. This is not the expected behavior, and the default type should NOT be boolean in the schema.

Please note this is also related to the PR: #1185

Why we need this:
When launching kubernetes nodes, it may become important to deploy instances (such as in the case of etcd) and ensure private ips only.

Note to reviewer:
Please feel free to help me make this cleaner/clearer and consistent with the expectations of this repo, which I am new to. Thank you for your time!

… of the flag which means the

ability to create a private instance when defining a public subnet id.
@alejandroEsc alejandroEsc changed the title AssociatePublicIpAddress logic changed a bit. resource_aws_instance: AssociatePublicIPAddress logic changed. Jul 21, 2017
@alejandroEsc alejandroEsc changed the title resource_aws_instance: AssociatePublicIPAddress logic changed. resource_aws_instance: AssociatePublicIPAddress flag logic changed. Jul 21, 2017
@alejandroEsc alejandroEsc changed the title resource_aws_instance: AssociatePublicIPAddress flag logic changed. resource_aws_instance: allow private instance in public subnet. Jul 21, 2017
@alejandroEsc
Copy link
Author

@radeksimko this resolves a bug tightly related to PR: #1185 mentioned above. Could I get an eta when this would be reviewed?

@grubernaut
Copy link
Contributor

Hi @alejandroEsc, thank you so much for the contribution!

For now, going to close this PR with the response given in #227, but happy to re-open and discuss further if needed.
Thanks again!

@grubernaut grubernaut closed this Aug 1, 2017
@alejandroEsc
Copy link
Author

@grubernaut please warn and contact me before closing PR and allow for a discussion. I would like to strongly ask you to revert and open this PR again. I have, as you said, left a comment on #227 as a response.

@ghost
Copy link

ghost commented Apr 11, 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 Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants