diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa41073..f9d83fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,3 +68,58 @@ jobs: go build -o semibase ./cmd/semibase ./semibase all --database semiplot_dev --expected-major 17 ./semibase all --database semiplot_dev --expected-major 17 + + # The socket path, which nothing else covers: the same binary, run the way + # the official image runs an init script. During init the server has + # listen_addresses empty and only /var/run/postgresql answers, superuser + # auth is local trust, and the script's own "set -e" makes the entrypoint + # abort on failure. So the container reaching a state where semiplot_reader + # selects over TCP proves create completed over the socket. + - name: Provision over a unix socket, as a postgres image init script + env: + SEMIBASE_WRITER_PASSWORD: ci-writer + SEMIBASE_READER_PASSWORD: ci-reader + # postgres:17-alpine is musl. The runner has a C toolchain, so an + # unset CGO_ENABLED defaults to 1 and links against glibc; the image + # then reports the binary as "not found", which reads as a missing + # file rather than a missing interpreter. release.yml builds the + # shipped Linux artifact the same way. + CGO_ENABLED: "0" + run: | + set -euo pipefail + go build -o semibase-linux ./cmd/semibase + if ! file semibase-linux | grep -q 'statically linked'; then + echo "::error::semibase-linux is not statically linked, so it cannot run on Alpine" + file semibase-linux + exit 1 + fi + cat > initdb-semibase.sh <<'SCRIPT' + #!/bin/sh + set -e + /semibase create --host /var/run/postgresql --database semiplot_dev --expected-major 17 + SCRIPT + # the entrypoint sources a non-executable init script instead of running it, + # and a binary without the bit set cannot run at all + chmod +x semibase-linux initdb-semibase.sh + cat > Dockerfile.initdb <<'DOCKERFILE' + FROM postgres:17-alpine + COPY semibase-linux /semibase + COPY initdb-semibase.sh /docker-entrypoint-initdb.d/10-semibase.sh + DOCKERFILE + docker build -f Dockerfile.initdb -t semibase-initdb . + # nothing is published: the host side of a published port is bound by + # docker-proxy before postgres listens, so it answers a probe too early + docker run -d --name semibase-initdb -e POSTGRES_PASSWORD=ci-super \ + -e SEMIBASE_WRITER_PASSWORD -e SEMIBASE_READER_PASSWORD semibase-initdb + # TCP inside the container opens only once the init scripts have run + for _ in $(seq 60); do + if docker exec semibase-initdb pg_isready -h 127.0.0.1 -q 2>/dev/null; then break; fi + if [ "$(docker inspect -f '{{.State.Running}}' semibase-initdb)" != true ]; then break; fi + sleep 2 + done + if ! docker exec -e PGPASSWORD=ci-reader semibase-initdb psql -h 127.0.0.1 \ + -U semiplot_reader -d semiplot_dev -tAc 'select count(*) from semiplot_tags'; then + echo "::error::provisioning over the unix socket did not complete" + docker logs semibase-initdb + exit 1 + fi diff --git a/.golangci.yml b/.golangci.yml index f81ab60..5c9e845 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,3 +16,6 @@ linters: - path: _test\.go linters: - gosec + - linters: + - revive + text: "^(exported|package-comments):" diff --git a/cmd/semibase/env_test.go b/cmd/semibase/env_test.go index d3a5319..ae06334 100644 --- a/cmd/semibase/env_test.go +++ b/cmd/semibase/env_test.go @@ -66,8 +66,8 @@ func TestLoadDotEnvAbsentFileIsSilent(t *testing.T) { func TestLoadDotEnvDirectoryReportsReadError(t *testing.T) { t.Chdir(t.TempDir()) - // On Windows os.Open succeeds on a directory; the read fails, exercising - // the scanner.Err() warning path. + // on Windows os.Open succeeds on a directory; the read fails, exercising + // the scanner.Err() warning path if err := os.Mkdir(dotEnvName, 0o755); err != nil { t.Fatal(err) } diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index 30526ba..47aeffe 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -1,4 +1,3 @@ -// Command semibase provisions the SemiBase PostgreSQL instance. package main import ( @@ -43,25 +42,18 @@ directory (flag wins over environment, environment wins over .env): Run 'semibase --help' for the command's flags. ` -// revision identifies the build; a release sets it with -// -ldflags "-X main.revision=...". Empty means "not embedded". +// set by a release with -ldflags "-X main.revision=..." var revision string -// phaseTimeout bounds every command. Provisioning is DDL on empty objects, so -// a generous fixed bound is enough. const phaseTimeout = 5 * time.Minute -// output receives the success reporting (help text, version, the final Done -// line) and errorOutput the usage and flag-parse reporting. Variables so tests -// capture the output instead of writing to the test log. +// variables so tests can capture the streams var ( output io.Writer = os.Stdout errorOutput io.Writer = os.Stderr ) -// commands is the single source for command validation and dispatch. A name -// missing here is an error, never a fallback to All. Production code never -// writes this map; only tests insert stub entries. +// only tests write to this map, to insert stubs var commands = map[string]func(context.Context, provision.Options) error{ "config": func(ctx context.Context, options provision.Options) error { return options.Config(ctx) }, "create": func(ctx context.Context, options provision.Options) error { return options.Create(ctx) }, @@ -77,9 +69,6 @@ func main() { os.Exit(exitCode) } -// run owns the whole command lifecycle and returns the exit code; main alone -// turns it into os.Exit. Ctrl+C cancels ctx, which cancels the in-flight -// query through pgx instead of hard-killing the process mid-DDL. func run(ctx context.Context, arguments []string) int { if len(arguments) == 0 { fmt.Fprint(errorOutput, usage) @@ -116,15 +105,11 @@ func run(ctx context.Context, arguments []string) int { fmt.Fprintln(errorOutput, "error:", err) return 1 } - fmt.Fprintf(output, "\nDone: %s completed against %s:%d/%s.\n", - command, options.Host, options.Port, options.Database) + fmt.Fprintf(output, "\nDone: %s completed against %s.\n", + command, options.Endpoint()) return 0 } -// resolveRevision returns the embedded ldflags revision verbatim when a -// release set it; otherwise it falls back to the VCS revision the Go -// toolchain recorded at build time. Test binaries and non-VCS builds resolve -// to "unknown". func resolveRevision(embedded string) string { if embedded != "" { return embedded @@ -156,13 +141,13 @@ func revisionFromSettings(settings []debug.BuildSetting) string { return vcsRevision } -// newFlagSet registers the shared command flags on a ContinueOnError set. -// Password flags default to empty so the usage output can only ever show the -// environment variable names, never their values. +// password flags default to empty so the usage output can only ever show the +// variable names, never their values func newFlagSet(command string, options *provision.Options) *flag.FlagSet { flags := flag.NewFlagSet(command, flag.ContinueOnError) flags.SetOutput(errorOutput) - flags.StringVar(&options.Host, "host", "localhost", "server host") + flags.StringVar(&options.Host, "host", "localhost", + "server host, or a unix socket directory such as /var/run/postgresql") flags.IntVar(&options.Port, "port", 5432, "server port") flags.StringVar(&options.Database, "database", "scada_archive", "archive database name") flags.StringVar(&options.SuperUser, "superuser", "postgres", "superuser role name") @@ -191,8 +176,6 @@ func parseOptions(command string, arguments []string) (provision.Options, error) return options, nil } -// resolvePasswordsFromEnvironment fills every password the flags left empty -// from its SEMIBASE_* variable. func resolvePasswordsFromEnvironment(options *provision.Options) { if options.SuperPassword == "" { options.SuperPassword = os.Getenv("SEMIBASE_SUPER_PASSWORD") @@ -207,10 +190,8 @@ func resolvePasswordsFromEnvironment(options *provision.Options) { const dotEnvName = ".env" -// loadDotEnv applies variables from a .env file in the working directory. -// Variables already present in the process environment win; flags win over both. -// A missing file is normal and silent; any other failure is reported so an -// unreadable file does not degrade into a misleading missing-password error. +// an unreadable file is reported so it does not degrade into a misleading +// missing-password error later func loadDotEnv() { file, err := os.Open(dotEnvName) if err != nil { @@ -250,8 +231,8 @@ func applyEnv(reader io.Reader) error { return scanner.Err() } -// Only a matching pair of surrounding quotes ("..." or '...') is removed, so -// a password that legitimately starts or ends with a quote passes intact. +// only a matching pair is removed, so a password that legitimately starts or +// ends with a quote passes intact func trimMatchingQuotes(value string) string { if len(value) >= 2 && (value[0] == '"' || value[0] == '\'') && value[len(value)-1] == value[0] { return value[1 : len(value)-1] diff --git a/cmd/semibase/main_test.go b/cmd/semibase/main_test.go index c5a10ee..3feee2e 100644 --- a/cmd/semibase/main_test.go +++ b/cmd/semibase/main_test.go @@ -21,8 +21,7 @@ var passwordEnvironmentNames = []string{ "SEMIBASE_READER_PASSWORD", } -// captureOutput redirects the package success stream into a buffer for the -// duration of the test. Tests mutating it must not run in parallel. +// callers must not use t.Parallel: the streams are package-level func captureOutput(t *testing.T) *bytes.Buffer { t.Helper() buffer := &bytes.Buffer{} @@ -32,7 +31,6 @@ func captureOutput(t *testing.T) *bytes.Buffer { return buffer } -// Same contract as captureOutput, for the error stream. func captureErrorOutput(t *testing.T) *bytes.Buffer { t.Helper() buffer := &bytes.Buffer{} @@ -42,9 +40,8 @@ func captureErrorOutput(t *testing.T) *bytes.Buffer { return buffer } -// unsetEnvForTest removes the variables for the duration of the test: -// t.Setenv registers restoration of the original value, and the Unsetenv -// right after actually clears the variable from the process environment. +// t.Setenv registers restoration of the original value, the Unsetenv right after +// clears the variable from the process environment func unsetEnvForTest(t *testing.T, names ...string) { t.Helper() for _, name := range names { @@ -263,7 +260,7 @@ func TestRunGivesCommandsADeadline(t *testing.T) { captureOutput(t) clearPasswordEnvironment(t) - // Mutates the shared commands map, so this test must not use t.Parallel. + // mutates the shared commands map, so no t.Parallel here var deadline time.Time var hasDeadline bool commands["stub-deadline"] = func(ctx context.Context, _ provision.Options) error { @@ -278,19 +275,40 @@ func TestRunGivesCommandsADeadline(t *testing.T) { if !hasDeadline { t.Fatal("command context carries no deadline") } - // The lower bound catches a dispatch bug wrapping commands in a - // near-zero timeout, which "at most phaseTimeout" alone would pass. + // the lower bound catches a dispatch bug wrapping commands in a near-zero + // timeout, which "at most phaseTimeout" alone would pass remaining := time.Until(deadline) if remaining > phaseTimeout || remaining < phaseTimeout/2 { t.Errorf("deadline is %v away, want close to %v", remaining, phaseTimeout) } } +// the Done line is the only place the endpoint reaches an operator on success, and the +// socket spelling of it is what the init-script path prints +func TestRunReportsTheEndpointItRanAgainst(t *testing.T) { + captureErrorOutput(t) + buffer := captureOutput(t) + clearPasswordEnvironment(t) + + // mutates the shared commands map, so no t.Parallel here + commands["stub-done"] = func(context.Context, provision.Options) error { return nil } + t.Cleanup(func() { delete(commands, "stub-done") }) + + arguments := []string{"stub-done", "--host", "/var/run/postgresql", "--database", "semiplot_dev"} + if got := run(context.Background(), arguments); got != 0 { + t.Fatalf("run(stub-done) = %d, want exit code 0", got) + } + want := "Done: stub-done completed against /var/run/postgresql/.s.PGSQL.5432 (semiplot_dev)." + if !strings.Contains(buffer.String(), want) { + t.Errorf("output %q does not contain %q", buffer.String(), want) + } +} + func TestRunReportsCommandFailure(t *testing.T) { buffer := captureErrorOutput(t) clearPasswordEnvironment(t) - // Mutates the shared commands map, so this test must not use t.Parallel. + // mutates the shared commands map, so no t.Parallel here commands["stub-failure"] = func(context.Context, provision.Options) error { return errors.New("stub-failure sentinel") } @@ -305,8 +323,6 @@ func TestRunReportsCommandFailure(t *testing.T) { } func TestCancelledContextReturnsPromptly(t *testing.T) { - // Create prints its step banner through the provision console; discard it - // so the test output stays clean. previous := provision.SetConsoleOutput(io.Discard) t.Cleanup(func() { provision.SetConsoleOutput(previous) }) diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index fa70404..75ce4c5 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -5,9 +5,9 @@ production instance alike. That is the point: a tool that has created the develo many times is proven; a tool written for commissioning and run once, on site, on the day it matters, is a liability. -The binary is self-contained: `pgx/v5` talks to the server directly over TCP, so neither -`psql.exe` nor any runtime needs to be present on the target machine. `sql/semiplot_tags.sql` -is embedded at build time. Installing the PostgreSQL engine itself is one documented `winget` +The binary is self-contained: `pgx/v5` speaks the wire protocol itself, over TCP or a unix +socket, so neither `psql.exe` nor any runtime needs to be present on the target machine. +`sql/semiplot_tags.sql` is embedded at build time. Installing the PostgreSQL engine itself is one documented `winget` line, deliberately outside the tool — OS package management is not database provisioning. ## Commands @@ -31,6 +31,33 @@ file in the working directory is read as the lowest-precedence source (flag > en existing credentials untouched. Flag usage output shows only the environment variable names, never their values. +## The connection target + +`--host` takes a hostname or a unix socket directory, and the split is the driver's own: +`pgconn.isAbsolutePath` calls a value a socket directory when it starts with `/` or is a +drive-letter path such as `C:\pgsock`. A socket directory cannot ride in the URL authority — +its slashes would end the authority — so the connection string moves it, and the port with it, +into the query, percent-encoded: + +``` +postgres://postgres:***@/scada_archive?host=%2Fvar%2Frun%2Fpostgresql&port=5432 +``` + +pgx then dials `/.s.PGSQL.` on the `unix` network. Widening the test past pgx's +would be a trap, not a courtesy: libpq also accepts a leading `@` for Linux's abstract namespace, +but pgx resolves such a host as a TCP name, so `semibase` leaves it on the TCP path. + +The socket form is what lets `create` run as an init script in the official `postgres` image, +whose entrypoint serves `/docker-entrypoint-initdb.d/` from a temporary server with +`listen_addresses` set to empty — reachable over the socket only. The Linux CI job runs exactly +that: it layers the freshly built binary onto `postgres:17-alpine` behind such an init script and +asserts the container reaches a state where the reader can query. + +Messages name the socket file rather than a URL — +`/var/run/postgresql/.s.PGSQL.5432 (scada_archive)`. The database is in parentheses because a +slash would make the socket file read as a directory and send the operator to a path that can +never exist. + ## Invariants the provision package owns The package that runs the SQL owns its guards; the CLI is a thin dispatcher. diff --git a/docs/deployment.md b/docs/deployment.md index 61824ed..b458aa0 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -100,11 +100,11 @@ winget install --id PostgreSQL.PostgreSQL.17 --exact ## Основные флаги -| Флаг | По умолчанию | Назначение | -| ------------------ | --------------- | --------------------------------- | -| `--host` | `localhost` | Адрес сервера | -| `--port` | `5432` | Порт сервера | -| `--database` | `scada_archive` | Имя архивной базы | -| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | +| Флаг | По умолчанию | Назначение | +| ------------------ | --------------- | ------------------------------------------------------------- | +| `--host` | `localhost` | Адрес сервера или каталог unix-сокета (`/var/run/postgresql`) | +| `--port` | `5432` | Порт сервера | +| `--database` | `scada_archive` | Имя архивной базы | +| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | Полный перечень — `semibase <команда> --help`. diff --git a/docs/plans/completed/20260814-linux-bench-provisioning.md b/docs/plans/completed/20260814-linux-bench-provisioning.md index 0941cb2..895a5d1 100644 --- a/docs/plans/completed/20260814-linux-bench-provisioning.md +++ b/docs/plans/completed/20260814-linux-bench-provisioning.md @@ -47,11 +47,12 @@ vanilla PostgreSQL 17 via `winget` (`PostgreSQL.PostgreSQL.17`), so the bench ru 2. [x] `GOOS=linux GOARCH=amd64 go build ./...` and `GOOS=darwin GOARCH=arm64 go build ./...` both exit 0; `GOOS=linux go vet ./...` exits 0. 3. [x] `go mod tidy` drops `golang.org/x/sys` — nothing else in the module uses it. -4. [ ] The Ubuntu CI job is green: build, unit tests, and the provisioning step — `all` run twice +4. [x] The Ubuntu CI job is green: build, unit tests, and the provisioning step — `all` run twice against the `postgres:17-alpine` service container, proving the bench path and idempotency in one step (`verify` inside `all` reports "writer has not run" as a state, not a failure). Lint runs once, on the Windows job — the same config over the same untagged code. -5. [ ] The Windows CI job stays green. + Confirmed 2026-08-14: master push run `31793374588` succeeded. +5. [x] The Windows CI job stays green. Same run. ## Implementation Steps @@ -81,7 +82,7 @@ vanilla PostgreSQL 17 via `winget` (`PostgreSQL.PostgreSQL.17`), so the bench ru - [x] `ubuntu-latest` job in `.github/workflows/ci.yml`: build, `go test -race`, and a `postgres:17-alpine` service container provisioned by running `all` twice - [x] the stale "does not build on a Linux runner" comment removed from `ci.yml` and `CLAUDE.md` -- [ ] first push shows both jobs green (acceptance items 4 and 5) +- [x] first push shows both jobs green (acceptance items 4 and 5) ### Task 4: Documentation — done @@ -94,8 +95,8 @@ vanilla PostgreSQL 17 via `winget` (`PostgreSQL.PostgreSQL.17`), so the bench ru ### Task 5: Close the plan -- [ ] CI green on master confirms acceptance items 4 and 5 -- [ ] move this plan to `docs/plans/completed/` +- [x] CI green on master confirms acceptance items 4 and 5 +- [x] move this plan to `docs/plans/completed/` ## Post-Completion diff --git a/embed.go b/embed.go index 4cd6ee5..4def537 100644 --- a/embed.go +++ b/embed.go @@ -1,9 +1,6 @@ -// Package semibase carries repository-level assets embedded into the binary. package semibase import _ "embed" -// SemiplotTagsSQL is the DDL for the one object SemiBase adds to the archive database. -// //go:embed sql/semiplot_tags.sql var SemiplotTagsSQL string diff --git a/internal/provision/config.go b/internal/provision/config.go index 517029f..21026d9 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -8,16 +8,11 @@ import ( "github.com/jackc/pgx/v5" ) -// setting is one ALTER SYSTEM delta. Order is kept for readable output. type setting struct { Name string Value string } -// settings are the configuration deltas. Every installation guarantees at -// least 8 GB of RAM, so the memory figures are constants sized to that floor -// and every machine runs the same configuration. The rationale for every -// value is in docs/architecture/configuration.md. var settings = []setting{ {"shared_buffers", "2GB"}, {"effective_cache_size", "4GB"}, @@ -32,11 +27,6 @@ var settings = []setting{ {"track_io_timing", "on"}, } -// Config writes the configuration deltas through ALTER SYSTEM and applies -// them with pg_reload_conf(). Every setting except shared_buffers is -// reload-context or weaker, so it takes effect immediately; shared_buffers -// waits for the next service restart, which Config reports through the -// server's own pending-restart list instead of touching the service. func (o Options) Config(ctx context.Context) error { if err := o.Validate(); err != nil { return err @@ -60,8 +50,8 @@ func (o Options) Config(ctx context.Context) error { if _, err := conn.Exec(ctx, "SELECT pg_reload_conf()"); err != nil { return fmt.Errorf("reloading the configuration: %w", err) } - // pg_reload_conf signals the reload asynchronously; the pause lets the - // server processes apply the new files before pending_restart is read. + // pg_reload_conf signals asynchronously; the pause lets the server processes apply + // the new files before pending_restart is read if _, err := conn.Exec(ctx, "SELECT pg_sleep(0.5)"); err != nil { return fmt.Errorf("waiting for the reload: %w", err) } diff --git a/internal/provision/console.go b/internal/provision/console.go index 3fa2542..f47d165 100644 --- a/internal/provision/console.go +++ b/internal/provision/console.go @@ -8,9 +8,6 @@ import ( var consoleWriter io.Writer = os.Stdout -// SetConsoleOutput redirects the console helpers to writer and returns the -// previous writer. Tests outside the package use it to keep step banners out -// of their output; production code never calls it. func SetConsoleOutput(writer io.Writer) io.Writer { previous := consoleWriter consoleWriter = writer diff --git a/internal/provision/provision.go b/internal/provision/provision.go index 4e44fca..ff7b509 100644 --- a/internal/provision/provision.go +++ b/internal/provision/provision.go @@ -1,6 +1,3 @@ -// Package provision creates and verifies the SemiBase PostgreSQL instance: -// server configuration, the archive database, the roles, the reader access -// chain, and semiplot_tags. package provision import ( @@ -17,20 +14,13 @@ import ( semibase "github.com/Semiteq/SemiBase" ) -// Roles of the archive database. The SCADA writes, the viewers read. -// semiplot_tags stays owned by the superuser; a dedicated owner role returns -// when a tag-editing mechanism exists to hold it. const ( WriterRole = "scada_writer" ReaderRole = "semiplot_reader" ) -// versionFloor is PostgreSQL 14, the oldest release with date_bin, which the -// readers' bucketing query uses. const versionFloor = 140000 -// Options carries everything the phases need. Zero passwords mean "leave the -// existing credential untouched". type Options struct { Host string Port int @@ -42,11 +32,8 @@ type Options struct { ExpectedMajor int } -// databaseNamePattern is the identifier gate in front of every DDL statement -// that interpolates the database name. var databaseNamePattern = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) -// Validate checks the invariants the phases rely on before any SQL runs. func (o Options) Validate() error { if !databaseNamePattern.MatchString(o.Database) { return fmt.Errorf("database name %q must match %s", o.Database, databaseNamePattern) @@ -54,8 +41,6 @@ func (o Options) Validate() error { return nil } -// All runs config, create, and verify in order. A writer that has not run yet -// is reported as a state, not a failure. func (o Options) All(ctx context.Context) error { if err := o.Config(ctx); err != nil { return err @@ -66,24 +51,63 @@ func (o Options) All(ctx context.Context) error { return o.verify(ctx, true) } -// Verify runs the post-writer checks and fails when the writer has not run. func (o Options) Verify(ctx context.Context) error { return o.verify(ctx, false) } -func (o Options) connect(ctx context.Context, database, user, password string) (*pgx.Conn, error) { +// pgx dials a unix socket exactly when pgconn.isAbsolutePath accepts the host. Widening this +// past pgx would route a host into the query parameter that the driver then resolves as a TCP +// name: libpq's extra "@" form for Linux's abstract namespace is that case, so it stays out. +func isSocketHost(host string) bool { + return strings.HasPrefix(host, "/") || isWindowsDrivePath(host) +} + +// pgconn.isAbsolutePath's drive-letter clause, character for character +func isWindowsDrivePath(host string) bool { + return len(host) >= 3 && host[0] >= 'A' && host[0] <= 'Z' && host[1] == ':' && host[2] == '\\' +} + +// a socket directory cannot ride in the URL authority: its slashes would end the authority +// and the rest of the path would be read as the URL path. libpq and pgx take the socket +// directory from the host query parameter instead, percent-encoded like any query value +func (o Options) connectionString(database, user, password string) string { target := url.URL{ Scheme: "postgres", User: url.UserPassword(user, password), - Host: net.JoinHostPort(o.Host, strconv.Itoa(o.Port)), Path: "/" + database, } - conn, err := pgx.Connect(ctx, target.String()) + if isSocketHost(o.Host) { + target.RawQuery = url.Values{ + "host": {o.Host}, + "port": {strconv.Itoa(o.Port)}, + }.Encode() + return target.String() + } + target.Host = net.JoinHostPort(o.Host, strconv.Itoa(o.Port)) + return target.String() +} + +// Endpoint names what a connection to the archive database targets, for messages. +func (o Options) Endpoint() string { + return o.endpoint(o.Database) +} + +// the socket file is /.s.PGSQL., so the database cannot follow it behind a +// slash: that would name a path under the socket file and send a reader of the message +// looking for a directory that can never exist +func (o Options) endpoint(database string) string { + if isSocketHost(o.Host) { + return fmt.Sprintf("%s/.s.PGSQL.%d (%s)", strings.TrimRight(o.Host, `/\`), o.Port, database) + } + return fmt.Sprintf("%s:%d/%s", o.Host, o.Port, database) +} + +func (o Options) connect(ctx context.Context, database, user, password string) (*pgx.Conn, error) { + conn, err := pgx.Connect(ctx, o.connectionString(database, user, password)) if err != nil { - return nil, fmt.Errorf("connecting to %s:%d/%s as %s: %w", o.Host, o.Port, database, user, err) + return nil, fmt.Errorf("connecting to %s as %s: %w", o.endpoint(database), user, err) } - // escapeLiteral's quote-doubling is complete only with this on (see its - // doc). USERSET, so no privilege is needed. + // escapeLiteral relies on this; USERSET, so no privilege is needed if _, err := conn.Exec(ctx, "SET standard_conforming_strings = on"); err != nil { _ = conn.Close(ctx) return nil, fmt.Errorf("setting standard_conforming_strings: %w", err) @@ -91,17 +115,14 @@ func (o Options) connect(ctx context.Context, database, user, password string) ( return conn, nil } -// escapeLiteral doubles single quotes. This is sufficient in every server mode -// only because connect forces standard_conforming_strings = on for the session; -// under that setting backslashes carry no escape meaning inside '...' literals. +// quote doubling alone is safe only because connect sets standard_conforming_strings = on, +// which strips backslashes of any escape meaning inside '...' literals func escapeLiteral(value string) string { return strings.ReplaceAll(value, "'", "''") } -// The statement builders below are the only places a database name or a -// password reaches DDL text. Role names are package constants, never user -// input, so only the database name and the password need escaping. - +// only the database name and the password reach DDL text here; role names are +// package constants, never user input func grantConnectStatement(database, role string) string { return fmt.Sprintf("GRANT CONNECT ON DATABASE %s TO %s", pgx.Identifier{database}.Sanitize(), role) } @@ -118,8 +139,6 @@ func alterRolePasswordStatement(name, password string) string { return fmt.Sprintf("ALTER ROLE %s PASSWORD '%s'", name, escapeLiteral(password)) } -// Create provisions the archive database, the roles, the grants, the -// default privileges, and semiplot_tags. Every step checks before it creates. func (o Options) Create(ctx context.Context) error { if err := o.Validate(); err != nil { return err @@ -176,13 +195,13 @@ func (o Options) Create(ctx context.Context) error { } defer archive.Close(ctx) - // PostgreSQL 15+ no longer grants CREATE on schema public to everyone; on 14 this is redundant. + // PostgreSQL 15+ no longer grants CREATE on schema public to everyone; on 14 this is redundant if _, err := archive.Exec(ctx, "GRANT CREATE ON SCHEMA public TO "+WriterRole); err != nil { return fmt.Errorf("granting CREATE on schema public: %w", err) } - // The reader access chain: trends/messages do not exist until the SCADA runs, so SELECT on - // them can only be granted ahead of time, through default privileges of their future owner. + // trends/messages do not exist until the SCADA runs, so SELECT on them can only be + // granted ahead of time, through default privileges of their future owner defaultPrivileges := fmt.Sprintf( "ALTER DEFAULT PRIVILEGES FOR ROLE %s IN SCHEMA public GRANT SELECT ON TABLES TO %s", WriterRole, ReaderRole) diff --git a/internal/provision/provision_test.go b/internal/provision/provision_test.go index 5d1bf06..9dfe0e4 100644 --- a/internal/provision/provision_test.go +++ b/internal/provision/provision_test.go @@ -2,6 +2,9 @@ package provision import ( "testing" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" ) func TestValidate(t *testing.T) { @@ -32,9 +35,8 @@ func TestValidate(t *testing.T) { } } -// The statement-builder tests pin the exact DDL text Create and ensureDatabase -// execute: a hostile database name comes out as one double-quoted identifier, -// inert as SQL. Dropping the Sanitize call at either site fails these. +// these pin the exact DDL text: a hostile database name comes out as one +// double-quoted identifier, inert as SQL. dropping Sanitize fails them func TestGrantConnectStatement(t *testing.T) { hostile := `archive"; DROP DATABASE postgres; --` want := `GRANT CONNECT ON DATABASE "archive""; DROP DATABASE postgres; --" TO scada_writer` @@ -63,8 +65,7 @@ func TestRoleStatementsEscapeThePassword(t *testing.T) { } } -// The backslash cases pin escapeLiteral's standard_conforming_strings -// contract. +// the backslash cases pin the standard_conforming_strings contract func TestEscapeLiteral(t *testing.T) { tests := []struct { name string @@ -87,3 +88,158 @@ func TestEscapeLiteral(t *testing.T) { }) } } + +// the socket form is what makes the binary usable as a postgres image init script: the +// entrypoint's temporary server sets listen_addresses to empty, so nothing answers on TCP +func TestConnectionString(t *testing.T) { + tests := []struct { + name string + options Options + want string + }{ + { + "tcp host", + Options{Host: "localhost", Port: 5432}, + "postgres://postgres:secret@localhost:5432/scada_archive", + }, + { + "tcp host on a custom port", + Options{Host: "db.internal", Port: 15432}, + "postgres://postgres:secret@db.internal:15432/scada_archive", + }, + { + "ipv6 literal keeps its brackets", + Options{Host: "::1", Port: 5432}, + "postgres://postgres:secret@[::1]:5432/scada_archive", + }, + { + "socket directory moves to the host parameter", + Options{Host: "/var/run/postgresql", Port: 5432}, + "postgres://postgres:secret@/scada_archive?host=%2Fvar%2Frun%2Fpostgresql&port=5432", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.options.connectionString("scada_archive", "postgres", "secret") + if got != tt.want { + t.Errorf("connectionString() = %q, want %q", got, tt.want) + } + }) + } +} + +// the golden strings above prove nothing on their own: ?host=/var/run/postgresql and +// ?host=%2Fvar%2Frun%2Fpostgresql parse to the same config. What has to hold is that the +// driver then dials the socket, which is the network pgconn derives from the parsed host +func TestConnectionStringIsDialedAsTheRightNetwork(t *testing.T) { + tests := []struct { + name string + options Options + wantHost string + wantNetwork string + }{ + {"tcp host", Options{Host: "localhost", Port: 5432}, "localhost", "tcp"}, + { + "socket directory", + Options{Host: "/var/run/postgresql", Port: 15432}, + "/var/run/postgresql", + "unix", + }, + { + "windows drive path", + Options{Host: `C:\pgsock`, Port: 5432}, + `C:\pgsock`, + "unix", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config, err := pgx.ParseConfig(tt.options.connectionString("scada_archive", "postgres", "p@ss word")) + if err != nil { + t.Fatalf("ParseConfig: %v", err) + } + if config.Host != tt.wantHost { + t.Errorf("parsed host = %q, want %q", config.Host, tt.wantHost) + } + if config.Port != uint16(tt.options.Port) { + t.Errorf("parsed port = %d, want %d", config.Port, tt.options.Port) + } + if config.Database != "scada_archive" { + t.Errorf("parsed database = %q, want %q", config.Database, "scada_archive") + } + if config.Password != "p@ss word" { + t.Errorf("parsed password = %q, want %q", config.Password, "p@ss word") + } + // the address is filepath.Join'd, so it differs by GOOS; only the network is portable + network, address := pgconn.NetworkAddress(config.Host, config.Port) + if network != tt.wantNetwork { + t.Errorf("dialled network = %q (address %q), want %q", network, address, tt.wantNetwork) + } + }) + } +} + +// the boundary of the socket branch is pgconn.isAbsolutePath and nothing wider: a host this +// says yes to and pgx says no to would be dialled as a TCP name found in the host parameter +func TestIsSocketHost(t *testing.T) { + tests := []struct { + host string + want bool + }{ + {"/var/run/postgresql", true}, + {"/", true}, + {`C:\pgsock`, true}, + {`C:\`, true}, + {`c:\pgsock`, false}, + {"C:/pgsock", false}, + {"C:pgsock", false}, + {"@abstract", false}, + {"localhost", false}, + {"::1", false}, + {"", false}, + } + for _, tt := range tests { + t.Run(tt.host, func(t *testing.T) { + if got := isSocketHost(tt.host); got != tt.want { + t.Errorf("isSocketHost(%q) = %v, want %v", tt.host, got, tt.want) + } + }) + } +} + +// the socket form must stay one quotable path: a database appended behind a slash would +// name a directory under the socket file, which cannot exist +func TestEndpoint(t *testing.T) { + tests := []struct { + name string + options Options + want string + }{ + {"tcp host", Options{Host: "localhost", Port: 5432}, "localhost:5432/scada_archive"}, + { + "socket directory names the socket file", + Options{Host: "/var/run/postgresql", Port: 5432}, + "/var/run/postgresql/.s.PGSQL.5432 (scada_archive)", + }, + { + "trailing slash does not double", + Options{Host: "/var/run/postgresql/", Port: 15432}, + "/var/run/postgresql/.s.PGSQL.15432 (scada_archive)", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.options.endpoint("scada_archive"); got != tt.want { + t.Errorf("endpoint() = %q, want %q", got, tt.want) + } + }) + } +} + +// Endpoint is the exported one, and it names the archive database rather than taking one +func TestEndpointUsesTheConfiguredDatabase(t *testing.T) { + options := Options{Host: "/var/run/postgresql", Port: 5432, Database: "semiplot_dev"} + if got, want := options.Endpoint(), "/var/run/postgresql/.s.PGSQL.5432 (semiplot_dev)"; got != want { + t.Errorf("Endpoint() = %q, want %q", got, want) + } +}