Skip to content

Make cluster security group idempotent [GH-49] #50

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Make cluster security group idempotent [GH-49] #50

wants to merge 3 commits into from

Conversation

@DownRangeDevOps
Copy link

@DownRangeDevOps DownRangeDevOps commented Oct 25, 2019

Resolves #49 and #20
aknysh
aknysh requested changes Oct 25, 2019
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

@techfishio
thanks, looks good and we need to do it (get rid of the inline SG rules).
Please make these two changes:

  1. Remove the egress inline rule as well and use aws_security_group_rule for it - we should not mix inline rules with aws_security_group_rule - TF does not like it

  2. Rebuild README by executing:

make init make readme/deps make readme

thanks
@DownRangeDevOps
Copy link
Author

DownRangeDevOps commented Oct 28, 2019

@aknysh make init hangs indefinitely for me. It seems that the curl command to download your build-harness isn't working as expected? No idea if this is on my end or not.

I updated the TF code to move the egress block to a rule.
@aknysh
Copy link
Member

aknysh commented Oct 28, 2019

@techfishio
If you open this in the browser, does it return the script https://git.io/build-harness
@DownRangeDevOps
Copy link
Author

DownRangeDevOps commented Oct 29, 2019

@aknysh nm, reboot fixed it.

Readme updated.
aknysh
aknysh reviewed Oct 29, 2019
View reviewed changes
aknysh
aknysh reviewed Oct 29, 2019
View reviewed changes
aknysh
aknysh reviewed Oct 29, 2019
View reviewed changes
aknysh
aknysh reviewed Oct 29, 2019
View reviewed changes
aknysh
aknysh requested changes Oct 29, 2019
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @techfishio
please see comments
@DownRangeDevOps
Copy link
Author

DownRangeDevOps commented Oct 30, 2019

@aknysh thanks. Updated.
@MSR-RyanFisher
Address code review comments
1a431a5 
* Ensure all resources respect `enabled` * Convert single item lists to strings where required
@MSR-RyanFisher
Add a variable to customize the Redis DNS name
c9b9365
@karlskewes karlskewes mentioned this pull request Nov 18, 2019
Closed
@legal90
Copy link

legal90 commented Dec 6, 2019

@aknysh Could you please review this PR?
aknysh
aknysh requested changes Dec 6, 2019
View reviewed changes
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @techfishio
please rebuild README by executing these commands:

make init make readme/deps make readme
@aknysh aknysh mentioned this pull request Dec 23, 2019
Merged
@aknysh aknysh closed this in #54 Dec 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@DownRangeDevOps @aknysh @legal90 @MSR-RyanFisher