Merge pull request #423 from kubernetes-sigs/service-annotations

Support for service level annotations

Author: bigkraig

docs/ingress-resources.md

@@ -36,9 +36,9 @@ The host field specifies the eventual Route 53-managed domain that will route to
 
 ## Annotations
 
-The ALB Ingress Controller is configured by Annotations on the `Ingress` resource object. Some are required and some are optional. All annotations use the namespace `alb.ingress.kubernetes.io/`.
+The ALB Ingress Controller is configured by Annotations on the `Ingress` and `Service` resource objects. Some are required and some are optional. All annotations use the namespace `alb.ingress.kubernetes.io/`.
 
-### Required Annotations
+### Required Ingress Annotations
 
 ```
 alb.ingress.kubernetes.io/scheme
@@ -48,7 +48,7 @@ Required annotations are:
 
 - **scheme**: Defines whether an ALB should be `internal` or `internet-facing`. See [Load balancer scheme](http://docs.aws.amazon.com/elasticloadbalancing/latest/userguide/how-elastic-load-balancing-works.html#load-balancer-scheme) in the AWS documentation for more details.
 
-### Optional Annotations
+### Optional Ingress Annotations
 
 ```
 alb.ingress.kubernetes.io/load-balancer-attributes
@@ -120,3 +120,22 @@ Optional annotations are:
 - **ip-address-type**: The IP address type thats used to either route IPv4 traffic only or to route both IPv4 and IPv6 traffic. Can be either `dualstack` or `ipv4`. When omitted `ipv4` is used.
 
 - **ssl-policy**: Defines the [Security Policy](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/create-https-listener.html#describe-ssl-policies) that should be assigned to the ALB, allowing you to control the protocol and ciphers.
+
+### Services
+
+A subset of these annotations are supported on Services. This is used to customize the Target Group created for the Service. If a Service has no annotations, the Target Group options will default to the same options configured on the Ingress.
+
+#### Optional Service Annotations
+
+```
+alb.ingress.kubernetes.io/backend-protocol
+alb.ingress.kubernetes.io/healthcheck-interval-seconds
+alb.ingress.kubernetes.io/healthcheck-path
+alb.ingress.kubernetes.io/healthcheck-port
+alb.ingress.kubernetes.io/healthcheck-protocol
+alb.ingress.kubernetes.io/healthcheck-timeout-seconds
+alb.ingress.kubernetes.io/healthy-threshold-count
+alb.ingress.kubernetes.io/unhealthy-threshold-count
+alb.ingress.kubernetes.io/success-codes
+alb.ingress.kubernetes.io/target-group-attributes
+```

pkg/alb/lb/loadbalancer.go

@@ -26,17 +26,20 @@ import (
 )
 
 type NewDesiredLoadBalancerOptions struct {
-	ALBNamePrefix        string
-	Namespace            string
-	IngressName          string
-	ExistingLoadBalancer *LoadBalancer
-	Logger               *log.Logger
-	Annotations          *annotations.Annotations
-	Tags                 util.Tags
-	Attributes           []*elbv2.LoadBalancerAttribute
-	IngressRules         []extensions.IngressRule
-	GetServiceNodePort   func(string, int32) (*int64, error)
-	GetNodes             func() util.AWSStringSlice
+	ALBNamePrefix         string
+	Namespace             string
+	IngressName           string
+	ExistingLoadBalancer  *LoadBalancer
+	Logger                *log.Logger
+	Annotations           *annotations.Annotations
+	AnnotationFactory     annotations.AnnotationFactory
+	IngressAnnotations    *map[string]string
+	Tags                  util.Tags
+	Attributes            []*elbv2.LoadBalancerAttribute
+	IngressRules          []extensions.IngressRule
+	GetServiceNodePort    func(string, int32) (*int64, error)
+	GetServiceAnnotations func(string, string) (*map[string]string, error)
+	GetNodes              func() util.AWSStringSlice
 }
 
 // NewDesiredLoadBalancer returns a new loadbalancer.LoadBalancer based on the opts provided.
@@ -102,13 +105,16 @@ func NewDesiredLoadBalancer(o *NewDesiredLoadBalancerOptions) (newLoadBalancer *
 		IngressRules:         o.IngressRules,
 		LoadBalancerID:       newLoadBalancer.id,
 		ExistingTargetGroups: existingtgs,
-		Annotations:          o.Annotations,
-		ALBNamePrefix:        o.ALBNamePrefix,
-		Namespace:            o.Namespace,
-		Tags:                 o.Tags,
-		Logger:               o.Logger,
-		GetServiceNodePort:   o.GetServiceNodePort,
-		GetNodes:             o.GetNodes,
+		// Annotations:           o.Annotations,
+		IngressAnnotations:    o.IngressAnnotations,
+		ALBNamePrefix:         o.ALBNamePrefix,
+		Namespace:             o.Namespace,
+		Tags:                  o.Tags,
+		Logger:                o.Logger,
+		GetServiceNodePort:    o.GetServiceNodePort,
+		GetServiceAnnotations: o.GetServiceAnnotations,
+		AnnotationFactory:     o.AnnotationFactory,
+		GetNodes:              o.GetNodes,
 	})
 
 	if err != nil {

pkg/alb/tg/targetgroups.go

@@ -121,16 +121,18 @@ func NewCurrentTargetGroups(o *NewCurrentTargetGroupsOptions) (TargetGroups, err
 }
 
 type NewDesiredTargetGroupsOptions struct {
-	IngressRules         []extensions.IngressRule
-	LoadBalancerID       string
-	ExistingTargetGroups TargetGroups
-	Annotations          *annotations.Annotations
-	ALBNamePrefix        string
-	Namespace            string
-	Tags                 util.Tags
-	Logger               *log.Logger
-	GetServiceNodePort   func(string, int32) (*int64, error)
-	GetNodes             func() util.AWSStringSlice
+	IngressRules          []extensions.IngressRule
+	LoadBalancerID        string
+	ExistingTargetGroups  TargetGroups
+	AnnotationFactory     annotations.AnnotationFactory
+	IngressAnnotations    *map[string]string
+	ALBNamePrefix         string
+	Namespace             string
+	Tags                  util.Tags
+	Logger                *log.Logger
+	GetServiceNodePort    func(string, int32) (*int64, error)
+	GetServiceAnnotations func(string, string) (*map[string]string, error)
+	GetNodes              func() util.AWSStringSlice
 }
 
 // NewDesiredTargetGroups returns a new targetgroups.TargetGroups based on an extensions.Ingress.
@@ -146,9 +148,20 @@ func NewDesiredTargetGroups(o *NewDesiredTargetGroupsOptions) (TargetGroups, err
 				return nil, err
 			}
 
+			tgAnnotations, err := mergeAnnotations(&mergeAnnotationsOptions{
+				AnnotationFactory:     o.AnnotationFactory,
+				IngressAnnotations:    o.IngressAnnotations,
+				Namespace:             o.Namespace,
+				ServiceName:           path.Backend.ServiceName,
+				GetServiceAnnotations: o.GetServiceAnnotations,
+			})
+			if err != nil {
+				return output, err
+			}
+
 			// Start with a new target group with a new Desired state.
 			targetGroup := NewDesiredTargetGroup(&NewDesiredTargetGroupOptions{
-				Annotations:    o.Annotations,
+				Annotations:    tgAnnotations,
 				Tags:           o.Tags,
 				ALBNamePrefix:  o.ALBNamePrefix,
 				LoadBalancerID: o.LoadBalancerID,
@@ -187,3 +200,39 @@ func NewDesiredTargetGroups(o *NewDesiredTargetGroupsOptions) (TargetGroups, err
 	}
 	return output, nil
 }
+
+type mergeAnnotationsOptions struct {
+	AnnotationFactory     annotations.AnnotationFactory
+	IngressAnnotations    *map[string]string
+	Namespace             string
+	ServiceName           string
+	GetServiceAnnotations func(string, string) (*map[string]string, error)
+}
+
+func mergeAnnotations(o *mergeAnnotationsOptions) (*annotations.Annotations, error) {
+	serviceAnnotations, err := o.GetServiceAnnotations(o.Namespace, o.ServiceName)
+	if err != nil {
+		return nil, err
+	}
+
+	mergedAnnotations := make(map[string]string)
+	for k, v := range *o.IngressAnnotations {
+		mergedAnnotations[k] = v
+	}
+
+	for k, v := range *serviceAnnotations {
+		mergedAnnotations[k] = v
+	}
+
+	tgAnnotations, err := o.AnnotationFactory.ParseAnnotations(&annotations.ParseAnnotationsOptions{
+		Annotations: mergedAnnotations,
+		Namespace:   o.Namespace,
+		ServiceName: o.ServiceName,
+	})
+
+	if err != nil {
+		msg := fmt.Errorf("Error parsing service annotations: %s", err.Error())
+		return nil, msg
+	}
+	return tgAnnotations, nil
+}

pkg/alb/tg/targetgroups_test.go

@@ -0,0 +1,81 @@
+package tg
+
+import (
+	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/annotations"
+)
+
+func TestMergeAnnotations(t *testing.T) {
+	var tests = []struct {
+		ingressAnnotations map[string]string
+		serviceAnnotations map[string]string
+		expected           annotations.Annotations
+		pass               bool
+	}{
+		{
+			map[string]string{
+				"alb.ingress.kubernetes.io/subnets":          "subnet-abcdfg",
+				"alb.ingress.kubernetes.io/scheme":           "internal",
+				"alb.ingress.kubernetes.io/healthcheck-path": "/ingressPath",
+			},
+			map[string]string{
+				"alb.ingress.kubernetes.io/healthcheck-path": "/servicePath",
+			},
+			annotations.Annotations{
+				HealthcheckPath: aws.String("/servicePath"),
+			},
+			true,
+		},
+		{
+			map[string]string{
+				"alb.ingress.kubernetes.io/subnets":          "subnet-abcdfg",
+				"alb.ingress.kubernetes.io/scheme":           "internal",
+				"alb.ingress.kubernetes.io/healthcheck-path": "/ingressPath",
+			},
+			map[string]string{},
+			annotations.Annotations{
+				HealthcheckPath: aws.String("/ingressPath"),
+			},
+			true,
+		},
+		{
+			map[string]string{
+				"alb.ingress.kubernetes.io/subnets":          "subnet-abcdfg",
+				"alb.ingress.kubernetes.io/scheme":           "internal",
+				"alb.ingress.kubernetes.io/healthcheck-path": "/ingressPath",
+			},
+			map[string]string{},
+			annotations.Annotations{
+				HealthcheckPath: aws.String("/"),
+			},
+			false,
+		},
+	}
+	vf := annotations.NewValidatingAnnotationFactory(&annotations.NewValidatingAnnotationFactoryOptions{
+		Validator:   annotations.FakeValidator{VpcId: "vpc-1"},
+		ClusterName: "clusterName"})
+
+	for i, tt := range tests {
+		a, err := mergeAnnotations(&mergeAnnotationsOptions{
+			AnnotationFactory:  vf,
+			IngressAnnotations: &tt.ingressAnnotations,
+			GetServiceAnnotations: func(string, string) (*map[string]string, error) {
+				return &tt.serviceAnnotations, nil
+			},
+		})
+
+		if err != nil && tt.pass {
+			t.Errorf("mergeAnnotations(%v): got %v expected %v, errored: %v", i, *a.HealthcheckPath, *tt.expected.HealthcheckPath, err)
+		}
+
+		if err == nil && tt.pass && *tt.expected.HealthcheckPath != *a.HealthcheckPath {
+			t.Errorf("mergeAnnotations(%v): expected %v, actual %v", i, *tt.expected.HealthcheckPath, *a.HealthcheckPath)
+		}
+
+		if err == nil && !tt.pass && *tt.expected.HealthcheckPath == *a.HealthcheckPath {
+			t.Errorf("mergeAnnotations(%v): not expected %v, actual %v", i, *tt.expected.HealthcheckPath, *a.HealthcheckPath)
+		}
+	}
+}

pkg/albingress/albingress.go

@@ -53,16 +53,18 @@ type NewALBIngressFromIngressOptions struct {
 	ClusterName           string
 	ALBNamePrefix         string
 	GetServiceNodePort    func(string, int32) (*int64, error)
+	GetServiceAnnotations func(string, string) (*map[string]string, error)
 	GetNodes              func() util.AWSStringSlice
 	Recorder              record.EventRecorder
 	ConnectionIdleTimeout *int64
+	AnnotationFactory     annotations.AnnotationFactory
 }
 
 // NewALBIngressFromIngress builds ALBIngress's based off of an Ingress object
 // https://godoc.org/k8s.io/kubernetes/pkg/apis/extensions#Ingress. Creates a new ingress object,
 // and looks up to see if a previous ingress object with the same id is known to the ALBController.
 // If there is an issue and the ingress is invalid, nil is returned.
-func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFactory annotations.AnnotationFactory) *ALBIngress {
+func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {
 	var err error
 
 	// Create newIngress ALBIngress object holding the resource details and some cluster information.
@@ -90,7 +92,11 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFact
 	}
 
 	// Load up the ingress with our current annotations.
-	newIngress.annotations, err = annotationFactory.ParseAnnotations(o.Ingress)
+	newIngress.annotations, err = o.AnnotationFactory.ParseAnnotations(&annotations.ParseAnnotationsOptions{
+		Annotations: o.Ingress.Annotations,
+		Namespace:   o.Ingress.Namespace,
+		IngressName: o.Ingress.Name,
+	})
 	if err != nil {
 		msg := fmt.Sprintf("Error parsing annotations: %s", err.Error())
 		newIngress.reconciled = false
@@ -110,16 +116,19 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions, annotationFact
 
 	// Assemble the load balancer
 	newIngress.loadBalancer, err = lb.NewDesiredLoadBalancer(&lb.NewDesiredLoadBalancerOptions{
-		ALBNamePrefix:        o.ALBNamePrefix,
-		Namespace:            o.Ingress.GetNamespace(),
-		ExistingLoadBalancer: newIngress.loadBalancer,
-		IngressName:          o.Ingress.Name,
-		IngressRules:         o.Ingress.Spec.Rules,
-		Logger:               newIngress.logger,
-		Annotations:          newIngress.annotations,
-		Tags:                 newIngress.Tags(o.ClusterName),
-		GetServiceNodePort:   o.GetServiceNodePort,
-		GetNodes:             o.GetNodes,
+		ALBNamePrefix:         o.ALBNamePrefix,
+		Namespace:             o.Ingress.GetNamespace(),
+		ExistingLoadBalancer:  newIngress.loadBalancer,
+		IngressName:           o.Ingress.Name,
+		IngressRules:          o.Ingress.Spec.Rules,
+		Logger:                newIngress.logger,
+		Annotations:           newIngress.annotations,
+		IngressAnnotations:    &o.Ingress.Annotations,
+		Tags:                  newIngress.Tags(o.ClusterName),
+		GetServiceNodePort:    o.GetServiceNodePort,
+		GetServiceAnnotations: o.GetServiceAnnotations,
+		GetNodes:              o.GetNodes,
+		AnnotationFactory:     o.AnnotationFactory,
 	})
 
 	if err != nil {
@@ -294,10 +303,6 @@ func (a *ALBIngress) Tags(clusterName string) []*elbv2.Tag {
 	return tags
 }
 
-type ReconcileOptions struct {
-	Eventf func(string, string, string, ...interface{})
-}
-
 // id returns an ingress id based off of a namespace and name
 func GenerateID(namespace, name string) string {
 	return fmt.Sprintf("%s/%s", namespace, name)

pkg/albingress/albingress_test.go

@@ -66,18 +66,24 @@ func TestNewALBIngressFromIngress(t *testing.T) {
 			nodePort := int64(8000)
 			return &nodePort, nil
 		},
+		GetServiceAnnotations: func(namespace string, serviceName string) (*map[string]string, error) {
+			a := make(map[string]string)
+			return &a, nil
+		},
 		GetNodes: func() types.AWSStringSlice {
 			instance1 := "i-1"
 			instance2 := "i-2"
 			return types.AWSStringSlice{&instance1, &instance2}
 		},
 		ClusterName:   "testCluster",
 		ALBNamePrefix: "albNamePrefix",
+		AnnotationFactory: annotations.NewValidatingAnnotationFactory(&annotations.NewValidatingAnnotationFactoryOptions{
+			Validator:   annotations.FakeValidator{VpcId: "vpc-1"},
+			ClusterName: "testCluster",
+		},
+		),
 	}
-	ingress := NewALBIngressFromIngress(
-		options,
-		annotations.NewValidatingAnnotationFactory(annotations.FakeValidator{VpcId: "vpc-1"}, "testCluster"),
-	)
+	ingress := NewALBIngressFromIngress(options)
 	if ingress == nil {
 		t.Errorf("NewALBIngressFromIngress returned nil")
 	}