Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Add support to azure provider for security group source CIDRs, and also other related drive by fixes #6855
Conversation
| @@ -333,6 +333,9 @@ type Firewaller interface { | ||
| // IngressRules returns the ingress rules applied to the whole environment. | ||
| // Must only be used if the environment was setup with the | ||
| // FwGlobal firewall mode. | ||
| + // If is expected that there be only one ingress rule result for a given |
| - rule, err = network.NewIngressRule("tcp", 80, 100) | ||
| - c.Assert(err, jc.ErrorIsNil) | ||
| + rule = network.MustNewIngressRule("tcp", 80, 80, "0.0.0.0/0") | ||
| + c.Assert( |
| + | ||
| + // Record the SourceAddressPrefix for the port range. | ||
| + remotePrefix := "0.0.0.0/0" | ||
| + if rule.Properties.SourceAddressPrefix != nil && *rule.Properties.SourceAddressPrefix != "*" { |
axw
Jan 23, 2017
Member
s/rule.Properties.SourceAddressPrefix != nil && *rule.Properties.SourceAddressPrefix/to.String(rule.Properties.SourceAddressPrefix)/
| + if rule.Properties.SourceAddressPrefix != nil && *rule.Properties.SourceAddressPrefix != "*" { | ||
| + remotePrefix = *rule.Properties.SourceAddressPrefix | ||
| + } | ||
| + var ( |
axw
Jan 23, 2017
Member
these don't need to be declared here? prefer if you declare them inside the loop, where they're needed
| for _, protocol := range protocols { | ||
| portRange.Protocol = protocol | ||
| - rules = append(rules, jujunetwork.RulesFromPortRanges(portRange)...) | ||
| + if sourceCIDRs, ok = portSourceCIDRs[portRange]; !ok { |
axw
Jan 23, 2017
Member
can we please just
sourceCIDRs, ok := portSourceCIDRs[portRange]
if !ok {
sourceCIDRs = &[]string{}
portSourceCIDRs[portRange] = sourceCIDRs
}
*sourceCIDRs = append(*sourceCIDRs, remotePrefix)| + rules = append(rules, rule) | ||
| + } | ||
| + jujunetwork.SortIngressRules(rules) | ||
| + fmt.Println(rules) |
| @@ -275,7 +275,7 @@ func (s *instanceSuite) TestMultipleInstanceAddresses(c *gc.C) { | ||
| )) | ||
| } | ||
| -func (s *instanceSuite) TestInstancePortsEmpty(c *gc.C) { | ||
| +func (s *instanceSuite) TestInstanceRulesEmpty(c *gc.C) { |
| @@ -284,7 +284,7 @@ func (s *instanceSuite) TestInstancePortsEmpty(c *gc.C) { | ||
| c.Assert(rules, gc.HasLen, 0) | ||
| } | ||
| -func (s *instanceSuite) TestInstancePorts(c *gc.C) { | ||
| +func (s *instanceSuite) TestInstanceRules(c *gc.C) { |
|
QA
|
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit fcf9f77
into
juju:develop
Jan 23, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wallyworld commentedJan 23, 2017
This PR adds support in the Azure provider for opening/closing ports with CIDR rules.
There's also cleanup from work landed previously.
The existing Juju firewaller does not yet know about handling the new source CIDR rules so the user visible functionality remains as it is now - ports are opened to the world.
QA - smoke test on AWS