fix(kubernetes): handle kubectl-diff exit 1

pkg/kubernetes/client/client.go

@@ -31,7 +31,7 @@ type Client interface {
 	// Info returns known informational data about the client. Best effort based,
 	// fields of `Info` that cannot be stocked with valuable data, e.g.
 	// due to an error, shall be left nil.
-	Info() (*Info, error)
+	Info() Info
 }
 
 // Info contains metadata about the client and its environment

pkg/kubernetes/client/diff.go

@@ -6,6 +6,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/Masterminds/semver"
 	"github.com/grafana/tanka/pkg/kubernetes/manifest"
 	"github.com/grafana/tanka/pkg/kubernetes/util"
 )
@@ -23,16 +24,15 @@ func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) {
 
 	raw := bytes.Buffer{}
 	cmd.Stdout = &raw
-	cmd.Stderr = FilterWriter{regexp.MustCompile(`exit status \d`)}
+
+	fw := FilterWriter{filters: []*regexp.Regexp{regexp.MustCompile(`exit status \d`)}}
+	cmd.Stderr = &fw
 
 	cmd.Stdin = strings.NewReader(ready.String())
 
 	err = cmd.Run()
-
-	// kubectl uses exit status 1 to tell us that there is a diff
-	if exitError, ok := err.(*exec.ExitError); ok && exitError.ExitCode() == 1 {
-	} else if err != nil {
-		return nil, err
+	if diffErr := parseDiffErr(err, fw.buf, k.Info().ClientVersion); diffErr != nil {
+		return nil, diffErr
 	}
 
 	s := raw.String()
@@ -52,6 +52,39 @@ func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) {
 	return nil, nil
 }
 
+// parseDiffErr handles the exit status code of `kubectl diff`. It returns err
+// when an error happened, nil otherwise.
+// "Differences found (exit status 1)" is not an error.
+//
+// kubectl >= 1.18:
+// 0: no error, no differences
+// 1: differences found
+// >1: error
+//
+// kubectl < 1.18:
+// 0: no error, no differences
+// 1: error OR differences found
+func parseDiffErr(err error, stderr string, version *semver.Version) error {
+	exitErr, ok := err.(*exec.ExitError)
+	if !ok {
+		// this error is not kubectl related
+		return err
+	}
+
+	// internal kubectl error
+	if exitErr.ExitCode() != 1 {
+		return err
+	}
+
+	// before 1.18 "exit status 1" meant error as well ... so we need to check stderr
+	if version.LessThan(semver.MustParse("1.18.0")) && stderr != "" {
+		return err
+	}
+
+	// differences found is not an error
+	return nil
+}
+
 func separateMissingNamespace(in manifest.List, exists map[string]bool) (ready, missingNamespace manifest.List) {
 	for _, r := range in {
 		// namespace does not exist, also ignore implicit default ("")

pkg/kubernetes/client/kubectl.go

@@ -21,6 +21,8 @@ type Kubectl struct {
 	context objx.Map
 	cluster objx.Map
 
+	info *Info
+
 	APIServer string
 }
 
@@ -32,11 +34,22 @@ func New(endpoint, namespace string) (*Kubectl, error) {
 	if err := k.setupContext(namespace); err != nil {
 		return nil, errors.Wrap(err, "finding usable context")
 	}
+
+	info, err := k.newInfo()
+	if err != nil {
+		return nil, errors.Wrap(err, "gathering client info")
+	}
+	k.info = info
+
 	return &k, nil
 }
 
 // Info returns known informational data about the client and its environment
-func (k Kubectl) Info() (*Info, error) {
+func (k Kubectl) Info() Info {
+	return *k.info
+}
+
+func (k Kubectl) newInfo() (*Info, error) {
 	client, server, err := k.version()
 	if err != nil {
 		return nil, errors.Wrap(err, "obtaining versions")
@@ -103,14 +116,18 @@ func (k Kubectl) Namespaces() (map[string]bool, error) {
 
 // FilterWriter is an io.Writer that discards every message that matches at
 // least one of the regular expressions.
-type FilterWriter []*regexp.Regexp
+type FilterWriter struct {
+	buf     string
+	filters []*regexp.Regexp
+}
 
-func (r FilterWriter) Write(p []byte) (n int, err error) {
-	for _, re := range r {
+func (r *FilterWriter) Write(p []byte) (n int, err error) {
+	for _, re := range r.filters {
 		if re.Match(p) {
 			// silently discard
 			return len(p), nil
 		}
 	}
+	r.buf += string(p)
 	return os.Stderr.Write(p)
 }

pkg/kubernetes/kubernetes.go

@@ -19,8 +19,7 @@ type Kubernetes struct {
 	Spec v1alpha1.Spec
 
 	// Client (kubectl)
-	ctl  client.Client
-	info client.Info
+	ctl client.Client
 
 	// Diffing
 	differs map[string]Differ // List of diff strategies
@@ -38,25 +37,18 @@ func New(s v1alpha1.Spec) (*Kubernetes, error) {
 		return nil, errors.Wrap(err, "creating client")
 	}
 
-	// obtain information about the client (including versions)
-	info, err := ctl.Info()
-	if err != nil {
-		return nil, err
-	}
-
 	// setup diffing
 	if s.DiffStrategy == "" {
 		s.DiffStrategy = "native"
 
-		if info.ServerVersion.LessThan(semver.MustParse("1.13.0")) {
+		if ctl.Info().ServerVersion.LessThan(semver.MustParse("1.13.0")) {
 			s.DiffStrategy = "subset"
 		}
 	}
 
 	k := Kubernetes{
 		Spec: s,
 		ctl:  ctl,
-		info: *info,
 		differs: map[string]Differ{
 			"native": ctl.DiffServerSide,
 			"subset": SubsetDiffer(ctl),
@@ -71,12 +63,9 @@ type ApplyOpts client.ApplyOpts
 
 // Apply receives a state object generated using `Reconcile()` and may apply it to the target system
 func (k *Kubernetes) Apply(state manifest.List, opts ApplyOpts) error {
-	info, err := k.ctl.Info()
-	if err != nil {
-		return err
-	}
 	alert := color.New(color.FgRed, color.Bold).SprintFunc()
 
+	info := k.ctl.Info()
 	if !opts.AutoApprove {
 		if err := cli.Confirm(
 			fmt.Sprintf(`Applying to namespace '%s' of cluster '%s' at '%s' using context '%s'.`,
@@ -126,7 +115,7 @@ func (k *Kubernetes) Diff(state manifest.List, opts DiffOpts) (*string, error) {
 
 // Info about the client, etc.
 func (k *Kubernetes) Info() client.Info {
-	return k.info
+	return k.ctl.Info()
 }
 
 func objectspec(m manifest.Manifest) string {