Prevent service-specific labels from being put on the revision templa…

…te (#672) See: knative/serving#6865 Signed-off-by: Doug Davis <[email protected]>
knative · Feb 17, 2020 · 83b926b · 83b926b
1 parent a220a88
commit 83b926b
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/pkg/serving/config_changes.go b/pkg/serving/config_changes.go
@@ -362,6 +362,12 @@ func UpdateResources(template *servingv1.RevisionTemplateSpec, requestsResourceL
 	return nil
 }
 
+// ServiceOnlyLabels should only appear on the Service and NOT on the
+// Revision template
+var ServiceOnlyLabels = map[string]struct{}{
+	serving.GroupName + "/visibility": {},
+}
+
 // UpdateLabels updates the labels identically on a service and template.
 // Does not overwrite the entire Labels field, only makes the requested updates
 func UpdateLabels(service *servingv1.Service, template *servingv1.RevisionTemplateSpec, toUpdate map[string]string, toRemove []string) error {
@@ -373,7 +379,11 @@ func UpdateLabels(service *servingv1.Service, template *servingv1.RevisionTempla
 	}
 	for key, value := range toUpdate {
 		service.ObjectMeta.Labels[key] = value
-		template.ObjectMeta.Labels[key] = value
+
+		// Only add it to the template if it's not in our ServiceOnly list
+		if _, ok := ServiceOnlyLabels[key]; !ok {
+			template.ObjectMeta.Labels[key] = value
+		}
 	}
 	for _, key := range toRemove {
 		delete(service.ObjectMeta.Labels, key)

diff --git a/pkg/serving/config_changes_test.go b/pkg/serving/config_changes_test.go
@@ -331,6 +331,25 @@ func TestUpdateLabelsNew(t *testing.T) {
 		"a": "foo",
 		"b": "bar",
 	}
+	tLabels := labels // revision template labels
+
+	// Only test service-specific labels if we have any
+	if len(ServiceOnlyLabels) != 0 {
+		// Make a copy of the expected labels so we can modify the original
+		// list w/o changing what's expected for the revsion template
+		tLabels = map[string]string{}
+		for k, v := range labels {
+			tLabels[k] = v
+		}
+
+		// Just add a random value from the list to make sure it doesn't show
+		// up in the revision template
+		for k := range ServiceOnlyLabels {
+			labels[k] = "testing"
+			break
+		}
+	}
+
 	err := UpdateLabels(service, template, labels, []string{})
 	assert.NilError(t, err)
 
@@ -340,8 +359,8 @@ func TestUpdateLabelsNew(t *testing.T) {
 	}
 
 	actual = template.ObjectMeta.Labels
-	if !reflect.DeepEqual(labels, actual) {
-		t.Fatalf("Template labels did not match expected %v found %v", labels, actual)
+	if !reflect.DeepEqual(tLabels, actual) {
+		t.Fatalf("Template labels did not match expected %v found %v", tLabels, actual)
 	}
 }