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.

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

Supporting more rules for egress #165

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Supporting more rules for egress #165

wants to merge 9 commits into from

Conversation

vale21
Copy link

@vale21 vale21 commented Apr 24, 2023

what

Support the ability to specify CIDR blocks and additional security groups for egress as well

why

improve security groups

references

Fixes #164

@vale21 vale21 requested review from a team as code owners April 24, 2023 14:07
aknysh
aknysh reviewed Apr 24, 2023
View reviewed changes
variables.tf Outdated Show resolved Hide resolved
aknysh
aknysh reviewed Apr 24, 2023
View reviewed changes
variables.tf Outdated Show resolved Hide resolved
aknysh
aknysh reviewed Apr 24, 2023
View reviewed changes
variables.tf Outdated Show resolved Hide resolved
vale21 and others added 2 commits April 24, 2023 16:16
@vale21 @aknysh
Update variables.tf
613f799 
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
@vale21 @aknysh
Update variables.tf
bd5430c 
Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
aknysh
aknysh reviewed Apr 24, 2023
View reviewed changes
variables.tf Outdated Show resolved Hide resolved
aknysh
aknysh reviewed Apr 24, 2023
View reviewed changes
main.tf Outdated Show resolved Hide resolved
aknysh
aknysh requested changes Apr 24, 2023
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.

@vale21 thanks for the PR.

Please see the comments.
Please also format all TF code by running terraform fmt.
And also, please run the following commands:

make init
make github/init
make readme

and commit the changes

thanks

@vale21 vale21 requested a review from a team as a code owner April 24, 2023 14:31
@vale21
Copy link
Author

vale21 commented Apr 24, 2023

Thank you @aknysh. Changes integrated

@aknysh
Copy link
Member

aknysh commented Apr 24, 2023

/test all

1 similar comment
@aknysh
Copy link
Member

aknysh commented Apr 24, 2023

/test all

@Nuru
Copy link
Sponsor Contributor

Nuru commented Apr 26, 2023

/test all

Nuru
Nuru previously approved these changes Apr 26, 2023
View reviewed changes
@vale21 vale21 requested a review from aknysh April 26, 2023 08:57
@vale21
Copy link
Author

vale21 commented May 4, 2023 •
edited

Hello guys. Any update on this please? Is something still pending? Can it be merged?

@srhopkins @florian0410 @aknysh

1 similar comment
@vale21
Copy link
Author

vale21 commented May 25, 2023

Hello guys. Any update on this please? Is something still pending? Can it be merged?

@srhopkins @florian0410 @aknysh

@vale21
Copy link
Author

vale21 commented Jun 20, 2023

@aknysh could you plese review the changes that I pushed? This PR is blocked since a lot of time

@vale21
Copy link
Author

vale21 commented Jul 17, 2023 •
edited

@aknysh @Nuru guys, I've incorporated the requested changes. can I have a review please?

@taliesins
Copy link

@aknysh and @Nuru this seems like an easy win. What do we need to do to get this merged in? I believe @vale21 has made all the requested changes.

@vale21 vale21 requested a review from Nuru August 30, 2023 13:06
@vale21
Copy link
Author

vale21 commented Aug 30, 2023

@vale21 thanks for the PR.

Please see the comments. Please also format all TF code by running terraform fmt. And also, please run the following commands:

make init
make github/init
make readme

and commit the changes

thanks

Hi @aknysh, this has been done a while ago. Can you review please?

@hans-d
Copy link

hans-d commented Mar 2, 2024

@aknysh can you re review?

@mergify Mergify
Copy link

mergify bot commented Mar 9, 2024

Thanks @vale21 for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added the triage Needs triage label Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srhopkins srhopkins Awaiting requested review from srhopkins srhopkins is a code owner automatically assigned from cloudposse/contributors

@florian0410 florian0410 Awaiting requested review from florian0410 florian0410 is a code owner automatically assigned from cloudposse/contributors

@aknysh aknysh Awaiting requested review from aknysh

@Nuru Nuru Awaiting requested review from Nuru

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
triage Needs triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot treat Security Group egress in the same way as we do with ingress
6 participants
@vale21 @aknysh @Nuru @taliesins @hans-d @cloudpossebot