diff --git a/pkg/api/generate.go b/pkg/api/generate.go new file mode 100644 index 00000000000..5447ff722d5 --- /dev/null +++ b/pkg/api/generate.go @@ -0,0 +1,74 @@ +/* +Copyright 2014 Google Inc. 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 api + +import ( + "fmt" + "math/rand" +) + +// NameGenerator generates names for objects. Some backends may have more information +// available to guide selection of new names and this interface hides those details. +type NameGenerator interface { + // GenerateName generates a valid name from the base name, adding a random suffix to the + // the base. If base is valid, the returned name must also be valid. The generator is + // responsible for knowing the maximum valid name length. + GenerateName(base string) string +} + +// GenerateName will resolve the object name of the provided ObjectMeta to a generated version if +// necessary. It expects that validation for ObjectMeta has already completed (that Base is a +// valid name) and that the NameGenerator generates a name that is also valid. +func GenerateName(u NameGenerator, meta *ObjectMeta) { + if len(meta.GenerateName) == 0 || len(meta.Name) != 0 { + return + } + meta.Name = u.GenerateName(meta.GenerateName) +} + +// simpleNameGenerator generates random names. +type simpleNameGenerator struct{} + +// SimpleNameGenerator is a generator that returns the name plus a random suffix of five alphanumerics +// when a name is requested. The string is guaranteed to not exceed the length of a standard Kubernetes +// name (63 characters) +var SimpleNameGenerator NameGenerator = simpleNameGenerator{} + +const ( + // TODO: make this flexible for non-core resources with alternate naming rules. + maxNameLength = 63 + randomLength = 5 + maxGeneratedNameLength = maxNameLength - randomLength +) + +func (simpleNameGenerator) GenerateName(base string) string { + if len(base) > maxGeneratedNameLength { + base = base[:maxGeneratedNameLength] + } + value := randSeq(randomLength) + return fmt.Sprintf("%s%s", base, value) +} + +var letters = []rune("abcdefghijklmnopqrstuvwxyz0123456789") + +func randSeq(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letters[rand.Intn(len(letters))] + } + return string(b) +} diff --git a/pkg/api/generate_test.go b/pkg/api/generate_test.go new file mode 100644 index 00000000000..7f943f21dac --- /dev/null +++ b/pkg/api/generate_test.go @@ -0,0 +1,79 @@ +/* +Copyright 2014 Google Inc. 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 api + +import ( + "strings" + "testing" +) + +type nameGeneratorFunc func(base string) string + +func (fn nameGeneratorFunc) GenerateName(base string) string { + return fn(base) +} + +func TestGenerateName(t *testing.T) { + testCases := []struct { + meta ObjectMeta + + base string + returned string + }{ + { + returned: "", + }, + { + meta: ObjectMeta{ + GenerateName: "test", + }, + base: "test", + returned: "test", + }, + { + meta: ObjectMeta{ + Name: "foo", + GenerateName: "test", + }, + base: "test", + returned: "foo", + }, + } + + for i, testCase := range testCases { + GenerateName(nameGeneratorFunc(func(base string) string { + if base != testCase.base { + t.Errorf("%d: unexpected call with base", i) + } + return "test" + }), &testCase.meta) + expect := testCase.returned + if expect != testCase.meta.Name { + t.Errorf("%d: unexpected name: %#v", i, testCase.meta) + } + } +} + +func TestSimpleNameGenerator(t *testing.T) { + meta := &ObjectMeta{ + GenerateName: "foo", + } + GenerateName(SimpleNameGenerator, meta) + if !strings.HasPrefix(meta.Name, "foo") || meta.Name == "foo" { + t.Errorf("unexpected name: %#v", meta) + } +} diff --git a/pkg/api/meta.go b/pkg/api/meta.go index 4bdfcdacd73..5782be6558f 100644 --- a/pkg/api/meta.go +++ b/pkg/api/meta.go @@ -17,6 +17,8 @@ limitations under the License. package api import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" ) @@ -24,6 +26,7 @@ import ( func FillObjectMetaSystemFields(ctx Context, meta *ObjectMeta) { meta.CreationTimestamp = util.Now() meta.UID = util.NewUUID() + meta.SelfLink = "" } // HasObjectMetaSystemFieldValues returns true if fields that are managed by the system on ObjectMeta have values. @@ -31,3 +34,17 @@ func HasObjectMetaSystemFieldValues(meta *ObjectMeta) bool { return !meta.CreationTimestamp.Time.IsZero() || len(meta.UID) != 0 } + +// GetObjectMetaPtr returns a pointer to a provided object's ObjectMeta. +// TODO: allow runtime.Unknown to extract this object +func ObjectMetaFor(obj runtime.Object) (*ObjectMeta, error) { + v, err := conversion.EnforcePtr(obj) + if err != nil { + return nil, err + } + var objectMeta *ObjectMeta + if err := runtime.FieldPtr(v, "ObjectMeta", &objectMeta); err != nil { + return nil, err + } + return objectMeta, nil +} diff --git a/pkg/api/meta/meta.go b/pkg/api/meta/meta.go index 1353083474d..ce11b272e39 100644 --- a/pkg/api/meta/meta.go +++ b/pkg/api/meta/meta.go @@ -373,36 +373,12 @@ func (a genericAccessor) SetAnnotations(annotations map[string]string) { *a.annotations = annotations } -// fieldPtr puts the address of fieldName, which must be a member of v, -// into dest, which must be an address of a variable to which this field's -// address can be assigned. -func fieldPtr(v reflect.Value, fieldName string, dest interface{}) error { - field := v.FieldByName(fieldName) - if !field.IsValid() { - return fmt.Errorf("couldn't find %v field in %#v", fieldName, v.Interface()) - } - v, err := conversion.EnforcePtr(dest) - if err != nil { - return err - } - field = field.Addr() - if field.Type().AssignableTo(v.Type()) { - v.Set(field) - return nil - } - if field.Type().ConvertibleTo(v.Type()) { - v.Set(field.Convert(v.Type())) - return nil - } - return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type()) -} - // extractFromTypeMeta extracts pointers to version and kind fields from an object func extractFromTypeMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "APIVersion", &a.apiVersion); err != nil { + if err := runtime.FieldPtr(v, "APIVersion", &a.apiVersion); err != nil { return err } - if err := fieldPtr(v, "Kind", &a.kind); err != nil { + if err := runtime.FieldPtr(v, "Kind", &a.kind); err != nil { return err } return nil @@ -410,25 +386,25 @@ func extractFromTypeMeta(v reflect.Value, a *genericAccessor) error { // extractFromObjectMeta extracts pointers to metadata fields from an object func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "Namespace", &a.namespace); err != nil { + if err := runtime.FieldPtr(v, "Namespace", &a.namespace); err != nil { return err } - if err := fieldPtr(v, "Name", &a.name); err != nil { + if err := runtime.FieldPtr(v, "Name", &a.name); err != nil { return err } - if err := fieldPtr(v, "UID", &a.uid); err != nil { + if err := runtime.FieldPtr(v, "UID", &a.uid); err != nil { return err } - if err := fieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { + if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { return err } - if err := fieldPtr(v, "SelfLink", &a.selfLink); err != nil { + if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { return err } - if err := fieldPtr(v, "Labels", &a.labels); err != nil { + if err := runtime.FieldPtr(v, "Labels", &a.labels); err != nil { return err } - if err := fieldPtr(v, "Annotations", &a.annotations); err != nil { + if err := runtime.FieldPtr(v, "Annotations", &a.annotations); err != nil { return err } return nil @@ -436,10 +412,10 @@ func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error { // extractFromObjectMeta extracts pointers to metadata fields from a list object func extractFromListMeta(v reflect.Value, a *genericAccessor) error { - if err := fieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { + if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { return err } - if err := fieldPtr(v, "SelfLink", &a.selfLink); err != nil { + if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { return err } return nil diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go new file mode 100644 index 00000000000..f7601c9668c --- /dev/null +++ b/pkg/api/rest/create.go @@ -0,0 +1,66 @@ +/* +Copyright 2014 Google Inc. 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 rest + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" +) + +// RESTCreateStrategy defines the minimum validation, accepted input, and +// name generation behavior to create an object that follows Kubernetes +// API conventions. +type RESTCreateStrategy interface { + runtime.ObjectTyper + // The name generate is used when the standard GenerateName field is set. + // The NameGenerator will be invoked prior to validation. + api.NameGenerator + + // ResetBeforeCreate is invoked on create before validation to remove any fields + // that may not be persisted. + ResetBeforeCreate(obj runtime.Object) + // Validate is invoked after default fields in the object have been filled in before + // the object is persisted. + Validate(obj runtime.Object) errors.ValidationErrorList +} + +// BeforeCreate ensures that common operations for all resources are performed on creation. It only returns +// errors that can be converted to api.Status. It invokes ResetBeforeCreate, then GenerateName, then Validate. +// It returns nil if the object should be created. +func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Object) error { + _, kind, err := strategy.ObjectVersionAndKind(obj) + if err != nil { + return errors.NewInternalError(err) + } + objectMeta, err := api.ObjectMetaFor(obj) + if err != nil { + return errors.NewInternalError(err) + } + + if !api.ValidNamespace(ctx, objectMeta) { + return errors.NewBadRequest("the namespace of the provided object does not match the namespace sent on the request") + } + strategy.ResetBeforeCreate(obj) + api.FillObjectMetaSystemFields(ctx, objectMeta) + api.GenerateName(strategy, objectMeta) + + if errs := strategy.Validate(obj); len(errs) > 0 { + return errors.NewInvalid(kind, objectMeta.Name, errs) + } + return nil +} diff --git a/pkg/api/rest/doc.go b/pkg/api/rest/doc.go new file mode 100644 index 00000000000..01d0e6ea553 --- /dev/null +++ b/pkg/api/rest/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2014 Google Inc. 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 rest defines common logic around changes to Kubernetes resources. +package rest diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go new file mode 100644 index 00000000000..1f8ef774470 --- /dev/null +++ b/pkg/api/rest/resttest/resttest.go @@ -0,0 +1,118 @@ +/* +Copyright 2014 Google Inc. 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 resttest + +import ( + "strings" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" +) + +type Tester struct { + *testing.T + storage apiserver.RESTStorage +} + +func New(t *testing.T, storage apiserver.RESTStorage) *Tester { + return &Tester{ + T: t, + storage: storage, + } +} + +func copyOrDie(obj runtime.Object) runtime.Object { + out, err := api.Scheme.Copy(obj) + if err != nil { + panic(err) + } + return out +} + +func (t *Tester) TestCreate(valid runtime.Object, invalid ...runtime.Object) { + t.TestCreateHasMetadata(copyOrDie(valid)) + t.TestCreateGeneratesName(copyOrDie(valid)) + t.TestCreateRejectsMismatchedNamespace(copyOrDie(valid)) + t.TestCreateInvokesValidation(invalid...) +} + +func (t *Tester) TestCreateHasMetadata(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.Name = "test" + objectMeta.Namespace = api.NamespaceDefault + + channel, err := t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if obj := <-channel; obj.Object == nil { + t.Fatalf("Unexpected object from channel: %#v", obj) + } + if !api.HasObjectMetaSystemFieldValues(objectMeta) { + t.Errorf("storage did not populate object meta field values") + } +} + +func (t *Tester) TestCreateGeneratesName(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.GenerateName = "test-" + + _, err = t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if objectMeta.Name == "test-" || !strings.HasPrefix(objectMeta.Name, "test-") { + t.Errorf("unexpected name: %#v", valid) + } +} + +func (t *Tester) TestCreateInvokesValidation(invalid ...runtime.Object) { + for i, obj := range invalid { + ctx := api.NewDefaultContext() + _, err := t.storage.(apiserver.RESTCreater).Create(ctx, obj) + if !errors.IsInvalid(err) { + t.Errorf("%d: Expected to get an invalid resource error, got %v", i, err) + } + } +} + +func (t *Tester) TestCreateRejectsMismatchedNamespace(valid runtime.Object) { + objectMeta, err := api.ObjectMetaFor(valid) + if err != nil { + t.Fatalf("object does not have ObjectMeta: %v\n%#v", err, valid) + } + + objectMeta.Namespace = "not-default" + + _, err = t.storage.(apiserver.RESTCreater).Create(api.NewDefaultContext(), valid) + if err == nil { + t.Errorf("Expected an error, but we didn't get one") + } else if strings.Contains(err.Error(), "Controller.Namespace does not match the provided context") { + t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error()) + } +} diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go new file mode 100644 index 00000000000..11b8428ada2 --- /dev/null +++ b/pkg/api/rest/types.go @@ -0,0 +1,47 @@ +/* +Copyright 2014 Google Inc. 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 rest + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" +) + +// rcStrategy implements behavior for Replication Controllers. +// TODO: move to a replicationcontroller specific package. +type rcStrategy struct { + runtime.ObjectTyper + api.NameGenerator +} + +// ReplicationControllers is the default logic that applies when creating and updating Replication Controller +// objects. +var ReplicationControllers RESTCreateStrategy = rcStrategy{api.Scheme, api.SimpleNameGenerator} + +// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. +func (rcStrategy) ResetBeforeCreate(obj runtime.Object) { + controller := obj.(*api.ReplicationController) + controller.Status = api.ReplicationControllerStatus{} +} + +// Validate validates a new replication controller. +func (rcStrategy) Validate(obj runtime.Object) errors.ValidationErrorList { + controller := obj.(*api.ReplicationController) + return validation.ValidateReplicationController(controller) +} diff --git a/pkg/api/types.go b/pkg/api/types.go index 3d72b98ed9e..c9f94215d95 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -74,7 +74,7 @@ type ListMeta struct { } // ObjectMeta is metadata that all persisted resources must have, which includes all objects -// users must create. A resource may have only one of {ObjectMeta, ListMeta}. +// users must create. type ObjectMeta struct { // Name is unique within a namespace. Name is required when creating resources, although // some resources may allow a client to request the generation of an appropriate name @@ -82,6 +82,19 @@ type ObjectMeta struct { // definition. Name string `json:"name,omitempty"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty"` + // Namespace defines the space within which name must be unique. An empty namespace is // equivalent to the "default" namespace, but "default" is the canonical representation. // Not all objects are required to be scoped to a namespace - the value of this field for diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 652080d8bc4..4c02861ea87 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -326,6 +326,19 @@ type TypeMeta struct { APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"` Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Annotations are unstructured key value data stored with a resource that may be set by // external tooling. They are not queryable and should be preserved when modifying // objects. diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index d2c52c4b3eb..2d00faff0aa 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -290,6 +290,19 @@ type TypeMeta struct { APIVersion string `json:"apiVersion,omitempty" description:"version of the schema the object should have"` Namespace string `json:"namespace,omitempty" description:"namespace to which the object belongs; must be a DNS_SUBDOMAIN; 'default' by default"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Annotations are unstructured key value data stored with a resource that may be set by // external tooling. They are not queryable and should be preserved when modifying // objects. diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 1088d68a956..761e1c58f20 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -82,6 +82,19 @@ type ObjectMeta struct { // definition. Name string `json:"name,omitempty"` + // GenerateName indicates that the name should be made unique by the server prior to persisting + // it. A non-empty value for the field indicates the name will be made unique (and the name + // returned to the client will be different than the name passed). The value of this field will + // be combined with a unique suffix on the server if the Name field has not been provided. + // The provided value must be valid within the rules for Name, and may be truncated by the length + // of the suffix required to make the value unique on the server. + // + // If this field is specified, and Name is not present, the server will NOT return a 409 if the + // generated name exists - instead, it will either return 201 Created or 500 with Reason + // TryAgainLater indicating a unique name could not be found in the time allotted, and the client + // should retry (optionally after the time indicated in the Retry-After header). + GenerateName string `json:"generateName,omitempty" description:"an optional prefix to use to generate a unique name; has the same validation rules as name; optional, and is applied only name if is not specified"` + // Namespace defines the space within which name must be unique. An empty namespace is // equivalent to the "default" namespace, but "default" is the canonical representation. // Not all objects are required to be scoped to a namespace - the value of this field for diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 93d62246ea7..e7f5d27828a 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -53,11 +53,24 @@ func ValidateAnnotations(annotations map[string]string, field string) errs.Valid } // ValidateNameFunc validates that the provided name is valid for a given resource type. -// Not all resources have the same validation rules for names. -type ValidateNameFunc func(name string) (bool, string) +// Not all resources have the same validation rules for names. Prefix is true if the +// name will have a value appended to it. +type ValidateNameFunc func(name string, prefix bool) (bool, string) + +// maskTrailingDash replaces the final character of a string with a subdomain safe +// value if is a dash. +func maskTrailingDash(name string) string { + if strings.HasSuffix(name, "-") { + return name[:len(name)-2] + "a" + } + return name +} // nameIsDNSSubdomain is a ValidateNameFunc for names that must be a DNS subdomain. -func nameIsDNSSubdomain(name string) (bool, string) { +func nameIsDNSSubdomain(name string, prefix bool) (bool, string) { + if prefix { + name = maskTrailingDash(name) + } if util.IsDNSSubdomain(name) { return true, "" } @@ -65,23 +78,36 @@ func nameIsDNSSubdomain(name string) (bool, string) { } // nameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label. -func nameIsDNS952Label(name string) (bool, string) { +func nameIsDNS952Label(name string, prefix bool) (bool, string) { + if prefix { + name = maskTrailingDash(name) + } if util.IsDNS952Label(name) { return true, "" } return false, "name must be lowercase letters, numbers, and dashes" } -// ValidateObjectMeta validates an object's metadata. +// ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already +// been performed. func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn ValidateNameFunc) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} + + if len(meta.GenerateName) != 0 { + if ok, qualifier := nameFn(meta.GenerateName, true); !ok { + allErrs = append(allErrs, errs.NewFieldInvalid("generateName", meta.GenerateName, qualifier)) + } + } + // if the generated name validates, but the calculated value does not, it's a problem with generation, and we + // report it here. This may confuse users, but indicates a programming bug and still must be validated. if len(meta.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", meta.Name)) } else { - if ok, qualifier := nameFn(meta.Name); !ok { + if ok, qualifier := nameFn(meta.Name, false); !ok { allErrs = append(allErrs, errs.NewFieldInvalid("name", meta.Name, qualifier)) } } + if requiresNamespace { if len(meta.Namespace) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("namespace", meta.Namespace)) @@ -96,10 +122,6 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...) allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...) - // Clear self link internally - // TODO: move to its own area - meta.SelfLink = "" - return allErrs } @@ -131,10 +153,6 @@ func ValidateObjectMetaUpdate(old, meta *api.ObjectMeta) errs.ValidationErrorLis allErrs = append(allErrs, ValidateLabels(meta.Labels, "labels")...) allErrs = append(allErrs, ValidateAnnotations(meta.Annotations, "annotations")...) - // Clear self link internally - // TODO: move to its own area - meta.SelfLink = "" - return allErrs } @@ -660,7 +678,7 @@ func ValidateBoundPod(pod *api.BoundPod) errs.ValidationErrorList { if len(pod.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", pod.Name)) } else { - if ok, qualifier := nameIsDNSSubdomain(pod.Name); !ok { + if ok, qualifier := nameIsDNSSubdomain(pod.Name, false); !ok { allErrs = append(allErrs, errs.NewFieldInvalid("name", pod.Name, qualifier)) } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 745e653ddea..210f4c72963 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -37,6 +37,30 @@ func expectPrefix(t *testing.T, prefix string, errs errors.ValidationErrorList) } } +// Ensure custom name functions are allowed +func TestValidateObjectMetaCustomName(t *testing.T) { + errs := ValidateObjectMeta(&api.ObjectMeta{Name: "test", GenerateName: "foo"}, false, func(s string, prefix bool) (bool, string) { + if s == "test" { + return true, "" + } + return false, "name-gen" + }) + if len(errs) != 1 { + t.Fatalf("unexpected errors: %v", errs) + } + if !strings.Contains(errs[0].Error(), "name-gen") { + t.Errorf("unexpected error message: %v", errs) + } +} + +// Ensure trailing slash is allowed in generate name +func TestValidateObjectMetaTrimsTrailingSlash(t *testing.T) { + errs := ValidateObjectMeta(&api.ObjectMeta{Name: "test", GenerateName: "foo-"}, false, nameIsDNSSubdomain) + if len(errs) != 0 { + t.Fatalf("unexpected errors: %v", errs) + } +} + func TestValidateLabels(t *testing.T) { successCases := []map[string]string{ {"simple": "bar"}, @@ -953,7 +977,7 @@ func TestValidateService(t *testing.T) { { name: "invalid id", svc: api.Service{ - ObjectMeta: api.ObjectMeta{Name: "123abc", Namespace: api.NamespaceDefault}, + ObjectMeta: api.ObjectMeta{Name: "-123abc", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Port: 8675, Selector: map[string]string{"foo": "bar"}, @@ -962,6 +986,38 @@ func TestValidateService(t *testing.T) { // Should fail because the ID is invalid. numErrs: 1, }, + { + name: "invalid generate.base", + svc: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + GenerateName: "-123abc", + Namespace: api.NamespaceDefault, + }, + Spec: api.ServiceSpec{ + Port: 8675, + Selector: map[string]string{"foo": "bar"}, + }, + }, + // Should fail because the Base value for generation is invalid + numErrs: 1, + }, + { + name: "invalid generateName", + svc: api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + GenerateName: "abc1234567abc1234567abc1234567abc1234567abc1234567abc1234567", + Namespace: api.NamespaceDefault, + }, + Spec: api.ServiceSpec{ + Port: 8675, + Selector: map[string]string{"foo": "bar"}, + }, + }, + // Should fail because the generate name type is invalid. + numErrs: 1, + }, { name: "missing port", svc: api.Service{ @@ -1426,7 +1482,9 @@ func TestValidateMinion(t *testing.T) { }, }, { - ObjectMeta: api.ObjectMeta{Name: "abc"}, + ObjectMeta: api.ObjectMeta{ + Name: "abc", + }, Status: api.NodeStatus{ HostIP: "something", }, diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 5171914af59..bd8f75e3259 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -21,12 +21,12 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" rc "github.com/GoogleCloudPlatform/kubernetes/pkg/controller" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" ) @@ -55,23 +55,14 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan apiserver.RE if !ok { return nil, fmt.Errorf("not a replication controller: %#v", obj) } - if !api.ValidNamespace(ctx, &controller.ObjectMeta) { - return nil, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context")) - } - if len(controller.Name) == 0 { - controller.Name = string(util.NewUUID()) + if err := rest.BeforeCreate(rest.ReplicationControllers, ctx, obj); err != nil { + return nil, err } - if errs := validation.ValidateReplicationController(controller); len(errs) > 0 { - return nil, errors.NewInvalid("replicationController", controller.Name, errs) - } - - api.FillObjectMetaSystemFields(ctx, &controller.ObjectMeta) return apiserver.MakeAsync(func() (runtime.Object, error) { - err := rs.registry.CreateController(ctx, controller) - if err != nil { - return nil, err + if err := rs.registry.CreateController(ctx, controller); err != nil { + return apiserver.RESTResult{}, err } return rs.registry.GetController(ctx, controller.Name) }), nil diff --git a/pkg/registry/controller/rest_test.go b/pkg/registry/controller/rest_test.go index dc5455959b1..ed76e04ed81 100644 --- a/pkg/registry/controller/rest_test.go +++ b/pkg/registry/controller/rest_test.go @@ -27,6 +27,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest/resttest" _ "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" @@ -106,6 +107,7 @@ func TestListControllerList(t *testing.T) { } } +// TODO: remove, this is sufficiently covered by other tests func TestControllerDecode(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} storage := REST{ @@ -140,6 +142,7 @@ func TestControllerDecode(t *testing.T) { } } +// TODO: this is sufficiently covered by other tetss func TestControllerParsing(t *testing.T) { expectedController := api.ReplicationController{ ObjectMeta: api.ObjectMeta{ @@ -228,6 +231,7 @@ var validPodTemplate = api.PodTemplate{ }, } +// TODO: remove, this is sufficiently covered by other tests func TestCreateController(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} mockPodRegistry := registrytest.PodRegistry{ @@ -274,6 +278,7 @@ func TestCreateController(t *testing.T) { } } +// TODO: remove, covered by TestCreate func TestControllerStorageValidatesCreate(t *testing.T) { mockRegistry := registrytest.ControllerRegistry{} storage := REST{ @@ -376,6 +381,32 @@ func TestFillCurrentState(t *testing.T) { } } +// TODO: remove, covered by TestCreate +func TestCreateControllerWithGeneratedName(t *testing.T) { + storage := NewREST(®istrytest.ControllerRegistry{}, nil) + controller := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Namespace: api.NamespaceDefault, + GenerateName: "rc-", + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + } + + ctx := api.NewDefaultContext() + _, err := storage.Create(ctx, controller) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if controller.Name == "rc-" || !strings.HasPrefix(controller.Name, "rc-") { + t.Errorf("unexpected name: %#v", controller) + } +} + +// TODO: remove, covered by TestCreate func TestCreateControllerWithConflictingNamespace(t *testing.T) { storage := REST{} controller := &api.ReplicationController{ @@ -389,7 +420,7 @@ func TestCreateControllerWithConflictingNamespace(t *testing.T) { } if err == nil { t.Errorf("Expected an error, but we didn't get one") - } else if strings.Index(err.Error(), "Controller.Namespace does not match the provided context") == -1 { + } else if strings.Contains(err.Error(), "Controller.Namespace does not match the provided context") { t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error()) } } @@ -411,3 +442,25 @@ func TestUpdateControllerWithConflictingNamespace(t *testing.T) { t.Errorf("Expected 'Controller.Namespace does not match the provided context' error, got '%v'", err.Error()) } } + +func TestCreate(t *testing.T) { + test := resttest.New(t, NewREST(®istrytest.ControllerRegistry{}, nil)) + test.TestCreate( + // valid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"a": "b"}, + Template: &validPodTemplate.Spec, + }, + }, + // invalid + &api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{}, + Template: &validPodTemplate.Spec, + }, + }, + ) +} diff --git a/pkg/runtime/helper.go b/pkg/runtime/helper.go index 8681912a869..aba69f3cb53 100644 --- a/pkg/runtime/helper.go +++ b/pkg/runtime/helper.go @@ -114,3 +114,27 @@ func SetList(list Object, objects []Object) error { items.Set(slice) return nil } + +// fieldPtr puts the address of fieldName, which must be a member of v, +// into dest, which must be an address of a variable to which this field's +// address can be assigned. +func FieldPtr(v reflect.Value, fieldName string, dest interface{}) error { + field := v.FieldByName(fieldName) + if !field.IsValid() { + return fmt.Errorf("couldn't find %v field in %#v", fieldName, v.Interface()) + } + v, err := conversion.EnforcePtr(dest) + if err != nil { + return err + } + field = field.Addr() + if field.Type().AssignableTo(v.Type()) { + v.Set(field) + return nil + } + if field.Type().ConvertibleTo(v.Type()) { + v.Set(field.Convert(v.Type())) + return nil + } + return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type()) +} diff --git a/pkg/runtime/types.go b/pkg/runtime/types.go index fec13030206..8f8a857c1a2 100644 --- a/pkg/runtime/types.go +++ b/pkg/runtime/types.go @@ -104,8 +104,9 @@ type RawExtension struct { // Unknown allows api objects with unknown types to be passed-through. This can be used // to deal with the API objects from a plug-in. Unknown objects still have functioning -// TypeMeta features-- kind, version, resourceVersion, etc. -// TODO: Not implemented yet! +// TypeMeta features-- kind, version, etc. +// TODO: Make this object have easy access to field based accessors and settors for +// metadata and field mutatation. type Unknown struct { TypeMeta `json:",inline"` // RawJSON will hold the complete JSON of the object which couldn't be matched