Add firewall CLI and facades #7826

Merged
merged 1 commit into from Sep 5, 2017

Conversation

Projects
None yet
4 participants
Owner

wallyworld commented Sep 5, 2017

Description of change

Add a new CLI for setting and listing firewall rules.
Most of this PR is api/apiserver boiler plate. There's also a driveby fix for the state firewall rule save api.
CMR will use the juju-application-offer rule to control allowed ingress for cross model relations.
Allowance has also been made for specifying rules ssh and controller api ingress (but those are not currently implemented).

QA steps

bootstrap
$ juju set-firewall-rule ssh --blacklist 10.0.0.0/8 --whitelist 192.168.0.0/16
$ juju firewall-rules
Service Whitelist subnets Blacklist subnets
ssh 192.168.0.0/16 10.0.0.0/8

Documentation changes

Doc will be needed when it's all done.

axw approved these changes Sep 5, 2017

cmd/juju/firewall/listrules.go
+ if err != nil {
+ return err
+ }
+ if len(rulesResult) == 0 {
@axw

axw Sep 5, 2017

Member

there's a bug saying that the output (for some other data type) should always be valid YAML/JSON if one of those is requested. can you please make them output an empty list?

state/firewallrules.go
-func (fw *firewallRulesState) Save(service WellKnownServiceType, whiteListCidrs, blackListCidrs []string) (*FirewallRule, error) {
- if err := service.validate(); err != nil {
- return nil, errors.Trace(err)
+func (fw *firewallRulesState) Save(rule *FirewallRule) error {
@axw

axw Sep 5, 2017

Member

please make it a non-pointer type. then you can be sure it's non-nil, and the caller can be sure that this method won't (can't) modify it

Owner

wallyworld commented Sep 5, 2017

$$merge$$

Contributor

jujubot commented Sep 5, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

Contributor

jujubot commented Sep 5, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/222

Owner

wallyworld commented Sep 5, 2017

$$merge$$

Contributor

jujubot commented Sep 5, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

@jujubot jujubot merged commit 077357b into juju:develop Sep 5, 2017

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
Owner

nskaggs commented Sep 5, 2017

@pmatulis This will need docs. @wallyworld we'll also need some CI around this.

@pmatulis pmatulis referenced this pull request in juju/docs Sep 5, 2017

Closed

[2.3] firewall CLI and facades #2050

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment