Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(context): Removing unneeded global context #649

Merged
merged 1 commit into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading