Remove code duplication of string to model.Protocol conversion (istio…

…#1866)

* added ConvertCaseInsensitiveStringToProtocol and ProtocolUnsupported

* remove code duplication in convertProtocol functions

* use ConvertCaseInsensitiveStringToProtocol instead of strings.ToUpper

in conversion of a string to model.Protocol, since there are
not-all-uppercase protocols like Mongo and Redis

* removed unused metadata values

* added TestConvertCaseInsensitiveStringToProtocol

* replaced shadowing variable

* Revert "replaced shadowing variable"

This reverts commit 6a9981f.

* replaced shadowing variable

pilot/model/service.go

@@ -112,8 +112,34 @@ const (
 	ProtocolMongo Protocol = "Mongo"
 	// ProtocolRedis declares that the port carries redis traffic
 	ProtocolRedis Protocol = "Redis"
+	// ProtocolUnsupported - value to signify that the protocol is unsupported
+	ProtocolUnsupported Protocol = "UnsupportedProtocol"
 )
 
+// ConvertCaseInsensitiveStringToProtocol converts a case-insensitive protocol to Protocol
+func ConvertCaseInsensitiveStringToProtocol(protocolAsString string) Protocol {
+	switch strings.ToLower(protocolAsString) {
+	case "tcp":
+		return ProtocolTCP
+	case "udp":
+		return ProtocolUDP
+	case "grpc":
+		return ProtocolGRPC
+	case "http":
+		return ProtocolHTTP
+	case "http2":
+		return ProtocolHTTP2
+	case "https":
+		return ProtocolHTTPS
+	case "mongo":
+		return ProtocolMongo
+	case "redis":
+		return ProtocolRedis
+	}
+
+	return ProtocolUnsupported
+}
+
 // IsHTTP is true for protocols that use HTTP as transport protocol
 func (p Protocol) IsHTTP() bool {
 	switch p {

pilot/model/service_test.go

@@ -177,3 +177,34 @@ func TestGetByPort(t *testing.T) {
 		t.Errorf("GetByPort(88) => want none but got %v, %t", port, exists)
 	}
 }
+
+func TestConvertCaseInsensitiveStringToProtocol(t *testing.T) {
+	var testPairs = []struct {
+		name string
+		out  Protocol
+	}{
+		{"tcp", ProtocolTCP},
+		{"http", ProtocolHTTP},
+		{"HTTP", ProtocolHTTP},
+		{"Http", ProtocolHTTP},
+		{"https", ProtocolHTTPS},
+		{"http2", ProtocolHTTP2},
+		{"grpc", ProtocolGRPC},
+		{"udp", ProtocolUDP},
+		{"Mongo", ProtocolMongo},
+		{"mongo", ProtocolMongo},
+		{"MONGO", ProtocolMongo},
+		{"Redis", ProtocolRedis},
+		{"redis", ProtocolRedis},
+		{"REDIS", ProtocolRedis},
+		{"", ProtocolUnsupported},
+		{"SMTP", ProtocolUnsupported},
+	}
+
+	for _, testPair := range testPairs {
+		out := ConvertCaseInsensitiveStringToProtocol(testPair.name)
+		if out != testPair.out {
+			t.Errorf("ConvertCaseInsensitiveStringToProtocol(%q) => %q, want %q", testPair.name, out, testPair.out)
+		}
+	}
+}

pilot/model/validation.go

@@ -877,7 +877,7 @@ func ValidateEgressRule(msg proto.Message) error {
 			errs = multierror.Append(errs, err)
 		}
 
-		if cidrDestinationService && Protocol(strings.ToUpper(port.Protocol)) != ProtocolTCP {
+		if cidrDestinationService && ConvertCaseInsensitiveStringToProtocol(port.Protocol) != ProtocolTCP {
 			errs = multierror.Append(errs, fmt.Errorf("Only TCP protocol can be defined for CIDR destination "+
 				"service notation. port: %d protocol: %s destination.service: %s",
 				port.Port, port.Protocol, destination.Service))

pilot/platform/consul/conversion.go

@@ -139,27 +139,10 @@ func parseHostname(hostname string) (name string, err error) {
 }
 
 func convertProtocol(name string) model.Protocol {
-	switch name {
-	case "tcp":
-		return model.ProtocolTCP
-	case "udp":
-		return model.ProtocolUDP
-	case "grpc":
-		return model.ProtocolGRPC
-	case "http":
-		return model.ProtocolHTTP
-	case "http2":
-		return model.ProtocolHTTP2
-	case "https":
-		return model.ProtocolHTTPS
-	case "mongo":
-		return model.ProtocolMongo
-	case "redis":
-		return model.ProtocolRedis
-	case "":
-		// fallthrough to default protocol
-	default:
+	protocol := model.ConvertCaseInsensitiveStringToProtocol(name)
+	if protocol == model.ProtocolUnsupported {
 		glog.Warningf("unsupported protocol value: %s", name)
+		return model.ProtocolTCP
 	}
-	return model.ProtocolTCP
+	return protocol
 }

pilot/platform/eureka/conversion.go

@@ -16,7 +16,6 @@ package eureka
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/golang/glog"
 
@@ -122,42 +121,15 @@ func convertPorts(instance *instance) model.PortList {
 
 const protocolMetadata = "istio.protocol" // metadata key for port protocol
 
-// supported protocol metadata values
-const (
-	metadataUDP   = "udp"
-	metadataTCP   = "tcp"
-	metadataHTTP  = "http"
-	metadataHTTP2 = "http2"
-	metadataHTTPS = "https"
-	metadataGRPC  = "grpc"
-	metadataMongo = "mongo"
-	metadataRedis = "redis"
-)
-
 func convertProtocol(md metadata) model.Protocol {
+	name := md[protocolMetadata]
+
 	if md != nil {
-		protocol := strings.ToLower(md[protocolMetadata])
-		switch protocol {
-		case metadataUDP:
-			return model.ProtocolUDP
-		case metadataTCP:
-			return model.ProtocolTCP
-		case metadataHTTP:
-			return model.ProtocolHTTP
-		case metadataHTTP2:
-			return model.ProtocolHTTP2
-		case metadataHTTPS:
-			return model.ProtocolHTTPS
-		case metadataGRPC:
-			return model.ProtocolGRPC
-		case metadataMongo:
-			return model.ProtocolMongo
-		case metadataRedis:
-			return model.ProtocolRedis
-		case "":
-			// fallthrough to default protocol
-		default:
-			glog.Warningf("unsupported protocol value: %s", protocol)
+		protocol := model.ConvertCaseInsensitiveStringToProtocol(name)
+		if protocol == model.ProtocolUnsupported {
+			glog.Warningf("unsupported protocol value: %s", name)
+		} else {
+			return protocol
 		}
 	}
 	return model.ProtocolTCP // default protocol

pilot/platform/eureka/conversion_test.go

@@ -192,13 +192,13 @@ func TestConvertProtocol(t *testing.T) {
 		{in: nil, out: model.ProtocolTCP},
 		{in: makeMetadata(""), out: model.ProtocolTCP},
 		{in: makeMetadata("HTCPCP"), out: model.ProtocolTCP},
-		{in: makeMetadata(metadataUDP), out: model.ProtocolUDP},
-		{in: makeMetadata(metadataHTTP), out: model.ProtocolHTTP},
-		{in: makeMetadata(metadataHTTP2), out: model.ProtocolHTTP2},
-		{in: makeMetadata(metadataHTTPS), out: model.ProtocolHTTPS},
-		{in: makeMetadata(metadataGRPC), out: model.ProtocolGRPC},
-		{in: makeMetadata(metadataMongo), out: model.ProtocolMongo},
-		{in: makeMetadata(metadataRedis), out: model.ProtocolRedis},
+		{in: makeMetadata("udp"), out: model.ProtocolUDP},
+		{in: makeMetadata("http"), out: model.ProtocolHTTP},
+		{in: makeMetadata("http2"), out: model.ProtocolHTTP2},
+		{in: makeMetadata("https"), out: model.ProtocolHTTPS},
+		{in: makeMetadata("grpc"), out: model.ProtocolGRPC},
+		{in: makeMetadata("mongo"), out: model.ProtocolMongo},
+		{in: makeMetadata("redis"), out: model.ProtocolRedis},
 	}
 
 	for _, tt := range protocolTests {
@@ -211,7 +211,7 @@ func TestConvertProtocol(t *testing.T) {
 func TestConvertLabels(t *testing.T) {
 	md := metadata{
 		"@class":         "java.util.Collections$EmptyMap",
-		protocolMetadata: metadataHTTP2,
+		protocolMetadata: "http2",
 		"kit":            "kat",
 		"spam":           "coolaid",
 	}

pilot/platform/kube/conversion.go

@@ -174,19 +174,9 @@ func ConvertProtocol(name string, proto v1.Protocol) model.Protocol {
 		if i >= 0 {
 			prefix = name[:i]
 		}
-		switch prefix {
-		case "grpc":
-			out = model.ProtocolGRPC
-		case "http":
-			out = model.ProtocolHTTP
-		case "http2":
-			out = model.ProtocolHTTP2
-		case "https":
-			out = model.ProtocolHTTPS
-		case "mongo":
-			out = model.ProtocolMongo
-		case "redis":
-			out = model.ProtocolRedis
+		protocol := model.ConvertCaseInsensitiveStringToProtocol(prefix)
+		if protocol != model.ProtocolUDP && protocol != model.ProtocolUnsupported {
+			out = protocol
 		}
 	}
 	return out

pilot/proxy/envoy/config.go

@@ -893,7 +893,7 @@ func buildEgressHTTPRoutes(mesh *proxyconfig.MeshConfig, node proxy.Node,
 
 	for _, rule := range egressRules {
 		for _, port := range rule.Ports {
-			protocol := model.Protocol(strings.ToUpper(port.Protocol))
+			protocol := model.ConvertCaseInsensitiveStringToProtocol(port.Protocol)
 			if protocol != model.ProtocolHTTP && protocol != model.ProtocolHTTPS &&
 				protocol != model.ProtocolHTTP2 && protocol != model.ProtocolGRPC {
 				continue