Merge pull request #749 from smarterclayton/generate_valid_names

Generate valid file Pod Names and test for them

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})
 	}

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) {

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
+// simpleSubdomainSafeHash generates a pod name for the given path that is
-// only in the range a-zA-Z0-9, making it suitable for DNS subdomain labels
+// suitable as a subdomain label.
-func simpleSubdomainSafeHash(s string) string {
+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 {
+	return fmt.Sprintf("%.15s%.30s", name, sha)
-		sha = sha[:20]
-	}
-	return sha
 }

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"}}},
+		{Version: "v1beta1", Containers: []api.Container{{Name: "1", Image: "foo"}}},
-		{ID: "", Containers: []api.Container{{Image: "bar"}}},
+		{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)

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: "", Containers: []api.Container{{Name: "1", Image: "foo"}}},
-		{Version: "v1beta1", ID: "bar"},
+		{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) {