From c17e70075933cea07d6c239fec18d4f30beae5be Mon Sep 17 00:00:00 2001
From: Gabe Rosenhouse <rosenhouse@gmail.com>
Date: Wed, 13 Jul 2016 22:24:34 -0400
Subject: [PATCH] pkg/skel: missing env var log lines appear in stderr

Previously, the log lines appeared in stdout before the JSON encoding of
the error message.  That would break JSON parsing of stdout.  Instead, we use
stderr for these unstructured logs, consistent with the CNI spec.
---
 pkg/skel/skel.go      |  5 +++--
 pkg/skel/skel_test.go | 22 ++++++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go
index 484ef374..bed405c3 100644
--- a/pkg/skel/skel.go
+++ b/pkg/skel/skel.go
@@ -40,6 +40,7 @@ type CmdArgs struct {
 type dispatcher struct {
 	Getenv func(string) string
 	Stdin  io.Reader
+	Stderr io.Writer
 }
 
 type reqForCmdEntry map[string]bool
@@ -106,8 +107,7 @@ func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) {
 	for _, v := range vars {
 		*v.val = t.Getenv(v.name)
 		if v.reqForCmd[cmd] && *v.val == "" {
-			log.Printf("%v env variable missing", v.name)
-			// TODO: test this logging ^^^  and log to stderr instead of stdout
+			fmt.Fprintf(t.Stderr, "%v env variable missing\n", v.name)
 			argsMissing = true
 		}
 	}
@@ -172,6 +172,7 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
 	caller := dispatcher{
 		Getenv: os.Getenv,
 		Stdin:  os.Stdin,
+		Stderr: os.Stderr,
 	}
 
 	err := caller.pluginMain(cmdAdd, cmdDel)
diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go
index 8a69d556..9974c9a5 100644
--- a/pkg/skel/skel_test.go
+++ b/pkg/skel/skel_test.go
@@ -15,6 +15,7 @@
 package skel
 
 import (
+	"bytes"
 	"errors"
 	"io"
 	"strings"
@@ -47,6 +48,7 @@ var _ = Describe("dispatching to the correct callback", func() {
 	var (
 		environment     map[string]string
 		stdin           io.Reader
+		stderr          *bytes.Buffer
 		cmdAdd, cmdDel  *fakeCmd
 		dispatch        *dispatcher
 		expectedCmdArgs *CmdArgs
@@ -62,9 +64,11 @@ var _ = Describe("dispatching to the correct callback", func() {
 			"CNI_PATH":        "/some/cni/path",
 		}
 		stdin = strings.NewReader(`{ "some": "config" }`)
+		stderr = &bytes.Buffer{}
 		dispatch = &dispatcher{
 			Getenv: func(key string) string { return environment[key] },
 			Stdin:  stdin,
+			Stderr: stderr,
 		}
 		cmdAdd = &fakeCmd{}
 		cmdDel = &fakeCmd{}
@@ -87,6 +91,7 @@ var _ = Describe("dispatching to the correct callback", func() {
 				Code: 100,
 				Msg:  "required env variables missing",
 			}))
+			Expect(stderr.String()).To(ContainSubstring(envVar + " env variable missing\n"))
 		} else {
 			Expect(err).NotTo(HaveOccurred())
 		}
@@ -117,6 +122,23 @@ var _ = Describe("dispatching to the correct callback", func() {
 			Entry("args", "CNI_ARGS", false),
 			Entry("path", "CNI_PATH", true),
 		)
+
+		Context("when multiple required env vars are missing", func() {
+			BeforeEach(func() {
+				delete(environment, "CNI_NETNS")
+				delete(environment, "CNI_IFNAME")
+				delete(environment, "CNI_PATH")
+			})
+
+			It("reports that all of them are missing, not just the first", func() {
+				Expect(dispatch.pluginMain(cmdAdd.Func, cmdDel.Func)).NotTo(Succeed())
+				log := stderr.String()
+				Expect(log).To(ContainSubstring("CNI_NETNS env variable missing\n"))
+				Expect(log).To(ContainSubstring("CNI_IFNAME env variable missing\n"))
+				Expect(log).To(ContainSubstring("CNI_PATH env variable missing\n"))
+
+			})
+		})
 	})
 
 	Context("when the CNI_COMMAND is DEL", func() {