Skip to content

Commit

Permalink
fix(k8s): consistent types in manifest.Manifest (#338)
Browse files Browse the repository at this point in the history
The manifest.Manifest is a `map[string]interface{}`, which sticks to
what `json.Unmarshal` returns, meaning it only includes concrete values,
`[]interface{}` and `map[string]interface{}`.

In a v0.10
change (https://github.com/grafana/tanka/blob/a6d5fde5aac69dc4d66bb57d5a501e52ba60fada/pkg/kubernetes/manifest/manifest.go#L248)
I accidentally broke that, without realizing it.

Up to #312, this didn't create any issues. Now however, because we
called `Annotations` on all objects, types weren't like they should have
been.

This caused subsetdiff to miss annotations, not computing the subset on
them at all.

Fixes #318
  • Loading branch information
sh0rez committed Aug 15, 2020
1 parent dfa544a commit 6548855
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 51 deletions.
31 changes: 8 additions & 23 deletions pkg/kubernetes/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,37 +219,22 @@ func (m Metadata) UID() string {
}

// Labels of the manifest
func (m Metadata) Labels() map[string]string {
return safeStringMap(m, "labels")
func (m Metadata) Labels() map[string]interface{} {
return safeMSI(m, "labels")
}

// Annotations of the manifest
func (m Metadata) Annotations() map[string]string {
return safeStringMap(m, "annotations")
func (m Metadata) Annotations() map[string]interface{} {
return safeMSI(m, "annotations")
}

// safeStringMap safely returns a string map:
// - returns if map[string]string
// - converts if map[string]interface{}
// - zeroes if anything else
func safeStringMap(m map[string]interface{}, key string) map[string]string {
func safeMSI(m map[string]interface{}, key string) map[string]interface{} {
switch t := m[key].(type) {
case map[string]string:
return t
case map[string]interface{}:
mss := make(map[string]string)
for k, v := range t {
s, ok := v.(string)
if !ok {
continue
}
mss[k] = s
}
m[key] = mss
return m[key].(map[string]string)
return t
default:
m[key] = make(map[string]string)
return m[key].(map[string]string)
m[key] = make(map[string]interface{})
return m[key].(map[string]interface{})
}
}

Expand Down
50 changes: 22 additions & 28 deletions pkg/kubernetes/subsetdiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"strings"

"github.com/pkg/errors"
yaml "gopkg.in/yaml.v3"

"github.com/grafana/tanka/pkg/kubernetes/client"
"github.com/grafana/tanka/pkg/kubernetes/manifest"
Expand Down Expand Up @@ -93,60 +92,55 @@ func subsetDiff(c client.Client, m manifest.Manifest) (*difference, error) {
return nil, errors.Wrap(err, "getting state from cluster")
}

should, err := yaml.Marshal(m)
if err != nil {
return nil, err
}
should := m.String()

is, err := yaml.Marshal(subset(m, rawIs))
if err != nil {
return nil, err
}
if string(is) == "{}\n" {
is = []byte("")
sub := subset(m, rawIs)
is := manifest.Manifest(sub).String()
if is == "{}\n" {
is = ""
}

return &difference{
name: name,
live: string(is),
merged: string(should),
live: is,
merged: should,
}, nil
}

// subset removes all keys from is, that are not present in should.
// It makes is a subset of should.
// subset removes all keys from big, that are not present in small.
// It makes big a subset of small.
// Kubernetes returns more keys than we can know about.
// This means, we need to remove all keys from the kubectl output, that are not present locally.
func subset(should, is map[string]interface{}) map[string]interface{} {
if should["namespace"] != nil {
is["namespace"] = should["namespace"]
func subset(small, big map[string]interface{}) map[string]interface{} {
if small["namespace"] != nil {
big["namespace"] = small["namespace"]
}

// just ignore the apiVersion for now, too much bloat
if should["apiVersion"] != nil && is["apiVersion"] != nil {
is["apiVersion"] = should["apiVersion"]
if small["apiVersion"] != nil && big["apiVersion"] != nil {
big["apiVersion"] = small["apiVersion"]
}

for k, v := range is {
if should[k] == nil {
delete(is, k)
for k, v := range big {
if _, ok := small[k]; !ok {
delete(big, k)
continue
}

switch b := v.(type) {
case map[string]interface{}:
if a, ok := should[k].(map[string]interface{}); ok {
is[k] = subset(a, b)
if a, ok := small[k].(map[string]interface{}); ok {
big[k] = subset(a, b)
}
case []map[string]interface{}:
for i := range b {
if a, ok := should[k].([]map[string]interface{}); ok {
if a, ok := small[k].([]map[string]interface{}); ok {
b[i] = subset(a[i], b[i])
}
}
case []interface{}:
for i := range b {
if a, ok := should[k].([]interface{}); ok {
if a, ok := small[k].([]interface{}); ok {
if i >= len(a) {
// slice in config shorter than in live. Abort, as there are no entries to diff anymore
break
Expand All @@ -168,5 +162,5 @@ func subset(should, is map[string]interface{}) map[string]interface{} {
}
}
}
return is
return big
}

0 comments on commit 6548855

Please sign in to comment.