You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by franklouwers <gi...@git.apache.org> on 2015/07/17 15:35:14 UTC

[GitHub] cloudstack pull request: Fix securitygroups ingress FW for protoco...

GitHub user franklouwers opened a pull request:

    https://github.com/apache/cloudstack/pull/601

    Fix securitygroups ingress FW for protocol any and 0.0.0.0/0

    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...

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/franklouwers/cloudstack bug/securitygroups_proto_all

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/601.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #601
    
----
commit 01a96aa664db72944b71c49b163a8eedf42ed88a
Author: Frank Louwers <fr...@openminds.be>
Date:   2015-07-17T13:33:07Z

    Fix securitygroups ingress FW for protocol any and 0.0.0.0/0

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by franklouwers <gi...@git.apache.org>.
Github user franklouwers commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123706604
  
    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. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123711497
  
    @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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by franklouwers <gi...@git.apache.org>.
Github user franklouwers commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123756479
  
    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....


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix securitygroups ingress FW for protoco...

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-122313669
  
    @remibergsma @NuxRo &others can you review this?
    @franklouwers this is one of the rare cases I think squashing makes sense ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/601#discussion_r35187102
  
    --- Diff: scripts/vm/network/security_group.py ---
    @@ -860,8 +860,10 @@ def add_network_rules(vm_name, vm_id, vm_ip, signature, seqno, vmMac, rules, vif
                     for ip in ips:
                         execute("iptables -I " + vmchain + " -p icmp --icmp-type " + range + " " + direction + " " + ip + " -j "+ action)
     
    -        if allow_any and protocol != 'all':
    -            if protocol != 'icmp':
    +        if allow_any
    --- End diff --
    
    missing ":"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123697692
  
    @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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/601


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123526526
  
    LGTM, though I've not tested this with a real host


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: CLOUDSTACK-8650: Fix securitygroups ingre...

Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-123995530
  
    Awesome @franklouwers ! Thanks for the details.
    
    LGTM :+1: Merging...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix securitygroups ingress FW for protoco...

Posted by franklouwers <gi...@git.apache.org>.
Github user franklouwers commented on the pull request:

    https://github.com/apache/cloudstack/pull/601#issuecomment-122309433
  
    JIRA ticket: https://issues.apache.org/jira/browse/CLOUDSTACK-8650


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---