Skip to content

fix: satisfy the linter (funlen, gocognit, nolintlint, usetesting) #1

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.

Sign up for GitHub

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

Merged
malpou merged 1 commit into from
Aug 1, 2026
Merged

fix: satisfy the linter (funlen, gocognit, nolintlint, usetesting) #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 54 additions & 31 deletions cmd/pgctl/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,46 +93,19 @@ func run(args []string) error {
return usage()
}

fs := flag.NewFlagSet(cmd, flag.ContinueOnError)
env := fs.String("env", "", "target database (a target named in the config)")
cfgPath := fs.String("config", "", "config file (default: $"+config.EnvVar+", else ./"+config.Default+")")
sshUser := fs.String("ssh-user", "", "ssh to the host as this user (default: the target's ssh_user, else you)")
to := fs.String("to", "", "restore: recover to this point in time (e.g. '2026-07-12 03:00:00')")
pgdata := fs.String("pgdata", defaultPGData, "restore: directory to restore into")
// In the recovery image, everything -- barman, postgres, psql -- is already
// here, and the host we would otherwise ssh to may be exactly what we lost.
local := fs.Bool("local", false, "run commands here instead of over ssh (set inside the recovery image)")
if err := fs.Parse(rest); err != nil {
return err
}

// Load and validate before anything reaches a database: a bad config must
// fail here, not halfway through a backup.
cfg, _, err := config.Load(*cfgPath)
if err != nil {
return err
}
target, err := cfg.LookupTarget(*env)
opts, err := parseFlags(cmd, rest)
if err != nil {
return err
}
// --ssh-user beats the target's ssh_user: who you are is a property of you,
// not of the database. Empty leaves it to ssh, which uses the local user.
user := cfg.SSHUser(*env)
if *sshUser != "" {
user = *sshUser
}

// Ctrl-C and CI cancellation cancel the context rather than killing the
// process, so the drill's teardown gets a chance to run. A leaked drill
// container or scratch dir on the production host is not acceptable.
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
defer stop()

var runner pg.Runner = pg.SSHRunner{Host: target.Host, User: user}
if *local || os.Getenv("PGCTL_LOCAL") == "1" {
runner = pg.LocalRunner{}
}
target := opts.target
runner := opts.runner
logger := log.Default()

switch cmd {
Expand All @@ -153,12 +126,62 @@ func run(args []string) error {
ctx, cancel := context.WithTimeout(ctx, restoreTimeout)
defer cancel()
logger.Printf("pgctl %s (%s), restoring %s", version, commit, target.Server)
return pg.Restore(ctx, runner, target, logger, *pgdata, *to, time.Now())
return pg.Restore(ctx, runner, target, logger, opts.pgdata, opts.to, time.Now())
default:
return usage()
}
}

// opts is everything a command needs once flags are parsed and the config is
// resolved: the target, the runner to reach it, and the restore-only flags.
type opts struct {
target pg.Target
runner pg.Runner
to string
pgdata string
}

// parseFlags parses the shared flag set, loads and validates the config, and
// resolves the target and runner. A bad config fails here, before anything
// reaches a database.
func parseFlags(cmd string, rest []string) (opts, error) {
fs := flag.NewFlagSet(cmd, flag.ContinueOnError)
env := fs.String("env", "", "target database (a target named in the config)")
cfgPath := fs.String("config", "", "config file (default: $"+config.EnvVar+", else ./"+config.Default+")")
sshUser := fs.String("ssh-user", "", "ssh to the host as this user (default: the target's ssh_user, else you)")
to := fs.String("to", "", "restore: recover to this point in time (e.g. '2026-07-12 03:00:00')")
pgdata := fs.String("pgdata", defaultPGData, "restore: directory to restore into")
// In the recovery image, everything -- barman, postgres, psql -- is already
// here, and the host we would otherwise ssh to may be exactly what we lost.
local := fs.Bool("local", false, "run commands here instead of over ssh (set inside the recovery image)")
if err := fs.Parse(rest); err != nil {
return opts{}, err
}

cfg, _, err := config.Load(*cfgPath)
if err != nil {
return opts{}, err
}
target, err := cfg.LookupTarget(*env)
if err != nil {
return opts{}, err
}

// --ssh-user beats the target's ssh_user: who you are is a property of you,
// not of the database. Empty leaves it to ssh, which uses the local user.
user := cfg.SSHUser(*env)
if *sshUser != "" {
user = *sshUser
}

var runner pg.Runner = pg.SSHRunner{Host: target.Host, User: user}
if *local || os.Getenv("PGCTL_LOCAL") == "1" {
runner = pg.LocalRunner{}
}

return opts{target: target, runner: runner, to: *to, pgdata: *pgdata}, nil
}

func listBackups(ctx context.Context, runner pg.Runner, target pg.Target) error {
ctx, cancel := context.WithTimeout(ctx, listTimeout)
defer cancel()
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func Load(flagPath string) (Config, string, error) {
return Config{}, "", err
}

data, err := os.ReadFile(path) //nolint:gosec // the path is the operator's, by definition
data, err := os.ReadFile(path)
if err != nil {
return Config{}, path, fmt.Errorf("reading %s: %w", path, err)
}
Expand Down
50 changes: 18 additions & 32 deletions internal/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,27 @@ func write(t *testing.T, dir, name, body string) string {
}

// chdir moves into a temp dir so ./pgctl.yaml resolution is testable without
// touching the repo.
// touching the repo. t.Chdir restores the previous directory automatically.
func chdir(t *testing.T, dir string) {
t.Helper()
old, err := os.Getwd()
t.Chdir(dir)
}

// loadFromPath asserts Load reads exactly wantPath and returns a config that
// still defines example-production. Keeps each resolution-order subtest to
// setup plus one call.
func loadFromPath(t *testing.T, flag, wantPath string) {
t.Helper()
cfg, path, err := Load(flag)
if err != nil {
t.Fatal(err)
}
if err := os.Chdir(dir); err != nil {
t.Fatal(err)
if path != wantPath {
t.Errorf("read %s, want %s", path, wantPath)
}
if _, ok := cfg.Targets["example-production"]; !ok {
t.Errorf("got targets %v, want the one from %s", cfg.targetNames(), wantPath)
}
t.Cleanup(func() { _ = os.Chdir(old) })
}

func TestResolutionOrder(t *testing.T) {
Expand All @@ -56,16 +66,7 @@ func TestResolutionOrder(t *testing.T) {
flagPath := write(t, dir, "flag.yaml", validConfig)
t.Setenv(EnvVar, envPath)

cfg, path, err := Load(flagPath)
if err != nil {
t.Fatal(err)
}
if path != flagPath {
t.Errorf("read %s, want %s", path, flagPath)
}
if _, ok := cfg.Targets["example-production"]; !ok {
t.Errorf("got targets %v, want the --config one", cfg.targetNames())
}
loadFromPath(t, flagPath, flagPath)
})

t.Run("PGCTL_CONFIG wins over ./pgctl.yaml", func(t *testing.T) {
Expand All @@ -75,30 +76,15 @@ func TestResolutionOrder(t *testing.T) {
envPath := write(t, dir, "env.yaml", validConfig)
t.Setenv(EnvVar, envPath)

cfg, path, err := Load("")
if err != nil {
t.Fatal(err)
}
if path != envPath {
t.Errorf("read %s, want %s", path, envPath)
}
if _, ok := cfg.Targets["example-production"]; !ok {
t.Errorf("got targets %v, want the $%s one", cfg.targetNames(), EnvVar)
}
loadFromPath(t, "", envPath)
})

t.Run("./pgctl.yaml is the last resort", func(t *testing.T) {
dir := t.TempDir()
chdir(t, dir)
write(t, dir, "pgctl.yaml", validConfig)

_, path, err := Load("")
if err != nil {
t.Fatal(err)
}
if path != Default {
t.Errorf("read %s, want ./%s", path, Default)
}
loadFromPath(t, "", Default)
})

t.Run("nothing anywhere names every path tried", func(t *testing.T) {
Expand Down
Loading