Fix error checking for pod name & NS

Previously it would just check if POD_NAME *or* POD_NAMESPACE was set, which could lead to very bizarre failure modes.
nicr9 · Jul 14, 2017 · ad583c3 · ad583c3
1 parent d416d02
commit ad583c3
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 5 deletions.
diff --git a/core/pkg/k8s/main.go b/core/pkg/k8s/main.go
@@ -117,7 +117,7 @@ func GetPodDetails(kubeClient clientset.Interface) (*PodInfo, error) {
 	podName := os.Getenv("POD_NAME")
 	podNs := os.Getenv("POD_NAMESPACE")
 
-	if podName == "" && podNs == "" {
+	if podName == "" || podNs == "" {
 		return nil, fmt.Errorf("unable to get POD information (missing POD_NAME or POD_NAMESPACE environment variable")
 	}
 

diff --git a/core/pkg/k8s/main_test.go b/core/pkg/k8s/main_test.go
@@ -297,14 +297,30 @@ func TestGetPodDetails(t *testing.T) {
 		t.Errorf("expected an error but returned nil")
 	}
 
-	// POD not exist
-	os.Setenv("POD_NAME", "testpod")
+	// POD_NAME not exist
+	os.Setenv("POD_NAME", "")
 	os.Setenv("POD_NAMESPACE", api.NamespaceDefault)
 	_, err2 := GetPodDetails(testclient.NewSimpleClientset())
 	if err2 == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
+	// POD_NAMESPACE not exist
+	os.Setenv("POD_NAME", "testpod")
+	os.Setenv("POD_NAMESPACE", "")
+	_, err3 := GetPodDetails(testclient.NewSimpleClientset())
+	if err3 == nil {
+		t.Errorf("expected an error but returned nil")
+	}
+
+	// POD not exist
+	os.Setenv("POD_NAME", "testpod")
+	os.Setenv("POD_NAMESPACE", api.NamespaceDefault)
+	_, err4 := GetPodDetails(testclient.NewSimpleClientset())
+	if err4 == nil {
+		t.Errorf("expected an error but returned nil")
+	}
+
 	// success to get PodInfo
 	fkClient := testclient.NewSimpleClientset(
 		&api.PodList{Items: []api.Pod{{
@@ -331,8 +347,8 @@ func TestGetPodDetails(t *testing.T) {
 			},
 		}}})
 
-	epi, err3 := GetPodDetails(fkClient)
-	if err3 != nil {
+	epi, err5 := GetPodDetails(fkClient)
+	if err5 != nil {
 		t.Errorf("expected a PodInfo but returned error")
 		return
 	}