feat: Adding labels flag as optional flag

Author: Felipe Amaral

README.md

@@ -223,7 +223,7 @@ Otherwise there's `Makefile`
 ```sh
 $ make
 make
-all                            Cean, build and pack
+all                            Clean, build and pack
 help                           Prints list of tasks
 build                          Build binary
 generate                       Go generate

cmd/kubent/main.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"io"
@@ -96,11 +97,13 @@ func getServerVersion(cv *judge.Version, collectors []collector.Collector) (*jud
 }
 
 func main() {
+	ctx := context.Background()
 	exitCode := EXIT_CODE_FAIL_GENERIC
 
 	configureGlobalLogging()
 
-	config, err := config.NewFromFlags()
+	config, ctx, err := config.NewFromFlags(ctx)
+
 	if err != nil {
 		log.Fatal().Err(err).Msg("failed to parse config flags")
 	}
@@ -156,7 +159,7 @@ func main() {
 		log.Fatal().Err(err).Str("name", "Rego").Msg("Failed to filter results")
 	}
 
-	err = outputResults(results, config.Output, config.OutputFile)
+	err = outputResults(results, config.Output, config.OutputFile, ctx)
 	if err != nil {
 		log.Fatal().Err(err).Msgf("Failed to output results")
 	}
@@ -180,14 +183,14 @@ func configureGlobalLogging() {
 	log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})
 }
 
-func outputResults(results []judge.Result, outputType string, outputFile string) error {
+func outputResults(results []judge.Result, outputType string, outputFile string, ctx context.Context) error {
 	printer, err := printer.NewPrinter(outputType, outputFile)
 	if err != nil {
 		return fmt.Errorf("failed to create printer: %v", err)
 	}
 	defer printer.Close()
 
-	err = printer.Print(results)
+	err = printer.Print(results, ctx)
 	if err != nil {
 		return fmt.Errorf("failed to print results: %v", err)
 	}

cmd/kubent/main_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -13,6 +14,7 @@ import (
 
 	"github.com/doitintl/kube-no-trouble/pkg/collector"
 	"github.com/doitintl/kube-no-trouble/pkg/config"
+	ctxKey "github.com/doitintl/kube-no-trouble/pkg/context"
 	"github.com/doitintl/kube-no-trouble/pkg/judge"
 
 	"github.com/rs/zerolog"
@@ -108,6 +110,7 @@ func TestMainExitCodes(t *testing.T) {
 	defer os.RemoveAll(tmpDir)
 
 	expectedJsonOutput, _ := os.ReadFile(filepath.Join(FIXTURES_DIR, "expected-json-output.json"))
+	expectedJsonOutputLabels, _ := os.ReadFile(filepath.Join(FIXTURES_DIR, "expected-json-output-labels.json"))
 	helm3FlagDisabled := "--helm3=false"
 	clusterFlagDisabled := "--cluster=false"
 	testCases := []struct {
@@ -121,6 +124,7 @@ func TestMainExitCodes(t *testing.T) {
 		{"success", []string{clusterFlagDisabled, helm3FlagDisabled}, 0, "", "", false},
 		{"errorBadFlag", []string{"-c=not-boolean"}, 2, "", "", false},
 		{"successFound", []string{"-o=json", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, string(expectedJsonOutput), "", false},
+		{"successFoundWithLabels", []string{"--labels=true", "-o=json", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1-labels.yaml")}, 0, string(expectedJsonOutputLabels), "", false},
 		{"exitErrorFlagNone", []string{clusterFlagDisabled, helm3FlagDisabled, "-e"}, 0, "", "", false},
 		{"exitErrorFlagFound", []string{clusterFlagDisabled, helm3FlagDisabled, "-e", "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 200, "", "", false},
 		{"version short flag set", []string{"-v"}, 0, "", "", false},
@@ -131,6 +135,7 @@ func TestMainExitCodes(t *testing.T) {
 		{"json-file", []string{"-o=json", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, "", filepath.Join(tmpDir, "json-file.out"), false},
 		{"text-file", []string{"-o=text", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, "", filepath.Join(tmpDir, "text-file.out"), false},
 		{"json-stdout", []string{"-o=json", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1.yaml")}, 0, string(expectedJsonOutput), "-", false},
+		{"json-stdout-with-labels", []string{"--labels=true", "-o=json", clusterFlagDisabled, helm3FlagDisabled, "-f=" + filepath.Join(FIXTURES_DIR, "deployment-v1beta1-labels.yaml")}, 0, string(expectedJsonOutputLabels), "-", false},
 		{"error-bad-file", []string{clusterFlagDisabled, helm3FlagDisabled}, 1, "", "/this/dir/is/unlikely/to/exist", false},
 		{"no-3rdparty-output", []string{clusterFlagDisabled, helm3FlagDisabled, "-l=disabled"}, 0, "", "", true},
 	}
@@ -286,9 +291,12 @@ func Test_outputResults(t *testing.T) {
 		{"bad-new-printer-file", args{testResults, "text", "/unlikely/to/exist/dir"}, true},
 	}
 
+	labelsFlag := false
+	ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag)
+
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile); (err != nil) != tt.wantErr {
+			if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile, ctx); (err != nil) != tt.wantErr {
 				t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
 			}
 		})

fixtures/deployment-v1beta1-labels.yaml

@@ -0,0 +1,21 @@
+apiVersion: apps/v1beta1
+kind: Deployment
+metadata:
+  name: nginx-deployment-old
+  labels:
+    app: nginx
+spec:
+  replicas: 3
+  selector:
+    matchLabels:
+      app: nginx
+  template:
+    metadata:
+      labels:
+        app: nginx
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.14.2
+        ports:
+        - containerPort: 80

fixtures/deployment-v1beta1.yaml

@@ -2,8 +2,6 @@ apiVersion: apps/v1beta1
 kind: Deployment
 metadata:
   name: nginx-deployment-old
-  labels:
-    app: nginx
 spec:
   replicas: 3
   selector:

fixtures/expected-json-output-labels.json

@@ -0,0 +1,14 @@
+[
+	{
+		"Name": "nginx-deployment-old",
+		"Namespace": "\u003cundefined\u003e",
+		"Kind": "Deployment",
+		"ApiVersion": "apps/v1beta1",
+		"RuleSet": "Deprecated APIs removed in 1.16",
+		"ReplaceWith": "apps/v1",
+		"Since": "1.9.0",
+		"Labels": {
+			"app": "nginx"
+		}
+	}
+]

fixtures/expected-json-output.json

@@ -6,6 +6,7 @@
 		"ApiVersion": "apps/v1beta1",
 		"RuleSet": "Deprecated APIs removed in 1.16",
 		"ReplaceWith": "apps/v1",
-		"Since": "1.9.0"
+		"Since": "1.9.0",
+		"Labels": {}
 	}
 ]

pkg/collector/file_test.go

@@ -52,7 +52,7 @@ func TestFileCollectorGet(t *testing.T) {
 				t.Errorf("Expected to get %d, got %d", len(tc.expected), len(manifests))
 			}
 
-			for i, _ := range manifests {
+			for i := range manifests {
 				if manifests[i]["kind"] != tc.expected[i] {
 					t.Errorf("Expected to get %s, instead got: %s", tc.expected[i], manifests[i]["kind"])
 				}

pkg/config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -9,14 +10,20 @@ import (
 	"strings"
 	"unicode"
 
+	ctxKey "github.com/doitintl/kube-no-trouble/pkg/context"
 	"github.com/doitintl/kube-no-trouble/pkg/judge"
-	"github.com/doitintl/kube-no-trouble/pkg/printer"
 	"k8s.io/client-go/tools/clientcmd"
 
 	"github.com/rs/zerolog"
 	flag "github.com/spf13/pflag"
 )
 
+const (
+	JSON = "json"
+	TEXT = "text"
+	CSV  = "csv"
+)
+
 type Config struct {
 	AdditionalKinds       []string
 	AdditionalAnnotations []string
@@ -33,12 +40,14 @@ type Config struct {
 	KubentVersion         bool
 }
 
-func NewFromFlags() (*Config, error) {
+func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
 	config := Config{
 		LogLevel:      ZeroLogLevel(zerolog.InfoLevel),
 		TargetVersion: &judge.Version{},
 	}
 
+	var labels bool
+
 	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")
@@ -52,19 +61,22 @@ func NewFromFlags() (*Config, error) {
 	flag.StringVarP(&config.OutputFile, "output-file", "O", "-", "output file, use - for stdout")
 	flag.VarP(&config.LogLevel, "log-level", "l", "set log level (trace, debug, info, warn, error, fatal, panic, disabled)")
 	flag.VarP(config.TargetVersion, "target-version", "t", "target K8s version in SemVer format (autodetected by default)")
+	flag.BoolVar(&labels, "labels", false, "print resource labels")
 
 	flag.Parse()
 
-	if _, err := printer.ParsePrinter(config.Output); err != nil {
-		return nil, fmt.Errorf("failed to validate argument output: %w", err)
+	newContext := context.WithValue(ctx, ctxKey.LABELS_CTX_KEY, &labels)
+
+	if !isValidOutputFormat(config.Output) {
+		return nil, nil, fmt.Errorf("failed to validate argument output: %s", config.Output)
 	}
 
 	if err := validateOutputFile(config.OutputFile); err != nil {
-		return nil, fmt.Errorf("failed to validate argument output-file: %w", err)
+		return nil, nil, fmt.Errorf("failed to validate argument output-file: %w", err)
 	}
 
 	if err := validateAdditionalResources(config.AdditionalKinds); err != nil {
-		return nil, fmt.Errorf("failed to validate arguments: %w", err)
+		return nil, nil, fmt.Errorf("failed to validate arguments: %w", err)
 	}
 
 	// This is a little ugly, but I think preferred to implementing
@@ -74,7 +86,18 @@ func NewFromFlags() (*Config, error) {
 		config.TargetVersion = nil
 	}
 
-	return &config, nil
+	return &config, newContext, nil
+}
+
+// Previuosly this was handled by a printer.go ParsePrinter function
+// but we need to avoid cycle imports in order to inject the additional flags
+func isValidOutputFormat(format string) bool {
+	switch format {
+	case JSON, TEXT, CSV:
+		return true
+	default:
+		return false
+	}
 }
 
 // validateAdditionalResources check that all resources are provided in full form

pkg/config/config_test.go

@@ -1,24 +1,28 @@
 package config
 
 import (
-	goversion "github.com/hashicorp/go-version"
+	"context"
 	"os"
 	"testing"
 
+	goversion "github.com/hashicorp/go-version"
+
 	"github.com/spf13/pflag"
 )
 
 func TestValidLogLevelFromFlags(t *testing.T) {
 	oldArgs := os.Args[1]
 	defer func() { os.Args[1] = oldArgs }()
+	ctx := context.Background()
 
 	var validLevels = []string{"trace", "debug", "info", "warn", "error", "fatal", "panic", "", "disabled"}
 	for i, level := range validLevels {
 		// reset for testing
 		pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
 
 		os.Args[1] = "--log-level=" + level
-		config, err := NewFromFlags()
+
+		config, _, err := NewFromFlags(ctx)
 
 		if err != nil {
 			t.Errorf("Flags parsing failed %s", err)
@@ -44,8 +48,9 @@ func TestInvalidLogLevelFromFlags(t *testing.T) {
 func TestNewFromFlags(t *testing.T) {
 	// reset for testing
 	pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
+	ctx := context.Background()
 
-	config, err := NewFromFlags()
+	config, _, err := NewFromFlags(ctx)
 
 	if err != nil {
 		t.Errorf("Flags parsing failed %s", err)
@@ -72,9 +77,9 @@ func TestValidateAdditionalResources(t *testing.T) {
 
 func TestValidateAdditionalResourcesFail(t *testing.T) {
 	testCases := [][]string{
-		[]string{"abcdef"},
-		[]string{""},
-		[]string{"test.v1.com"},
+		{"abcdef"},
+		{""},
+		{"test.v1.com"},
 	}
 
 	for _, tc := range testCases {
@@ -90,6 +95,7 @@ func TestTargetVersion(t *testing.T) {
 	validVersions := []string{
 		"1.16", "1.16.3", "1.2.3",
 	}
+	ctx := context.Background()
 
 	oldArgs := os.Args[1]
 	defer func() { os.Args[1] = oldArgs }()
@@ -99,7 +105,7 @@ func TestTargetVersion(t *testing.T) {
 		pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
 
 		os.Args[1] = "--target-version=" + v
-		config, err := NewFromFlags()
+		config, _, err := NewFromFlags(ctx)
 
 		if err != nil {
 			t.Errorf("Flags parsing failed %s", err)
@@ -120,7 +126,7 @@ func TestTargetVersionInvalid(t *testing.T) {
 	invalidVersions := []string{
 		"1.blah", "nope",
 	}
-
+	ctx := context.Background()
 	oldArgs := os.Args[1]
 	defer func() { os.Args[1] = oldArgs }()
 
@@ -129,7 +135,7 @@ func TestTargetVersionInvalid(t *testing.T) {
 		pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ContinueOnError)
 
 		os.Args[1] = "--target-version=" + v
-		config, _ := NewFromFlags()
+		config, _, _ := NewFromFlags(ctx)
 
 		if config.TargetVersion != nil {
 			t.Errorf("expected --target-version flag parsing to fail for: %s", v)
@@ -141,7 +147,7 @@ func TestContext(t *testing.T) {
 	validContexts := []string{
 		"my-context",
 	}
-
+	ctx := context.Background()
 	oldArgs := os.Args[1]
 	defer func() { os.Args[1] = oldArgs }()
 
@@ -150,7 +156,7 @@ func TestContext(t *testing.T) {
 		pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
 
 		os.Args[1] = "--context=" + context
-		config, err := NewFromFlags()
+		config, _, err := NewFromFlags(ctx)
 
 		if err != nil {
 			t.Errorf("Flags parsing failed %s", err)