Merge pull request #828 from roberthbailey/config

Add validation when processing pod manifests from a URL.
This commit is contained in:
Daniel Smith 2014-08-21 14:34:25 -07:00
commit 3ab35c63f3
2 changed files with 125 additions and 105 deletions

View File

@ -66,23 +66,18 @@ func (s *SourceURL) extractFromURL() error {
} }
// First try as if it's a single manifest // First try as if it's a single manifest
var pod kubelet.Pod var manifest api.ContainerManifest
singleErr := yaml.Unmarshal(data, &pod.Manifest) singleErr := yaml.Unmarshal(data, &manifest)
// TODO: replace with validation
if singleErr == nil && pod.Manifest.Version == "" {
// If data is a []ContainerManifest, trying to put it into a ContainerManifest
// will not give an error but also won't set any of the fields.
// Our docs say that the version field is mandatory, so using that to judge wether
// this was actually successful.
singleErr = fmt.Errorf("got blank version field")
}
if singleErr == nil { if singleErr == nil {
name := pod.Manifest.ID if errs := api.ValidateManifest(&manifest); len(errs) > 0 {
if name == "" { singleErr = fmt.Errorf("invalid manifest: %v", errs)
name = "1" }
}
if singleErr == nil {
pod := kubelet.Pod{Name: manifest.ID, Manifest: manifest}
if pod.Name == "" {
pod.Name = "1"
} }
pod.Name = name
s.updates <- kubelet.PodUpdate{[]kubelet.Pod{pod}, kubelet.SET} s.updates <- kubelet.PodUpdate{[]kubelet.Pod{pod}, kubelet.SET}
return nil return nil
} }
@ -93,18 +88,21 @@ func (s *SourceURL) extractFromURL() error {
// We're not sure if the person reading the logs is going to care about the single or // We're not sure if the person reading the logs is going to care about the single or
// multiple manifest unmarshalling attempt, so we need to put both in the logs, as is // multiple manifest unmarshalling attempt, so we need to put both in the logs, as is
// done at the end. Hence not returning early here. // done at the end. Hence not returning early here.
if multiErr == nil && len(manifests) > 0 && manifests[0].Version == "" { if multiErr == nil {
multiErr = fmt.Errorf("got blank version field") for _, manifest := range manifests {
if errs := api.ValidateManifest(&manifest); len(errs) > 0 {
multiErr = fmt.Errorf("invalid manifest: %v", errs)
break
}
}
} }
if multiErr == nil { if multiErr == nil {
pods := []kubelet.Pod{} pods := []kubelet.Pod{}
for i := range manifests { for i, manifest := range manifests {
pod := kubelet.Pod{Manifest: manifests[i]} pod := kubelet.Pod{Name: manifest.ID, Manifest: manifest}
name := pod.Manifest.ID if pod.Name == "" {
if name == "" { pod.Name = fmt.Sprintf("%d", i+1)
name = fmt.Sprintf("%d", i+1)
} }
pod.Name = name
pods = append(pods, pod) pods = append(pods, pod)
} }
s.updates <- kubelet.PodUpdate{pods, kubelet.SET} s.updates <- kubelet.PodUpdate{pods, kubelet.SET}
@ -113,5 +111,5 @@ func (s *SourceURL) extractFromURL() error {
return fmt.Errorf("%v: received '%v', but couldn't parse as a "+ return fmt.Errorf("%v: received '%v', but couldn't parse as a "+
"single manifest (%v: %+v) or as multiple manifests (%v: %+v).\n", "single manifest (%v: %+v) or as multiple manifests (%v: %+v).\n",
s.url, string(data), singleErr, pod.Manifest, multiErr, manifests) s.url, string(data), singleErr, manifest, multiErr, manifests)
} }

View File

@ -41,97 +41,119 @@ func TestURLErrorNotExistNoUpdate(t *testing.T) {
func TestExtractFromHttpBadness(t *testing.T) { func TestExtractFromHttpBadness(t *testing.T) {
ch := make(chan interface{}, 1) ch := make(chan interface{}, 1)
c := SourceURL{"http://localhost:49575/_not_found_", ch} c := SourceURL{"http://localhost:49575/_not_found_", ch}
err := c.extractFromURL() if err := c.extractFromURL(); err == nil {
if err == nil {
t.Errorf("Expected error") t.Errorf("Expected error")
} }
expectEmptyChannel(t, ch) expectEmptyChannel(t, ch)
} }
func TestExtractFromHttpSingle(t *testing.T) { func TestExtractInvalidManifest(t *testing.T) {
manifests := []api.ContainerManifest{ var testCases = []struct {
{Version: "v1beta1", ID: "foo"}, desc string
manifests interface{}
}{
{
desc: "No version",
manifests: []api.ContainerManifest{{Version: ""}},
},
{
desc: "Invalid version",
manifests: []api.ContainerManifest{{Version: "v1betta2"}},
},
{
desc: "Invalid volume name",
manifests: []api.ContainerManifest{
{Version: "v1beta1", Volumes: []api.Volume{{Name: "_INVALID_"}}},
},
},
{
desc: "Duplicate volume names",
manifests: []api.ContainerManifest{
{
Version: "v1beta1",
Volumes: []api.Volume{{Name: "repeated"}, {Name: "repeated"}},
},
},
},
} }
// Taking a single-manifest from a URL allows kubelet to be used for _, testCase := range testCases {
// in the implementation of google's container VM image. data, err := json.Marshal(testCase.manifests)
data, err := json.Marshal(manifests[0]) if err != nil {
t.Fatalf("%s: Some weird json problem: %v", testCase.desc, err)
fakeHandler := util.FakeHandler{ }
StatusCode: 200, fakeHandler := util.FakeHandler{
ResponseBody: string(data), StatusCode: 200,
} ResponseBody: string(data),
testServer := httptest.NewServer(&fakeHandler) }
ch := make(chan interface{}, 1) testServer := httptest.NewServer(&fakeHandler)
c := SourceURL{testServer.URL, ch} ch := make(chan interface{}, 1)
c := SourceURL{testServer.URL, ch}
err = c.extractFromURL() if err := c.extractFromURL(); err == nil {
if err != nil { t.Errorf("%s: Expected error", testCase.desc)
t.Errorf("Unexpected error: %v", err)
}
update := (<-ch).(kubelet.PodUpdate)
expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "foo", Manifest: manifests[0]})
if !reflect.DeepEqual(expected, update) {
t.Errorf("Expected: %#v, Got: %#v", expected, update)
}
}
func TestExtractFromHttpMultiple(t *testing.T) {
manifests := []api.ContainerManifest{
{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 {
t.Fatalf("Some weird json problem: %v", err)
}
fakeHandler := util.FakeHandler{
StatusCode: 200,
ResponseBody: string(data),
}
testServer := httptest.NewServer(&fakeHandler)
ch := make(chan interface{}, 1)
c := SourceURL{testServer.URL, ch}
err = c.extractFromURL()
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
update := (<-ch).(kubelet.PodUpdate)
expected := CreatePodUpdate(kubelet.SET, kubelet.Pod{Name: "1", Manifest: manifests[0]}, kubelet.Pod{Name: "bar", Manifest: manifests[1]})
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) { func TestExtractFromHTTP(t *testing.T) {
manifests := []api.ContainerManifest{} var testCases = []struct {
data, err := json.Marshal(manifests) desc string
if err != nil { manifests interface{}
t.Fatalf("Some weird json problem: %v", err) expected kubelet.PodUpdate
}{
{
desc: "Single manifest",
manifests: api.ContainerManifest{Version: "v1beta1", ID: "foo"},
expected: CreatePodUpdate(kubelet.SET,
kubelet.Pod{
Name: "foo",
Manifest: api.ContainerManifest{Version: "v1beta1", ID: "foo"},
}),
},
{
desc: "Multiple manifests",
manifests: []api.ContainerManifest{
{Version: "v1beta1", ID: "", Containers: []api.Container{{Name: "1", Image: "foo"}}},
{Version: "v1beta1", ID: "bar", Containers: []api.Container{{Name: "1", Image: "foo"}}},
},
expected: CreatePodUpdate(kubelet.SET,
kubelet.Pod{
Name: "1",
Manifest: api.ContainerManifest{Version: "v1beta1", ID: "", Containers: []api.Container{{Name: "1", Image: "foo"}}},
},
kubelet.Pod{
Name: "bar",
Manifest: api.ContainerManifest{Version: "v1beta1", ID: "bar", Containers: []api.Container{{Name: "1", Image: "foo"}}},
}),
},
{
desc: "Empty Array",
manifests: []api.ContainerManifest{},
expected: CreatePodUpdate(kubelet.SET),
},
} }
for _, testCase := range testCases {
fakeHandler := util.FakeHandler{ data, err := json.Marshal(testCase.manifests)
StatusCode: 200, if err != nil {
ResponseBody: string(data), t.Fatalf("%s: Some weird json problem: %v", testCase.desc, err)
} }
testServer := httptest.NewServer(&fakeHandler) fakeHandler := util.FakeHandler{
ch := make(chan interface{}, 1) StatusCode: 200,
c := SourceURL{testServer.URL, ch} ResponseBody: string(data),
}
err = c.extractFromURL() testServer := httptest.NewServer(&fakeHandler)
if err != nil { ch := make(chan interface{}, 1)
t.Errorf("Unexpected error: %v", err) c := SourceURL{testServer.URL, ch}
} if err := c.extractFromURL(); err != nil {
update := (<-ch).(kubelet.PodUpdate) t.Errorf("%s: Unexpected error: %v", testCase.desc, err)
expected := CreatePodUpdate(kubelet.SET) }
if !reflect.DeepEqual(expected, update) { update := (<-ch).(kubelet.PodUpdate)
t.Errorf("Expected: %#v, Got: %#v", expected, update) if !reflect.DeepEqual(testCase.expected, update) {
t.Errorf("%s: Expected: %#v, Got: %#v", testCase.desc, testCase.expected, update)
}
for i := range update.Pods {
if errs := kubelet.ValidatePod(&update.Pods[i]); len(errs) != 0 {
t.Errorf("%s: Expected no validation errors on %#v, Got %#v", testCase.desc, update.Pods[i], errs)
}
}
} }
} }