Skip to content

Commit

Permalink
chore(context): Remove unneeded global context
Browse files Browse the repository at this point in the history
Signed-off-by: dark0dave <[email protected]>
  • Loading branch information
dark0dave committed Nov 9, 2024
1 parent 358c6cb commit 9106626
Show file tree
Hide file tree
Showing 16 changed files with 174 additions and 260 deletions.
2 changes: 2 additions & 0 deletions .mise.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tools]
go = "1.23.0"
17 changes: 10 additions & 7 deletions cmd/kubent/main.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package main

import (
"context"
"flag"
"fmt"
"io"
Expand Down Expand Up @@ -97,12 +96,11 @@ func getServerVersion(cv *judge.Version, collectors []collector.Collector) (*jud
}

func main() {
ctx := context.Background()
exitCode := EXIT_CODE_FAIL_GENERIC

configureGlobalLogging()

config, ctx, err := config.NewFromFlags(ctx)
config, err := config.NewFromFlags()

if err != nil {
log.Fatal().Err(err).Msg("failed to parse config flags")
Expand Down Expand Up @@ -159,7 +157,12 @@ func main() {
log.Fatal().Err(err).Str("name", "Rego").Msg("Failed to filter results")
}

err = outputResults(results, config.Output, config.OutputFile, ctx)
options, err := printer.NewPrinterOptions(config.OutputFile, config.ShowLabels)
if err != nil {
log.Fatal().Err(err).Msgf("Failed to create output file")
}

err = outputResults(results, config.Output, options)
if err != nil {
log.Fatal().Err(err).Msgf("Failed to output results")
}
Expand All @@ -183,14 +186,14 @@ func configureGlobalLogging() {
log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})
}

func outputResults(results []judge.Result, outputType string, outputFile string, ctx context.Context) error {
printer, err := printer.NewPrinter(outputType, outputFile)
func outputResults(results []judge.Result, outputType string, options *printer.PrinterOptions) error {
printer, err := printer.NewPrinter(outputType, options)
if err != nil {
return fmt.Errorf("failed to create printer: %v", err)
}
defer printer.Close()

err = printer.Print(results, ctx)
err = printer.Print(results)
if err != nil {
return fmt.Errorf("failed to print results: %v", err)
}
Expand Down
15 changes: 7 additions & 8 deletions cmd/kubent/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
Expand All @@ -14,8 +13,8 @@ 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/doitintl/kube-no-trouble/pkg/printer"

"github.com/rs/zerolog"
)
Expand Down Expand Up @@ -288,15 +287,15 @@ func Test_outputResults(t *testing.T) {
}{
{"good", args{testResults, "text", "-"}, false},
{"bad-new-printer-type", args{testResults, "unknown", "-"}, true},
{"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, ctx); (err != nil) != tt.wantErr {
options, err := printer.NewPrinterOptions(tt.args.outputFile, false)
if err != nil {
t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
}
if err = outputResults(tt.args.results, tt.args.outputType, options); (err != nil) != tt.wantErr {
t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
}
})
Expand All @@ -305,6 +304,6 @@ func Test_outputResults(t *testing.T) {

func Test_configureGlobalLogging(t *testing.T) {
// just make sure the method runs, this is mostly covered
//by the Test_MainExitCodes
// by the Test_MainExitCodes
configureGlobalLogging()
}
24 changes: 9 additions & 15 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"context"
"errors"
"fmt"
"io/fs"
Expand All @@ -10,7 +9,6 @@ import (
"strings"
"unicode"

ctxKey "github.com/doitintl/kube-no-trouble/pkg/context"
"github.com/doitintl/kube-no-trouble/pkg/judge"
"k8s.io/client-go/tools/clientcmd"

Expand Down Expand Up @@ -38,16 +36,15 @@ type Config struct {
OutputFile string
TargetVersion *judge.Version
KubentVersion bool
ShowLabels bool
}

func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
func NewFromFlags() (*Config, 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")
Expand All @@ -57,26 +54,23 @@ func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
flag.BoolVar(&config.Helm3, "helm3", true, "enable Helm v3 collector")
flag.StringSliceVarP(&config.Filenames, "filename", "f", []string{}, "manifests to check, use - for stdin")
flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", os.Getenv(clientcmd.RecommendedConfigPathEnvVar), "path to the kubeconfig file")
flag.StringVarP(&config.Output, "output", "o", "text", "output format - [text|json|csv]")
flag.StringVarP(&config.Output, "output", "o", TEXT, "output format - [text|json|csv]")
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.BoolVar(&config.ShowLabels, "labels", false, "print resource labels")
flag.Parse()

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)
return nil, fmt.Errorf("failed to validate argument output: %s", config.Output)
}

if err := validateOutputFile(config.OutputFile); err != nil {
return nil, nil, fmt.Errorf("failed to validate argument output-file: %w", err)
return nil, fmt.Errorf("failed to validate argument output-file: %w", err)
}

if err := validateAdditionalResources(config.AdditionalKinds); err != nil {
return nil, nil, fmt.Errorf("failed to validate arguments: %w", err)
return nil, fmt.Errorf("failed to validate arguments: %w", err)
}

// This is a little ugly, but I think preferred to implementing
Expand All @@ -86,10 +80,10 @@ func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
config.TargetVersion = nil
}

return &config, newContext, nil
return &config, nil
}

// Previuosly this was handled by a printer.go ParsePrinter function
// Previously 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 {
Expand Down
17 changes: 6 additions & 11 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package config

import (
"context"
"os"
"testing"

Expand All @@ -13,7 +12,6 @@ import (
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 {
Expand All @@ -22,7 +20,7 @@ func TestValidLogLevelFromFlags(t *testing.T) {

os.Args[1] = "--log-level=" + level

config, _, err := NewFromFlags(ctx)
config, err := NewFromFlags()

if err != nil {
t.Errorf("Flags parsing failed %s", err)
Expand All @@ -48,9 +46,8 @@ 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(ctx)
config, err := NewFromFlags()

if err != nil {
t.Errorf("Flags parsing failed %s", err)
Expand Down Expand Up @@ -95,7 +92,6 @@ 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 }()
Expand All @@ -105,7 +101,7 @@ func TestTargetVersion(t *testing.T) {
pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)

os.Args[1] = "--target-version=" + v
config, _, err := NewFromFlags(ctx)
config, err := NewFromFlags()

if err != nil {
t.Errorf("Flags parsing failed %s", err)
Expand All @@ -126,7 +122,7 @@ func TestTargetVersionInvalid(t *testing.T) {
invalidVersions := []string{
"1.blah", "nope",
}
ctx := context.Background()

oldArgs := os.Args[1]
defer func() { os.Args[1] = oldArgs }()

Expand All @@ -135,7 +131,7 @@ func TestTargetVersionInvalid(t *testing.T) {
pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ContinueOnError)

os.Args[1] = "--target-version=" + v
config, _, _ := NewFromFlags(ctx)
config, _ := NewFromFlags()

if config.TargetVersion != nil {
t.Errorf("expected --target-version flag parsing to fail for: %s", v)
Expand All @@ -147,7 +143,6 @@ func TestContext(t *testing.T) {
validContexts := []string{
"my-context",
}
ctx := context.Background()
oldArgs := os.Args[1]
defer func() { os.Args[1] = oldArgs }()

Expand All @@ -156,7 +151,7 @@ func TestContext(t *testing.T) {
pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)

os.Args[1] = "--context=" + context
config, _, err := NewFromFlags(ctx)
config, err := NewFromFlags()

if err != nil {
t.Errorf("Flags parsing failed %s", err)
Expand Down
5 changes: 0 additions & 5 deletions pkg/context/context-keys.go

This file was deleted.

18 changes: 6 additions & 12 deletions pkg/printer/csv.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package printer

import (
"context"
"encoding/csv"
"fmt"
"sort"
Expand All @@ -14,8 +13,8 @@ type csvPrinter struct {
}

// newCSVPrinter creates new CSV printer that prints to given output file
func newCSVPrinter(outputFileName string) (Printer, error) {
cp, err := newCommonPrinter(outputFileName)
func newCSVPrinter(options *PrinterOptions) (Printer, error) {
cp, err := newCommonPrinter(options)
if err != nil {
return nil, fmt.Errorf("failed to create new common printer: %w", err)
}
Expand All @@ -30,7 +29,7 @@ func (c *csvPrinter) Close() error {
}

// Print will print results in CSV format
func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error {
func (c *csvPrinter) Print(results []judge.Result) error {

sort.Slice(results, func(i, j int) bool {
return results[i].Name < results[j].Name
Expand All @@ -45,7 +44,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error {
return results[i].RuleSet < results[j].RuleSet
})

w := csv.NewWriter(c.commonPrinter.outputFile)
w := csv.NewWriter(c.commonPrinter.options.outputFile)

fields := []string{
"api_version",
Expand All @@ -57,12 +56,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error {
"rule_set",
}

labels, err := shouldShowLabels(ctx)
if err != nil {
return fmt.Errorf("failed to get labels: %w", err)
}

if labels != nil && *labels {
if c.options.showLabels {
fields = append(fields, "labels")
}

Expand All @@ -79,7 +73,7 @@ func (c *csvPrinter) Print(results []judge.Result, ctx context.Context) error {
r.RuleSet,
}

if labels != nil && *labels {
if c.options.showLabels {
row = append(row, mapToCommaSeparatedString(r.Labels))
}

Expand Down
Loading

0 comments on commit 9106626

Please sign in to comment.