Skip to content

Commit 1afe0af

Browse files
dark0davedark0dave
authored
Merge pull request #649 from doitintl/fix/removeContext
fix(context): Removing unneeded global context
2 parents 358c6cb + 9106626 commit 1afe0af

16 files changed

+174
-260
lines changed

‎.mise.toml

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[tools]
2+
go = "1.23.0"

‎cmd/kubent/main.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package main
22

33
import (
4-
"context"
54
"flag"
65
"fmt"
76
"io"
@@ -97,12 +96,11 @@ func getServerVersion(cv *judge.Version, collectors []collector.Collector) (*jud
9796
}
9897

9998
func main() {
100-
ctx := context.Background()
10199
exitCode := EXIT_CODE_FAIL_GENERIC
102100

103101
configureGlobalLogging()
104102

105-
config, ctx, err := config.NewFromFlags(ctx)
103+
config, err := config.NewFromFlags()
106104

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

162-
err = outputResults(results, config.Output, config.OutputFile, ctx)
160+
options, err := printer.NewPrinterOptions(config.OutputFile, config.ShowLabels)
161+
if err != nil {
162+
log.Fatal().Err(err).Msgf("Failed to create output file")
163+
}
164+
165+
err = outputResults(results, config.Output, options)
163166
if err != nil {
164167
log.Fatal().Err(err).Msgf("Failed to output results")
165168
}
@@ -183,14 +186,14 @@ func configureGlobalLogging() {
183186
log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr})
184187
}
185188

186-
func outputResults(results []judge.Result, outputType string, outputFile string, ctx context.Context) error {
187-
printer, err := printer.NewPrinter(outputType, outputFile)
189+
func outputResults(results []judge.Result, outputType string, options *printer.PrinterOptions) error {
190+
printer, err := printer.NewPrinter(outputType, options)
188191
if err != nil {
189192
return fmt.Errorf("failed to create printer: %v", err)
190193
}
191194
defer printer.Close()
192195

193-
err = printer.Print(results, ctx)
196+
err = printer.Print(results)
194197
if err != nil {
195198
return fmt.Errorf("failed to print results: %v", err)
196199
}

‎cmd/kubent/main_test.go

+7-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package main
22

33
import (
44
"bytes"
5-
"context"
65
"encoding/base64"
76
"encoding/json"
87
"errors"
@@ -14,8 +13,8 @@ import (
1413

1514
"github.com/doitintl/kube-no-trouble/pkg/collector"
1615
"github.com/doitintl/kube-no-trouble/pkg/config"
17-
ctxKey "github.com/doitintl/kube-no-trouble/pkg/context"
1816
"github.com/doitintl/kube-no-trouble/pkg/judge"
17+
"github.com/doitintl/kube-no-trouble/pkg/printer"
1918

2019
"github.com/rs/zerolog"
2120
)
@@ -288,15 +287,15 @@ func Test_outputResults(t *testing.T) {
288287
}{
289288
{"good", args{testResults, "text", "-"}, false},
290289
{"bad-new-printer-type", args{testResults, "unknown", "-"}, true},
291-
{"bad-new-printer-file", args{testResults, "text", "/unlikely/to/exist/dir"}, true},
292290
}
293291

294-
labelsFlag := false
295-
ctx := context.WithValue(context.Background(), ctxKey.LABELS_CTX_KEY, &labelsFlag)
296-
297292
for _, tt := range tests {
298293
t.Run(tt.name, func(t *testing.T) {
299-
if err := outputResults(tt.args.results, tt.args.outputType, tt.args.outputFile, ctx); (err != nil) != tt.wantErr {
294+
options, err := printer.NewPrinterOptions(tt.args.outputFile, false)
295+
if err != nil {
296+
t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
297+
}
298+
if err = outputResults(tt.args.results, tt.args.outputType, options); (err != nil) != tt.wantErr {
300299
t.Errorf("unexpected error - got: %v, wantErr: %v", err, tt.wantErr)
301300
}
302301
})
@@ -305,6 +304,6 @@ func Test_outputResults(t *testing.T) {
305304

306305
func Test_configureGlobalLogging(t *testing.T) {
307306
// just make sure the method runs, this is mostly covered
308-
//by the Test_MainExitCodes
307+
// by the Test_MainExitCodes
309308
configureGlobalLogging()
310309
}

‎pkg/config/config.go

+9-15
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package config
22

33
import (
4-
"context"
54
"errors"
65
"fmt"
76
"io/fs"
@@ -10,7 +9,6 @@ import (
109
"strings"
1110
"unicode"
1211

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

@@ -38,16 +36,15 @@ type Config struct {
3836
OutputFile string
3937
TargetVersion *judge.Version
4038
KubentVersion bool
39+
ShowLabels bool
4140
}
4241

43-
func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
42+
func NewFromFlags() (*Config, error) {
4443
config := Config{
4544
LogLevel: ZeroLogLevel(zerolog.InfoLevel),
4645
TargetVersion: &judge.Version{},
4746
}
4847

49-
var labels bool
50-
5148
flag.StringSliceVarP(&config.AdditionalKinds, "additional-kind", "a", []string{}, "additional kinds of resources to report in Kind.version.group.com format")
5249
flag.StringSliceVarP(&config.AdditionalAnnotations, "additional-annotation", "A", []string{}, "additional annotations that should be checked to determine the last applied config")
5350
flag.BoolVarP(&config.Cluster, "cluster", "c", true, "enable Cluster collector")
@@ -57,26 +54,23 @@ func NewFromFlags(ctx context.Context) (*Config, context.Context, error) {
5754
flag.BoolVar(&config.Helm3, "helm3", true, "enable Helm v3 collector")
5855
flag.StringSliceVarP(&config.Filenames, "filename", "f", []string{}, "manifests to check, use - for stdin")
5956
flag.StringVarP(&config.Kubeconfig, "kubeconfig", "k", os.Getenv(clientcmd.RecommendedConfigPathEnvVar), "path to the kubeconfig file")
60-
flag.StringVarP(&config.Output, "output", "o", "text", "output format - [text|json|csv]")
57+
flag.StringVarP(&config.Output, "output", "o", TEXT, "output format - [text|json|csv]")
6158
flag.StringVarP(&config.OutputFile, "output-file", "O", "-", "output file, use - for stdout")
6259
flag.VarP(&config.LogLevel, "log-level", "l", "set log level (trace, debug, info, warn, error, fatal, panic, disabled)")
6360
flag.VarP(config.TargetVersion, "target-version", "t", "target K8s version in SemVer format (autodetected by default)")
64-
flag.BoolVar(&labels, "labels", false, "print resource labels")
65-
61+
flag.BoolVar(&config.ShowLabels, "labels", false, "print resource labels")
6662
flag.Parse()
6763

68-
newContext := context.WithValue(ctx, ctxKey.LABELS_CTX_KEY, &labels)
69-
7064
if !isValidOutputFormat(config.Output) {
71-
return nil, nil, fmt.Errorf("failed to validate argument output: %s", config.Output)
65+
return nil, fmt.Errorf("failed to validate argument output: %s", config.Output)
7266
}
7367

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

7872
if err := validateAdditionalResources(config.AdditionalKinds); err != nil {
79-
return nil, nil, fmt.Errorf("failed to validate arguments: %w", err)
73+
return nil, fmt.Errorf("failed to validate arguments: %w", err)
8074
}
8175

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

89-
return &config, newContext, nil
83+
return &config, nil
9084
}
9185

92-
// Previuosly this was handled by a printer.go ParsePrinter function
86+
// Previously this was handled by a printer.go ParsePrinter function
9387
// but we need to avoid cycle imports in order to inject the additional flags
9488
func isValidOutputFormat(format string) bool {
9589
switch format {

‎pkg/config/config_test.go

+6-11
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package config
22

33
import (
4-
"context"
54
"os"
65
"testing"
76

@@ -13,7 +12,6 @@ import (
1312
func TestValidLogLevelFromFlags(t *testing.T) {
1413
oldArgs := os.Args[1]
1514
defer func() { os.Args[1] = oldArgs }()
16-
ctx := context.Background()
1715

1816
var validLevels = []string{"trace", "debug", "info", "warn", "error", "fatal", "panic", "", "disabled"}
1917
for i, level := range validLevels {
@@ -22,7 +20,7 @@ func TestValidLogLevelFromFlags(t *testing.T) {
2220

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

25-
config, _, err := NewFromFlags(ctx)
23+
config, err := NewFromFlags()
2624

2725
if err != nil {
2826
t.Errorf("Flags parsing failed %s", err)
@@ -48,9 +46,8 @@ func TestInvalidLogLevelFromFlags(t *testing.T) {
4846
func TestNewFromFlags(t *testing.T) {
4947
// reset for testing
5048
pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
51-
ctx := context.Background()
5249

53-
config, _, err := NewFromFlags(ctx)
50+
config, err := NewFromFlags()
5451

5552
if err != nil {
5653
t.Errorf("Flags parsing failed %s", err)
@@ -95,7 +92,6 @@ func TestTargetVersion(t *testing.T) {
9592
validVersions := []string{
9693
"1.16", "1.16.3", "1.2.3",
9794
}
98-
ctx := context.Background()
9995

10096
oldArgs := os.Args[1]
10197
defer func() { os.Args[1] = oldArgs }()
@@ -105,7 +101,7 @@ func TestTargetVersion(t *testing.T) {
105101
pflag.CommandLine = pflag.NewFlagSet(os.Args[0], pflag.ExitOnError)
106102

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

110106
if err != nil {
111107
t.Errorf("Flags parsing failed %s", err)
@@ -126,7 +122,7 @@ func TestTargetVersionInvalid(t *testing.T) {
126122
invalidVersions := []string{
127123
"1.blah", "nope",
128124
}
129-
ctx := context.Background()
125+
130126
oldArgs := os.Args[1]
131127
defer func() { os.Args[1] = oldArgs }()
132128

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

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

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

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

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

161156
if err != nil {
162157
t.Errorf("Flags parsing failed %s", err)

‎pkg/context/context-keys.go

-5
This file was deleted.

‎pkg/printer/csv.go

+6-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package printer
22

33
import (
4-
"context"
54
"encoding/csv"
65
"fmt"
76
"sort"
@@ -14,8 +13,8 @@ type csvPrinter struct {
1413
}
1514

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

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

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

48-
w := csv.NewWriter(c.commonPrinter.outputFile)
47+
w := csv.NewWriter(c.commonPrinter.options.outputFile)
4948

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

60-
labels, err := shouldShowLabels(ctx)
61-
if err != nil {
62-
return fmt.Errorf("failed to get labels: %w", err)
63-
}
64-
65-
if labels != nil && *labels {
59+
if c.options.showLabels {
6660
fields = append(fields, "labels")
6761
}
6862

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

82-
if labels != nil && *labels {
76+
if c.options.showLabels {
8377
row = append(row, mapToCommaSeparatedString(r.Labels))
8478
}
8579

0 commit comments

Comments
 (0)