Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional notes to firewall rules #65

Merged
merged 2 commits into from
Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions cmd/commands_firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,15 @@ func firewallGroupList(cmd *cli.Cmd) {
}

func firewallRuleCreate(cmd *cli.Cmd) {
cmd.Spec = "-g -n ((--tcp --port) | (--udp --port) | --icmp | --gre)"
cmd.Spec = "-g -n ((--tcp --port) | (--udp --port) | --icmp | --gre) [--notes]"
gid := cmd.StringOpt("g group-id", "", "Firewall group ID (see <firewall group list>)")
cidr := cmd.StringOpt("n network", "0.0.0.0/0", "IPv4/IPv6 network in CIDR notation")
tcp := cmd.BoolOpt("tcp", false, "TCP protocol")
udp := cmd.BoolOpt("udp", false, "UDP protocol")
icmp := cmd.BoolOpt("icmp", false, "ICMP protocol")
gre := cmd.BoolOpt("gre", false, "GRE protocol")
port := cmd.StringOpt("port", "", "Port number or port range (TCP/UDP only)")
notes := cmd.StringOpt("notes", "", "Optional note")

cmd.Action = func() {
var protocol string
Expand All @@ -110,15 +111,15 @@ func firewallRuleCreate(cmd *cli.Cmd) {
log.Fatalf("Invalid network CIDR: %s", *cidr)
}

ruleNum, err := GetClient().CreateFirewallRule(*gid, protocol, *port, network)
ruleNum, err := GetClient().CreateFirewallRule(*gid, protocol, *port, network, *notes)
if err != nil {
log.Fatal(err)
}

fmt.Printf("Firewall rule created\n\n")
lengths := []int{10, 10, 10, 12, 20}
tabsPrint(columns{"GROUP_ID", "RULE_NUM", "PROTOCOL", "PORT", "NETWORK"}, lengths)
tabsPrint(columns{*gid, ruleNum, protocol, *port, network}, lengths)
lengths := []int{10, 10, 10, 12, 20, 30}
tabsPrint(columns{"GROUP_ID", "RULE_NUM", "PROTOCOL", "PORT", "NETWORK", "NOTES"}, lengths)
tabsPrint(columns{*gid, ruleNum, protocol, *port, network, *notes}, lengths)
tabsFlush()
}
}
Expand Down Expand Up @@ -154,15 +155,16 @@ func firewallRuleList(cmd *cli.Cmd) {
return
}

lengths := []int{10, 10, 8, 12, 20}
tabsPrint(columns{"RULE_NUM", "ACTION", "PROTOCOL", "PORT", "NETWORK"}, lengths)
lengths := []int{10, 10, 8, 12, 20, 30}
tabsPrint(columns{"RULE_NUM", "ACTION", "PROTOCOL", "PORT", "NETWORK", "NOTES"}, lengths)
for _, r := range rules {
tabsPrint(columns{
r.RuleNumber,
r.Action,
r.Protocol,
r.Port,
r.Network.String(),
r.Notes,
}, lengths)
}
tabsFlush()
Expand Down
8 changes: 7 additions & 1 deletion lib/firewall.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type FirewallRule struct {
Protocol string `json:"protocol"`
Port string `json:"port"`
Network *net.IPNet
Notes string `json:"notes"`
}

type firewallGroups []FirewallGroup
Expand Down Expand Up @@ -83,6 +84,7 @@ func (r *FirewallRule) UnmarshalJSON(data []byte) (err error) {
r.Action = fmt.Sprintf("%v", fields["action"])
r.Protocol = fmt.Sprintf("%v", fields["protocol"])
r.Port = fmt.Sprintf("%v", fields["port"])
r.Notes = fmt.Sprintf("%v", fields["notes"])
subnet := fmt.Sprintf("%v", fields["subnet"])
if subnet == "<nil>" {
subnet = ""
Expand Down Expand Up @@ -198,7 +200,7 @@ func (c *Client) GetFirewallRules(groupID string) ([]FirewallRule, error) {
// protocol must be one of: "icmp", "tcp", "udp", "gre"
// port can be a port number or colon separated port range (TCP/UDP only)
func (c *Client) CreateFirewallRule(groupID, protocol, port string,
network *net.IPNet) (int, error) {
network *net.IPNet, notes string) (int, error) {
Copy link
Contributor Author

@nstapelbroek nstapelbroek Mar 19, 2019

Choose a reason for hiding this comment

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

After reading #40 I think we should maybe create a CreateFirewallRuleWithNote and leave this method signature untouched. What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

No, I think it's fine this way. We'll just bump the major version number since its a breaking aPI change, but not really a big deal. Anyone using external golang code as a library should vendor his dependencies imho, and thus not have any problem with such changes. 😄

ip := network.IP.String()
maskBits, _ := network.Mask.Size()
if ip == "<nil>" {
Expand Down Expand Up @@ -230,6 +232,10 @@ func (c *Client) CreateFirewallRule(groupID, protocol, port string,
values.Add("port", port)
}

if len(notes) > 0 {
values.Add("notes", notes)
}

var result FirewallRule
err := c.post(`firewall/rule_create`, values, &result)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion lib/firewall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func Test_Firewall_CreateRule_Ok(t *testing.T) {
defer server.Close()

_, netw, _ := net.ParseCIDR("10.234.22.0/24")
num, err := client.CreateFirewallRule("1a", "tcp", "80", netw)
num, err := client.CreateFirewallRule("1a", "tcp", "80", netw, "some-example-note")
if err != nil {
t.Error(err)
}
Expand Down