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

fix: Add vpc_config.cluster_security_group output as primary cluster security group id #828

Merged

Conversation

slimm609
Copy link
Contributor

PR o'clock

Add cluster security group output as "primary cluster security group id".

This is a short term fix to provide the cluster security group id until 1.16 is released and node groups can be refactored to move cluster security group id to additional security group id. With 1.16 release, 1.13 will no longer be maintained which will cause the issues with backwards compatibility to go away because the old node groups will no longer be supported.

Description

#799
and
#792

Checklist

@slimm609 slimm609 changed the title Add cluster security group id output as additional output fix: Add cluster security group id output as additional output Apr 11, 2020
@barryib
Copy link
Member

barryib commented Apr 13, 2020

Thanks @slimm609 for opening a new suggestion for this issue. Holding this PR a bit so we can move forward on #799 discussion

Copy link
Member

@barryib barryib left a comment

Choose a reason for hiding this comment

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

@slimm609 Thanks again for pushing this. Can you please add some description on cluster_security_group_id as @dpiddockcmp proposed #799 (comment) ?

outputs.tf Outdated Show resolved Hide resolved
@barryib barryib removed the on hold label Apr 22, 2020
@barryib barryib requested a review from dpiddockcmp April 23, 2020 15:15
Copy link
Member

@barryib barryib left a comment

Choose a reason for hiding this comment

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

LGTM

@dpiddockcmp please review.

Copy link
Contributor

@dpiddockcmp dpiddockcmp left a comment

Choose a reason for hiding this comment

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

Looks good to me too 👍

@barryib barryib changed the title fix: Add cluster security group id output as additional output fix: Add vpc_config.cluster_security_group output as primary cluster security group id Apr 23, 2020
@barryib barryib merged commit 9c2a718 into terraform-aws-modules:master Apr 23, 2020
@barryib
Copy link
Member

barryib commented Apr 23, 2020

Awsome. Thanks @slimm609 and @dpiddockcmp.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
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.

3 participants