Merge tags on service and ingress

Author: oliviassss

pkg/ingress/model_build_tags.go

@@ -49,14 +49,19 @@ func (t *defaultModelBuildTask) buildIngressResourceTags(ing ClassifiedIngress)
 
 // buildIngressBackendResourceTags builds the AWS Tags used for a single Ingress and Backend. e.g. TargetGroup.
 // Note: the Tags specified via IngressClass takes higher priority than tags specified via annotation on Ingress or Service.
-//		 the Tags annotation of Service takes higher priority than annotation of Ingress. (TODO: we might consider change this behavior to merge tags instead)
+//		 the target group will have the merged tags specified by the annotations of both Ingress and Service
+// 		 the Tags annotation of Service takes higher priority if there is conflict between the tags of Ingress and Service
 func (t *defaultModelBuildTask) buildIngressBackendResourceTags(ing ClassifiedIngress, backend *corev1.Service) (map[string]string, error) {
-	mergedAnnotations := algorithm.MergeStringMap(backend.Annotations, ing.Ing.Annotations)
-	var annotationTags map[string]string
-	if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &annotationTags, mergedAnnotations); err != nil {
+	var backendAnnotationTags map[string]string
+	var ingressAnnotationTags map[string]string
+	if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &backendAnnotationTags, backend.Annotations); err != nil {
 		return nil, err
 	}
-	if err := t.validateTagCollisionWithExternalManagedTags(annotationTags); err != nil {
+	if _, err := t.annotationParser.ParseStringMapAnnotation(annotations.IngressSuffixTags, &ingressAnnotationTags, ing.Ing.Annotations); err != nil {
+		return nil, err
+	}
+	mergedAnnotationTags := algorithm.MergeStringMap(backendAnnotationTags, ingressAnnotationTags)
+	if err := t.validateTagCollisionWithExternalManagedTags(mergedAnnotationTags); err != nil {
 		return nil, errors.Wrapf(err, "failed build tags for Ingress %v and Service %v",
 			k8s.NamespacedName(ing.Ing).String(), k8s.NamespacedName(backend).String())
 	}
@@ -66,7 +71,7 @@ func (t *defaultModelBuildTask) buildIngressBackendResourceTags(ing ClassifiedIn
 		return nil, err
 	}
 
-	return algorithm.MergeStringMap(ingClassTags, annotationTags), nil
+	return algorithm.MergeStringMap(ingClassTags, mergedAnnotationTags), nil
 }
 
 // buildIngressClassResourceTags builds the AWS Tags for a IngressClass.

pkg/ingress/model_build_tags_test.go

@@ -276,6 +276,7 @@ func Test_defaultModelBuildTask_buildIngressBackendResourceTags(t *testing.T) {
 			want: map[string]string{
 				"tag-c": "value-c1",
 				"tag-d": "value-d1",
+				"tag-e": "value-e",
 			},
 		},
 		{
@@ -403,6 +404,59 @@ func Test_defaultModelBuildTask_buildIngressBackendResourceTags(t *testing.T) {
 				"tag-e": "value-e",
 			},
 		},
+		{
+			name: "non-empty annotation tags from Ingress and Service, non-empty IngressClass tags",
+			fields: fields{
+				externalManagedTags: sets.NewString("tag-a", "tag-b"),
+			},
+			args: args{
+				ing: ClassifiedIngress{
+					Ing: &networking.Ingress{
+						ObjectMeta: metav1.ObjectMeta{
+							Namespace: "awesome-ns",
+							Name:      "awesome-ing",
+							Annotations: map[string]string{
+								"alb.ingress.kubernetes.io/tags": "tag-c=value-c,tag-d=value-d, tag-f=value-f",
+							},
+						},
+					},
+					IngClassConfig: ClassConfiguration{
+						IngClassParams: &elbv2api.IngressClassParams{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "awesome-class",
+							},
+							Spec: elbv2api.IngressClassParamsSpec{
+								Tags: []elbv2api.Tag{
+									{
+										Key:   "tag-d",
+										Value: "value-d1",
+									},
+									{
+										Key:   "tag-e",
+										Value: "value-e",
+									},
+								},
+							},
+						},
+					},
+				},
+				backend: &corev1.Service{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "awesome-ns",
+						Name:      "awesome-svc",
+						Annotations: map[string]string{
+							"alb.ingress.kubernetes.io/tags": "tag-c=value-c,tag-d=value-d2",
+						},
+					},
+				},
+			},
+			want: map[string]string{
+				"tag-c": "value-c",
+				"tag-d": "value-d1",
+				"tag-e": "value-e",
+				"tag-f": "value-f",
+			},
+		},
 		{
 			name: "empty tags from Ingress & Service, empty tags from IngressClass",
 			fields: fields{

test/e2e/ingress/multi_path_backend.go

@@ -257,7 +257,7 @@ func (s *multiPathBackendStack) buildIngressResource(ns *corev1.Namespace, ingID
 		backendSVC := svcByBackendID[pathCFG.BackendID]
 		exact := networking.PathTypeExact
 		ing.Spec.Rules[0].HTTP.Paths = append(ing.Spec.Rules[0].HTTP.Paths, networking.HTTPIngressPath{
-			Path: pathCFG.Path,
+			Path:     pathCFG.Path,
 			PathType: &exact,
 			Backend: networking.IngressBackend{
 				Service: &networking.IngressServiceBackend{