Repository: grafana/tanka
Pull request #684 (2022-03-21 19:06:18)

Fix `target must be a non-nil pointer`
With an API server for which I'm not authentified with correctly, `tanka` would panic with `target must be a non-nil pointer` because of a call to `errors.As`. Since we are actually only checking the type of the error and not its content, `errors.Is` can be used here. The It could also be moved to a variable to avoid creating a new instance each time just to check if it's of the right type.

This was happening trying to show the diff because it seems I'm not configured correctly.

```
WARNING: version difference between client (1.23) and server (1.20) exceeds the supported minor version skew of +/-1
Error from server (Forbidden): namespaces "default" is forbidden: User "<email>" cannot get resource "namespaces" in API group "" in the namespace "default": requires one of ["container.namespaces.get"] permission(s).
panic: errors: target must be a non-nil pointer
```


```
panic: errors: target must be a non-nil pointer

goroutine 1 [running]:
errors.As({0x10130dae8, 0x140000b2000}, {0x1012c0800, 0x101794360})
	/usr/local/go/src/errors/wrap.go:85 +0x3e0
github.com/pkg/errors.As(...)
	/Users/maoueh/go/pkg/mod/github.com/pkg/errors@v0.9.1/go113.go:31
github.com/grafana/tanka/pkg/kubernetes.(*Kubernetes).Diff(0x140004961a0, {0x140003f39d0, 0x2, 0x2}, {0x0?, 0x0?, {0x0?, 0x0?}})
	/Users/maoueh/work/github/grafana_tanka/pkg/kubernetes/diff.go:31 +0x4b8
github.com/grafana/tanka/pkg/tanka.Diff({0x16effebc0?, 0x140001edc58?}, {{{0x0, 0x140003227b0, 0x140003227e0, {0x0, 0x0, 0x0}, {0x0, 0x0}, ...}, ...}, ...})
	/Users/maoueh/work/github/grafana_tanka/pkg/tanka/workflow.go:122 +0xdc
main.diffCmd.func1(0x140001c4800?, {0x140002f8390, 0x1, 0x1?})
	/Users/maoueh/work/github/grafana_tanka/cmd/tk/workflow.go:158 +0x19c
github.com/go-clix/cli.(*Command).execute(0x140002afea0, {0x140002f8370, 0x1, 0x1})
	/Users/maoueh/go/pkg/mod/github.com/go-clix/cli@v0.2.0/command.go:118 +0x2b4
github.com/go-clix/cli.(*Command).Execute(0x140002af0e0)
	/Users/maoueh/go/pkg/mod/github.com/go-clix/cli@v0.2.0/command.go:76 +0xd4
main.main()
	/Users/maoueh/work/github/grafana_tanka/cmd/tk/main.go:58 +0x5bc
```

maoueh: @sh0rez You are right, it's a nuance I was not aware of the `errors.Is`, only works if the `error` checked is the same instance indeed, source code of `errors.Is` is clear there.

Changed my fix to keep `errors.As` must now using a pointer receiver.

Patch:
diff --git a/cmd/tk/workflow.go b/cmd/tk/workflow.go
index bc19a06e..733e8cc4 100644
--- a/cmd/tk/workflow.go
+++ b/cmd/tk/workflow.go
@@ -41,6 +41,7 @@ func applyCmd() *cli.Command {
 	cmd.Flags().BoolVar(&opts.Validate, "validate", true, "validation of resources (kubectl --validate=false)")
 	cmd.Flags().BoolVar(&opts.AutoApprove, "dangerous-auto-approve", false, "skip interactive approval. Only for automation!")
 	cmd.Flags().StringVar(&opts.DryRun, "dry-run", "", `--dry-run parameter to pass down to kubectl, must be "none", "server", or "client"`)
+	cmd.Flags().BoolVar(&opts.ServerSide, "server-side", false, `pass --server-side flag down to kubectl`)
 
 	vars := workflowFlags(cmd.Flags())
 	getJsonnetOpts := jsonnetFlags(cmd.Flags())
diff --git a/pkg/kubernetes/client/apply.go b/pkg/kubernetes/client/apply.go
index 83a40cb2..421aed01 100644
--- a/pkg/kubernetes/client/apply.go
+++ b/pkg/kubernetes/client/apply.go
@@ -25,6 +25,10 @@ func (k Kubectl) applyCtl(data manifest.List, opts ApplyOpts) *exec.Cmd {
 		argv = append(argv, dryRun)
 	}
 
+	if opts.ServerSide {
+		argv = append(argv, "--server-side")
+	}
+
 	return k.ctl("apply", argv...)
 }
 
diff --git a/pkg/kubernetes/client/client.go b/pkg/kubernetes/client/client.go
index 7b88cef6..9c8be1eb 100644
--- a/pkg/kubernetes/client/client.go
+++ b/pkg/kubernetes/client/client.go
@@ -53,6 +53,9 @@ type ApplyOpts struct {
 
 	// DryRun string passed to kubectl as --dry-run=<DryRun>
 	DryRun string
+
+	// ServerSide flag passed to kubectl as --server-side
+	ServerSide bool
 }
 
 // DeleteOpts allow to specify additional parameters for delete operations
diff --git a/pkg/kubernetes/diff.go b/pkg/kubernetes/diff.go
index acbfdb9e..d57759b8 100644
--- a/pkg/kubernetes/diff.go
+++ b/pkg/kubernetes/diff.go
@@ -28,7 +28,7 @@ Please upgrade kubectl to at least version 1.18.1.`)
 		for _, namespace := range resourceNamespaces {
 			_, err = k.ctl.Namespace(namespace)
 			if err != nil {
-				if errors.As(err, client.ErrNamespaceNotFound{}) {
+				if errors.As(err, &client.ErrNamespaceNotFound{}) {
 					continue
 				}
 				return nil, errors.Wrap(err, "retrieving namespaces")
diff --git a/pkg/tanka/workflow.go b/pkg/tanka/workflow.go
index 6d6e3fa3..8795ba8a 100644
--- a/pkg/tanka/workflow.go
+++ b/pkg/tanka/workflow.go
@@ -27,6 +27,8 @@ type ApplyOpts struct {
 	Validate bool
 	// DryRun string passed to kubectl as --dry-run=<DryRun>
 	DryRun string
+	// ServerSide bool passed to kubectl as --server-side
+	ServerSide bool
 }
 
 // Apply parses the environment at the given directory (a `baseDir`) and applies
@@ -67,9 +69,10 @@ func Apply(baseDir string, opts ApplyOpts) error {
 	}
 
 	return kube.Apply(l.Resources, kubernetes.ApplyOpts{
-		Force:    opts.Force,
-		Validate: opts.Validate,
-		DryRun:   opts.DryRun,
+		Force:      opts.Force,
+		Validate:   opts.Validate,
+		DryRun:     opts.DryRun,
+		ServerSide: opts.ServerSide,
 	})
 }
 


Test patch:
diff --git a/pkg/kubernetes/client/apply_test.go b/pkg/kubernetes/client/apply_test.go
index 193075b7..6436ac01 100644
--- a/pkg/kubernetes/client/apply_test.go
+++ b/pkg/kubernetes/client/apply_test.go
@@ -58,6 +58,13 @@ func TestKubectl_applyCtl(t *testing.T) {
 			expectedArgs:   []string{"--dry-run=server"},
 			unExpectedArgs: []string{"--validate=false"},
 		},
+		{
+			name: "test server-side",
+			args: args{
+				opts: ApplyOpts{ServerSide: true},
+			},
+			expectedArgs: []string{"--server-side"},
+		},
 	}
 
 	for _, tt := range tests {