Fixed attribute logic

docs/ingress-resources.md

@@ -54,7 +54,6 @@ Required annotations are:
 alb.ingress.kubernetes.io/load-balancer-attributes
 alb.ingress.kubernetes.io/backend-protocol
 alb.ingress.kubernetes.io/certificate-arn
-alb.ingress.kubernetes.io/connection-idle-timeout
 alb.ingress.kubernetes.io/healthcheck-interval-seconds
 alb.ingress.kubernetes.io/healthcheck-path
 alb.ingress.kubernetes.io/healthcheck-port
@@ -81,8 +80,6 @@ Optional annotations are:
 
 - **certificate-arn**: Enables HTTPS and uses the certificate defined, based on arn, stored in your [AWS Certificate Manager](https://aws.amazon.com/certificate-manager).
 
-- **connection-idle-timeout**: Sets the connection idle timeout setting for the ALB. This is a global setting for the ALB.
-
 - **healthcheck-interval-seconds**: The approximate amount of time, in seconds, between health checks of an individual target. The default is 15 seconds.
 
 - **healthcheck-path**: The ping path that is the destination on the targets for health checks. The default is /.

pkg/alb/lb/types.go

@@ -44,7 +44,6 @@ type options struct {
 }
 
 type opts struct {
-	idleTimeout       *int64
 	ports             portList
 	inboundCidrs      util.Cidrs
 	webACLId          *string
@@ -61,7 +60,6 @@ const (
 	schemeModified
 	attributesModified
 	managedSecurityGroupsModified
-	connectionIdleTimeoutModified
 	ipAddressTypeModified
 	webACLAssociationModified
 )

pkg/alb/rs/rule_test.go

@@ -354,6 +354,8 @@ func TestRuleReconcile(t *testing.T) {
 }
 
 func genTG(arn, svcname string) *tg.TargetGroup {
+	albelbv2.ELBV2svc = mockedELBV2{}
+
 	t, _ := tg.NewCurrentTargetGroup(&tg.NewCurrentTargetGroupOptions{
 		ALBNamePrefix:  "pfx",
 		LoadBalancerID: "nnnnn",
@@ -755,3 +757,7 @@ func (m mockedELBV2) ModifyRule(input *elbv2.ModifyRuleInput) (*elbv2.ModifyRule
 func (m mockedELBV2) DeleteRule(input *elbv2.DeleteRuleInput) (*elbv2.DeleteRuleOutput, error) {
 	return &m.DeleteRuleOutput, m.DeleteRuleError
 }
+
+func (m mockedELBV2) DescribeTargetGroupAttributesFiltered(s *string) (albelbv2.TargetGroupAttributes, error) {
+	return nil, nil
+}

pkg/alb/tg/targetgroup.go

@@ -121,12 +121,18 @@ func NewCurrentTargetGroup(o *NewCurrentTargetGroupOptions) (*TargetGroup, error
 		return nil, fmt.Errorf("The Target Group %s does not have the proper tags, can't import: %s", *o.TargetGroup.TargetGroupArn, err.Error())
 	}
 
+	attrs, err := albelbv2.ELBV2svc.DescribeTargetGroupAttributesFiltered(o.TargetGroup.TargetGroupArn)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to retrieve attributes for Target Group. Error: %s", err.Error())
+	}
+
 	return &TargetGroup{
-		ID:      id,
-		SvcName: svcName,
-		logger:  o.Logger,
-		tags:    tags{current: tgTags},
-		tg:      tg{current: o.TargetGroup},
+		ID:         id,
+		SvcName:    svcName,
+		logger:     o.Logger,
+		attributes: attributes{current: attrs},
+		tags:       tags{current: tgTags},
+		tg:         tg{current: o.TargetGroup},
 	}, nil
 }
 
@@ -234,7 +240,8 @@ func (t *TargetGroup) create(rOpts *ReconcileOptions) error {
 func (t *TargetGroup) modify(mods tgChange, rOpts *ReconcileOptions) error {
 	desired := t.tg.desired
 	if mods&paramsModified != 0 {
-		in := &elbv2.ModifyTargetGroupInput{
+		t.logger.Infof("Modifying target group parameters.")
+		o, err := albelbv2.ELBV2svc.ModifyTargetGroup(&elbv2.ModifyTargetGroupInput{
 			HealthCheckIntervalSeconds: desired.HealthCheckIntervalSeconds,
 			HealthCheckPath:            desired.HealthCheckPath,
 			HealthCheckPort:            desired.HealthCheckPort,
@@ -244,13 +251,11 @@ func (t *TargetGroup) modify(mods tgChange, rOpts *ReconcileOptions) error {
 			Matcher:                    desired.Matcher,
 			TargetGroupArn:             t.CurrentARN(),
 			UnhealthyThresholdCount:    desired.UnhealthyThresholdCount,
-		}
-		o, err := albelbv2.ELBV2svc.ModifyTargetGroup(in)
+		})
 		if err != nil {
 			rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error modifying target group %s: %s", t.ID, err.Error())
-			t.logger.Errorf("Failed TargetGroup modification. ARN: %s | Error: %s.",
+			return fmt.Errorf("Failed TargetGroup modification. ARN: %s | Error: %s",
 				*t.CurrentARN(), err.Error())
-			return err
 		}
 		t.tg.current = o.TargetGroups[0]
 		// AmazonAPI doesn't return an empty HealthCheckPath.
@@ -259,46 +264,45 @@ func (t *TargetGroup) modify(mods tgChange, rOpts *ReconcileOptions) error {
 
 	// check/change tags
 	if mods&tagsModified != 0 {
+		t.logger.Infof("Modifying target group tags.")
 		if err := albelbv2.ELBV2svc.UpdateTags(t.CurrentARN(), t.tags.current, t.tags.desired); err != nil {
 			rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error changing tags on target group %s: %s", t.ID, err.Error())
-			t.logger.Errorf("Failed TargetGroup modification. Unable to modify tags. ARN: %s | Error: %s.",
+			return fmt.Errorf("Failed TargetGroup modification. Unable to modify tags. ARN: %s | Error: %s",
 				*t.CurrentARN(), err.Error())
-			return err
 		}
 		t.tags.current = t.tags.desired
 	}
 
 	if mods&targetsModified != 0 {
+		t.logger.Infof("Modifying target group targets.")
 		additions := util.Difference(t.targets.desired, t.targets.current)
 		removals := util.Difference(t.targets.current, t.targets.desired)
 
 		// check/change targets
 		if len(additions) > 0 {
 			if err := t.registerTargets(additions, rOpts); err != nil {
 				rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error adding targets to target group %s: %s", t.ID, err.Error())
-				t.logger.Infof("Failed TargetGroup modification. Unable to add targets: %s.", err.Error())
-				return err
+				return fmt.Errorf("Failed TargetGroup modification. Unable to add targets: %s", err.Error())
 			}
 		}
 		if len(removals) > 0 {
 			if err := t.deregisterTargets(removals, rOpts); err != nil {
 				rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error removing targets from target group %s: %s", t.ID, err.Error())
-				t.logger.Infof("Failed TargetGroup modification. Unable to remove targets: %s.", err.Error())
-				return err
+				return fmt.Errorf("Failed TargetGroup modification. Unable to remove targets: %s", err.Error())
 			}
 		}
 		t.targets.current = t.targets.desired
 	}
 
 	if mods&attributesModified != 0 {
+		t.logger.Infof("Modifying target group attributes.")
 		aOpts := &elbv2.ModifyTargetGroupAttributesInput{
 			Attributes:     t.attributes.desired,
 			TargetGroupArn: t.CurrentARN(),
 		}
 		if _, err := albelbv2.ELBV2svc.ModifyTargetGroupAttributes(aOpts); err != nil {
 			rOpts.Eventf(api.EventTypeWarning, "ERROR", "Error modifying attributes in target group %s: %s", t.ID, err.Error())
-			t.logger.Infof("Failed TargetGroup modification. Unable to change attributes: %s.", err.Error())
-			return err
+			return fmt.Errorf("Failed TargetGroup modification. Unable to change attributes: %s", err.Error())
 		}
 		t.attributes.current = t.attributes.desired
 	}
@@ -367,15 +371,12 @@ func (t *TargetGroup) needsModification() tgChange {
 		changes |= tagsModified
 	}
 
-	if !reflect.DeepEqual(t.attributes.current.Sorted(), t.attributes.desired.Sorted()) {
+	if !reflect.DeepEqual(t.attributes.current.Filtered().Sorted(), t.attributes.desired.Filtered().Sorted()) {
 		t.logger.Debugf("Attributes need to be changed")
 		changes |= attributesModified
 	}
 
 	return changes
-	// These fields require a rebuild and are enforced via TG name hash
-	//	Port *int64 `min:"1" type:"integer"`
-	//	Protocol *string `type:"string" enum:"ProtocolEnum"`
 }
 
 // Registers Targets (ec2 instances) to TargetGroup, must be called when Current != Desired

pkg/annotations/annotations.go

@@ -27,7 +27,6 @@ var cache = ccache.New(ccache.Configure())
 const (
 	backendProtocolKey            = "alb.ingress.kubernetes.io/backend-protocol"
 	certificateArnKey             = "alb.ingress.kubernetes.io/certificate-arn"
-	connectionIdleTimeoutKey      = "alb.ingress.kubernetes.io/connection-idle-timeout"
 	webACLIdKey                   = "alb.ingress.kubernetes.io/web-acl-id"
 	webACLIdAltKey                = "alb.ingress.kubernetes.io/waf-acl-id"
 	healthcheckIntervalSecondsKey = "alb.ingress.kubernetes.io/healthcheck-interval-seconds"
@@ -61,7 +60,6 @@ const (
 type Annotations struct {
 	BackendProtocol            *string
 	CertificateArn             *string
-	ConnectionIdleTimeout      *int64
 	WebACLId                   *string
 	HealthcheckIntervalSeconds *int64
 	HealthcheckPath            *string
@@ -82,7 +80,7 @@ type Annotations struct {
 	TargetGroupAttributes      albelbv2.TargetGroupAttributes
 	SslPolicy                  *string
 	VPCID                      *string
-	LoadBalancerAttributes     []*elbv2.LoadBalancerAttribute
+	LoadBalancerAttributes     albelbv2.LoadBalancerAttributes
 }
 
 type PortData struct {
@@ -139,7 +137,6 @@ func (vf *ValidatingAnnotationFactory) ParseAnnotations(opts *ParseAnnotationsOp
 	a := new(Annotations)
 	for _, err := range []error{
 		a.setBackendProtocol(annotations),
-		a.setConnectionIdleTimeout(annotations),
 		a.setCertificateArn(annotations, vf.validator),
 		a.setHealthcheckIntervalSeconds(annotations),
 		a.setHealthcheckPath(annotations),
@@ -171,7 +168,6 @@ func (vf *ValidatingAnnotationFactory) ParseAnnotations(opts *ParseAnnotationsOp
 }
 
 func (a *Annotations) setLoadBalancerAttributes(annotations map[string]string) error {
-	var attrs []*elbv2.LoadBalancerAttribute
 	var badAttrs []string
 	v, ok := annotations[loadbalancerAttributesKey]
 	if !ok {
@@ -189,12 +185,8 @@ func (a *Annotations) setLoadBalancerAttributes(annotations map[string]string) e
 			badAttrs = append(badAttrs, *rawAttr)
 			continue
 		}
-		attrs = append(attrs, &elbv2.LoadBalancerAttribute{
-			Key:   aws.String(parts[0]),
-			Value: aws.String(parts[1]),
-		})
+		a.LoadBalancerAttributes.Set(parts[0], parts[1])
 	}
-	a.LoadBalancerAttributes = attrs
 
 	if len(badAttrs) > 0 {
 		return fmt.Errorf("Unable to parse `%s` into Key=Value pair(s)", strings.Join(badAttrs, ", "))
@@ -224,22 +216,6 @@ func (a *Annotations) setCertificateArn(annotations map[string]string, validator
 	return nil
 }
 
-func (a *Annotations) setConnectionIdleTimeout(annotations map[string]string) error {
-	i, err := strconv.ParseInt(annotations[connectionIdleTimeoutKey], 10, 64)
-	if err != nil {
-		if annotations[connectionIdleTimeoutKey] != "" {
-			return err
-		}
-		return nil
-	}
-	// aws only accepts a range of 1-3600 seconds
-	if i < 1 || i > 3600 {
-		return fmt.Errorf("Invalid connection idle timeout provided must be within 1-3600 seconds. Was: %d", i)
-	}
-	a.ConnectionIdleTimeout = aws.Int64(i)
-	return nil
-}
-
 func (a *Annotations) setHealthcheckIntervalSeconds(annotations map[string]string) error {
 	i, err := strconv.ParseInt(annotations[healthcheckIntervalSecondsKey], 10, 64)
 	if err != nil {
@@ -677,16 +653,11 @@ func (a *Annotations) setTags(annotations map[string]string) error {
 	}
 	return nil
 }
+
 func (a *Annotations) setTargetGroupAttributes(annotations map[string]string) error {
 	var badAttrs []string
 	rawAttrs := util.NewAWSStringSlice(annotations[targetGroupAttributesKey])
 
-	a.TargetGroupAttributes.Set("deregistration_delay.timeout_seconds", "300")
-	a.TargetGroupAttributes.Set("slow_start.duration_seconds", "0")
-	a.TargetGroupAttributes.Set("stickiness.enabled", "false")
-	a.TargetGroupAttributes.Set("stickiness.lb_cookie.duration_seconds", "86400")
-	a.TargetGroupAttributes.Set("stickiness.type", "lb_cookie")
-
 	for _, rawAttr := range rawAttrs {
 		parts := strings.Split(*rawAttr, "=")
 		switch {

pkg/annotations/annotations_test.go

@@ -184,21 +184,6 @@ func TestHealthcheckSecondsValidation(t *testing.T) {
 	}
 }
 
-// Should fail when idle timeout is not in range 1-3600. Should succeed otherwise.
-func TestConnectionIdleTimeoutValidation(t *testing.T) {
-	a := &Annotations{}
-
-	err := a.setConnectionIdleTimeout(map[string]string{connectionIdleTimeoutKey: "15"})
-	if err != nil || a.ConnectionIdleTimeout == aws.Int64(0) {
-		t.Error("Failed to set connection idle timeout when value was correct.")
-	}
-
-	err = a.setConnectionIdleTimeout(map[string]string{connectionIdleTimeoutKey: "3700"})
-	if err == nil {
-		t.Error("Succeeded setting connection idle timeout when value was incorrect")
-	}
-}
-
 func TestSetLoadBalancerAttributes(t *testing.T) {
 	var tests = []struct {
 		annotations map[string]string
@@ -299,7 +284,7 @@ func TestSetTargetGroupAttributes(t *testing.T) {
 	annotations := &Annotations{}
 	attributes := map[string]string{targetGroupAttributesKey: "deregistration_delay.timeout_seconds=60,stickiness.enabled=true"}
 	err := annotations.setTargetGroupAttributes(attributes)
-	if err != nil || len(annotations.TargetGroupAttributes) != 5 {
+	if err != nil || len(annotations.TargetGroupAttributes) != 2 {
 		t.Errorf("setTargetGroupAttributes - number of attributes incorrect")
 	}