From 20901a6478d4b3320c72fdbd2cde5fa5040689f9 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 21 Jan 2016 08:57:59 +0100 Subject: [PATCH] Add a check to see if the port value is valid Without this additional check it could happen that one of the firewall resources would panic is given an unexpected port value. --- .../resource_cloudstack_egress_firewall.go | 20 +++++++++++-------- .../resource_cloudstack_firewall.go | 20 +++++++++++-------- .../resource_cloudstack_network_acl_rule.go | 18 ++++++++++------- builtin/providers/cloudstack/resources.go | 3 +++ 4 files changed, 38 insertions(+), 23 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go index 41909f90e..6119a2441 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go @@ -2,7 +2,6 @@ package cloudstack import ( "fmt" - "regexp" "strconv" "strings" "sync" @@ -198,9 +197,6 @@ func createEgressFirewallRule( // Create an empty schema.Set to hold all processed ports ports := &schema.Set{F: schema.HashString} - // Define a regexp for parsing the port - re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`) - for _, port := range ps.List() { if _, ok := uuids[port.(string)]; ok { ports.Add(port) @@ -208,7 +204,7 @@ func createEgressFirewallRule( continue } - m := re.FindStringSubmatch(port.(string)) + m := splitPorts.FindStringSubmatch(port.(string)) startPort, err := strconv.Atoi(m[1]) if err != nil { @@ -536,7 +532,7 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte protocol := rule["protocol"].(string) if protocol != "tcp" && protocol != "udp" && protocol != "icmp" { return fmt.Errorf( - "%s is not a valid protocol. Valid options are 'tcp', 'udp' and 'icmp'", protocol) + "%q is not a valid protocol. Valid options are 'tcp', 'udp' and 'icmp'", protocol) } if protocol == "icmp" { @@ -549,9 +545,17 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte "Parameter icmp_code is a required parameter when using protocol 'icmp'") } } else { - if _, ok := rule["ports"]; !ok { + if ports, ok := rule["ports"].(*schema.Set); ok { + for _, port := range ports.List() { + m := splitPorts.FindStringSubmatch(port.(string)) + if m == nil { + return fmt.Errorf( + "%q is not a valid port value. Valid options are '80' or '80-90'", port.(string)) + } + } + } else { return fmt.Errorf( - "Parameter port is a required parameter when using protocol 'tcp' or 'udp'") + "Parameter ports is a required parameter when *not* using protocol 'icmp'") } } diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_firewall.go index b8f92b553..7bb60ef28 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_firewall.go @@ -2,7 +2,6 @@ package cloudstack import ( "fmt" - "regexp" "strconv" "strings" "sync" @@ -199,9 +198,6 @@ func createFirewallRule( // Create an empty schema.Set to hold all processed ports ports := &schema.Set{F: schema.HashString} - // Define a regexp for parsing the port - re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`) - for _, port := range ps.List() { if _, ok := uuids[port.(string)]; ok { ports.Add(port) @@ -209,7 +205,7 @@ func createFirewallRule( continue } - m := re.FindStringSubmatch(port.(string)) + m := splitPorts.FindStringSubmatch(port.(string)) startPort, err := strconv.Atoi(m[1]) if err != nil { @@ -537,7 +533,7 @@ func verifyFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{ protocol := rule["protocol"].(string) if protocol != "tcp" && protocol != "udp" && protocol != "icmp" { return fmt.Errorf( - "%s is not a valid protocol. Valid options are 'tcp', 'udp' and 'icmp'", protocol) + "%q is not a valid protocol. Valid options are 'tcp', 'udp' and 'icmp'", protocol) } if protocol == "icmp" { @@ -550,9 +546,17 @@ func verifyFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{ "Parameter icmp_code is a required parameter when using protocol 'icmp'") } } else { - if _, ok := rule["ports"]; !ok { + if ports, ok := rule["ports"].(*schema.Set); ok { + for _, port := range ports.List() { + m := splitPorts.FindStringSubmatch(port.(string)) + if m == nil { + return fmt.Errorf( + "%q is not a valid port value. Valid options are '80' or '80-90'", port.(string)) + } + } + } else { return fmt.Errorf( - "Parameter port is a required parameter when using protocol 'tcp' or 'udp'") + "Parameter ports is a required parameter when *not* using protocol 'icmp'") } } diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go index 2a5542755..66dcc772e 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go @@ -2,7 +2,6 @@ package cloudstack import ( "fmt" - "regexp" "strconv" "strings" "sync" @@ -224,9 +223,6 @@ func createNetworkACLRule( // Create an empty schema.Set to hold all processed ports ports := &schema.Set{F: schema.HashString} - // Define a regexp for parsing the port - re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`) - for _, port := range ps.List() { if _, ok := uuids[port.(string)]; ok { ports.Add(port) @@ -234,7 +230,7 @@ func createNetworkACLRule( continue } - m := re.FindStringSubmatch(port.(string)) + m := splitPorts.FindStringSubmatch(port.(string)) startPort, err := strconv.Atoi(m[1]) if err != nil { @@ -607,7 +603,15 @@ func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interfac case "all": // No additional test are needed, so just leave this empty... case "tcp", "udp": - if _, ok := rule["ports"]; !ok { + if ports, ok := rule["ports"].(*schema.Set); ok { + for _, port := range ports.List() { + m := splitPorts.FindStringSubmatch(port.(string)) + if m == nil { + return fmt.Errorf( + "%q is not a valid port value. Valid options are '80' or '80-90'", port.(string)) + } + } + } else { return fmt.Errorf( "Parameter ports is a required parameter when *not* using protocol 'icmp'") } @@ -615,7 +619,7 @@ func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interfac _, err := strconv.ParseInt(protocol, 0, 0) if err != nil { return fmt.Errorf( - "%s is not a valid protocol. Valid options are 'tcp', 'udp', "+ + "%q is not a valid protocol. Valid options are 'tcp', 'udp', "+ "'icmp', 'all' or a valid protocol number", protocol) } } diff --git a/builtin/providers/cloudstack/resources.go b/builtin/providers/cloudstack/resources.go index 4182b947f..8d7090e1f 100644 --- a/builtin/providers/cloudstack/resources.go +++ b/builtin/providers/cloudstack/resources.go @@ -14,6 +14,9 @@ import ( // UnlimitedResourceID is a "special" ID to define an unlimited resource const UnlimitedResourceID = "-1" +// Define a regexp for parsing the port +var splitPorts = regexp.MustCompile(`^(\d+)(?:-(\d+))?$`) + type retrieveError struct { name string value string