From 5c1e198be44b9d8fd0601eca78ed5616117f756b Mon Sep 17 00:00:00 2001
From: Dan Gerdesmeier <dangerd@google.com>
Date: Thu, 5 Dec 2019 15:17:38 -0800
Subject: [PATCH] Add upgrade test for new defaults (#6165)

* Add upgrade test for new defaults

This adds a new test using BYO Revision name to make sure that the
immutability contract is not violated by adding new defaults to the
Service template.

Fixes #6077

* CR comments
---
 test/upgrade/service_postupgrade_test.go | 28 ++++++++++++++++++++++--
 test/upgrade/service_preupgrade_test.go  | 21 ++++++++++++++++++
 test/upgrade/upgrade.go                  |  2 ++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/test/upgrade/service_postupgrade_test.go b/test/upgrade/service_postupgrade_test.go
index 3a122391705b..dea7d01facf3 100644
--- a/test/upgrade/service_postupgrade_test.go
+++ b/test/upgrade/service_postupgrade_test.go
@@ -22,10 +22,13 @@ import (
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/ptr"
 	ptest "knative.dev/pkg/test"
+	"knative.dev/serving/pkg/apis/serving/v1"
 	serviceresourcenames "knative.dev/serving/pkg/reconciler/service/resources/names"
 	"knative.dev/serving/test"
 	"knative.dev/serving/test/e2e"
+	v1test "knative.dev/serving/test/v1"
 	v1a1test "knative.dev/serving/test/v1alpha1"
 )
 
@@ -39,11 +42,32 @@ func TestRunLatestServicePostUpgradeFromScaleToZero(t *testing.T) {
 	updateService(scaleToZeroServiceName, t)
 }
 
+// TestBYORevisionPostUpgrade attempts to update the RouteSpec of a Service using BYO Revision name. This
+// test is meant to catch new defaults that break the immutability of BYO Revision name.
+func TestBYORevisionPostUpgrade(t *testing.T) {
+	t.Parallel()
+	clients := e2e.Setup(t)
+	names := test.ResourceNames{
+		Service: byoServiceName,
+	}
+
+	if _, err := v1test.UpdateServiceRouteSpec(t, clients, names, v1.RouteSpec{
+		Traffic: []v1.TrafficTarget{{
+			Tag:          "example-tag",
+			RevisionName: byoRevName,
+			Percent:      ptr.Int64(100),
+		}},
+	}); err != nil {
+		t.Fatalf("Failed to update Service: %v", err)
+	}
+}
+
 func updateService(serviceName string, t *testing.T) {
 	t.Helper()
 	clients := e2e.Setup(t)
-	var names test.ResourceNames
-	names.Service = serviceName
+	names := test.ResourceNames{
+		Service: serviceName,
+	}
 
 	t.Logf("Getting service %q", names.Service)
 	svc, err := clients.ServingAlphaClient.Services.Get(names.Service, metav1.GetOptions{})
diff --git a/test/upgrade/service_preupgrade_test.go b/test/upgrade/service_preupgrade_test.go
index e28eab9ebacb..9be0cb736c8d 100644
--- a/test/upgrade/service_preupgrade_test.go
+++ b/test/upgrade/service_preupgrade_test.go
@@ -22,10 +22,12 @@ import (
 	"testing"
 
 	"knative.dev/serving/pkg/apis/autoscaling"
+	"knative.dev/serving/pkg/apis/serving/v1"
 	revisionresourcenames "knative.dev/serving/pkg/reconciler/revision/resources/names"
 	rtesting "knative.dev/serving/pkg/testing/v1alpha1"
 	"knative.dev/serving/test"
 	"knative.dev/serving/test/e2e"
+	v1test "knative.dev/serving/test/v1"
 	v1a1test "knative.dev/serving/test/v1alpha1"
 )
 
@@ -72,3 +74,22 @@ func TestRunLatestServicePreUpgradeAndScaleToZero(t *testing.T) {
 		t.Fatalf("Could not scale to zero: %v", err)
 	}
 }
+
+// TestBYORevisionUpgrade creates a Service that uses the BYO Revision name functionality. This test
+// is meant to catch new defaults that break bring your own revision name immutability.
+func TestBYORevisionPreUpgrade(t *testing.T) {
+	t.Parallel()
+	clients := e2e.Setup(t)
+	names := test.ResourceNames{
+		Service: byoServiceName,
+		Image:   test.PizzaPlanet1,
+	}
+
+	_, err := v1test.CreateServiceReady(t, clients, &names,
+		func(svc *v1.Service) {
+			svc.Spec.ConfigurationSpec.Template.Name = byoRevName
+		})
+	if err != nil {
+		t.Fatalf("Failed to create Service: %v", err)
+	}
+}
diff --git a/test/upgrade/upgrade.go b/test/upgrade/upgrade.go
index b2c446332668..bcb360209d7c 100644
--- a/test/upgrade/upgrade.go
+++ b/test/upgrade/upgrade.go
@@ -35,6 +35,8 @@ const (
 	// multiple "go test" invocations.
 	serviceName            = "pizzaplanet-upgrade-service"
 	scaleToZeroServiceName = "scale-to-zero-upgrade-service"
+	byoServiceName         = "byo-revision-name-upgrade-test"
+	byoRevName             = byoServiceName + "-" + "rev1"
 )
 
 // Shamelessly cribbed from conformance/service_test.