diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index d48f7183a4b..2ba2ab7d07a 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -2008,3 +2008,24 @@ func ValidatePodLogOptions(opts *api.PodLogOptions) errs.ValidationErrorList { } return allErrs } + +// ValidateLoadBalancerStatus validates required fields on a LoadBalancerStatus +func ValidateLoadBalancerStatus(status *api.LoadBalancerStatus) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + for _, ingress := range status.Ingress { + if len(ingress.IP) > 0 { + if isIP := (net.ParseIP(ingress.IP) != nil); !isIP { + allErrs = append(allErrs, errs.NewFieldInvalid("ingress.ip", ingress.IP, "must be an IP address")) + } + } + if len(ingress.Hostname) > 0 { + if valid, errMsg := NameIsDNSSubdomain(ingress.Hostname, false); !valid { + allErrs = append(allErrs, errs.NewFieldInvalid("ingress.hostname", ingress.Hostname, errMsg)) + } + if isIP := (net.ParseIP(ingress.Hostname) != nil); isIP { + allErrs = append(allErrs, errs.NewFieldInvalid("ingress.hostname", ingress.Hostname, "must be a DNS name, not ip address")) + } + } + } + return allErrs +} diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 33a38d24792..a1b3f96f965 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -443,6 +443,14 @@ func ValidateIngressUpdate(oldIngress, ingress *extensions.Ingress) errs.Validat return allErrs } +// ValidateIngressStatusUpdate tests if required fields in the Ingress are set when updating status. +func ValidateIngressStatusUpdate(ingress, oldIngress *extensions.Ingress) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + allErrs = append(allErrs, apivalidation.ValidateObjectMetaUpdate(&ingress.ObjectMeta, &oldIngress.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, apivalidation.ValidateLoadBalancerStatus(&ingress.Status.LoadBalancer).Prefix("status.loadBalancer")...) + return allErrs +} + func validateIngressRules(IngressRules []extensions.IngressRule) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} if len(IngressRules) == 0 { diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index 1f9b7f31c9f..ddbfa0f881b 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -942,6 +942,98 @@ func TestValidateIngress(t *testing.T) { } } +func TestValidateIngressStatusUpdate(t *testing.T) { + defaultBackend := extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: util.IntOrString{Kind: util.IntstrInt, IntVal: 80}, + } + + newValid := func() extensions.Ingress { + return extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + ResourceVersion: "9", + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: util.IntOrString{Kind: util.IntstrInt, IntVal: 80}, + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/foo", + Backend: defaultBackend, + }, + }, + }, + }, + }, + }, + }, + Status: extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "127.0.0.1", Hostname: "foo.bar.com"}, + }, + }, + }, + } + } + oldValue := newValid() + newValue := newValid() + newValue.Status = extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "127.0.0.2", Hostname: "foo.com"}, + }, + }, + } + invalidIP := newValid() + invalidIP.Status = extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "abcd", Hostname: "foo.com"}, + }, + }, + } + invalidHostname := newValid() + invalidHostname.Status = extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "127.0.0.1", Hostname: "127.0.0.1"}, + }, + }, + } + + errs := ValidateIngressStatusUpdate(&newValue, &oldValue) + if len(errs) != 0 { + t.Errorf("Unexpected error %v", errs) + } + + errorCases := map[string]extensions.Ingress{ + "status.loadBalancer.ingress.ip: invalid value": invalidIP, + "status.loadBalancer.ingress.hostname: invalid value": invalidHostname, + } + for k, v := range errorCases { + errs := ValidateIngressStatusUpdate(&v, &oldValue) + if len(errs) == 0 { + t.Errorf("expected failure for %s", k) + } else { + s := strings.Split(k, ":") + err := errs[0].(*errors.ValidationError) + if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) { + t.Errorf("unexpected error: %v, expected: %s", errs[0], k) + } + } + } +} + func TestValidateClusterAutoscaler(t *testing.T) { successCases := []extensions.ClusterAutoscaler{ { diff --git a/pkg/master/master.go b/pkg/master/master.go index 0f886fe8c5a..d62afadcd55 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -1032,7 +1032,7 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion { daemonSetStorage, daemonSetStatusStorage := daemonetcd.NewREST(dbClient("daemonsets")) deploymentStorage := deploymentetcd.NewStorage(dbClient("deployments")) jobStorage, jobStatusStorage := jobetcd.NewREST(dbClient("jobs")) - ingressStorage := ingressetcd.NewREST(dbClient("ingress")) + ingressStorage, ingressStatusStorage := ingressetcd.NewREST(dbClient("ingress")) thirdPartyControl := ThirdPartyController{ master: m, @@ -1058,6 +1058,7 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion { strings.ToLower("jobs"): jobStorage, strings.ToLower("jobs/status"): jobStatusStorage, strings.ToLower("ingress"): ingressStorage, + strings.ToLower("ingress/status"): ingressStatusStorage, } expMeta := latest.GroupOrDie("extensions") diff --git a/pkg/registry/ingress/etcd/etcd.go b/pkg/registry/ingress/etcd/etcd.go index b9f3c61650c..bd5203d40fc 100644 --- a/pkg/registry/ingress/etcd/etcd.go +++ b/pkg/registry/ingress/etcd/etcd.go @@ -38,7 +38,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against replication controllers. -func NewREST(s storage.Interface) *REST { +func NewREST(s storage.Interface) (*REST, *StatusREST) { store := &etcdgeneric.Etcd{ NewFunc: func() runtime.Object { return &extensions.Ingress{} }, @@ -72,6 +72,21 @@ func NewREST(s storage.Interface) *REST { Storage: s, } - - return &REST{store} + statusStore := *store + statusStore.UpdateStrategy = ingress.StatusStrategy + return &REST{store}, &StatusREST{store: &statusStore} +} + +// StatusREST implements the REST endpoint for changing the status of an ingress +type StatusREST struct { + store *etcdgeneric.Etcd +} + +func (r *StatusREST) New() runtime.Object { + return &extensions.Ingress{} +} + +// Update alters the status subset of an object. +func (r *StatusREST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) { + return r.store.Update(ctx, obj) } diff --git a/pkg/registry/ingress/etcd/etcd_test.go b/pkg/registry/ingress/etcd/etcd_test.go index b5ecd92b380..62ea1c981a0 100755 --- a/pkg/registry/ingress/etcd/etcd_test.go +++ b/pkg/registry/ingress/etcd/etcd_test.go @@ -29,10 +29,10 @@ import ( "k8s.io/kubernetes/pkg/util" ) -func newStorage(t *testing.T) (*REST, *tools.FakeEtcdClient) { +func newStorage(t *testing.T) (*REST, *StatusREST, *tools.FakeEtcdClient) { etcdStorage, fakeClient := registrytest.NewEtcdStorage(t, "extensions") - ingressStorage := NewREST(etcdStorage) - return ingressStorage, fakeClient + ingressStorage, statusStorage := NewREST(etcdStorage) + return ingressStorage, statusStorage, fakeClient } var ( @@ -107,7 +107,7 @@ func validIngress() *extensions.Ingress { } func TestCreate(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) ingress := validIngress() noDefaultBackendAndRules := validIngress() @@ -125,7 +125,7 @@ func TestCreate(t *testing.T) { } func TestUpdate(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) test.TestUpdate( // valid @@ -161,25 +161,25 @@ func TestUpdate(t *testing.T) { } func TestDelete(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) test.TestDelete(validIngress()) } func TestGet(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) test.TestGet(validIngress()) } func TestList(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) test.TestList(validIngress()) } func TestWatch(t *testing.T) { - storage, fakeClient := newStorage(t) + storage, _, fakeClient := newStorage(t) test := registrytest.New(t, fakeClient, storage.Etcd) test.TestWatch( validIngress(), @@ -201,3 +201,5 @@ func TestWatch(t *testing.T) { }, ) } + +// TODO TestUpdateStatus diff --git a/pkg/registry/ingress/strategy.go b/pkg/registry/ingress/strategy.go index 6bad71419de..f6cc8c9d785 100644 --- a/pkg/registry/ingress/strategy.go +++ b/pkg/registry/ingress/strategy.go @@ -47,6 +47,7 @@ func (ingressStrategy) NamespaceScoped() bool { // PrepareForCreate clears the status of an Ingress before creation. func (ingressStrategy) PrepareForCreate(obj runtime.Object) { ingress := obj.(*extensions.Ingress) + // create cannot set status ingress.Status = extensions.IngressStatus{} ingress.Generation = 1 @@ -56,7 +57,8 @@ func (ingressStrategy) PrepareForCreate(obj runtime.Object) { func (ingressStrategy) PrepareForUpdate(obj, old runtime.Object) { newIngress := obj.(*extensions.Ingress) oldIngress := old.(*extensions.Ingress) - //TODO: Clear Ingress status once we have a sub-resource. + // Update is not allowed to set status + newIngress.Status = oldIngress.Status // Any changes to the spec increment the generation number, any changes to the // status should reflect the generation number of the corresponding object. @@ -114,3 +116,22 @@ func MatchIngress(label labels.Selector, field fields.Selector) generic.Matcher }, } } + +type ingressStatusStrategy struct { + ingressStrategy +} + +var StatusStrategy = ingressStatusStrategy{Strategy} + +// PrepareForUpdate clears fields that are not allowed to be set by end users on update of status +func (ingressStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { + newIngress := obj.(*extensions.Ingress) + oldIngress := old.(*extensions.Ingress) + // status changes are not allowed to update spec + newIngress.Spec = oldIngress.Spec +} + +// ValidateUpdate is the default update validation for an end user updating status +func (ingressStatusStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { + return validation.ValidateIngressStatusUpdate(obj.(*extensions.Ingress), old.(*extensions.Ingress)) +} diff --git a/pkg/registry/ingress/strategy_test.go b/pkg/registry/ingress/strategy_test.go new file mode 100644 index 00000000000..74efc3caa24 --- /dev/null +++ b/pkg/registry/ingress/strategy_test.go @@ -0,0 +1,130 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ingress + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/extensions" + "k8s.io/kubernetes/pkg/util" +) + +func newIngress() extensions.Ingress { + defaultBackend := extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: util.IntOrString{Kind: util.IntstrInt, IntVal: 80}, + } + return extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "default-backend", + ServicePort: util.IntOrString{Kind: util.IntstrInt, IntVal: 80}, + }, + Rules: []extensions.IngressRule{ + { + Host: "foo.bar.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/foo", + Backend: defaultBackend, + }, + }, + }, + }, + }, + }, + }, + Status: extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "127.0.0.1"}, + }, + }, + }, + } +} + +func TestIngressStrategy(t *testing.T) { + ctx := api.NewDefaultContext() + if !Strategy.NamespaceScoped() { + t.Errorf("Ingress must be namespace scoped") + } + if Strategy.AllowCreateOnUpdate() { + t.Errorf("Ingress should not allow create on update") + } + + ingress := newIngress() + Strategy.PrepareForCreate(&ingress) + if len(ingress.Status.LoadBalancer.Ingress) != 0 { + t.Error("Ingress should not allow setting status on create") + } + errs := Strategy.Validate(ctx, &ingress) + if len(errs) != 0 { + t.Errorf("Unexpected error validating %v", errs) + } + invalidIngress := newIngress() + invalidIngress.ResourceVersion = "4" + invalidIngress.Spec = extensions.IngressSpec{} + Strategy.PrepareForUpdate(&invalidIngress, &ingress) + errs = Strategy.ValidateUpdate(ctx, &invalidIngress, &ingress) + if len(errs) == 0 { + t.Errorf("Expected a validation error") + } + if invalidIngress.ResourceVersion != "4" { + t.Errorf("Incoming resource version on update should not be mutated") + } +} + +func TestIngressStatusStrategy(t *testing.T) { + ctx := api.NewDefaultContext() + if !StatusStrategy.NamespaceScoped() { + t.Errorf("Ingress must be namespace scoped") + } + if StatusStrategy.AllowCreateOnUpdate() { + t.Errorf("Ingress should not allow create on update") + } + oldIngress := newIngress() + newIngress := newIngress() + oldIngress.ResourceVersion = "4" + newIngress.ResourceVersion = "4" + newIngress.Spec.Backend.ServiceName = "ignore" + newIngress.Status = extensions.IngressStatus{ + LoadBalancer: api.LoadBalancerStatus{ + Ingress: []api.LoadBalancerIngress{ + {IP: "127.0.0.2"}, + }, + }, + } + StatusStrategy.PrepareForUpdate(&newIngress, &oldIngress) + if newIngress.Status.LoadBalancer.Ingress[0].IP != "127.0.0.2" { + t.Errorf("Ingress status updates should allow change of status fields") + } + if newIngress.Spec.Backend.ServiceName != "default-backend" { + t.Errorf("PrepareForUpdate should have preserved old spec") + } + errs := StatusStrategy.ValidateUpdate(ctx, &newIngress, &oldIngress) + if len(errs) != 0 { + t.Errorf("Unexpected error %v", errs) + } +}