Merge pull request #4772 from svanharmelen/b-fix-panic

provider/cloudstack: add a check to see if the port value is valid

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(
-				"Parameter port is a required parameter when using protocol 'tcp' or 'udp'")
+						"%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'")
 		}
 	}
 

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(
-				"Parameter port is a required parameter when using protocol 'tcp' or 'udp'")
+						"%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'")
 		}
 	}
 

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)
 		}
 	}

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