Skip to content

Commit

Permalink
Updates to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
derekwaynecarr committed Oct 1, 2014
1 parent f385939 commit 6625e58
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 23 deletions.
4 changes: 2 additions & 2 deletions examples/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ func validateObject(obj runtime.Object) (errors []error) {
errors = append(errors, validateObject(&t.Items[i])...)
}
case *api.Service:
api.ValidNamespaceOnCreateOrUpdate(ctx, &t.JSONBase)
api.ValidNamespace(ctx, &t.JSONBase)
errors = validation.ValidateService(t)
case *api.ServiceList:
for i := range t.Items {
errors = append(errors, validateObject(&t.Items[i])...)
}
case *api.Pod:
api.ValidNamespaceOnCreateOrUpdate(ctx, &t.JSONBase)
api.ValidNamespace(ctx, &t.JSONBase)
errors = validation.ValidateManifest(&t.DesiredState.Manifest)
case *api.PodList:
for i := range t.Items {
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ func NamespaceFrom(ctx Context) (string, bool) {
return namespace, ok
}

// ValidNamespaceOnCreateOrUpdate returns false if the namespace on the context differs from the resource. If the resource has no namespace, it is set to the value in the context.
func ValidNamespaceOnCreateOrUpdate(ctx Context, resource *JSONBase) bool {
// ValidNamespace returns false if the namespace on the context differs from the resource. If the resource has no namespace, it is set to the value in the context.
func ValidNamespace(ctx Context, resource *JSONBase) bool {
ns, ok := NamespaceFrom(ctx)
if len(resource.Namespace) == 0 {
resource.Namespace = ns
Expand Down
15 changes: 11 additions & 4 deletions pkg/api/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,31 @@ func TestNamespaceContext(t *testing.T) {
if api.NamespaceDefault != result {
t.Errorf("Expected: %v, Actual: %v", api.NamespaceDefault, result)
}

ctx = api.NewContext()
result, ok = api.NamespaceFrom(ctx)
if ok {
t.Errorf("Should not be ok because there is no namespace on the context")
}
}

func TestValidNamespaceOnCreateOrUpdate(t *testing.T) {
// TestValidNamespace validates that namespace rules are enforced on a resource prior to create or update
func TestValidNamespace(t *testing.T) {
ctx := api.NewDefaultContext()
namespace, _ := api.NamespaceFrom(ctx)
resource := api.ReplicationController{}
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &resource.JSONBase) {
if !api.ValidNamespace(ctx, &resource.JSONBase) {
t.Errorf("expected success")
}
if namespace != resource.Namespace {
t.Errorf("expected resource to have the default namespace assigned during validation")
}
resource = api.ReplicationController{JSONBase: api.JSONBase{Namespace: "other"}}
if api.ValidNamespaceOnCreateOrUpdate(ctx, &resource.JSONBase) {
if api.ValidNamespace(ctx, &resource.JSONBase) {
t.Errorf("Expected error that resource and context errors do not match because resource has different namespace")
}
ctx = api.NewContext()
if api.ValidNamespaceOnCreateOrUpdate(ctx, &resource.JSONBase) {
if api.ValidNamespace(ctx, &resource.JSONBase) {
t.Errorf("Expected error that resource and context errors do not match since context has no namespace")
}
}
9 changes: 4 additions & 5 deletions pkg/registry/controller/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package controller

import (
stderrs "errors"
"fmt"
"time"

Expand Down Expand Up @@ -60,8 +59,8 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje
if !ok {
return nil, fmt.Errorf("not a replication controller: %#v", obj)
}
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &controller.JSONBase) {
return nil, errors.NewConflict("controller", controller.Namespace, stderrs.New("Controller.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &controller.JSONBase) {
return nil, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context"))
}

if len(controller.ID) == 0 {
Expand Down Expand Up @@ -133,8 +132,8 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Obje
if !ok {
return nil, fmt.Errorf("not a replication controller: %#v", obj)
}
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &controller.JSONBase) {
return nil, errors.NewConflict("controller", controller.Namespace, stderrs.New("Controller.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &controller.JSONBase) {
return nil, errors.NewConflict("controller", controller.Namespace, fmt.Errorf("Controller.Namespace does not match the provided context"))
}
if errs := validation.ValidateReplicationController(controller); len(errs) > 0 {
return nil, errors.NewInvalid("replicationController", controller.ID, errs)
Expand Down
9 changes: 4 additions & 5 deletions pkg/registry/pod/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package pod

import (
stderrs "errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -70,8 +69,8 @@ func NewREST(config *RESTConfig) *REST {

func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) {
pod := obj.(*api.Pod)
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &pod.JSONBase) {
return nil, errors.NewConflict("pod", pod.Namespace, stderrs.New("Pod.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &pod.JSONBase) {
return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context"))
}
pod.DesiredState.Manifest.UUID = uuid.NewUUID().String()
if len(pod.ID) == 0 {
Expand Down Expand Up @@ -162,8 +161,8 @@ func (*REST) New() runtime.Object {

func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) {
pod := obj.(*api.Pod)
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &pod.JSONBase) {
return nil, errors.NewConflict("pod", pod.Namespace, stderrs.New("Pod.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &pod.JSONBase) {
return nil, errors.NewConflict("pod", pod.Namespace, fmt.Errorf("Pod.Namespace does not match the provided context"))
}
if errs := validation.ValidatePod(pod); len(errs) > 0 {
return nil, errors.NewInvalid("pod", pod.ID, errs)
Expand Down
9 changes: 4 additions & 5 deletions pkg/registry/service/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package service

import (
stderrs "errors"
"fmt"
"math/rand"
"strconv"
Expand Down Expand Up @@ -53,8 +52,8 @@ func NewREST(registry Registry, cloud cloudprovider.Interface, machines minion.R

func (rs *REST) Create(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) {
srv := obj.(*api.Service)
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &srv.JSONBase) {
return nil, errors.NewConflict("service", srv.Namespace, stderrs.New("Service.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &srv.JSONBase) {
return nil, errors.NewConflict("service", srv.Namespace, fmt.Errorf("Service.Namespace does not match the provided context"))
}
if errs := validation.ValidateService(srv); len(errs) > 0 {
return nil, errors.NewInvalid("service", srv.ID, errs)
Expand Down Expand Up @@ -169,8 +168,8 @@ func GetServiceEnvironmentVariables(ctx api.Context, registry Registry, machine

func (rs *REST) Update(ctx api.Context, obj runtime.Object) (<-chan runtime.Object, error) {
srv := obj.(*api.Service)
if !api.ValidNamespaceOnCreateOrUpdate(ctx, &srv.JSONBase) {
return nil, errors.NewConflict("service", srv.Namespace, stderrs.New("Service.Namespace does not match the provided context"))
if !api.ValidNamespace(ctx, &srv.JSONBase) {
return nil, errors.NewConflict("service", srv.Namespace, fmt.Errorf("Service.Namespace does not match the provided context"))
}
if errs := validation.ValidateService(srv); len(errs) > 0 {
return nil, errors.NewInvalid("service", srv.ID, errs)
Expand Down

0 comments on commit 6625e58

Please sign in to comment.