Skip to content

Commit 1230f42

Browse files
committed
feat(lint): Fix golangci-lint warnings
This commit addresses a large number of golangci-lint warnings across the codebase. The fixes span multiple categories of linters. Key changes include: - Error Handling: Ensured that all error return values are checked, particularly from `io.Copy` and `flag.Value.Set`. - Code Formatting: Corrected formatting issues with `gofumpt`. - Vet and Staticcheck: - Resolved a potential context leak in the GCS output by ensuring the cancel function is always called. - Replaced deprecated `io/ioutil` with `io` and `os` packages. - Replaced deprecated `zap.OnFatal` with `zap.WithFatalHook`. - Fixed an issue in TCP and TLS outputs where the write buffer was being modified, which is unsafe. A new buffer is now used. - Simplified various struct field selectors and boolean expressions. - Revive Linter: - Added comments to numerous exported functions, types, and methods to improve documentation. - Removed unused function parameters and variables. - Corrected function signatures to place `context.Context` as the first argument. - Code Organization: - Added package-level comments to all packages. - Renamed the `internal/output/tls` and `internal/output/udp` packages to match their dir names. - Added go-licenser and golangci-lint as go.mod tool dependencies. Removed tools.go. The only remaining linter warning is for the TODO in the udp output.
1 parent 9196166 commit 1230f42

File tree

31 files changed

+1446
-230
lines changed

31 files changed

+1446
-230
lines changed

.changelog/158.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
```release-note:bug
2+
Fixes a bug in the webhook output where an error when reading the response body was ignored.
3+
```
4+
5+
```release-note:bug
6+
Resolved a potential context leak in the GCS output by ensuring the cancel function is always called.
7+
```

.golangci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ linters:
6363
staticcheck:
6464
checks:
6565
- all
66+
- '-QF1008' # https://staticcheck.dev/docs/checks/#QF1008
6667
exclusions:
6768
generated: lax
6869
presets:

Makefile

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
LICENSE := ASL2-Short
22
VERSION ?= local
33

4-
GOIMPORTS := go run golang.org/x/tools/cmd/goimports
4+
GOLANGCI_LINT:= go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint
55
GOLICENSER := go run github.com/elastic/go-licenser
66

77
check-fmt:
88
@${GOLICENSER} -d -license ${LICENSE}
9-
@${GOIMPORTS} -l -e -local github.com/elastic . | read && echo "Code differs from gofmt's style. Run 'gofmt -w .'" 1>&2 && exit 1 || true
9+
@${GOLANGCI_LINT} fmt --diff > /dev/null || (echo "Please run 'make fmt' to fix the formatting issues" 1>&2 && exit 1)
1010

1111
docker:
1212
docker build -t docker.elastic.co/observability/stream:${VERSION} .
1313

1414
fmt:
1515
${GOLICENSER} -license ${LICENSE}
16-
${GOIMPORTS} -l -w -local github.com/elastic .
16+
${GOLANGCI_LINT} fmt ./...
1717

18-
.PHONY: check-fmt docker fmt
18+
lint:
19+
@${GOLANGCI_LINT} run ./...
20+
21+
.PHONY: check-fmt docker fmt lint

go.mod

Lines changed: 239 additions & 36 deletions
Large diffs are not rendered by default.

go.sum

Lines changed: 927 additions & 93 deletions
Large diffs are not rendered by default.

internal/cmdutil/validate.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// Elasticsearch B.V. licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information.
44

5+
// Package cmdutil provides utility functions and argument validators
6+
// to facilitate the creation and validation of CLI command arguments
7+
// when using the spf13/cobra library.
58
package cmdutil
69

710
import (
@@ -42,6 +45,10 @@ func RegularFiles(_ *cobra.Command, args []string) error {
4245
return nil
4346
}
4447

48+
// ExpandGlobPatternsFromArgs expands each argument in args as a glob pattern,
49+
// returning a slice containing all matching file paths. If any pattern is
50+
// invalid, an error is returned. Patterns that do not match any files are
51+
// silently ignored.
4552
func ExpandGlobPatternsFromArgs(args []string) ([]string, error) {
4653
var paths []string
4754
for _, pat := range args {

internal/command/httpserver.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,15 @@ func newHTTPServerRunner(options *httpserver.Options, logger *zap.Logger) *cobra
2626
},
2727
}
2828

29-
r.cmd.RunE = func(_ *cobra.Command, args []string) error {
29+
r.cmd.RunE = func(_ *cobra.Command, _ []string) error {
3030
r.logger = logger.Sugar().With("address", options.Addr)
3131
return r.Run()
3232
}
3333

3434
return r.cmd
3535
}
3636

37+
// Run executes the http-server command.
3738
func (r *httpServerRunner) Run() error {
3839
r.logger.Debug("mock server running...")
3940
server, err := httpserver.New(r.opts, r.logger)

internal/command/log.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ func newLogRunner(options *output.Options, logger *zap.Logger) *cobra.Command {
3939
return r.cmd
4040
}
4141

42+
// Run executes the log command.
4243
func (r *logRunner) Run(args []string) error {
43-
out, err := output.Initialize(r.out, r.logger, r.cmd.Context())
44+
out, err := output.Initialize(r.cmd.Context(), r.out, r.logger)
4445
if err != nil {
4546
return err
4647
}

internal/command/pcap.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ func newPCAPRunner(options *output.Options, logger *zap.Logger) *cobra.Command {
3838
return r.cmd
3939
}
4040

41+
// Run executes the pcap command.
4142
func (r *pcapRunner) Run(files []string) error {
42-
out, err := output.Initialize(r.out, r.logger, r.cmd.Context())
43+
out, err := output.Initialize(r.cmd.Context(), r.out, r.logger)
4344
if err != nil {
4445
return err
4546
}

internal/command/root.go

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
// Elasticsearch B.V. licenses this file to you under the Apache 2.0 License.
33
// See the LICENSE file in the project root for more information.
44

5+
// Package command provides the CLI interface and subcommands for the stream
6+
// utility. It defines the root command and all supported subcommands,
7+
// configuring command-line flags, argument validation, and the main entry point
8+
// for invoking stream's features.
59
package command
610

711
import (
@@ -38,6 +42,8 @@ import (
3842
_ "github.com/elastic/stream/internal/output/webhook"
3943
)
4044

45+
// Execute calls ExecuteContext with a context that is cancelled when
46+
// SIGINT is received.
4147
func Execute() error {
4248
c := make(chan os.Signal, 1)
4349
signal.Notify(c, os.Interrupt)
@@ -50,6 +56,9 @@ func Execute() error {
5056
return ExecuteContext(ctx)
5157
}
5258

59+
// ExecuteContext executes the stream command. It sets up the command-line
60+
// flags and initializes them with data from environment variables, before
61+
// executing the command.
5362
func ExecuteContext(ctx context.Context) error {
5463
logger, err := log.NewLogger()
5564
if err != nil {
@@ -97,10 +106,10 @@ func ExecuteContext(ctx context.Context) error {
97106
rootCmd.PersistentFlags().StringVar(&opts.KafkaOptions.Topic, "kafka-topic", "test", "Kafka topic name")
98107

99108
// GCS output flags.
100-
rootCmd.PersistentFlags().StringVar(&opts.GcsOptions.Bucket, "gcs-bucket", "testbucket", "GCS Bucket name")
101-
rootCmd.PersistentFlags().StringVar(&opts.GcsOptions.Object, "gcs-object", "testobject", "GCS Object name")
102-
rootCmd.PersistentFlags().StringVar(&opts.GcsOptions.ObjectContentType, "gcs-content-type", "application/json", "The Content type of the object to be uploaded to GCS.")
103-
rootCmd.PersistentFlags().StringVar(&opts.GcsOptions.ProjectID, "gcs-projectid", "testproject", "GCS Project name")
109+
rootCmd.PersistentFlags().StringVar(&opts.GCSOptions.Bucket, "gcs-bucket", "testbucket", "GCS Bucket name")
110+
rootCmd.PersistentFlags().StringVar(&opts.GCSOptions.Object, "gcs-object", "testobject", "GCS Object name")
111+
rootCmd.PersistentFlags().StringVar(&opts.GCSOptions.ObjectContentType, "gcs-content-type", "application/json", "The Content type of the object to be uploaded to GCS.")
112+
rootCmd.PersistentFlags().StringVar(&opts.GCSOptions.ProjectID, "gcs-projectid", "testproject", "GCS Project name")
104113

105114
// Lumberjack output flags.
106115
rootCmd.PersistentFlags().BoolVar(&opts.LumberjackOptions.ParseJSON, "lumberjack-parse-json", false, "Parse the input data as JSON and send the structured data as a Lumberjack batch.")
@@ -122,20 +131,20 @@ func ExecuteContext(ctx context.Context) error {
122131
rootCmd.AddCommand(versionCmd)
123132

124133
// Add common start-up delay logic.
125-
rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
134+
rootCmd.PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
126135
return multierr.Combine(
127-
waitForStartSignal(&opts, cmd.Context(), logger),
128-
waitForDelay(&opts, cmd.Context(), logger),
136+
waitForStartSignal(cmd.Context(), &opts, logger),
137+
waitForDelay(cmd.Context(), &opts, logger),
129138
)
130139
}
131140

132141
// Automatically set flags based on environment variables.
133-
rootCmd.PersistentFlags().VisitAll(setFlagFromEnv)
142+
rootCmd.PersistentFlags().VisitAll(setFlagFromEnv(logger))
134143

135144
return rootCmd.ExecuteContext(ctx)
136145
}
137146

138-
func waitForStartSignal(opts *output.Options, parent context.Context, logger *zap.Logger) error {
147+
func waitForStartSignal(ctx context.Context, opts *output.Options, logger *zap.Logger) error {
139148
if opts.StartSignal == "" {
140149
return nil
141150
}
@@ -147,30 +156,34 @@ func waitForStartSignal(opts *output.Options, parent context.Context, logger *za
147156

148157
// Wait for the signal or the command context to be done.
149158
logger.Sugar().Infow("Waiting for signal.", "start-signal", opts.StartSignal)
150-
startCtx, _ := osctx.WithSignal(parent, os.Signal(num))
159+
startCtx, _ := osctx.WithSignal(ctx, os.Signal(num))
151160
<-startCtx.Done()
152161
return nil
153162
}
154163

155-
func waitForDelay(opts *output.Options, parent context.Context, logger *zap.Logger) error {
164+
func waitForDelay(ctx context.Context, opts *output.Options, logger *zap.Logger) error {
156165
if opts.Delay <= 0 {
157166
return nil
158167
}
159168

160169
logger.Sugar().Infow("Delaying connection.", "delay", opts.Delay)
161-
if err := timed.Wait(parent, opts.Delay); err != nil {
170+
if err := timed.Wait(ctx, opts.Delay); err != nil {
162171
return fmt.Errorf("delay waiting period was interrupted: %w", err)
163172
}
164173
return nil
165174
}
166175

167-
func setFlagFromEnv(flag *pflag.Flag) {
168-
envVar := strings.ToUpper(flag.Name)
169-
envVar = strings.ReplaceAll(envVar, "-", "_")
170-
envVar = "STREAM_" + envVar
171-
172-
flag.Usage = fmt.Sprintf("%v [env %v]", flag.Usage, envVar)
173-
if value := os.Getenv(envVar); value != "" {
174-
flag.Value.Set(value)
176+
func setFlagFromEnv(l *zap.Logger) func(*pflag.Flag) {
177+
return func(flag *pflag.Flag) {
178+
envVar := strings.ToUpper(flag.Name)
179+
envVar = strings.ReplaceAll(envVar, "-", "_")
180+
envVar = "STREAM_" + envVar
181+
182+
flag.Usage = fmt.Sprintf("%v [env %v]", flag.Usage, envVar)
183+
if value := os.Getenv(envVar); value != "" {
184+
if err := flag.Value.Set(value); err != nil {
185+
l.Error("Failed to set flag from env", zap.String("env", envVar), zap.Error(err))
186+
}
187+
}
175188
}
176189
}

0 commit comments

Comments
 (0)