Allow controlling spec.host via a new permission

In order to simplify control over custom routes (routes with a
non-default spec.host), make using a custom host field require the
"create" verb on "routes/custom-host". By default, admin and edit within
a project will get this permission. Users are also prohibited from
setting or altering certificates without this permission.

Integrators who want to prohibit this can drop the permission from the
role and only allow cluster administrators to fill that field.

pkg/cmd/server/bootstrappolicy/policy.go

@@ -355,6 +355,8 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 				authorizationapi.NewRule(read...).Groups(quotaGroup, legacyQuotaGroup).Resources("appliedclusterresourcequotas").RuleOrDie(),
 
 				authorizationapi.NewRule(readWrite...).Groups(routeGroup, legacyRouteGroup).Resources("routes").RuleOrDie(),
+				// admins can create routes with custom hosts
+				authorizationapi.NewRule("create").Groups(routeGroup, legacyRouteGroup).Resources("routes/custom-host").RuleOrDie(),
 				authorizationapi.NewRule(read...).Groups(routeGroup, legacyRouteGroup).Resources("routes/status").RuleOrDie(),
 				// an admin can run routers that write back conditions to the route
 				authorizationapi.NewRule("update").Groups(routeGroup, legacyRouteGroup).Resources("routes/status").RuleOrDie(),
@@ -413,6 +415,8 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole {
 				authorizationapi.NewRule(read...).Groups(quotaGroup, legacyQuotaGroup).Resources("appliedclusterresourcequotas").RuleOrDie(),
 
 				authorizationapi.NewRule(readWrite...).Groups(routeGroup, legacyRouteGroup).Resources("routes").RuleOrDie(),
+				// editors can create routes with custom hosts
+				authorizationapi.NewRule("create").Groups(routeGroup, legacyRouteGroup).Resources("routes/custom-host").RuleOrDie(),
 				authorizationapi.NewRule(read...).Groups(routeGroup, legacyRouteGroup).Resources("routes/status").RuleOrDie(),
 
 				authorizationapi.NewRule(readWrite...).Groups(templateGroup, legacyTemplateGroup).Resources("templates", "templateconfigs", "processedtemplates", "templateinstances").RuleOrDie(),

pkg/cmd/server/origin/master.go

@@ -631,11 +631,6 @@ func (c *MasterConfig) GetRestStorage() map[schema.GroupVersion]map[string]rest.
 	checkStorageErr(err)
 	deployConfigRegistry := deployconfigregistry.NewRegistry(deployConfigStorage)
 
-	routeAllocator := c.RouteAllocator()
-
-	routeStorage, routeStatusStorage, err := routeetcd.NewREST(c.RESTOptionsGetter, routeAllocator)
-	checkStorageErr(err)
-
 	hostSubnetStorage, err := hostsubnetetcd.NewREST(c.RESTOptionsGetter)
 	checkStorageErr(err)
 	netNamespaceStorage, err := netnamespaceetcd.NewREST(c.RESTOptionsGetter)
@@ -721,6 +716,10 @@ func (c *MasterConfig) GetRestStorage() map[schema.GroupVersion]map[string]rest.
 	imageStreamImageStorage := imagestreamimage.NewREST(imageRegistry, imageStreamRegistry)
 	imageStreamImageRegistry := imagestreamimage.NewRegistry(imageStreamImageStorage)
 
+	routeAllocator := c.RouteAllocator()
+	routeStorage, routeStatusStorage, err := routeetcd.NewREST(c.RESTOptionsGetter, routeAllocator, subjectAccessReviewRegistry)
+	checkStorageErr(err)
+
 	buildGenerator := &buildgenerator.BuildGenerator{
 		Client: buildgenerator.Client{
 			GetBuildConfigFunc:      buildConfigRegistry.GetBuildConfig,

pkg/ingress/admission/ingress_admission.go

@@ -7,10 +7,15 @@ import (
 	"io"
 	"reflect"
 
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	kextensions "k8s.io/kubernetes/pkg/apis/extensions"
 
+	oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
 	configlatest "github.com/openshift/origin/pkg/cmd/server/api/latest"
 	"github.com/openshift/origin/pkg/ingress/admission/api"
 )
@@ -31,9 +36,12 @@ func init() {
 
 type ingressAdmission struct {
 	*admission.Handler
-	config *api.IngressAdmissionConfig
+	config     *api.IngressAdmissionConfig
+	authorizer authorizer.Authorizer
 }
 
+var _ = oadmission.WantsAuthorizer(&ingressAdmission{})
+
 func NewIngressAdmission(config *api.IngressAdmissionConfig) *ingressAdmission {
 	return &ingressAdmission{
 		Handler: admission.NewHandler(admission.Create, admission.Update),
@@ -60,19 +68,77 @@ func readConfig(reader io.Reader) (*api.IngressAdmissionConfig, error) {
 	return config, nil
 }
 
+func (r *ingressAdmission) SetAuthorizer(a authorizer.Authorizer) {
+	r.authorizer = a
+}
+
+func (r *ingressAdmission) Validate() error {
+	if r.authorizer == nil {
+		return fmt.Errorf("%s needs an Openshift Authorizer", IngressAdmission)
+	}
+	return nil
+}
+
 func (r *ingressAdmission) Admit(a admission.Attributes) error {
-	if a.GetResource().GroupResource() == kextensions.Resource("ingresses") && a.GetOperation() == admission.Update {
-		if r.config == nil || r.config.AllowHostnameChanges == false {
-			oldIngress, ok := a.GetOldObject().(*kextensions.Ingress)
-			if !ok {
-				return nil
-			}
-			newIngress, ok := a.GetObject().(*kextensions.Ingress)
-			if !ok {
-				return nil
+	if a.GetResource().GroupResource() == kextensions.Resource("ingresses") {
+		switch a.GetOperation() {
+		case admission.Create:
+			if ingress, ok := a.GetObject().(*kextensions.Ingress); ok {
+				// if any rules have a host, check whether the user has permission to set them
+				for i, rule := range ingress.Spec.Rules {
+					if len(rule.Host) > 0 {
+						attr := authorizer.AttributesRecord{
+							User:            a.GetUserInfo(),
+							Verb:            "create",
+							Namespace:       a.GetNamespace(),
+							Resource:        "routes",
+							Subresource:     "custom-host",
+							APIGroup:        "route.openshift.io",
+							ResourceRequest: true,
+						}
+						kind := schema.GroupKind{Group: a.GetResource().Group, Kind: a.GetResource().Resource}
+						allow, _, err := r.authorizer.Authorize(attr)
+						if err != nil {
+							return errors.NewInvalid(kind, ingress.Name, field.ErrorList{field.InternalError(field.NewPath("spec", "rules").Index(i), err)})
+						}
+						if !allow {
+							return errors.NewInvalid(kind, ingress.Name, field.ErrorList{field.Forbidden(field.NewPath("spec", "rules").Index(i), "you do not have permission to set host fields in ingress rules")})
+						}
+						break
+					}
+				}
 			}
-			if !haveHostnamesChanged(oldIngress, newIngress) {
-				return fmt.Errorf("cannot change hostname")
+		case admission.Update:
+			if r.config == nil || r.config.AllowHostnameChanges == false {
+				oldIngress, ok := a.GetOldObject().(*kextensions.Ingress)
+				if !ok {
+					return nil
+				}
+				newIngress, ok := a.GetObject().(*kextensions.Ingress)
+				if !ok {
+					return nil
+				}
+				if !haveHostnamesChanged(oldIngress, newIngress) {
+					attr := authorizer.AttributesRecord{
+						User:            a.GetUserInfo(),
+						Verb:            "update",
+						Namespace:       a.GetNamespace(),
+						Name:            a.GetName(),
+						Resource:        "routes",
+						Subresource:     "custom-host",
+						APIGroup:        "route.openshift.io",
+						ResourceRequest: true,
+					}
+					kind := schema.GroupKind{Group: a.GetResource().Group, Kind: a.GetResource().Resource}
+					allow, _, err := r.authorizer.Authorize(attr)
+					if err != nil {
+						return errors.NewInvalid(kind, newIngress.Name, field.ErrorList{field.InternalError(field.NewPath("spec", "rules"), err)})
+					}
+					if allow {
+						return nil
+					}
+					return fmt.Errorf("cannot change hostname")
+				}
 			}
 		}
 	}

pkg/ingress/admission/ingress_admission_test.go

@@ -5,13 +5,22 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
 	kextensions "k8s.io/kubernetes/pkg/apis/extensions"
 
 	"github.com/openshift/origin/pkg/ingress/admission/api"
 )
 
-func TestAdmission(t *testing.T) {
+type fakeAuthorizer struct {
+	allow bool
+	err   error
+}
+
+func (a *fakeAuthorizer) Authorize(authorizer.Attributes) (bool, string, error) {
+	return a.allow, "", a.err
+}
 
+func TestAdmission(t *testing.T) {
 	var newIngress *kextensions.Ingress
 	var oldIngress *kextensions.Ingress
 
@@ -21,6 +30,7 @@ func TestAdmission(t *testing.T) {
 		oldHost, newHost string
 		op               admission.Operation
 		admit            bool
+		allow            bool
 	}{
 		{
 			admit:    true,
@@ -51,6 +61,15 @@ func TestAdmission(t *testing.T) {
 			oldHost:  "bar.com",
 			testName: "changing hostname should fail",
 		},
+		{
+			admit:    true,
+			allow:    true,
+			config:   emptyConfig(),
+			op:       admission.Update,
+			newHost:  "foo.com",
+			oldHost:  "bar.com",
+			testName: "changing hostname should succeed if the user has permission",
+		},
 		{
 			admit:    false,
 			config:   nil,
@@ -74,6 +93,22 @@ func TestAdmission(t *testing.T) {
 			newHost:  "foo.com",
 			testName: "add new hostname with upstream rules",
 		},
+		{
+			admit:    false,
+			allow:    false,
+			config:   emptyConfig(),
+			op:       admission.Create,
+			newHost:  "foo.com",
+			testName: "setting the host should require permission",
+		},
+		{
+			admit:    true,
+			allow:    true,
+			config:   emptyConfig(),
+			op:       admission.Create,
+			newHost:  "foo.com",
+			testName: "setting the host should pass if user has permission",
+		},
 	}
 	for _, test := range tests {
 		if len(test.newHost) > 0 {
@@ -94,6 +129,7 @@ func TestAdmission(t *testing.T) {
 			}
 		}
 		handler := NewIngressAdmission(test.config)
+		handler.SetAuthorizer(&fakeAuthorizer{allow: test.allow})
 
 		if len(test.oldHost) > 0 {
 			//Provides the previous state of an ingress object

pkg/route/api/validation/validation.go

@@ -11,7 +11,6 @@ import (
 	kvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/api/validation"
-	kval "k8s.io/kubernetes/pkg/api/validation"
 
 	oapi "github.com/openshift/origin/pkg/api"
 	cmdutil "github.com/openshift/origin/pkg/cmd/util"
@@ -21,7 +20,7 @@ import (
 // ValidateRoute tests if required fields in the route are set.
 func ValidateRoute(route *routeapi.Route) field.ErrorList {
 	//ensure meta is set properly
-	result := kval.ValidateObjectMeta(&route.ObjectMeta, true, oapi.GetNameValidationFunc(kval.ValidatePodName), field.NewPath("metadata"))
+	result := validation.ValidateObjectMeta(&route.ObjectMeta, true, oapi.GetNameValidationFunc(validation.ValidatePodName), field.NewPath("metadata"))
 
 	specPath := field.NewPath("spec")
 
@@ -94,7 +93,6 @@ func ValidateRoute(route *routeapi.Route) field.ErrorList {
 
 func ValidateRouteUpdate(route *routeapi.Route, older *routeapi.Route) field.ErrorList {
 	allErrs := validation.ValidateObjectMetaUpdate(&route.ObjectMeta, &older.ObjectMeta, field.NewPath("metadata"))
-	allErrs = append(allErrs, validation.ValidateImmutableField(route.Spec.Host, older.Spec.Host, field.NewPath("spec", "host"))...)
 	allErrs = append(allErrs, validation.ValidateImmutableField(route.Spec.WildcardPolicy, older.Spec.WildcardPolicy, field.NewPath("spec", "wildcardPolicy"))...)
 	allErrs = append(allErrs, ValidateRoute(route)...)
 	return allErrs

pkg/route/api/validation/validation_test.go

@@ -1113,7 +1113,7 @@ func TestValidateRouteUpdate(t *testing.T) {
 				},
 			},
 			change:         func(route *api.Route) { route.Spec.Host = "" },
-			expectedErrors: 1,
+			expectedErrors: 0, // now controlled by rbac
 		},
 		{
 			route: &api.Route{
@@ -1131,7 +1131,7 @@ func TestValidateRouteUpdate(t *testing.T) {
 				},
 			},
 			change:         func(route *api.Route) { route.Spec.Host = "other" },
-			expectedErrors: 1,
+			expectedErrors: 0, // now controlled by rbac
 		},
 		{
 			route: &api.Route{

pkg/route/registry/route/etcd/etcd.go

@@ -20,8 +20,8 @@ type REST struct {
 }
 
 // NewREST returns a RESTStorage object that will work against routes.
-func NewREST(optsGetter restoptions.Getter, allocator route.RouteAllocator) (*REST, *StatusREST, error) {
-	strategy := rest.NewStrategy(allocator)
+func NewREST(optsGetter restoptions.Getter, allocator route.RouteAllocator, sarClient rest.SubjectAccessReviewInterface) (*REST, *StatusREST, error) {
+	strategy := rest.NewStrategy(allocator, sarClient)
 
 	store := &registry.Store{
 		Copier:            kapi.Scheme,

pkg/route/registry/route/etcd/etcd_test.go

@@ -11,6 +11,7 @@ import (
 	etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
 
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
 	routetypes "github.com/openshift/origin/pkg/route"
 	"github.com/openshift/origin/pkg/route/api"
 	_ "github.com/openshift/origin/pkg/route/api/install"
@@ -34,9 +35,20 @@ func (a *testAllocator) GenerateHostname(*api.Route, *api.RouterShard) string {
 	return a.Hostname
 }
 
+type testSAR struct {
+	allow bool
+	err   error
+	sar   *authorizationapi.SubjectAccessReview
+}
+
+func (t *testSAR) CreateSubjectAccessReview(ctx apirequest.Context, subjectAccessReview *authorizationapi.SubjectAccessReview) (*authorizationapi.SubjectAccessReviewResponse, error) {
+	t.sar = subjectAccessReview
+	return &authorizationapi.SubjectAccessReviewResponse{Allowed: t.allow}, t.err
+}
+
 func newStorage(t *testing.T, allocator routetypes.RouteAllocator) (*REST, *etcdtesting.EtcdTestServer) {
 	etcdStorage, server := registrytest.NewEtcdStorage(t, "")
-	storage, _, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), allocator)
+	storage, _, err := NewREST(restoptions.NewSimpleGetter(etcdStorage), allocator, &testSAR{allow: true})
 	if err != nil {
 		t.Fatal(err)
 	}