Merge pull request #14818 from smarterclayton/backcompat_for_routes

Preserve backwards compatibilty for old routes for destination CA

Author: smarterclayton

pkg/cmd/server/origin/legacy.go

@@ -7,6 +7,8 @@ import (
 
 	buildconfigetcd "github.com/openshift/origin/pkg/build/registry/buildconfig/etcd"
 	deploymentconfigetcd "github.com/openshift/origin/pkg/deploy/registry/deployconfig/etcd"
+	routeregistry "github.com/openshift/origin/pkg/route/registry/route"
+	routeetcd "github.com/openshift/origin/pkg/route/registry/route/etcd"
 )
 
 var (
@@ -204,6 +206,11 @@ func LegacyStorage(storage map[schema.GroupVersion]map[string]rest.Storage) map[
 					store := *restStorage.Store
 					restStorage.DeleteStrategy = orphanByDefault(store.DeleteStrategy)
 					legacyStorage[resource] = &deploymentconfigetcd.REST{Store: &store}
+				case "routes":
+					restStorage := s.(*routeetcd.REST)
+					store := *restStorage.Store
+					store.Decorator = routeregistry.DecorateLegacyRouteWithEmptyDestinationCACertificates
+					legacyStorage[resource] = &routeetcd.REST{Store: &store}
 
 				default:
 					legacyStorage[resource] = s

pkg/route/registry/route/strategy.go

@@ -52,13 +52,15 @@ func (routeStrategy) NamespaceScoped() bool {
 func (s routeStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) {
 	route := obj.(*routeapi.Route)
 	route.Status = routeapi.RouteStatus{}
+	stripEmptyDestinationCACertificate(route)
 }
 
 func (s routeStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime.Object) {
 	route := obj.(*routeapi.Route)
 	oldRoute := old.(*routeapi.Route)
-	route.Status = oldRoute.Status
 
+	route.Status = oldRoute.Status
+	stripEmptyDestinationCACertificate(route)
 	// Ignore attempts to clear the spec Host
 	// Prevents "immutable field" errors when applying the same route definition used to create
 	if len(route.Spec.Host) == 0 {
@@ -218,6 +220,56 @@ func (routeStatusStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runti
 	return validation.ValidateRouteStatusUpdate(obj.(*routeapi.Route), old.(*routeapi.Route))
 }
 
+const emptyDestinationCertificate = `-----BEGIN COMMENT-----
+This is an empty PEM file created to provide backwards compatibility
+for reencrypt routes that have no destinationCACertificate. This 
+content will only appear for routes accessed via /oapi/v1/routes.
+-----END COMMENT-----
+`
+
+// stripEmptyDestinationCACertificate removes the empty destinationCACertificate if it matches
+// the current route destination CA certificate.
+func stripEmptyDestinationCACertificate(route *routeapi.Route) {
+	tls := route.Spec.TLS
+	if tls == nil || tls.Termination != routeapi.TLSTerminationReencrypt {
+		return
+	}
+	if tls.DestinationCACertificate == emptyDestinationCertificate {
+		tls.DestinationCACertificate = ""
+	}
+}
+
+// DecorateLegacyRouteWithEmptyDestinationCACertificates is used for /oapi/v1 route endpoints
+// to prevent legacy clients from seeing an empty destination CA certificate for reencrypt routes,
+// which the 'route.openshift.io/v1' endpoint allows. These values are injected in REST responses
+// and stripped in PrepareForCreate and PrepareForUpdate.
+func DecorateLegacyRouteWithEmptyDestinationCACertificates(obj runtime.Object) error {
+	switch t := obj.(type) {
+	case *routeapi.Route:
+		tls := t.Spec.TLS
+		if tls == nil || tls.Termination != routeapi.TLSTerminationReencrypt {
+			return nil
+		}
+		if len(tls.DestinationCACertificate) == 0 {
+			tls.DestinationCACertificate = emptyDestinationCertificate
+		}
+		return nil
+	case *routeapi.RouteList:
+		for i := range t.Items {
+			tls := t.Items[i].Spec.TLS
+			if tls == nil || tls.Termination != routeapi.TLSTerminationReencrypt {
+				continue
+			}
+			if len(tls.DestinationCACertificate) == 0 {
+				tls.DestinationCACertificate = emptyDestinationCertificate
+			}
+		}
+		return nil
+	default:
+		return fmt.Errorf("unknown type passed to %T", obj)
+	}
+}
+
 // GetAttrs returns labels and fields of a given object for filtering purposes
 func GetAttrs(obj runtime.Object) (objLabels labels.Set, objFields fields.Set, err error) {
 	route, ok := obj.(*routeapi.Route)

pkg/route/registry/route/strategy_test.go

@@ -1,6 +1,7 @@
 package route
 
 import (
+	"reflect"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -65,6 +66,47 @@ func TestEmptyHostDefaulting(t *testing.T) {
 	}
 }
 
+func TestEmptyDefaultCACertificate(t *testing.T) {
+	testCases := []struct {
+		route *routeapi.Route
+	}{
+		{
+			route: &routeapi.Route{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:       "foo",
+					Name:            "myroute",
+					UID:             types.UID("abc"),
+					ResourceVersion: "1",
+				},
+				Spec: routeapi.RouteSpec{
+					Host: "myhost.com",
+				},
+			},
+		},
+	}
+	for i, testCase := range testCases {
+		copied, _ := kapi.Scheme.Copy(testCase.route)
+		if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*routeapi.Route)); err != nil {
+			t.Errorf("%d: unexpected error: %v", i, err)
+			continue
+		}
+		routeStrategy{}.PrepareForCreate(nil, copied.(*routeapi.Route))
+		if !reflect.DeepEqual(testCase.route, copied) {
+			t.Errorf("%d: unexpected change: %#v", i, copied)
+			continue
+		}
+		if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*routeapi.Route)); err != nil {
+			t.Errorf("%d: unexpected error: %v", i, err)
+			continue
+		}
+		routeStrategy{}.PrepareForUpdate(nil, copied.(*routeapi.Route), &routeapi.Route{})
+		if !reflect.DeepEqual(testCase.route, copied) {
+			t.Errorf("%d: unexpected change: %#v", i, copied)
+			continue
+		}
+	}
+}
+
 func TestHostWithWildcardPolicies(t *testing.T) {
 	ctx := apirequest.NewContext()
 	ctx = apirequest.WithUser(ctx, &user.DefaultInfo{Name: "bob"})

test/cmd/routes.sh

@@ -32,6 +32,12 @@ os::cmd::expect_success 'oc delete routes foo'
 os::cmd::expect_success_and_text 'oc create route edge --service foo --port=8080' 'created'
 os::cmd::expect_success_and_text 'oc create route edge --service bar --port=9090' 'created'
 
+# verify that reencrypt routes with no destination CA return the stub PEM block on the old API
+project="$(oc project -q)"
+os::cmd::expect_success_and_text     'oc create route reencrypt --service baz --port=9090' 'created'
+os::cmd::expect_success_and_text     'oc get --raw /oapi/v1/namespaces/${project}/routes/baz' 'This is an empty PEM file'
+os::cmd::expect_success_and_not_text 'oc get --raw /apis/route.openshift.io/v1/namespaces/${project}/routes/baz' 'This is an empty PEM file'
+
 os::cmd::expect_success_and_text 'oc set route-backends foo' 'routes/foo'
 os::cmd::expect_success_and_text 'oc set route-backends foo' 'Service'
 os::cmd::expect_success_and_text 'oc set route-backends foo' '100'