Skip to content

CLOUDSTACK-8650: Fix securitygroups ingress FW for protocol any and 0.0.0.0/0#601

Closed
franklouwers wants to merge 2 commits into
apache:masterfrom
franklouwers:bug/securitygroups_proto_all
Closed

CLOUDSTACK-8650: Fix securitygroups ingress FW for protocol any and 0.0.0.0/0#601
franklouwers wants to merge 2 commits into
apache:masterfrom
franklouwers:bug/securitygroups_proto_all

Conversation

@franklouwers

@franklouwers franklouwers commented Jul 17, 2015

Copy link
Copy Markdown

When using security groups, adding an ingress rule for protocol "any" with source address 0.0.0.0/0, resulted in no action (as the 0.0.0.0/0 entry was stripped from the array of source ips, but unlike icmp/tcp/udp, no special action was set for the handling of the allow_any flag.

This oneliner only removes 0.0.0.0/0 from the list if the protocol isn't any...

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-rats #78 SUCCESS
This pull request looks good

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-requests #776 SUCCESS
This pull request looks good

@franklouwers franklouwers force-pushed the bug/securitygroups_proto_all branch from 61c6d3e to e9c6233 Compare July 17, 2015 15:13
@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-analysis #11 SUCCESS
This pull request looks good

@franklouwers

franklouwers commented Jul 17, 2015

Copy link
Copy Markdown
Author

JIRA ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-8650

@DaanHoogland

DaanHoogland commented Jul 17, 2015

Copy link
Copy Markdown
Contributor

@remibergsma @NuxRo &others can you review this?
@franklouwers this is one of the rare cases I think squashing makes sense ;)

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-rats #83 SUCCESS
This pull request looks good

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-rats #84 SUCCESS
This pull request looks good

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-requests #781 SUCCESS
This pull request looks good

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-requests #782 SUCCESS
This pull request looks good

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-analysis #16 ABORTED

@asfbot

asfbot commented Jul 17, 2015

Copy link
Copy Markdown

cloudstack-pull-analysis #17 UNSTABLE
Looks like there's a problem with this pull request

CLOUDSTACK-8650: Fix securitygroups ingress FW for protocol any and 0…
2fa35c2 
….0.0.0/0

Change way 0.0.0.0/0 + all is handles, as per feedback in Slack channel
@franklouwers franklouwers force-pushed the bug/securitygroups_proto_all branch from e9c6233 to 2fa35c2 Compare July 20, 2015 08:24
@franklouwers franklouwers changed the title Fix securitygroups ingress FW for protocol any and 0.0.0.0/0 CLOUDSTACK-8650: Fix securitygroups ingress FW for protocol any and 0.0.0.0/0 Jul 20, 2015
@yadvr

yadvr commented Jul 22, 2015

Copy link
Copy Markdown
Member

LGTM, though I've not tested this with a real host

resmo
resmo reviewed Jul 22, 2015
View reviewed changes
Comment thread scripts/vm/network/security_group.py Outdated

@resmo resmo Jul 22, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing ":"

@wilderrodrigues

wilderrodrigues commented Jul 22, 2015

Copy link
Copy Markdown
Contributor

@bhaisaab I was going to ask @franklouwers how did he test his changes, since I would like to test them before giving a LGTM.

Actually, I think you were the only one to LGTM it. :)

Cheers,
Wilder

@franklouwers

franklouwers commented Jul 22, 2015

Copy link
Copy Markdown
Author

All,

I'll check the typo (I know how it happend) later this afternoon. Will also provide logs both before (bad behaviour: rule not installed) and after (good thing: rule installed) later today.

@yadvr

yadvr commented Jul 22, 2015

Copy link
Copy Markdown
Member

@wilderrodrigues yeah, as I said I've not tested it. Just had a glance at the code, but good that @resmo pointed out the typo.

@asfbot

asfbot commented Jul 22, 2015

Copy link
Copy Markdown

cloudstack-pull-rats #104 SUCCESS
This pull request looks good

@franklouwers

franklouwers commented Jul 22, 2015

Copy link
Copy Markdown
Author

See updated commit to fix the missing : .

See also https://gist.github.com/franklouwers/d5061b4ef50e2b4253fe with logs of what works, what doesn't work, and how this PR makes it work....

@asfbot

asfbot commented Jul 22, 2015

Copy link
Copy Markdown

cloudstack-pull-requests #802 UNSTABLE
Looks like there's a problem with this pull request

@asfbot

asfbot commented Jul 22, 2015

Copy link
Copy Markdown

cloudstack-pull-analysis #37 UNSTABLE
Looks like there's a problem with this pull request

@wilderrodrigues

wilderrodrigues commented Jul 23, 2015

Copy link
Copy Markdown
Contributor

Awesome @franklouwers ! Thanks for the details.

LGTM 👍 Merging...

@asfgit asfgit closed this in d8f37c5 Jul 23, 2015
maneesha-p pushed a commit to maneesha-p/cloudstack that referenced this pull request Jul 31, 2015
Sorry about the typo
bc2025e 
Signed-off-by: wilderrodrigues <wrodrigues@schubergphilis.com>

This closes apache#601
franklouwers pushed a commit to openminds/cloudstack that referenced this pull request Oct 5, 2015
Backport of apache#601
569568b
yadvr pushed a commit that referenced this pull request Jan 20, 2021
@davidjumani @yadvr
src: assorted bug fixes (#594)
58dddc2 
Fixes:
#588
#589
#590
#591
#601
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@franklouwers @asfbot @DaanHoogland @yadvr @wilderrodrigues @resmo