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

Add ability to specify additional security groups #59

Merged

Conversation

adamcrews
Copy link
Contributor

No description provided.

variables.tf Outdated

variable "vpc_security_group_ids" {
type = list(string)
description = "Additional security group ids to apply to the cluster, in addition to the default security group composed from `allowed_cidr_blocks`"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description = "Additional security group ids to apply to the cluster, in addition to the default security group composed from `allowed_cidr_blocks`"
description = "Additional security group IDs to apply to the cluster, in addition to the provisioned default security group with ingress traffic from existing CIDR blocks and existing security groups

aknysh
aknysh previously requested changes Feb 6, 2020
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.

@adamcrews thanks again!
please see comments and rebuild README

@adamcrews adamcrews force-pushed the feature/secondary_security_groups branch from 5e5fb7e to eba1d9c Compare February 6, 2020 18:58
@adamcrews adamcrews requested a review from aknysh February 6, 2020 18:58
@maximmi
Copy link
Contributor

maximmi commented Feb 11, 2020

/codefresh run test

@maximmi
Copy link
Contributor

maximmi commented Feb 11, 2020

@adamcrews please, run terraform fmt. Tests failed due to variables.tf formatting

Copy link
Contributor

@maximmi maximmi left a comment

Choose a reason for hiding this comment

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

@adamcrews please, run terraform fmt. Tests failed due to variables.tf formatting

@adamcrews adamcrews force-pushed the feature/secondary_security_groups branch from edffc60 to 2395a03 Compare February 11, 2020 21:07
@adamcrews
Copy link
Contributor Author

Fixed the fmt error, and rebased since there was another PR merged before this one.

@maximmi
Copy link
Contributor

maximmi commented Feb 11, 2020

/codefresh run test

Copy link
Contributor

@maximmi maximmi left a comment

Choose a reason for hiding this comment

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

@adamcrews thanks

@adamcrews adamcrews removed the request for review from aknysh February 11, 2020 23:03
@maximmi maximmi dismissed aknysh’s stale review February 11, 2020 23:07

comments were addressed

@maximmi maximmi merged commit d95acc1 into cloudposse:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants