Merge #245 #247

245: Don't panic on error r=mkmik a=mkmik

Currently when `kubeseal` wants to report an error, we just `panic` from main, which is ugly.
This change also weaves CLI flags into a `run` function so that we can test that in isolation.

247: Include upstream error in kubeseal --rotate and --validate r=mkmik a=mkmik



Co-authored-by: Marko Mikulicic <[email protected]>

Author: bors[bot]

cmd/kubeseal/main.go

@@ -128,7 +128,7 @@ func prettyEncoder(codecs runtimeserializer.CodecFactory, mediaType string, gv r
 func openCertFile(certFile string) (io.ReadCloser, error) {
 	f, err := os.Open(certFile)
 	if err != nil {
-		return nil, fmt.Errorf("Error reading %s: %v", certFile, err)
+		return nil, err
 	}
 	return f, nil
 }
@@ -139,14 +139,14 @@ func openCertHTTP(c corev1.CoreV1Interface, namespace, name string) (io.ReadClos
 		ProxyGet("http", name, "", "/v1/cert.pem", nil).
 		Stream()
 	if err != nil {
-		return nil, fmt.Errorf("Error fetching certificate: %v", err)
+		return nil, fmt.Errorf("cannot fetch certificate: %v", err)
 	}
 	return f, nil
 }
 
-func openCert() (io.ReadCloser, error) {
-	if *certFile != "" {
-		return openCertFile(*certFile)
+func openCert(certFile string) (io.ReadCloser, error) {
+	if certFile != "" {
+		return openCertFile(certFile)
 	}
 
 	conf, err := clientConfig.ClientConfig()
@@ -233,9 +233,9 @@ func validateSealedSecret(in io.Reader, namespace, name string) error {
 	res := req.Do()
 	if err := res.Error(); err != nil {
 		if status, ok := err.(*k8serrors.StatusError); ok && status.Status().Code == http.StatusConflict {
-			return fmt.Errorf("Unable to decrypt sealed secret")
+			return fmt.Errorf("unable to decrypt sealed secret")
 		}
-		return fmt.Errorf("Error occurred while validating sealed secret")
+		return fmt.Errorf("cannot validate sealed secret: %v", err)
 	}
 
 	return nil
@@ -267,9 +267,9 @@ func rotateSealedSecret(in io.Reader, out io.Writer, codecs runtimeserializer.Co
 	res := req.Do()
 	if err := res.Error(); err != nil {
 		if status, ok := err.(*k8serrors.StatusError); ok && status.Status().Code == http.StatusConflict {
-			return fmt.Errorf("Unable to rotate secret")
+			return fmt.Errorf("unable to rotate secret")
 		}
-		return fmt.Errorf("Error occurred while rotating secret")
+		return fmt.Errorf("cannot rotate secret: %v", err)
 	}
 	body, err := res.Raw()
 	if err != nil {
@@ -311,49 +311,45 @@ func sealedSecretOutput(out io.Writer, codecs runtimeserializer.CodecFactory, ss
 	return nil
 }
 
-func main() {
-	flag.Parse()
-	goflag.CommandLine.Parse([]string{})
-
-	if *printVersion {
-		fmt.Printf("kubeseal version: %s\n", VERSION)
-		return
+func run(w io.Writer, controllerNs, controllerName, certFile string, printVersion, validateSecret, rotate, dumpCert bool) error {
+	if printVersion {
+		fmt.Fprintf(w, "kubeseal version: %s\n", VERSION)
+		return nil
 	}
 
-	if *validateSecret {
-		err := validateSealedSecret(os.Stdin, *controllerNs, *controllerName)
-		if err != nil {
-			panic(err.Error())
-		}
-		return
+	if validateSecret {
+		return validateSealedSecret(os.Stdin, controllerNs, controllerName)
 	}
 
-	if *rotate {
-		if err := rotateSealedSecret(os.Stdin, os.Stdout, scheme.Codecs, *controllerNs, *controllerName); err != nil {
-			panic(err.Error())
-		}
-		return
+	if rotate {
+		return rotateSealedSecret(os.Stdin, os.Stdout, scheme.Codecs, controllerNs, controllerName)
 	}
 
-	f, err := openCert()
+	f, err := openCert(certFile)
 	if err != nil {
-		panic(err.Error())
+		return err
 	}
 	defer f.Close()
 
-	if *dumpCert {
-		if _, err := io.Copy(os.Stdout, f); err != nil {
-			panic(err.Error())
-		}
-		return
+	if dumpCert {
+		_, err := io.Copy(os.Stdout, f)
+		return err
 	}
 
 	pubKey, err := parseKey(f)
 	if err != nil {
-		panic(err.Error())
+		return err
 	}
 
-	if err := seal(os.Stdin, os.Stdout, scheme.Codecs, pubKey); err != nil {
-		panic(err.Error())
+	return seal(os.Stdin, os.Stdout, scheme.Codecs, pubKey)
+}
+
+func main() {
+	flag.Parse()
+	goflag.CommandLine.Parse([]string{})
+
+	if err := run(os.Stdout, *controllerNs, *controllerName, *certFile, *printVersion, *validateSecret, *rotate, *dumpCert); err != nil {
+		fmt.Fprintf(os.Stderr, "error: %v\n", err)
+		os.Exit(1)
 	}
 }

cmd/kubeseal/main_test.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -98,13 +98,9 @@ func TestParseKey(t *testing.T) {
 }
 
 func TestOpenCertFile(t *testing.T) {
-	*certFile = tmpfile(t, []byte(testCert))
-	defer func() {
-		os.Remove(*certFile)
-		*certFile = ""
-	}()
+	certFile := tmpfile(t, []byte(testCert))
 
-	f, err := openCert()
+	f, err := openCert(certFile)
 	if err != nil {
 		t.Fatalf("Error reading test cert file: %v", err)
 	}
@@ -178,3 +174,24 @@ func TestSeal(t *testing.T) {
 	}
 	// NB: See sealedsecret_test.go for e2e crypto test
 }
+
+func TestVersion(t *testing.T) {
+	var buf strings.Builder
+	err := run(&buf, "", "", "", true, false, false, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if got, want := buf.String(), "kubeseal version: UNKNOWN\n"; got != want {
+		t.Errorf("got: %q, want: %q", got, want)
+	}
+}
+
+func TestMainError(t *testing.T) {
+	const badFileName = "/?this/file/cannot/possibly/exist/can/it?"
+	err := run(ioutil.Discard, "", "", badFileName, false, false, false, false)
+
+	if err == nil || !os.IsNotExist(err) {
+		t.Fatalf("expecting not exist error, got: %v", err)
+	}
+}

integration/integration_suite_test.go

@@ -12,7 +12,7 @@ import (
 	"os/exec"
 	"testing"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -80,14 +80,14 @@ func containsString(haystack []string, needle string) bool {
 	return false
 }
 
-func runKubeseal(flags []string, input io.Reader, output io.Writer) error {
+func runKubeseal(flags []string, input io.Reader, output io.Writer, opts ...runAppOpt) error {
 	args := []string{}
 	if *kubeconfig != "" && !containsString(flags, "--kubeconfig") {
 		args = append(args, "--kubeconfig", *kubeconfig)
 	}
 	args = append(args, flags...)
 
-	return runApp(*kubesealBin, args, input, output)
+	return runApp(*kubesealBin, args, input, output, opts...)
 }
 
 type interruptableReader struct {
@@ -129,17 +129,34 @@ func runController(flags []string, input io.Reader, output io.Writer) error {
 	return runApp(*controllerBin, flags, input, output)
 }
 
-func runApp(app string, flags []string, input io.Reader, output io.Writer) error {
+type runAppOpt func(*runAppOpts)
+
+type runAppOpts struct {
+	stderr io.Writer
+}
+
+func runAppWithStderr(w io.Writer) runAppOpt {
+	return func(o *runAppOpts) { o.stderr = w }
+}
+
+func runApp(app string, flags []string, input io.Reader, output io.Writer, opts ...runAppOpt) error {
+	options := runAppOpts{
+		stderr: GinkgoWriter,
+	}
+	for _, o := range opts {
+		o(&options)
+	}
+
 	fmt.Fprintf(GinkgoWriter, "Running %q %q\n", app, flags)
 	cmd := exec.Command(app, flags...)
 	cmd.Stdin = input
 	cmd.Stdout = output
-	cmd.Stderr = GinkgoWriter
+	cmd.Stderr = options.stderr
 
 	return cmd.Run()
 }
 
-func runKubesealWith(flags []string, input runtime.Object) (runtime.Object, error) {
+func runKubesealWith(flags []string, input runtime.Object, opts ...runAppOpt) (runtime.Object, error) {
 	enc := scheme.Codecs.LegacyCodec(v1.SchemeGroupVersion)
 	indata, err := runtime.Encode(enc, input)
 	if err != nil {
@@ -150,7 +167,7 @@ func runKubesealWith(flags []string, input runtime.Object) (runtime.Object, erro
 
 	outbuf := bytes.Buffer{}
 
-	if err := runKubeseal(flags, bytes.NewReader(indata), &outbuf); err != nil {
+	if err := runKubeseal(flags, bytes.NewReader(indata), &outbuf, opts...); err != nil {
 		return nil, err
 	}
 

integration/kubeseal_test.go

@@ -284,3 +284,23 @@ var _ = Describe("kubeseal --verify", func() {
 	})
 
 })
+
+var _ = Describe("kubeseal --cert", func() {
+	var input io.Reader
+	var output *bytes.Buffer
+	var args []string
+
+	BeforeEach(func() {
+		args = []string{"--cert", "/?this/file/cannot/possibly/exist/right?"}
+		output = &bytes.Buffer{}
+	})
+
+	JustBeforeEach(func() {
+		err := runKubeseal(args, input, ioutil.Discard, runAppWithStderr(output))
+		Expect(err).To(HaveOccurred())
+	})
+
+	It("should return an error", func() {
+		Expect(output.String()).Should(MatchRegexp("^error:.*no such file or directory"))
+	})
+})