Carve out annotation parsing and add errors package, carved out backe…

…nd https settings (#226)

go.mod

@@ -35,6 +35,7 @@ require (
 	github.com/onsi/gomega v1.5.0
 	github.com/openzipkin/zipkin-go v0.1.3 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
+	github.com/pkg/errors v0.8.0
 	github.com/prometheus/client_golang v0.9.3-0.20190127221311-3c4408c8b829 // indirect
 	github.com/spf13/pflag v1.0.3
 	golang.org/x/sys v0.0.0-20190307162637-572b51eaf722 // indirect

go.sum

@@ -100,6 +100,7 @@ github.com/openzipkin/zipkin-go v0.1.1/go.mod h1:NtoC/o8u3JlF1lSlyPNswIbeQH9bJTm
 github.com/openzipkin/zipkin-go v0.1.3/go.mod h1:NtoC/o8u3JlF1lSlyPNswIbeQH9bJTmOf0Erfk+hxe8=
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
+github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_golang v0.8.0/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=

pkg/annotations/ingress_annotations.go

@@ -1,7 +1,16 @@
+// -------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+// --------------------------------------------------------------------------------------------
+
 package annotations
 
 import (
+	"strconv"
+
 	"k8s.io/api/extensions/v1beta1"
+
+	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/errors"
 )
 
 const (
@@ -24,26 +33,58 @@ const (
 	ApplicationGatewayIngressClass = "azure/application-gateway"
 )
 
-// BackendPathPrefix override path
-func BackendPathPrefix(ing *v1beta1.Ingress) string {
-	val, _ := ing.Annotations[BackendPathPrefixKey]
-	return val
-}
-
 // IngressClass ingress class
-func IngressClass(ing *v1beta1.Ingress) string {
-	val, _ := ing.Annotations[IngressClassKey]
-	return val
+func IngressClass(ing *v1beta1.Ingress) (string, error) {
+	return parseString(ing, IngressClassKey)
 }
 
 // IsApplicationGatewayIngress checks if the Ingress resource can be handled by the Application Gateway ingress controller.
-func IsApplicationGatewayIngress(ing *v1beta1.Ingress) bool {
-	controllerName := ing.Annotations[IngressClassKey]
-	return controllerName == ApplicationGatewayIngressClass
+func IsApplicationGatewayIngress(ing *v1beta1.Ingress) (bool, error) {
+	controllerName, err := parseString(ing, IngressClassKey)
+	return controllerName == ApplicationGatewayIngressClass, err
 }
 
 // IsSslRedirect for HTTP end points.
-func IsSslRedirect(ing *v1beta1.Ingress) bool {
-	val, ok := ing.Annotations[SslRedirectKey]
-	return ok && val == "true"
+func IsSslRedirect(ing *v1beta1.Ingress) (bool, error) {
+	return parseBool(ing, SslRedirectKey)
+}
+
+// BackendPathPrefix override path
+func BackendPathPrefix(ing *v1beta1.Ingress) (string, error) {
+	return parseString(ing, BackendPathPrefixKey)
+}
+
+func parseBool(ing *v1beta1.Ingress, name string) (bool, error) {
+	val, ok := ing.Annotations[name]
+	if ok {
+		boolVal, err := strconv.ParseBool(val)
+		if err != nil {
+			return false, errors.NewInvalidAnnotationContent(name, val)
+		}
+		return boolVal, nil
+	}
+	return false, errors.ErrMissingAnnotations
+}
+
+func parseString(ing *v1beta1.Ingress, name string) (string, error) {
+	val, ok := ing.Annotations[name]
+	if ok {
+		return val, nil
+	}
+
+	return "", errors.ErrMissingAnnotations
+}
+
+func parseInt32(ing *v1beta1.Ingress, name string) (int32, error) {
+	val, ok := ing.Annotations[name]
+	if ok {
+		intVal, err := strconv.Atoi(val)
+		if err == nil {
+			int32Val := int32(intVal)
+			return int32Val, nil
+		}
+		return 0, err
+	}
+
+	return 0, errors.ErrMissingAnnotations
 }

pkg/annotations/ingress_annotations_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/errors"
 	"k8s.io/api/extensions/v1beta1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -19,24 +20,94 @@ var ingress = v1beta1.Ingress{
 	},
 }
 
-func TestIsSslRedirectYes(t *testing.T) {
-	redirectKey := "true"
-	ingress.Annotations[SslRedirectKey] = redirectKey
-	if !IsSslRedirect(&ingress) {
-		t.Error(fmt.Sprintf("IsSslRedirect is expected to return true since %s = %s", SslRedirectKey, redirectKey))
+const (
+	NoError = "Expected to return %s and no error. Returned %v and %v."
+	Error   = "Expected to return error %s. Returned %v and %v."
+)
+
+func TestParseBoolTrue(t *testing.T) {
+	key := "key"
+	value := "true"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseBool(&ingress, key)
+	if !parsedVal || err != nil {
+		t.Error(fmt.Sprintf(NoError, value, parsedVal, err))
+	}
+}
+
+func TestParseBoolFalse(t *testing.T) {
+	key := "key"
+	value := "false"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseBool(&ingress, key)
+	if parsedVal || err != nil {
+		t.Error(fmt.Sprintf(NoError, value, parsedVal, err))
+	}
+}
+
+func TestParseBoolInvalid(t *testing.T) {
+	key := "key"
+	value := "nope"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseBool(&ingress, key)
+	if !errors.IsInvalidContent(err) {
+		t.Error(fmt.Sprintf(Error, err, parsedVal, err))
+	}
+}
+
+func TestParseBoolMissingKey(t *testing.T) {
+	key := "key"
+	delete(ingress.Annotations, key)
+	parsedVal, err := parseBool(&ingress, key)
+	if !errors.IsMissingAnnotations(err) || parsedVal {
+		t.Error(fmt.Sprintf(Error, errors.ErrMissingAnnotations, parsedVal, err))
 	}
 }
 
-func TestIsSslRedirectNo(t *testing.T) {
-	redirectKey := "nope"
-	ingress.Annotations[SslRedirectKey] = redirectKey
-	if IsSslRedirect(&ingress) {
-		t.Error(fmt.Sprintf("IsSslRedirect is expected to return false since %s = %s", SslRedirectKey, redirectKey))
+func TestParseInt32(t *testing.T) {
+	key := "key"
+	value := "20"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseInt32(&ingress, key)
+	if err != nil || fmt.Sprint(parsedVal) != value {
+		t.Error(fmt.Sprintf(NoError, value, parsedVal, err))
 	}
 }
-func TestIsSslRedirectMissingKey(t *testing.T) {
-	delete(ingress.Annotations, SslRedirectKey)
-	if IsSslRedirect(&ingress) {
-		t.Error(fmt.Sprintf("IsSslRedirect is expected to return false since there is no %s annotation", SslRedirectKey))
+
+func TestParseInt32Invalid(t *testing.T) {
+	key := "key"
+	value := "20asd"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseInt32(&ingress, key)
+	if errors.IsInvalidContent(err) {
+		t.Error(fmt.Sprintf(Error, err, parsedVal, err))
+	}
+}
+
+func TestParseInt32MissingKey(t *testing.T) {
+	key := "key"
+	delete(ingress.Annotations, key)
+	parsedVal, err := parseInt32(&ingress, key)
+	if !errors.IsMissingAnnotations(err) || parsedVal != 0 {
+		t.Error(fmt.Sprintf(Error, errors.ErrMissingAnnotations, parsedVal, err))
+	}
+}
+
+func TestParseString(t *testing.T) {
+	key := "key"
+	value := "/path"
+	ingress.Annotations[key] = value
+	parsedVal, err := parseString(&ingress, key)
+	if parsedVal != value || err != nil {
+		t.Error(fmt.Sprintf(NoError, value, parsedVal, err))
+	}
+}
+
+func TestParseStringMissingKey(t *testing.T) {
+	key := "key"
+	delete(ingress.Annotations, key)
+	parsedVal, err := parseString(&ingress, key)
+	if !errors.IsMissingAnnotations(err) {
+		t.Error(fmt.Sprintf(Error, errors.ErrMissingAnnotations, parsedVal, err))
 	}
 }

pkg/appgw/appgw_test.go

@@ -207,7 +207,8 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() {
 			ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &network.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
 				Protocol: network.HTTP,
 				Port:     &backendPort,
-				Path:     to.StringPtr(""),
+				Path:     nil,
+				HostName: nil,
 				Probe:    resourceRef(probeID),
 			},
 		}
@@ -492,7 +493,7 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() {
 					ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &network.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
 						Protocol: network.HTTP,
 						Port:     &servicePort,
-						Path:     to.StringPtr(""),
+						Path:     nil,
 						Probe:    resourceRef(appGwIdentifier.probeID(defaultProbeName)),
 					},
 				}
@@ -715,6 +716,7 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() {
 						Port:     &backendPort,
 						Path:     to.StringPtr("/test"),
 						Probe:    resourceRef(probeID),
+						HostName: nil,
 					},
 				}
 

pkg/appgw/backendhttpsettings.go

@@ -147,24 +147,7 @@ func (builder *appGwConfigBuilder) BackendHTTPSettingsCollection(ingressList [](
 		}
 
 		builder.serviceBackendPairMap[backendID] = uniquePair
-
-		probeName := builder.probesMap[backendID].Name
-		probeID := builder.appGwIdentifier.probeID(*probeName)
-		httpSettingsName := generateHTTPSettingsName(backendID.serviceFullName(), backendID.Backend.ServicePort.String(), uniquePair.BackendPort, backendID.Ingress.Name)
-		glog.Infof("Created a new HTTP setting w/ name: %s\n", httpSettingsName)
-		httpSettingsPort := uniquePair.BackendPort
-		backendPathPrefix := to.StringPtr(annotations.BackendPathPrefix(backendID.Ingress))
-		httpSettings := network.ApplicationGatewayBackendHTTPSettings{
-			Etag: to.StringPtr("*"),
-			Name: &httpSettingsName,
-			ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &network.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
-				Protocol: network.HTTP,
-				Port:     &httpSettingsPort,
-				Path:     backendPathPrefix,
-				Probe:    resourceRef(probeID),
-			},
-		}
-		// other settings should come from annotations
+		httpSettings := builder.generateHTTPSettings(backendID, uniquePair.BackendPort)
 		httpSettingsCollection[*httpSettings.Name] = httpSettings
 		builder.backendHTTPSettingsMap[backendID] = &httpSettings
 	}
@@ -178,3 +161,26 @@ func (builder *appGwConfigBuilder) BackendHTTPSettingsCollection(ingressList [](
 
 	return builder, nil
 }
+
+func (builder *appGwConfigBuilder) generateHTTPSettings(backendID backendIdentifier, port int32) network.ApplicationGatewayBackendHTTPSettings {
+	probeName := builder.probesMap[backendID].Name
+	probeID := builder.appGwIdentifier.probeID(*probeName)
+	httpSettingsName := generateHTTPSettingsName(backendID.serviceFullName(), backendID.Backend.ServicePort.String(), port, backendID.Ingress.Name)
+	glog.Infof("Created a new HTTP setting w/ name: %s\n", httpSettingsName)
+
+	httpSettings := network.ApplicationGatewayBackendHTTPSettings{
+		Etag: to.StringPtr("*"),
+		Name: &httpSettingsName,
+		ApplicationGatewayBackendHTTPSettingsPropertiesFormat: &network.ApplicationGatewayBackendHTTPSettingsPropertiesFormat{
+			Protocol: network.HTTP,
+			Port:     &port,
+			Probe:    resourceRef(probeID),
+		},
+	}
+
+	pathPrefix, err := annotations.BackendPathPrefix(backendID.Ingress)
+	if err == nil {
+		httpSettings.Path = to.StringPtr(pathPrefix)
+	}
+	return httpSettings
+}

pkg/appgw/health_probes.go

@@ -86,8 +86,9 @@ func (builder *appGwConfigBuilder) generateHealthProbe(backendID backendIdentifi
 		probe.Host = to.StringPtr(backendID.Rule.Host)
 	}
 
-	if len(annotations.BackendPathPrefix(backendID.Ingress)) != 0 {
-		probe.Path = to.StringPtr(annotations.BackendPathPrefix(backendID.Ingress))
+	pathPrefix, err := annotations.BackendPathPrefix(backendID.Ingress)
+	if err == nil {
+		probe.Path = to.StringPtr(pathPrefix)
 	} else if backendID.Path != nil && len(backendID.Path.Path) != 0 {
 		probe.Path = to.StringPtr(backendID.Path.Path)
 	}

pkg/appgw/ingress_rules.go

@@ -38,7 +38,8 @@ func (builder *appGwConfigBuilder) processIngressRules(ingress *v1beta1.Ingress)
 		}
 
 		// Enable HTTP only if HTTPS is not configured OR if ingress annotated with 'ssl-redirect'
-		if annotations.IsSslRedirect(ingress) || !httpsAvailable {
+		sslRedirect, _ := annotations.IsSslRedirect(ingress)
+		if sslRedirect || !httpsAvailable {
 			listenerID := generateListenerID(&rule, network.HTTP, nil)
 			frontendPorts[listenerID.FrontendPort] = nil
 			felAzConfig := listenerAzConfig{

pkg/appgw/requestroutingrules.go

@@ -6,9 +6,10 @@
 package appgw
 
 import (
-	"github.com/golang/glog"
 	"strconv"
 
+	"github.com/golang/glog"
+
 	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/annotations"
 	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
@@ -154,7 +155,8 @@ func (builder *appGwConfigBuilder) RequestRoutingRules(ingressList [](*v1beta1.I
 					defaultAddressPoolID, defaultHTTPSettingsID)
 
 				// If ingress is annotated with "ssl-redirect" - setup redirection configuration.
-				if annotations.IsSslRedirect(ingress) {
+				sslRedirect, _ := annotations.IsSslRedirect(ingress)
+				if sslRedirect {
 					builder.modifyPathRulesForRedirection(ingress, urlPathMaps[listenerHTTPID])
 				}
 			}