refactor: use pinned golangci-lint Docker image for linting (#55)
All checks were successful
check / check (push) Successful in 5s
All checks were successful
check / check (push) Successful in 5s
Closes [issue #50](#50) ## Summary Refactors the Dockerfile to use a separate lint stage with a pinned golangci-lint Docker image, following the pattern used by [sneak/pixa](https://git.eeqj.de/sneak/pixa). This replaces the previous approach of installing golangci-lint via curl in the builder stage. ## Changes ### Dockerfile - **New `lint` stage** using `golangci/golangci-lint:v2.11.3` (Debian-based, pinned by sha256 digest) as a separate build stage - **Builder stage** depends on lint via `COPY --from=lint /src/go.sum /dev/null` — build won't proceed unless linting passes - **Go bumped** from 1.24 to 1.26.1 (`golang:1.26.1-bookworm`, pinned by sha256) - **golangci-lint bumped** from v1.64.8 to v2.11.3 - All three Docker images (golangci-lint, golang, alpine) pinned by sha256 digest - Debian-based golangci-lint image used (not Alpine) because mattn/go-sqlite3 CGO does not compile on musl (off64_t) ### Linter Config (.golangci.yml) - Migrated from v1 to v2 format (`version: "2"` added) - Removed linters no longer available in v2: `gofmt` (handled by `make fmt-check`), `gosimple` (merged into `staticcheck`), `typecheck` (always-on in v2) - Same set of linters enabled — no rules weakened ### Code Fixes (all lint issues from v2 upgrade) - Added package comments to all packages - Added doc comments to all exported types, functions, and methods - Fixed unchecked errors flagged by `errcheck` (sqlDB.Close, os.Setenv in tests, resp.Body.Close, fmt.Fprint) - Fixed unused parameters flagged by `revive` (renamed to `_`) - Fixed `gosec` G120 warnings: added `http.MaxBytesReader` before `r.ParseForm()` calls - Fixed `staticcheck` QF1012: replaced `WriteString(fmt.Sprintf(...))` with `fmt.Fprintf` - Fixed `staticcheck` QF1003: converted if/else chain to tagged switch - Renamed `DeliveryTask` → `Task` to avoid package stutter (`delivery.Task` instead of `delivery.DeliveryTask`) - Renamed shadowed builtin `max` parameter to `upperBound` in `cryptoRandInt` - Used `t.Setenv` instead of `os.Setenv` in tests (auto-restores) ### README.md - Updated version requirements: Go 1.26+, golangci-lint v2.11+ - Updated Dockerfile description in project structure ## Verification `docker build .` passes cleanly — formatting check, linting, all tests, and build all succeed. Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de> Reviewed-on: #55 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #55.
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
// Package config loads application configuration from environment variables.
|
||||
package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
@@ -17,19 +19,29 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
// EnvironmentDev represents development environment
|
||||
// EnvironmentDev represents development environment.
|
||||
EnvironmentDev = "dev"
|
||||
// EnvironmentProd represents production environment
|
||||
// EnvironmentProd represents production environment.
|
||||
EnvironmentProd = "prod"
|
||||
|
||||
// defaultPort is the default HTTP listen port.
|
||||
defaultPort = 8080
|
||||
)
|
||||
|
||||
// nolint:revive // ConfigParams is a standard fx naming convention
|
||||
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
|
||||
// contains an unrecognised value.
|
||||
var ErrInvalidEnvironment = errors.New("invalid environment")
|
||||
|
||||
//nolint:revive // ConfigParams is a standard fx naming convention.
|
||||
type ConfigParams struct {
|
||||
fx.In
|
||||
|
||||
Globals *globals.Globals
|
||||
Logger *logger.Logger
|
||||
}
|
||||
|
||||
// Config holds all application configuration loaded from
|
||||
// environment variables.
|
||||
type Config struct {
|
||||
DataDir string
|
||||
Debug bool
|
||||
@@ -43,56 +55,67 @@ type Config struct {
|
||||
log *slog.Logger
|
||||
}
|
||||
|
||||
// IsDev returns true if running in development environment
|
||||
// IsDev returns true if running in development environment.
|
||||
func (c *Config) IsDev() bool {
|
||||
return c.Environment == EnvironmentDev
|
||||
}
|
||||
|
||||
// IsProd returns true if running in production environment
|
||||
// IsProd returns true if running in production environment.
|
||||
func (c *Config) IsProd() bool {
|
||||
return c.Environment == EnvironmentProd
|
||||
}
|
||||
|
||||
// envString returns the value of the named environment variable, or
|
||||
// an empty string if not set.
|
||||
// envString returns the value of the named environment variable,
|
||||
// or an empty string if not set.
|
||||
func envString(key string) string {
|
||||
return os.Getenv(key)
|
||||
}
|
||||
|
||||
// envBool returns the value of the named environment variable parsed as a
|
||||
// boolean. Returns defaultValue if not set.
|
||||
// envBool returns the value of the named environment variable
|
||||
// parsed as a boolean. Returns defaultValue if not set.
|
||||
func envBool(key string, defaultValue bool) bool {
|
||||
if v := os.Getenv(key); v != "" {
|
||||
return strings.EqualFold(v, "true") || v == "1"
|
||||
}
|
||||
|
||||
return defaultValue
|
||||
}
|
||||
|
||||
// envInt returns the value of the named environment variable parsed as an
|
||||
// integer. Returns defaultValue if not set or unparseable.
|
||||
// envInt returns the value of the named environment variable
|
||||
// parsed as an integer. Returns defaultValue if not set or
|
||||
// unparseable.
|
||||
func envInt(key string, defaultValue int) int {
|
||||
if v := os.Getenv(key); v != "" {
|
||||
if i, err := strconv.Atoi(v); err == nil {
|
||||
i, err := strconv.Atoi(v)
|
||||
if err == nil {
|
||||
return i
|
||||
}
|
||||
}
|
||||
|
||||
return defaultValue
|
||||
}
|
||||
|
||||
// nolint:revive // lc parameter is required by fx even if unused
|
||||
// New creates a Config by reading environment variables.
|
||||
//
|
||||
//nolint:revive // lc parameter is required by fx even if unused.
|
||||
func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
log := params.Logger.Get()
|
||||
|
||||
// Determine environment from WEBHOOKER_ENVIRONMENT env var, default to dev
|
||||
// Determine environment from WEBHOOKER_ENVIRONMENT env var,
|
||||
// default to dev
|
||||
environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
|
||||
if environment == "" {
|
||||
environment = EnvironmentDev
|
||||
}
|
||||
|
||||
// Validate environment
|
||||
if environment != EnvironmentDev && environment != EnvironmentProd {
|
||||
return nil, fmt.Errorf("WEBHOOKER_ENVIRONMENT must be either '%s' or '%s', got '%s'",
|
||||
EnvironmentDev, EnvironmentProd, environment)
|
||||
if environment != EnvironmentDev &&
|
||||
environment != EnvironmentProd {
|
||||
return nil, fmt.Errorf(
|
||||
"%w: WEBHOOKER_ENVIRONMENT must be '%s' or '%s', got '%s'",
|
||||
ErrInvalidEnvironment,
|
||||
EnvironmentDev, EnvironmentProd, environment,
|
||||
)
|
||||
}
|
||||
|
||||
// Load configuration values from environment variables
|
||||
@@ -103,15 +126,16 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
Environment: environment,
|
||||
MetricsUsername: envString("METRICS_USERNAME"),
|
||||
MetricsPassword: envString("METRICS_PASSWORD"),
|
||||
Port: envInt("PORT", 8080),
|
||||
Port: envInt("PORT", defaultPort),
|
||||
SentryDSN: envString("SENTRY_DSN"),
|
||||
log: log,
|
||||
params: ¶ms,
|
||||
}
|
||||
|
||||
// Set default DataDir. All SQLite databases (main application DB
|
||||
// and per-webhook event DBs) live here. The same default is used
|
||||
// regardless of environment; override with DATA_DIR if needed.
|
||||
// Set default DataDir. All SQLite databases (main application
|
||||
// DB and per-webhook event DBs) live here. The same default is
|
||||
// used regardless of environment; override with DATA_DIR if
|
||||
// needed.
|
||||
if s.DataDir == "" {
|
||||
s.DataDir = "/var/lib/webhooker"
|
||||
}
|
||||
@@ -128,7 +152,8 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
||||
"maintenanceMode", s.MaintenanceMode,
|
||||
"dataDir", s.DataDir,
|
||||
"hasSentryDSN", s.SentryDSN != "",
|
||||
"hasMetricsAuth", s.MetricsUsername != "" && s.MetricsPassword != "",
|
||||
"hasMetricsAuth",
|
||||
s.MetricsUsername != "" && s.MetricsPassword != "",
|
||||
)
|
||||
|
||||
return s, nil
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
package config
|
||||
package config_test
|
||||
|
||||
import (
|
||||
"os"
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/fx"
|
||||
"go.uber.org/fx/fxtest"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/globals"
|
||||
"sneak.berlin/go/webhooker/internal/logger"
|
||||
)
|
||||
@@ -22,121 +23,143 @@ func TestEnvironmentConfig(t *testing.T) {
|
||||
isProd bool
|
||||
}{
|
||||
{
|
||||
name: "default is dev",
|
||||
envValue: "",
|
||||
envVars: map[string]string{},
|
||||
expectError: false,
|
||||
isDev: true,
|
||||
isProd: false,
|
||||
name: "default is dev",
|
||||
isDev: true,
|
||||
isProd: false,
|
||||
},
|
||||
{
|
||||
name: "explicit dev",
|
||||
envValue: "dev",
|
||||
envVars: map[string]string{},
|
||||
expectError: false,
|
||||
isDev: true,
|
||||
isProd: false,
|
||||
name: "explicit dev",
|
||||
envValue: "dev",
|
||||
isDev: true,
|
||||
isProd: false,
|
||||
},
|
||||
{
|
||||
name: "explicit prod",
|
||||
envValue: "prod",
|
||||
envVars: map[string]string{},
|
||||
expectError: false,
|
||||
isDev: false,
|
||||
isProd: true,
|
||||
name: "explicit prod",
|
||||
envValue: "prod",
|
||||
isDev: false,
|
||||
isProd: true,
|
||||
},
|
||||
{
|
||||
name: "invalid environment",
|
||||
envValue: "staging",
|
||||
envVars: map[string]string{},
|
||||
expectError: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Set environment variable if specified
|
||||
// Cannot use t.Parallel() here because t.Setenv
|
||||
// is incompatible with parallel subtests.
|
||||
if tt.envValue != "" {
|
||||
os.Setenv("WEBHOOKER_ENVIRONMENT", tt.envValue)
|
||||
defer os.Unsetenv("WEBHOOKER_ENVIRONMENT")
|
||||
t.Setenv(
|
||||
"WEBHOOKER_ENVIRONMENT", tt.envValue,
|
||||
)
|
||||
} else {
|
||||
os.Unsetenv("WEBHOOKER_ENVIRONMENT")
|
||||
require.NoError(t, os.Unsetenv(
|
||||
"WEBHOOKER_ENVIRONMENT",
|
||||
))
|
||||
}
|
||||
|
||||
// Set additional environment variables
|
||||
for k, v := range tt.envVars {
|
||||
os.Setenv(k, v)
|
||||
defer os.Unsetenv(k)
|
||||
t.Setenv(k, v)
|
||||
}
|
||||
|
||||
if tt.expectError {
|
||||
// Use regular fx.New for error cases since fxtest doesn't expose errors the same way
|
||||
var cfg *Config
|
||||
app := fx.New(
|
||||
fx.NopLogger, // Suppress fx logs in tests
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
assert.Error(t, app.Err())
|
||||
testEnvironmentConfigError(t)
|
||||
} else {
|
||||
// Use fxtest for success cases
|
||||
var cfg *Config
|
||||
app := fxtest.New(
|
||||
t,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
testEnvironmentConfigSuccess(
|
||||
t, tt.isDev, tt.isProd,
|
||||
)
|
||||
require.NoError(t, app.Err())
|
||||
app.RequireStart()
|
||||
defer app.RequireStop()
|
||||
|
||||
assert.Equal(t, tt.isDev, cfg.IsDev())
|
||||
assert.Equal(t, tt.isProd, cfg.IsProd())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func testEnvironmentConfigError(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
var cfg *config.Config
|
||||
|
||||
app := fx.New(
|
||||
fx.NopLogger,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
config.New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
|
||||
assert.Error(t, app.Err())
|
||||
}
|
||||
|
||||
func testEnvironmentConfigSuccess(
|
||||
t *testing.T,
|
||||
isDev, isProd bool,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
var cfg *config.Config
|
||||
|
||||
app := fxtest.New(
|
||||
t,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
config.New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
require.NoError(t, app.Err())
|
||||
|
||||
app.RequireStart()
|
||||
|
||||
defer app.RequireStop()
|
||||
|
||||
assert.Equal(t, isDev, cfg.IsDev())
|
||||
assert.Equal(t, isProd, cfg.IsProd())
|
||||
}
|
||||
|
||||
func TestDefaultDataDir(t *testing.T) {
|
||||
// Verify that when DATA_DIR is unset, the default is /var/lib/webhooker
|
||||
// regardless of the environment setting.
|
||||
for _, env := range []string{"", "dev", "prod"} {
|
||||
name := env
|
||||
if name == "" {
|
||||
name = "unset"
|
||||
}
|
||||
t.Run("env="+name, func(t *testing.T) {
|
||||
if env != "" {
|
||||
os.Setenv("WEBHOOKER_ENVIRONMENT", env)
|
||||
defer os.Unsetenv("WEBHOOKER_ENVIRONMENT")
|
||||
} else {
|
||||
os.Unsetenv("WEBHOOKER_ENVIRONMENT")
|
||||
}
|
||||
os.Unsetenv("DATA_DIR")
|
||||
|
||||
var cfg *Config
|
||||
t.Run("env="+name, func(t *testing.T) {
|
||||
// Cannot use t.Parallel() here because t.Setenv
|
||||
// is incompatible with parallel subtests.
|
||||
if env != "" {
|
||||
t.Setenv("WEBHOOKER_ENVIRONMENT", env)
|
||||
} else {
|
||||
require.NoError(t, os.Unsetenv(
|
||||
"WEBHOOKER_ENVIRONMENT",
|
||||
))
|
||||
}
|
||||
|
||||
require.NoError(t, os.Unsetenv("DATA_DIR"))
|
||||
|
||||
var cfg *config.Config
|
||||
|
||||
app := fxtest.New(
|
||||
t,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
New,
|
||||
config.New,
|
||||
),
|
||||
fx.Populate(&cfg),
|
||||
)
|
||||
require.NoError(t, app.Err())
|
||||
|
||||
app.RequireStart()
|
||||
|
||||
defer app.RequireStop()
|
||||
|
||||
assert.Equal(t, "/var/lib/webhooker", cfg.DataDir)
|
||||
assert.Equal(
|
||||
t, "/var/lib/webhooker", cfg.DataDir,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user