Skip to content

Commit 3371845

Browse files
authored
fix(kubernetes): properly handle diff namespaces (#237)
* feat(kubernetes): client api-resources Adds a method to the client to query all known api-resources from the server * feat(kubernetes): separate diffable resources * feat(kubernetes): use separated resources Previous commit added support for separating resources in groups. This commit uses these new groups to provide a better diff experience. Especially a soon-created namespace won't cause an error anymore. * refactor: remove old test file The tested code is superseded by other tested code, we don't need that anymore * test(kubernetes): UnmarshalTable Adds a unit test to ensure UnmarshalTable behaves properly. Also modified the behaviour a bit for correctness. * doc(kubernetes): client.Resources * style: make golint happy
1 parent 9af9d7b commit 3371845

File tree

10 files changed

+554
-148
lines changed

10 files changed

+554
-148
lines changed

docs/docs/env-vars.md

+5
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,8 @@ route: "/env-vars"
99

1010
**Description**: Path to the `kubectl` tool executable
1111
**Default**: `$PATH/kubectl`
12+
13+
### TANKA_KUBECTL_TRACE
14+
15+
**Description**: Print all calls to `kubectl`
16+
**Default**: `false`

pkg/kubernetes/client/client.go

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ type Client interface {
2424

2525
// Namespaces the cluster currently has
2626
Namespaces() (map[string]bool, error)
27+
// Resources returns all known api-resources of the cluster
28+
Resources() (Resources, error)
2729

2830
// Info returns known informational data about the client. Best effort based,
2931
// fields of `Info` that cannot be stocked with valuable data, e.g.

pkg/kubernetes/client/diff.go

+5-33
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,11 @@ import (
99
"github.com/Masterminds/semver"
1010

1111
"github.com/grafana/tanka/pkg/kubernetes/manifest"
12-
"github.com/grafana/tanka/pkg/kubernetes/util"
1312
)
1413

1514
// DiffServerSide takes the desired state and computes the differences on the
1615
// server, returning them in `diff(1)` format
1716
func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) {
18-
ns, err := k.Namespaces()
19-
if err != nil {
20-
return nil, err
21-
}
22-
23-
ready, missing := separateMissingNamespace(data, ns)
2417
cmd := k.ctl("diff", "-f", "-")
2518

2619
raw := bytes.Buffer{}
@@ -29,28 +22,19 @@ func (k Kubectl) DiffServerSide(data manifest.List) (*string, error) {
2922
fw := FilterWriter{filters: []*regexp.Regexp{regexp.MustCompile(`exit status \d`)}}
3023
cmd.Stderr = &fw
3124

32-
cmd.Stdin = strings.NewReader(ready.String())
25+
cmd.Stdin = strings.NewReader(data.String())
3326

34-
err = cmd.Run()
27+
err := cmd.Run()
3528
if diffErr := parseDiffErr(err, fw.buf, k.Info().ClientVersion); diffErr != nil {
3629
return nil, diffErr
3730
}
3831

3932
s := raw.String()
40-
for _, r := range missing {
41-
d, err := util.DiffStr(util.DiffName(r), "", r.String())
42-
if err != nil {
43-
return nil, err
44-
}
45-
s += d
46-
}
47-
48-
if s != "" {
49-
return &s, nil
33+
if s == "" {
34+
return nil, nil
5035
}
5136

52-
// no diff -> nil
53-
return nil, nil
37+
return &s, nil
5438
}
5539

5640
// parseDiffErr handles the exit status code of `kubectl diff`. It returns err
@@ -85,15 +69,3 @@ func parseDiffErr(err error, stderr string, version *semver.Version) error {
8569
// differences found is not an error
8670
return nil
8771
}
88-
89-
func separateMissingNamespace(in manifest.List, exists map[string]bool) (ready, missingNamespace manifest.List) {
90-
for _, r := range in {
91-
// namespace does not exist, also ignore implicit default ("")
92-
if ns := r.Metadata().Namespace(); ns != "" && !exists[ns] {
93-
missingNamespace = append(missingNamespace, r)
94-
continue
95-
}
96-
ready = append(ready, r)
97-
}
98-
return
99-
}

pkg/kubernetes/client/diff_test.go

-92
This file was deleted.

pkg/kubernetes/client/exec.go

+4
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ func (k Kubectl) ctl(action string, args ...string) *exec.Cmd {
3232
cmd := kubectlCmd(argv...)
3333
cmd.Env = patchKubeconfig(k.nsPatch, os.Environ())
3434

35+
if os.Getenv("TANKA_KUBECTL_TRACE") != "" {
36+
fmt.Println(cmd.String())
37+
}
38+
3539
return cmd
3640
}
3741

pkg/kubernetes/client/resources.go

+128
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package client
2+
3+
import (
4+
"bytes"
5+
"encoding/json"
6+
"fmt"
7+
"os"
8+
"regexp"
9+
"strings"
10+
11+
"github.com/pkg/errors"
12+
13+
"github.com/grafana/tanka/pkg/kubernetes/manifest"
14+
)
15+
16+
// Resources the Kubernetes API knows
17+
type Resources []Resource
18+
19+
// Namespaced returns whether a resource is namespace-specific or cluster-wide
20+
func (r Resources) Namespaced(m manifest.Manifest) bool {
21+
for _, res := range r {
22+
if m.Kind() == res.Kind {
23+
return res.Namespaced
24+
}
25+
}
26+
27+
return false
28+
}
29+
30+
// Resource is a Kubernetes API Resource
31+
type Resource struct {
32+
APIGroup string `json:"APIGROUP"`
33+
Kind string `json:"KIND"`
34+
Name string `json:"NAME"`
35+
Namespaced bool `json:"NAMESPACED,string"`
36+
Shortnames string `json:"SHORTNAMES"`
37+
}
38+
39+
// Resources returns all API resources known to the server
40+
func (k Kubectl) Resources() (Resources, error) {
41+
cmd := k.ctl("api-resources", "--cached")
42+
var out bytes.Buffer
43+
cmd.Stdout = &out
44+
cmd.Stderr = os.Stderr
45+
46+
if err := cmd.Run(); err != nil {
47+
return nil, err
48+
}
49+
50+
var res Resources
51+
if err := UnmarshalTable(out.String(), &res); err != nil {
52+
return nil, errors.Wrap(err, "parsing table")
53+
}
54+
55+
return res, nil
56+
}
57+
58+
// UnmarshalTable unmarshals a raw CLI table into ptr. `json` package is used
59+
// for getting the dat into the ptr, `json:` struct tags can be used.
60+
func UnmarshalTable(raw string, ptr interface{}) error {
61+
raw = strings.TrimSpace(raw)
62+
63+
lines := strings.Split(raw, "\n")
64+
if len(lines) == 0 {
65+
return ErrorNoHeader
66+
}
67+
68+
headerStr := lines[0]
69+
// headers are ALL-CAPS
70+
if !regexp.MustCompile(`^[A-Z\s]+$`).MatchString(headerStr) {
71+
return ErrorNoHeader
72+
}
73+
74+
lines = lines[1:]
75+
76+
spc := regexp.MustCompile(`[A-Z]+\s*`)
77+
header := spc.FindAllString(headerStr, -1)
78+
79+
tbl := make([]map[string]string, 0, len(lines))
80+
for _, l := range lines {
81+
elems := splitRow(l, header)
82+
if len(elems) != len(header) {
83+
return ErrorElementsMismatch{Header: len(header), Row: len(elems)}
84+
}
85+
86+
row := make(map[string]string)
87+
for i, e := range elems {
88+
key := strings.TrimSpace(header[i])
89+
row[key] = strings.TrimSpace(e)
90+
}
91+
tbl = append(tbl, row)
92+
}
93+
94+
j, err := json.Marshal(tbl)
95+
if err != nil {
96+
return err
97+
}
98+
99+
return json.Unmarshal(j, ptr)
100+
}
101+
102+
// ErrorNoHeader occurs when the table lacks an ALL-CAPS header
103+
var ErrorNoHeader = fmt.Errorf("table has no header")
104+
105+
// ErrorElementsMismatch occurs when a row has a different count of elements
106+
// than it's header
107+
type ErrorElementsMismatch struct {
108+
Header, Row int
109+
}
110+
111+
func (e ErrorElementsMismatch) Error() string {
112+
return fmt.Sprintf("header and row have different element count: %v != %v", e.Header, e.Row)
113+
}
114+
115+
func splitRow(s string, header []string) (elems []string) {
116+
pos := 0
117+
for i, h := range header {
118+
if i == len(header)-1 {
119+
elems = append(elems, s[pos:])
120+
continue
121+
}
122+
123+
lim := len(h)
124+
elems = append(elems, s[pos:pos+lim])
125+
pos += lim
126+
}
127+
return elems
128+
}
+76
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package client
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
func TestUnmarshalTable(t *testing.T) {
12+
cases := []struct {
13+
name string
14+
tbl string
15+
dest interface{}
16+
want interface{}
17+
err error
18+
}{
19+
{
20+
name: "normal",
21+
tbl: strings.TrimSpace(tblNormal),
22+
want: &Resources{
23+
{APIGroup: "apps", Name: "Deployment", Namespaced: true},
24+
{APIGroup: "networking", Name: "Ingress", Namespaced: true},
25+
{APIGroup: "", Name: "Namespace", Namespaced: false},
26+
{APIGroup: "extensions", Name: "DaemonSet", Namespaced: true},
27+
},
28+
dest: &Resources{},
29+
},
30+
{
31+
name: "empty",
32+
tbl: strings.TrimSpace(tblEmpty),
33+
want: &Resources{},
34+
dest: &Resources{
35+
{APIGroup: "apps", Name: "Deployment", Namespaced: true},
36+
},
37+
},
38+
{
39+
name: "no-header",
40+
tbl: strings.TrimSpace(tblNoHeader),
41+
err: ErrorNoHeader,
42+
},
43+
{
44+
name: "nothing",
45+
tbl: tblNothing,
46+
err: ErrorNoHeader,
47+
},
48+
}
49+
50+
for _, c := range cases {
51+
t.Run(c.name, func(t *testing.T) {
52+
err := UnmarshalTable(c.tbl, c.dest)
53+
require.Equal(t, c.err, err)
54+
assert.Equal(t, c.want, c.dest)
55+
})
56+
}
57+
}
58+
59+
var tblNormal = `
60+
APIGROUP NAME NAMESPACED
61+
apps Deployment true
62+
networking Ingress true
63+
Namespace false
64+
extensions DaemonSet true
65+
`
66+
67+
var tblEmpty = `
68+
APIGROUP NAME NAMESPACED
69+
`
70+
71+
var tblNoHeader = `
72+
apps Deployment true
73+
networking Ingress true
74+
`
75+
76+
var tblNothing = ``

0 commit comments

Comments
 (0)