Repository: doitintl/kube-no-trouble
Pull request #323 (2022-07-15 09:24:00)

feat: Support for checking additional annotations
Currently, kubent only checks the `kubectl.kubernetes.io/last-applied-configuration` annotation for deprecated APIs, which is set by kubectl. If a different tool like [kapp](https://github.com/vmware-tanzu/carvel-kapp) is used, which might set other annotations, kubent won't detect those.

This PR adds support for checking additional annotations. It can be activated via the `--additional-annotations` flag.

**Example:**
```shell
kubent -t v1.22 --additional-annotations kapp.k14s.io/original
```

The change is backward compatible, as the `kubectl.kubernetes.io/last-applied-configuration` annotation will always be checked first. Only if that annotation is not set the additional annotations are tried.

AndreasSko: @stepanstipl would you be able to have a look at this PR if you got some time? What do you think about this idea? :slightly_smiling_face: 
stepanstipl: Hi @AndreasSko thanks, I was not aware of the kapp, but I like this, it only makes sense, and the PR looks good at first sight. 👍  So that's great 🎉 , thanks a lot for contributing.

I've included it in the current milestone, so will try to get to it and review it a bit more in detail soon - please allow me some time for that - but let's try to get it in with the next release.

AndreasSko: Thank you, @stepanstipl for your good review. I appreciate the feedback!
I adjusted the PR (hopefully I didn't forget anything :smile: :see_no_evil:) and it would be ready for another review :slightly_smiling_face: 

Patch:
diff --git a/README.md b/README.md
index 20515a1..6c43661 100644
--- a/README.md
+++ b/README.md
@@ -76,19 +76,24 @@ You can list all the configuration options available using `--help` switch:
 ```sh
 $./kubent -h
 Usage of ./kubent:
-  -a, --additional-kind strings   additional kinds of resources to report in Kind.version.group.com format
-  -c, --cluster                   enable Cluster collector (default true)
-  -x, --context string            kubeconfig context
-  -e, --exit-error                exit with non-zero code when issues are found
-  -f, --filename strings          manifests to check, use - for stdin
-      --helm2                     enable Helm v2 collector (default true)
-      --helm3                     enable Helm v3 collector (default true)
-  -k, --kubeconfig string         path to the kubeconfig file
-  -l, --log-level string          set log level (trace, debug, info, warn, error, fatal, panic, disabled) (default "info")
-  -o, --output string             output format - [text|json] (default "text")
-  -t, --target-version string     target K8s version in SemVer format (autodetected by default)
-  -v, --version                   prints the version of kubent and exits
+  -A, --additional-annotation strings   additional annotations that should be checked to determine the last applied config
+  -a, --additional-kind strings         additional kinds of resources to report in Kind.version.group.com format
+  -c, --cluster                         enable Cluster collector (default true)
+  -x, --context string                  kubeconfig context
+  -e, --exit-error                      exit with non-zero code when issues are found
+  -f, --filename strings                manifests to check, use - for stdin
+      --helm2                           enable Helm v2 collector (default true)
+      --helm3                           enable Helm v3 collector (default true)
+  -k, --kubeconfig string               path to the kubeconfig file
+  -l, --log-level string                set log level (trace, debug, info, warn, error, fatal, panic, disabled) (default "info")
+  -o, --output string                   output format - [text|json] (default "text")
+  -t, --target-version string           target K8s version in SemVer format (autodetected by default)
+  -v, --version                         prints the version of kubent and exits
+
 ```
+- *`--additional-annotation`*
+  Check additional annotations for the last applied configuration. This can be useful if a resource was applied
+  with a tool other than kubectl. The flag can be used multiple times.
 
 - *`-a, --additional-kind`*
   Tells `kubent` to flag additional custom resources when found in the specified version. The flag can be used multiple
diff --git a/cmd/kubent/main.go b/cmd/kubent/main.go
index fe56eb6..10426d2 100644
--- a/cmd/kubent/main.go
+++ b/cmd/kubent/main.go
@@ -59,7 +59,8 @@ func storeCollector(collector collector.Collector, err error, collectors []colle
 func initCollectors(config *config.Config) []collector.Collector {
 	collectors := []collector.Collector{}
 	if config.Cluster {
-		collector, err := collector.NewClusterCollector(&collector.ClusterOpts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context}, config.AdditionalKinds, generateUserAgent())
+		collector, err := collector.NewClusterCollector(&collector.ClusterOpts{Kubeconfig: config.Kubeconfig, KubeContext: config.Context},
+			config.AdditionalKinds, config.AdditionalAnnotations, generateUserAgent())
 		collectors = storeCollector(collector, err, collectors)
 	}
 
diff --git a/fixtures/fake-deployment-v1beta1-with-kapp-annotation.yaml b/fixtures/fake-deployment-v1beta1-with-kapp-annotation.yaml
new file mode 100644
index 0000000..46f94ab
--- /dev/null
+++ b/fixtures/fake-deployment-v1beta1-with-kapp-annotation.yaml
@@ -0,0 +1,71 @@
+# fake client resource are a bit special
+# apiVersion need to match versions we're collection, i.e. apps/v1
+# true version is taken from kubectl.kubernetes.io/last-applied-configuration
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  annotations:
+    deployment.kubernetes.io/revision: "1"
+    kapp.k14s.io/original: |
+      {"apiVersion":"apps/v1beta1","kind":"Deployment","metadata":{"annotations":{},"labels":{"app":"nginx"},"name":"nginx-deployment-old","namespace":"default"},"spec":{"replicas":3,"selector":{"matchLabels":{"app":"nginx"}},"template":{"metadata":{"labels":{"app":"nginx"}},"spec":{"containers":[{"image":"nginx:1.14.2","name":"nginx","ports":[{"containerPort":80}]}]}}}}
+  creationTimestamp: "2020-09-04T19:51:38Z"
+  generation: 1
+  labels:
+    app: nginx
+  name: nginx-deployment-old
+  namespace: default
+  resourceVersion: "647052"
+  selfLink: /apis/extensions/v1beta1/namespaces/default/deployments/nginx-deployment-old
+  uid: 93674123-01f1-4a10-a829-ff0751cf13da
+spec:
+  progressDeadlineSeconds: 600
+  replicas: 3
+  revisionHistoryLimit: 2
+  selector:
+    matchLabels:
+      app: nginx
+  strategy:
+    rollingUpdate:
+      maxSurge: 25%
+      maxUnavailable: 25%
+    type: RollingUpdate
+  template:
+    metadata:
+      creationTimestamp: null
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - image: nginx:1.14.2
+        imagePullPolicy: IfNotPresent
+        name: nginx
+        ports:
+        - containerPort: 80
+          protocol: TCP
+        resources: {}
+        terminationMessagePath: /dev/termination-log
+        terminationMessagePolicy: File
+      dnsPolicy: ClusterFirst
+      restartPolicy: Always
+      schedulerName: default-scheduler
+      securityContext: {}
+      terminationGracePeriodSeconds: 30
+status:
+  availableReplicas: 3
+  conditions:
+  - lastTransitionTime: "2020-09-04T19:51:48Z"
+    lastUpdateTime: "2020-09-04T19:51:48Z"
+    message: Deployment has minimum availability.
+    reason: MinimumReplicasAvailable
+    status: "True"
+    type: Available
+  - lastTransitionTime: "2020-09-04T19:51:38Z"
+    lastUpdateTime: "2020-09-04T19:51:48Z"
+    message: ReplicaSet "nginx-deployment-old-7fd6966748" has successfully progressed.
+    reason: NewReplicaSetAvailable
+    status: "True"
+    type: Progressing
+  observedGeneration: 1
+  readyReplicas: 3
+  replicas: 3
+  updatedReplicas: 3
diff --git a/pkg/collector/cluster.go b/pkg/collector/cluster.go
index 68624e3..0d7159f 100644
--- a/pkg/collector/cluster.go
+++ b/pkg/collector/cluster.go
@@ -17,8 +17,9 @@ const CLUSTER_COLLECTOR_NAME = "Cluster"
 type ClusterCollector struct {
 	*commonCollector
 	*kubeCollector
-	clientSet           dynamic.Interface
-	additionalResources []schema.GroupVersionResource
+	clientSet             dynamic.Interface
+	additionalResources   []schema.GroupVersionResource
+	additionalAnnotations []string
 }
 
 type ClusterOpts struct {
@@ -28,15 +29,17 @@ type ClusterOpts struct {
 	DiscoveryClient discovery.DiscoveryInterface
 }
 
-func NewClusterCollector(opts *ClusterOpts, additionalKinds []string, userAgent string) (*ClusterCollector, error) {
+func NewClusterCollector(opts *ClusterOpts, additionalKinds []string, additionalAnnotations []string, userAgent string) (
+	*ClusterCollector, error) {
 	kubeCollector, err := newKubeCollector(opts.Kubeconfig, opts.KubeContext, opts.DiscoveryClient, userAgent)
 	if err != nil {
 		return nil, err
 	}
 
 	collector := &ClusterCollector{
-		kubeCollector:   kubeCollector,
-		commonCollector: newCommonCollector(CLUSTER_COLLECTOR_NAME),
+		kubeCollector:         kubeCollector,
+		commonCollector:       newCommonCollector(CLUSTER_COLLECTOR_NAME),
+		additionalAnnotations: additionalAnnotations,
 	}
 
 	if opts.ClientSet == nil {
@@ -116,7 +119,7 @@ func (c *ClusterCollector) Get() ([]map[string]interface{}, error) {
 		}
 
 		for _, r := range rs.Items {
-			if jsonManifest, ok := r.GetAnnotations()["kubectl.kubernetes.io/last-applied-configuration"]; ok {
+			if jsonManifest, ok := c.getLastAppliedConfig(r.GetAnnotations()); ok {
 				var manifest map[string]interface{}
 
 				err := json.Unmarshal([]byte(jsonManifest), &manifest)
@@ -131,3 +134,14 @@ func (c *ClusterCollector) Get() ([]map[string]interface{}, error) {
 
 	return results, nil
 }
+
+func (c *ClusterCollector) getLastAppliedConfig(resourceAnnotations map[string]string) (string, bool) {
+	annotations := append([]string{"kubectl.kubernetes.io/last-applied-configuration"}, c.additionalAnnotations...)
+	for _, annotation := range annotations {
+		if jsonManifest, ok := resourceAnnotations[annotation]; ok {
+			return jsonManifest, ok
+		}
+	}
+
+	return "", false
+}
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 3ba7418..c4ae2ed 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -13,18 +13,19 @@ import (
 )
 
 type Config struct {
-	AdditionalKinds []string
-	Cluster         bool
-	Context         string
-	ExitError       bool
-	Filenames       []string
-	Helm2           bool
-	Helm3           bool
-	Kubeconfig      string
-	LogLevel        ZeroLogLevel
-	Output          string
-	TargetVersion   *judge.Version
-	KubentVersion   bool
+	AdditionalKinds       []string
+	AdditionalAnnotations []string
+	Cluster               bool
+	Context               string
+	ExitError             bool
+	Filenames             []string
+	Helm2                 bool
+	Helm3                 bool
+	Kubeconfig            string
+	LogLevel              ZeroLogLevel
+	Output                string
+	TargetVersion         *judge.Version
+	KubentVersion         bool
 }
 
 func NewFromFlags() (*Config, error) {
@@ -34,6 +35,7 @@ func NewFromFlags() (*Config, error) {
 	}
 
 	flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format")
+	flag.StringSliceVarP(&config.AdditionalAnnotations, "additional-annotation", "A", []string{}, "additional annotations that should be checked to determine the last applied config")
 	flag.BoolVarP(&config.Cluster, "cluster", "c", true, "enable Cluster collector")
 	flag.StringVarP(&config.Context, "context", "x", "", "kubeconfig context")
 	flag.BoolVarP(&config.ExitError, "exit-error", "e", false, "exit with non-zero code when issues are found")


Test patch:
diff --git a/pkg/collector/cluster_test.go b/pkg/collector/cluster_test.go
index cbac045..c0355cd 100644
--- a/pkg/collector/cluster_test.go
+++ b/pkg/collector/cluster_test.go
@@ -16,7 +16,7 @@ import (
 
 func TestNewClusterCollectorBadPath(t *testing.T) {
 	testOpts := ClusterOpts{Kubeconfig: "bad path"}
-	_, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
+	_, err := NewClusterCollector(&testOpts, []string{}, []string{}, USER_AGENT)
 
 	if !strings.Contains(err.Error(), "no configuration has been provided") {
 		if err != nil {
@@ -36,7 +36,7 @@ func TestNewClusterCollectorValidEmptyCollector(t *testing.T) {
 		ClientSet:       clientset,
 		DiscoveryClient: &discoveryClient,
 	}
-	collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
+	collector, err := NewClusterCollector(&testOpts, []string{}, []string{}, USER_AGENT)
 
 	if err != nil {
 		t.Fatalf("Should have parsed config instead got: %s", err)
@@ -55,7 +55,7 @@ func TestNewClusterCollectorFakeClient(t *testing.T) {
 	discoveryClient := discoveryFake.FakeDiscovery{}
 	testOpts := ClusterOpts{ClientSet: clientset, DiscoveryClient: &discoveryClient}
 
-	collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
+	collector, err := NewClusterCollector(&testOpts, []string{}, []string{}, USER_AGENT)
 	if err != nil {
 		t.Fatalf("failed to create cluster collector from fake client: %s", err)
 	}
@@ -69,15 +69,42 @@ func TestNewClusterCollectorFakeClient(t *testing.T) {
 
 func TestClusterCollectorGetFake(t *testing.T) {
 	testCases := []struct {
-		name     string
-		input    []string // file list
-		expected int      // number of manifests
+		name                  string
+		input                 []string // file list
+		additionalAnnotations []string
+		expected              int // number of manifests
 	}{
-		{"empty", []string{}, 0},
-		{"withoutAnnotation", []string{"fake-deployment-v1beta1-no-annotation.yaml"}, 0},
-		{"one", []string{"fake-deployment-v1beta1-with-annotation.yaml"}, 1},
-		{"multiple", []string{"fake-deployment-v1beta1-with-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"}, 2},
-		{"mixed", []string{"fake-deployment-v1beta1-no-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"}, 1},
+		{
+			name:     "empty",
+			input:    []string{},
+			expected: 0,
+		},
+		{
+			name:     "withoutAnnotation",
+			input:    []string{"fake-deployment-v1beta1-no-annotation.yaml"},
+			expected: 0,
+		},
+		{
+			name:     "one",
+			input:    []string{"fake-deployment-v1beta1-with-annotation.yaml"},
+			expected: 1,
+		},
+		{
+			name:     "multiple",
+			input:    []string{"fake-deployment-v1beta1-with-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"},
+			expected: 2,
+		},
+		{
+			name:     "mixed",
+			input:    []string{"fake-deployment-v1beta1-no-annotation.yaml", "fake-ingress-v1beta1-with-annotation.yaml"},
+			expected: 1,
+		},
+		{
+			name:                  "kappAnnotation",
+			input:                 []string{"fake-deployment-v1beta1-with-kapp-annotation.yaml"},
+			additionalAnnotations: []string{"kapp.k14s.io/original"},
+			expected:              1,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -104,7 +131,7 @@ func TestClusterCollectorGetFake(t *testing.T) {
 			discoveryClient := discoveryFake.FakeDiscovery{}
 			testOpts := ClusterOpts{ClientSet: clientset, DiscoveryClient: &discoveryClient}
 
-			collector, err := NewClusterCollector(&testOpts, []string{}, USER_AGENT)
+			collector, err := NewClusterCollector(&testOpts, []string{}, tc.additionalAnnotations, USER_AGENT)
 
 			if err != nil {
 				t.Errorf("failed to create collector from fake client: %s", err)
@@ -121,3 +148,70 @@ func TestClusterCollectorGetFake(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterCollector_getLastAppliedConfig(t *testing.T) {
+	tests := []struct {
+		name                string
+		c                   *ClusterCollector
+		resourceAnnotations map[string]string
+		wantManifest        string
+		wantOk              bool
+	}{
+		{
+			name: "Default annotation",
+			c:    &ClusterCollector{},
+			resourceAnnotations: map[string]string{
+				"kubectl.kubernetes.io/last-applied-configuration": "Some config",
+				"another-annotation": "Bla",
+			},
+			wantManifest: "Some config",
+			wantOk:       true,
+		},
+		{
+			name: "Try kubectl annotation first",
+			c: &ClusterCollector{
+				additionalAnnotations: []string{"kapp.k14s.io/original"},
+			},
+			resourceAnnotations: map[string]string{
+				"kubectl.kubernetes.io/last-applied-configuration": "Some config",
+				"kapp.k14s.io/original":                            "Kapp config",
+				"another-annotation":                               "Bla",
+			},
+			wantManifest: "Some config",
+			wantOk:       true,
+		},
+		{
+			name: "Use additional annotation",
+			c: &ClusterCollector{
+				additionalAnnotations: []string{"kapp.k14s.io/original"},
+			},
+			resourceAnnotations: map[string]string{
+				"kapp.k14s.io/original": "Kapp config",
+				"another-annotation":    "Bla",
+			},
+			wantManifest: "Kapp config",
+			wantOk:       true,
+		},
+		{
+			name: "No annotation found",
+			c: &ClusterCollector{
+				additionalAnnotations: []string{},
+			},
+			resourceAnnotations: map[string]string{
+				"kapp.k14s.io/original": "Kapp config",
+				"another-annotation":    "Bla",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			manifest, ok := tt.c.getLastAppliedConfig(tt.resourceAnnotations)
+			if manifest != tt.wantManifest {
+				t.Errorf("ClusterCollector.getLastAppliedConfig() got = %v, want %v", manifest, tt.wantManifest)
+			}
+			if ok != tt.wantOk {
+				t.Errorf("ClusterCollector.getLastAppliedConfig() got1 = %v, want %v", ok, tt.wantOk)
+			}
+		})
+	}
+}