cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] bwsw commented on issue #2355: CLOUDSTACK-10177: Only pass IPv6 address to Security Group Python scr?
Date Thu, 01 Jan 1970 00:00:00 GMT
bwsw commented on issue #2355: CLOUDSTACK-10177: Only pass IPv6 address to Security Group Python
scr?
URL: https://github.com/apache/cloudstack/pull/2355#issuecomment-349971327
 
 
   @wido I found your commit eaffe3a13bdbcc03ed4af1f1115c147f62e3de10 in wido/ipv6-icmpv6all
with current PR I think they togeter make ipv6 in 4.10 functional, unfortunately your PR wasn't
included in 4.10 by unknown reason. 
   I suppose it's wise to keep current PR, because together:
   1. this PR (to merge to 4.11)
   2. wido/ipv6-icmpv6all (merged to 4.11)
   3. CLOUDSTACK-10138 (merged to 4.11, optional)
   
   does all necessary IPv6+sg work. But I still feel that in security_group.py code should
include try-except:
   ```python
           for ip in rule['ipv4']:
               try:
                   if protocol == 'all':
                       execute('iptables -I ' + vmchain + ' -m state --state NEW ' + direction
+ ' ' + ip + ' -j ' + action)
                   elif protocol != 'icmp':
                       execute('iptables -I ' + vmchain + ' -p ' + protocol + ' -m ' + protocol
+ ' --dport ' + range + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
                   else:
                       execute('iptables -I ' + vmchain + ' -p icmp --icmp-type ' + range
+ ' ' + direction + ' ' + ip + ' -j ' + action)
               except:
                   pass
   
           for ip in rule['ipv6']:
               try:
                   if protocol == 'all':
                       execute('ip6tables -I ' + vmchain + ' -m state --state NEW ' + direction
+ ' ' + ip + ' -j ' + action)
                   elif 'icmp' != protocol:
                       execute('ip6tables -I ' + vmchain + ' -p ' + protocol + ' -m ' + protocol
+ ' --dport ' + range + ' -m state --state NEW ' + direction + ' ' + ip + ' -j ' + action)
                   else:
                       if range == 'any':
                           execute('ip6tables -I ' + vmchain + ' -p icmpv6 ' + direction +
' ' + ip + ' -j ' + action)
                       else:
                           execute('ip6tables -I ' + vmchain + ' -p icmpv6 --icmpv6-type '
+ range + ' ' + direction + ' ' + ip + ' -j ' + action)
               except:
                   pass
   ```
   at least because user can pass wrong icmp type/code and we should handle it at least skipping
but not failing all security group configuration. So if you could improve current PR with
try-catch I think it's more reasonable than diving into: https://github.com/apache/cloudstack/pull/2320
as it includes several cherry-picks which already in master and it is quite messy and I believe
it shouldn't be merged to master. Just for those who willing to make 4.10 work.
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message