Skip to content

fix: address golangci-lint warnings#158

Open
andrewkroh wants to merge 15 commits into
elastic:mainfrom
andrewkroh:fix/linter-warnings
Open

fix: address golangci-lint warnings#158
andrewkroh wants to merge 15 commits into
elastic:mainfrom
andrewkroh:fix/linter-warnings

Conversation

@andrewkroh
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh commented Jul 9, 2025

This commit addresses a large number of golangci-lint warnings across the codebase.
The fixes span multiple categories of linters.

  • Error Handling: Ensured that all error return values are checked, particularly from io.Copy and flag.Value.Set.
  • 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 all exported functions, types, and methods to improve documentation.
    • Replace unused function parameters with blank identifier (_).
    • Changed 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.

@andrewkroh andrewkroh force-pushed the fix/linter-warnings branch 5 times, most recently from 8523016 to ba94778 Compare July 15, 2025 18:08
@andrewkroh andrewkroh marked this pull request as ready for review July 15, 2025 18:12
@andrewkroh andrewkroh added the Team:Security-Service Integrations Team:Security-Service Integrations label Aug 19, 2025
I think this reduces clarity when there are multiple embedded structs.
Replace tools.go with go mod tool directives.

go get -tool github.com/golangci/golangci-lint/v2/cmd/golangci-lint
go get -tool github.com/elastic/go-licenser
go mod tidy
Rename GcsOptions to GCSOptions.
Fixed an issue in TCP and TLS outputs where the write buffer was being modified, which is unsafe. A new buffer is now used.
Resolved a potential context leak in the GCS output by ensuring the cancel function is always called.
@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

Comment thread go.mod
@andrewkroh andrewkroh requested a review from a team November 7, 2025 21:13
// KafkaOptions holds configuration for the Kafka output.
type KafkaOptions struct {
Topic string // Topic name. Will create it if not exists.
Topic string // Topic is the Kafka topic name. It will be created if it does not exist.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Topic is the Kafka topic name. The topic will be created if it does not exist."

("It" is Topic string on first reading)

Similar below.

Comment thread internal/output/util.go
Comment on lines +17 to +22
// Initialize creates and configures a new Output using the provided Options and
// logger, then attempts to establish a connection with retries. It returns the
// connected Output or an error if the connection could not be established within
// the allowed retries or if the provided context is canceled. The logger is used
// for informational and debug messages during initialization and connection
// attempts.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right (comment is in gutter).

}

r.cmd.RunE = func(_ *cobra.Command, args []string) error {
r.cmd.RunE = func(_ *cobra.Command, _ []string) error {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, this could be r.cmd.RunE = func(*cobra.Command, []string) error {. No need for change.

Comment thread internal/output/util.go
// the allowed retries or if the provided context is canceled. The logger is used
// for informational and debug messages during initialization and connection
// attempts.
func Initialize(ctx context.Context, opts *Options, logger *zap.SugaredLogger) (Output, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes it.

Comment on lines +85 to +89
// Add a newline for framing.
buf := make([]byte, len(b)+1)
copy(buf, b)
buf[len(b)] = '\n'
return o.conn.Write(buf)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In both cases this could be return o.conn.Write(append(b[:len(b):len(b)], '\n')) with safety.

Comment on lines +63 to +64
if (opts.TLSCertificate != "" || opts.TLSKey != "") &&
(opts.TLSCertificate == "" || opts.TLSKey == "") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	if (opts.TLSCertificate == "") != (opts.TLSKey == "") {

or better

certIsZero := opts.TLSCertificate == ""
keyIsZero := opts.TLSKey == ""
if certIsZero != keyIsZero {

if addr != "" {
h, err = url.Parse(addr)
if err != nil {
cancel()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rather than doing this, NewClient should accept a context.Context and the caller should have the responsibility to handle this.

if _, err = io.Copy(&buf, resp.Body); err != nil {
return 0, fmt.Errorf("failed to read response body: %w", err)
}
defer resp.Body.Close()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the copy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Security-Service Integrations Team:Security-Service Integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants