From 9df28d4c226629d447d9a718de573a5642519dd6 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 21 Jun 2017 18:23:13 -0400 Subject: [PATCH] Preserve backwards compatibilty for old routes for destination CA OpenShift 3.6 allows destinationCACertificates on the new route.openshift.io group for reencrypt routes to be empty. To preserve backwards compatibility for the existing route API, set a simple "no-op" PEM into the returned REST response, and strip it if a client round trips it. A v1 client that tries to send an empty destinationCACertificate will be allowed to do so, but will get back a response that includes the empty PEM file. --- pkg/cmd/server/origin/legacy.go | 7 +++ pkg/route/registry/route/strategy.go | 55 ++++++++++++++++++++++- pkg/route/registry/route/strategy_test.go | 42 +++++++++++++++++ test/cmd/routes.sh | 6 +++ 4 files changed, 109 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/server/origin/legacy.go b/pkg/cmd/server/origin/legacy.go index e51878eb4edb..070976ad2686 100644 --- a/pkg/cmd/server/origin/legacy.go +++ b/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 diff --git a/pkg/route/registry/route/strategy.go b/pkg/route/registry/route/strategy.go index 38a664e13ea9..f4aa366cbe33 100644 --- a/pkg/route/registry/route/strategy.go +++ b/pkg/route/registry/route/strategy.go @@ -52,13 +52,16 @@ func (routeStrategy) NamespaceScoped() bool { func (s routeStrategy) PrepareForCreate(ctx apirequest.Context, obj runtime.Object) { route := obj.(*api.Route) route.Status = api.RouteStatus{} + + stripEmptyDestinationCACertificate(route) } func (s routeStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime.Object) { route := obj.(*api.Route) oldRoute := old.(*api.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 +221,56 @@ func (routeStatusStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runti return validation.ValidateRouteStatusUpdate(obj.(*api.Route), old.(*api.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 *api.Route) { + tls := route.Spec.TLS + if tls == nil || tls.Termination != api.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 *api.Route: + tls := t.Spec.TLS + if tls == nil || tls.Termination != api.TLSTerminationReencrypt { + return nil + } + if len(tls.DestinationCACertificate) == 0 { + tls.DestinationCACertificate = emptyDestinationCertificate + } + return nil + case *api.RouteList: + for i := range t.Items { + tls := t.Items[i].Spec.TLS + if tls == nil || tls.Termination != api.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.(*api.Route) diff --git a/pkg/route/registry/route/strategy_test.go b/pkg/route/registry/route/strategy_test.go index cc6a4a9a2e7e..c6cdc1cd6aab 100644 --- a/pkg/route/registry/route/strategy_test.go +++ b/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 *api.Route + }{ + { + route: &api.Route{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "myroute", + UID: types.UID("abc"), + ResourceVersion: "1", + }, + Spec: api.RouteSpec{ + Host: "myhost.com", + }, + }, + }, + } + for i, testCase := range testCases { + copied, _ := kapi.Scheme.Copy(testCase.route) + if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*api.Route)); err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + continue + } + routeStrategy{}.PrepareForCreate(nil, copied.(*api.Route)) + if !reflect.DeepEqual(testCase.route, copied) { + t.Errorf("%d: unexpected change: %#v", i, copied) + continue + } + if err := DecorateLegacyRouteWithEmptyDestinationCACertificates(copied.(*api.Route)); err != nil { + t.Errorf("%d: unexpected error: %v", i, err) + continue + } + routeStrategy{}.PrepareForUpdate(nil, copied.(*api.Route), &api.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"}) diff --git a/test/cmd/routes.sh b/test/cmd/routes.sh index dc6339cdb82c..2f4681c2683f 100755 --- a/test/cmd/routes.sh +++ b/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'