From a92e1aa1bf857cf1b7b1a6e2f99c9d71ff06fd69 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 1 Jul 2014 13:01:39 -0700 Subject: [PATCH] First piece of validation I'm adding pieces incrementally to make sure we get full testing of each piece. Make syncLoop() private --- pkg/api/validation.go | 57 ++++++++++++++++++++++++++++++++++++++ pkg/api/validation_test.go | 50 +++++++++++++++++++++++++++++++++ pkg/kubelet/kubelet.go | 21 +++++++++----- 3 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 pkg/api/validation.go create mode 100644 pkg/api/validation_test.go diff --git a/pkg/api/validation.go b/pkg/api/validation.go new file mode 100644 index 00000000000..b3da4772856 --- /dev/null +++ b/pkg/api/validation.go @@ -0,0 +1,57 @@ +/* +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" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" +) + +func isSupportedManifestVersion(value string) bool { + switch value { + case "v1beta1", "v1beta2": + return true + } + return false +} + +func isInvalid(field string, value interface{}) error { + return fmt.Errorf("%s is invalid: '%v'", field, value) +} + +func isNotSupported(field string, value interface{}) error { + return fmt.Errorf("%s is not supported: '%v'", field, value) +} + +// ValidateManifest tests that the specified ContainerManifest has valid data. +// This includes checking formatting and uniqueness. It also canonicalizes the +// structure by setting default values and implementing any backwards-compatibility +// tricks. +func ValidateManifest(manifest *ContainerManifest) error { + if len(manifest.Version) == 0 { + return isInvalid("ContainerManifest.Version", manifest.Version) + } + if !isSupportedManifestVersion(manifest.Version) { + return isNotSupported("ContainerManifest.Version", manifest.Version) + } + if len(manifest.ID) > 255 || !util.IsDNSSubdomain(manifest.ID) { + return isInvalid("ContainerManifest.ID", manifest.ID) + } + // TODO(thockin): finish validation. + return nil +} diff --git a/pkg/api/validation_test.go b/pkg/api/validation_test.go new file mode 100644 index 00000000000..a36ba4c633c --- /dev/null +++ b/pkg/api/validation_test.go @@ -0,0 +1,50 @@ +/* +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" +) + +func TestValidateManifest(t *testing.T) { + successCases := []ContainerManifest{ + {Version: "v1beta1", ID: "abc"}, + {Version: "v1beta1", ID: "123"}, + {Version: "v1beta1", ID: "abc.123.do-re-mi"}, + } + for _, manifest := range successCases { + err := ValidateManifest(&manifest) + if err != nil { + t.Errorf("expected success: %v", err) + } + } + + errorCases := map[string]ContainerManifest{ + "empty version": {Version: "", ID: "abc"}, + "invalid version": {Version: "bogus", ID: "abc"}, + "zero-length id": {Version: "v1beta1", ID: ""}, + "id > 255 characters": {Version: "v1beta1", ID: strings.Repeat("a", 256)}, + "id not a DNS subdomain": {Version: "v1beta1", ID: "a.b.c."}, + } + for k, v := range errorCases { + err := ValidateManifest(&v) + if err == nil { + t.Errorf("expected failure for %s", k) + } + } +} diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 7e2430fdbef..d2ef9251933 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -141,7 +141,7 @@ func (kl *Kubelet) RunKubelet(config_path, manifest_url, etcd_servers, address, } go util.Forever(func() { s.ListenAndServe() }, 0) } - kl.RunSyncLoop(updateChannel, kl) + kl.syncLoop(updateChannel, kl) } // Interface implemented by Kubelet, for testability @@ -730,13 +730,13 @@ func (kl *Kubelet) SyncManifests(config []api.ContainerManifest) error { return err } -// runSyncLoop is the main loop for processing changes. It watches for changes from +// syncLoop is the main loop for processing changes. It watches for changes from // four channels (file, etcd, server, and http) and creates a union of them. For // any new change seen, will run a sync against desired state and running state. If // no changes are seen to the configuration, will synchronize the last known desired // state every sync_frequency seconds. // Never returns. -func (kl *Kubelet) RunSyncLoop(updateChannel <-chan manifestUpdate, handler SyncHandler) { +func (kl *Kubelet) syncLoop(updateChannel <-chan manifestUpdate, handler SyncHandler) { last := make(map[string][]api.ContainerManifest) for { select { @@ -746,12 +746,19 @@ func (kl *Kubelet) RunSyncLoop(updateChannel <-chan manifestUpdate, handler Sync case <-time.After(kl.SyncFrequency): } - manifests := []api.ContainerManifest{} - for _, m := range last { - manifests = append(manifests, m...) + allManifests := []api.ContainerManifest{} + for src, srcManifests := range last { + for i := range srcManifests { + m := &srcManifests[i] + if err := api.ValidateManifest(m); err != nil { + glog.Warningf("Manifest from %s failed validation, ignoring: %v", src, err) + continue + } + } + allManifests = append(allManifests, srcManifests...) } - err := handler.SyncManifests(manifests) + err := handler.SyncManifests(allManifests) if err != nil { glog.Errorf("Couldn't sync containers : %v", err) }