Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve backwards compatibilty for old routes for destination CA #14818

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkg/cmd/server/origin/legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liggitt I haven't used the decorator before. Does it play nicely the various storage caches involved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

things are copied before they come out of the cache, because of self link (which is always set)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it's good to double check. We've used Decorators for the last 3 years with ImageStreams, so not terribly worried.

legacyStorage[resource] = &routeetcd.REST{Store: &store}

default:
legacyStorage[resource] = s
Expand Down
54 changes: 53 additions & 1 deletion pkg/route/registry/route/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
42 changes: 42 additions & 0 deletions pkg/route/registry/route/strategy_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"reflect"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -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"})
Expand Down
6 changes: 6 additions & 0 deletions test/cmd/routes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down