From 10564e447bcb69bdda57f5a92d65f89cae597c49 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Sun, 3 Aug 2014 16:55:34 -0400 Subject: [PATCH] Generate valid file Pod Names and test for them Ensure that pods coming from the unit test configuration validate, and that proper names are generated. Lowercases source filenames, strips all but a-z0-9, and tests for consistent results. --- pkg/kubelet/config/etcd.go | 2 +- pkg/kubelet/config/etcd_test.go | 10 ++++++++-- pkg/kubelet/config/file.go | 22 ++++++++++++---------- pkg/kubelet/config/file_test.go | 30 ++++++++++++++++++++++++++++-- pkg/kubelet/config/http_test.go | 9 +++++++-- 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/pkg/kubelet/config/etcd.go b/pkg/kubelet/config/etcd.go index ef3210f5936..57baa5bf697 100644 --- a/pkg/kubelet/config/etcd.go +++ b/pkg/kubelet/config/etcd.go @@ -118,7 +118,7 @@ func responseToPods(response *etcd.Response) ([]kubelet.Pod, error) { for i, manifest := range manifests.Items { name := manifest.ID if name == "" { - name = fmt.Sprintf("_%d", i+1) + name = fmt.Sprintf("%d", i+1) } pods = append(pods, kubelet.Pod{Name: name, Manifest: manifest}) } diff --git a/pkg/kubelet/config/etcd_test.go b/pkg/kubelet/config/etcd_test.go index 0a302f0978d..854e88bdbc6 100644 --- a/pkg/kubelet/config/etcd_test.go +++ b/pkg/kubelet/config/etcd_test.go @@ -74,11 +74,12 @@ func TestGetEtcdNoData(t *testing.T) { func TestGetEtcd(t *testing.T) { fakeClient := tools.MakeFakeEtcdClient(t) ch := make(chan interface{}, 1) + manifest := api.ContainerManifest{ID: "foo", Version: "v1beta1", Containers: []api.Container{{Name: "1", Image: "foo"}}} fakeClient.Data["/registry/hosts/machine/kubelet"] = tools.EtcdResponseWithError{ R: &etcd.Response{ Node: &etcd.Node{ Value: api.EncodeOrDie(&api.ContainerManifestList{ - Items: []api.ContainerManifest{{ID: "foo"}}, + Items: []api.ContainerManifest{manifest}, }), ModifiedIndex: 1, }, @@ -94,10 +95,15 @@ func TestGetEtcd(t *testing.T) { t.Errorf("Expected %#v, Got %#v", 2, lastIndex) } update := (<-ch).(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "foo", Manifest: api.ContainerManifest{ID: "foo"}}) + expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "foo", Manifest: manifest}) if !reflect.DeepEqual(expected, update) { t.Errorf("Expected %#v, Got %#v", expected, update) } + for i := range update.Pods { + if errs := kubelet.ValidatePod(&update.Pods[i]); len(errs) != 0 { + t.Errorf("Expected no validation errors on %#v, Got %#v", update.Pods[i], errs) + } + } } func TestWatchEtcd(t *testing.T) { diff --git a/pkg/kubelet/config/file.go b/pkg/kubelet/config/file.go index 6128b8d8bfd..3c935307785 100644 --- a/pkg/kubelet/config/file.go +++ b/pkg/kubelet/config/file.go @@ -19,12 +19,14 @@ package config import ( "crypto/sha1" - "encoding/base64" + "encoding/base32" "fmt" "io/ioutil" "os" "path/filepath" + "regexp" "sort" + "strings" "time" "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet" @@ -134,16 +136,16 @@ func extractFromFile(name string) (kubelet.Pod, error) { return pod, nil } -var simpleSubdomainSafeEncoding = base64.NewEncoding("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345678900") +var simpleSubdomainSafeEncoding = base32.NewEncoding("0123456789abcdefghijklmnopqrstuv") +var unsafeDNSLabelReplacement = regexp.MustCompile("[^a-z0-9]+") -// simpleSubdomainSafeHash generates a compact hash of the input that uses characters -// only in the range a-zA-Z0-9, making it suitable for DNS subdomain labels -func simpleSubdomainSafeHash(s string) string { +// simpleSubdomainSafeHash generates a pod name for the given path that is +// suitable as a subdomain label. +func simpleSubdomainSafeHash(path string) string { + name := strings.ToLower(filepath.Base(path)) + name = unsafeDNSLabelReplacement.ReplaceAllString(name, "") hasher := sha1.New() - hasher.Write([]byte(s)) + hasher.Write([]byte(path)) sha := simpleSubdomainSafeEncoding.EncodeToString(hasher.Sum(nil)) - if len(sha) > 20 { - sha = sha[:20] - } - return sha + return fmt.Sprintf("%.15s%.30s", name, sha) } diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index f505c3b0c84..cb41669e851 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -148,8 +148,8 @@ func TestExtractFromEmptyDir(t *testing.T) { func TestExtractFromDir(t *testing.T) { manifests := []api.ContainerManifest{ - {ID: "", Containers: []api.Container{{Image: "foo"}}}, - {ID: "", Containers: []api.Container{{Image: "bar"}}}, + {Version: "v1beta1", Containers: []api.Container{{Name: "1", Image: "foo"}}}, + {Version: "v1beta1", Containers: []api.Container{{Name: "2", Image: "bar"}}}, } files := make([]*os.File, len(manifests)) @@ -193,6 +193,32 @@ func TestExtractFromDir(t *testing.T) { if !reflect.DeepEqual(expected, update) { t.Errorf("Expected %#v, Got %#v", expected, update) } + for i := range update.Pods { + if errs := kubelet.ValidatePod(&update.Pods[i]); len(errs) != 0 { + t.Errorf("Expected no validation errors on %#v, Got %#v", update.Pods[i], errs) + } + } +} + +func TestSubdomainSafeName(t *testing.T) { + type Case struct { + Input string + Expected string + } + testCases := []Case{ + {"/some/path/invalidUPPERCASE", "invaliduppercasa6hlenc0vpqbbdtt26ghneqsq3pvud"}, + {"/some/path/_-!%$#&@^&*(){}", "nvhc03p016m60huaiv3avts372rl2p"}, + } + for _, testCase := range testCases { + value := simpleSubdomainSafeHash(testCase.Input) + if value != testCase.Expected { + t.Errorf("Expected %s, Got %s", testCase.Expected, value) + } + value2 := simpleSubdomainSafeHash(testCase.Input) + if value != value2 { + t.Errorf("Value for %s was not stable across runs: %s %s", testCase.Input, value, value2) + } + } } // These are used for testing extract json (below) diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index bf5efb5add4..4e08b45fc79 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -77,8 +77,8 @@ func TestExtractFromHttpSingle(t *testing.T) { func TestExtractFromHttpMultiple(t *testing.T) { manifests := []api.ContainerManifest{ - {Version: "v1beta1", ID: ""}, - {Version: "v1beta1", ID: "bar"}, + {Version: "v1beta1", ID: "", Containers: []api.Container{{Name: "1", Image: "foo"}}}, + {Version: "v1beta1", ID: "bar", Containers: []api.Container{{Name: "1", Image: "foo"}}}, } data, err := json.Marshal(manifests) if err != nil { @@ -103,6 +103,11 @@ func TestExtractFromHttpMultiple(t *testing.T) { if !reflect.DeepEqual(expected, update) { t.Errorf("Expected: %#v, Got: %#v", expected, update) } + for i := range update.Pods { + if errs := kubelet.ValidatePod(&update.Pods[i]); len(errs) != 0 { + t.Errorf("Expected no validation errors on %#v, Got %#v", update.Pods[i], errs) + } + } } func TestExtractFromHttpEmptyArray(t *testing.T) {