diff --git a/CHANGELOG.md b/CHANGELOG.md index a595f7c..2f4b6c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- `PKGPROXY_HOST` env var to set the listen address without passing `--host` on the command line - Container image now runs `serve` by default and loads bundled config from `$KO_DATA_PATH` ### Changed diff --git a/Makefile b/Makefile index 56ae7b6..258ace4 100644 --- a/Makefile +++ b/Makefile @@ -150,7 +150,7 @@ ci-build: ## To be called to build the application binary in a CI pipeline $(info ******************************************************) $(info ********** EXECUTING 'ci-build' MAKE TARGET **********) $(info ******************************************************) - CGO_ENABLED=$(CGO_ENABLED) go build $(GO_BUILD_ARGS) $(GO_BUILD_ARGS_EXTRA) -ldflags '$(LDFLAGS)' -o $(LOCALBIN)$(NAME) . + CGO_ENABLED=$(CGO_ENABLED) go build $(GO_BUILD_ARGS) $(GO_BUILD_ARGS_EXTRA) -ldflags '$(LDFLAGS)' -o $(LOCALBIN)/$(NAME) . .PHONY: run run: format vet generate ## Run the application from your host diff --git a/README.md b/README.md index 41cca54..489289e 100644 --- a/README.md +++ b/README.md @@ -15,12 +15,12 @@ PKGPROXY_CONFIG=./configs/pkgproxy.yaml go run github.com/ganto/pkgproxy serve Run the application via a container engine (e.g. [Podman](https://podman.io/)): ```shell -podman run --rm -p 8080:8080 --volume ./cache:/ko-app/cache:z ghcr.io/ganto/pkgproxy serve --host 0.0.0.0 +podman run --rm -p 8080:8080 -e PKGPROXY_HOST=0.0.0.0 --volume ./cache:/ko-app/cache:z ghcr.io/ganto/pkgproxy ``` To use a custom `pkgproxy.yaml`, bind-mount it into the container: ```shell -podman run --rm -p 8080:8080 --volume ./cache:/ko-app/cache:z --volume ./pkgproxy.yaml:/ko-app/pkgproxy.yaml ghcr.io/ganto/pkgproxy serve --host 0.0.0.0 +podman run --rm -p 8080:8080 -e PKGPROXY_HOST=0.0.0.0 --volume ./cache:/ko-app/cache:z --volume ./pkgproxy.yaml:/ko-app/pkgproxy.yaml ghcr.io/ganto/pkgproxy ``` ## Server Configuration @@ -31,11 +31,13 @@ podman run --rm -p 8080:8080 --volume ./cache:/ko-app/cache:z --volume ./pkgprox |------|--------------|---------|-------------| | `--config, -c` | `PKGPROXY_CONFIG` | `./pkgproxy.yaml` | Path to the repository config file | | `--cachedir` | | `cache` | Path to the local cache directory | -| `--host` | | `localhost` | Listen address | +| `--host` | `PKGPROXY_HOST` | `localhost` | Listen address | | `--port` | | `8080` | Listen port | | `--public-host` | `PKGPROXY_PUBLIC_HOST` | | Public hostname (or `host:port`) shown in landing page config snippets. When set, the listen port is not appended. Useful when running behind a reverse proxy. | | `--debug` | | `false` | Enable debug logging | +Any flag with an env variable listed above can be set via the environment instead of passing the flag. + ## Repository Configuration An example repository configuration can be found at [configs/pkgproxy.yaml](configs/pkgproxy.yaml). diff --git a/cmd/serve.go b/cmd/serve.go index f4a4bd0..da61b66 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -27,6 +27,7 @@ var ( const ( defaultAddress = "localhost" defaultPort = 8080 + hostEnvVar = "PKGPROXY_HOST" publicHostEnvVar = "PKGPROXY_PUBLIC_HOST" ) @@ -35,7 +36,8 @@ func newServeCommand() *cobra.Command { Use: "serve", Args: cobra.ArbitraryArgs, Short: "Start forward proxy", - PersistentPreRunE: func(_ *cobra.Command, _ []string) error { + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + listenAddress = resolveListenHost(cmd.Flag("host").Changed, listenAddress, os.Getenv(hostEnvVar)) return initConfig() }, RunE: startServer, @@ -48,6 +50,17 @@ func newServeCommand() *cobra.Command { return c } +// resolveListenHost determines the listen host using flag → env var → default precedence. +func resolveListenHost(flagChanged bool, flagValue, envValue string) string { + if flagChanged { + return flagValue + } + if envValue != "" { + return envValue + } + return defaultAddress +} + // resolvePublicAddr determines the address rendered in landing page config snippets. // The CLI flag takes precedence over the environment variable. If neither is set, // the listen host:port is used. diff --git a/cmd/serve_test.go b/cmd/serve_test.go index 0e6b3a5..21ae48c 100644 --- a/cmd/serve_test.go +++ b/cmd/serve_test.go @@ -8,6 +8,58 @@ import ( "github.com/stretchr/testify/assert" ) +func TestResolveListenHost(t *testing.T) { + tests := []struct { + name string + flagChanged bool + flagValue string + envValue string + want string + }{ + { + name: "flag changed wins over env var", + flagChanged: true, + flagValue: "192.168.10.4", + envValue: "10.0.0.1", + want: "192.168.10.4", + }, + { + name: "flag changed wins even when value equals default", + flagChanged: true, + flagValue: "localhost", + envValue: "0.0.0.0", + want: "localhost", + }, + { + name: "env var used when flag unchanged", + flagChanged: false, + flagValue: "localhost", + envValue: "0.0.0.0", + want: "0.0.0.0", + }, + { + name: "empty env var falls through to default", + flagChanged: false, + flagValue: "localhost", + envValue: "", + want: "localhost", + }, + { + name: "neither set returns default", + flagChanged: false, + flagValue: "localhost", + envValue: "", + want: "localhost", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := resolveListenHost(tt.flagChanged, tt.flagValue, tt.envValue) + assert.Equal(t, tt.want, got) + }) + } +} + func TestResolvePublicAddr(t *testing.T) { tests := []struct { name string diff --git a/openspec/changes/archive/2026-05-10-host-env-var/.openspec.yaml b/openspec/changes/archive/2026-05-10-host-env-var/.openspec.yaml new file mode 100644 index 0000000..1b75776 --- /dev/null +++ b/openspec/changes/archive/2026-05-10-host-env-var/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-25 diff --git a/openspec/changes/archive/2026-05-10-host-env-var/design.md b/openspec/changes/archive/2026-05-10-host-env-var/design.md new file mode 100644 index 0000000..fc7aa21 --- /dev/null +++ b/openspec/changes/archive/2026-05-10-host-env-var/design.md @@ -0,0 +1,81 @@ +## Context + +The previous `container-default-config` change (archived 2026-04-19) made `serve` the default subcommand and taught `initConfig` to find the bundled `pkgproxy.yaml` via `KO_DATA_PATH`. The remaining rough edge is the listen address: `--host` defaults to `localhost`, which is unreachable through a port-mapped container, so users still have to type `serve --host 0.0.0.0`. + +The binary doesn't currently read any env var for the listen address. Adding `PKGPROXY_HOST` gives operators a way to control the listen address via the environment — useful both for `podman run -e PKGPROXY_HOST=0.0.0.0 …` and for any orchestrator that sets env vars at deploy time. + +The existing CLI already has two flag→env precedents: + +- `--public-host` ↔ `PKGPROXY_PUBLIC_HOST` — uses empty-string-as-unset because the flag has no default. +- `--config` ↔ `PKGPROXY_CONFIG` — uses value-equals-default because the flag has a meaningful default. + +`--host` falls into the same category as `--config`. Mirroring `PKGPROXY_CONFIG`'s value-equals-default trick would carry forward its known edge case (a user typing `--host localhost` would be indistinguishable from "no flag passed"). Cobra exposes a cleaner primitive — `cmd.Flag(name).Changed` — that distinguishes "user typed the default" from "default applied". This change adopts that primitive. + +## Goals / Non-Goals + +**Goals:** +- `podman run -p 8080:8080 -e PKGPROXY_HOST=0.0.0.0 ghcr.io/ganto/pkgproxy` starts a working server reachable through the port mapping without passing CLI arguments. +- `podman run -p 8080:8080 -e PKGPROXY_HOST=127.0.0.1 ghcr.io/ganto/pkgproxy` lets the operator narrow the listen address without rebuilding the image or passing CLI arguments. +- Local development (`make run`, `go run .`, `bin/pkgproxy serve`) keeps `localhost` as the listen default — no change to muscle memory, no surprise binding to a public interface. +- Explicit `--host` always wins, including when it equals the built-in default. `--host localhost` means "really listen on localhost". + +**Non-Goals:** +- Adding `PKGPROXY_PORT`. Symmetric and harmless, but the 8080 default already works in containers; YAGNI. +- Changing the binary's CLI default for `--host` or baking `0.0.0.0` into the published image config. Operators set the listen address via the env var at runtime. +- Migrating `PKGPROXY_CONFIG` to the same `Flag.Changed` pattern. Worth doing later for consistency, but out of scope here. +- FHS-style env-var conventions (`PKGPROXY_LISTEN_ADDRESS`, `PKGPROXY_BIND_HOST`, etc.). The chosen name `PKGPROXY_HOST` is symmetric with the flag and matches the project's existing naming. + +## Decisions + +### D1. Detect "user passed --host" via `cmd.Flag("host").Changed` + +The resolution helper takes either the Cobra command (so it can call `cmd.Flag("host").Changed`) or a pre-extracted `bool changed` argument. The latter is preferred because it makes the helper trivially unit-testable without constructing a Cobra command. + +```go +// in cmd/serve.go +const hostEnvVar = "PKGPROXY_HOST" + +func resolveListenHost(flagChanged bool, flagValue, envValue string) string { + if flagChanged { + return flagValue + } + if envValue != "" { + return envValue + } + return defaultAddress +} +``` + +Wiring lives in `PersistentPreRunE` (which already calls `initConfig`): + +```go +PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + listenAddress = resolveListenHost( + cmd.Flag("host").Changed, + listenAddress, + os.Getenv(hostEnvVar), + ) + return initConfig() +}, +``` + +`startServer` keeps reading `listenAddress` unchanged. + +**Alternatives considered:** +- _Mirror `PKGPROXY_CONFIG`'s value-equals-default trick._ Rejected. Carries forward the same edge case the user explicitly chose to avoid for new code: `--host localhost` would silently be overridden by `PKGPROXY_HOST=192.168.0.10`, which is the wrong behavior. +- _Use viper for full env-var binding._ Rejected. Two flag→env mappings (this one and the existing `PKGPROXY_PUBLIC_HOST`/`PKGPROXY_CONFIG`) don't justify a new dependency. + +### D2. Empty-string env var is treated as "unset" + +`os.Getenv` returns `""` for unset and for explicitly-set-to-empty. Both are treated as "fall through to the default". Distinguishing them with `os.LookupEnv` would let a user "unset" the env var by passing `-e PKGPROXY_HOST=` — but that's never useful here (an empty listen address would fail to bind), and the simpler rule keeps parity with `resolvePublicAddr`. + +### D3. The CLI default stays `localhost`; operators set the listen address at runtime + +`defaultAddress` in `cmd/serve.go` is unchanged. Local builds and `make run` keep their current behavior. Container users who need the server reachable on all interfaces pass `-e PKGPROXY_HOST=0.0.0.0` at `podman run` time — no Go-side conditional logic, no image-baked default. + +This is deliberate: developers running `bin/pkgproxy serve` from a checkout never accidentally bind to a public interface, and the published image carries no opinionated default for the listen address. + +## Risks / Trade-offs + +- **`cmd.Flag("host").Changed` is package-global** → `listenAddress` is set as a package-level `var`, and `Changed` reflects what Cobra parsed *for the current invocation*. In tests that reuse the same root command across calls, `Changed` could leak between iterations. → Mitigation: write the unit tests against `resolveListenHost(changed, flagValue, envValue)` directly so the helper has no Cobra dependency. End-to-end behavior of `PersistentPreRunE` is exercised through normal command runs, not table-driven tests. +- **Inconsistency with `PKGPROXY_CONFIG`** → `--config` keeps the value-equals-default heuristic; `--host` uses `Flag.Changed`. Two patterns in the same package is mildly ugly. → Mitigation: documented in this design as a deliberate one-way ratchet — new code uses the better primitive; the existing `PKGPROXY_CONFIG` can be migrated later under a separate change. diff --git a/openspec/changes/archive/2026-05-10-host-env-var/proposal.md b/openspec/changes/archive/2026-05-10-host-env-var/proposal.md new file mode 100644 index 0000000..6b6133c --- /dev/null +++ b/openspec/changes/archive/2026-05-10-host-env-var/proposal.md @@ -0,0 +1,29 @@ +## Why + +The published container image still requires users to type `serve --host 0.0.0.0` because the `--host` flag defaults to `localhost`, which is unreachable through a port-mapped container. This change adds a `PKGPROXY_HOST` environment variable so operators can control the listen address via the environment without rewriting the command line. The previous `container-default-config` change closed half the gap by making `serve` the default subcommand and finding the bundled config; this change adds the missing env-var hook for the listen address. + +## What Changes + +- Add a new `PKGPROXY_HOST` environment variable that is consulted by `serve` whenever the user has not explicitly set `--host` on the command line. Resolution chain: `--host` flag (when set by the user) → `PKGPROXY_HOST` (when set and non-empty) → built-in default `localhost`. +- Use Cobra's `cmd.Flag("host").Changed` to detect explicit user input rather than the value-equals-default heuristic used by `PKGPROXY_CONFIG`. This makes `--host localhost` distinguishable from "no flag passed". +- Update README.md flags table to document the new env var, add a short note that env-var entries in the table can substitute for the corresponding flag, and replace the `serve --host 0.0.0.0` argument in the existing container-run examples with `-e PKGPROXY_HOST=0.0.0.0` so the env-var path becomes the recommended way to make a containerized pkgproxy reachable. +- Add a `[Unreleased]` CHANGELOG entry. +- `PKGPROXY_PORT` is **not** introduced. The 8080 default is acceptable in containers and the smaller surface is preferred. + +## Capabilities + +### New Capabilities +- `listen-address-config`: How `serve` resolves its listen address from the `--host` flag, the `PKGPROXY_HOST` environment variable, and the built-in default. + +### Modified Capabilities +_None._ The existing `default-subcommand` and `config-discovery` specs are unaffected; this change is purely additive. + +## Impact + +- `cmd/serve.go` — Add `hostEnvVar = "PKGPROXY_HOST"` constant and a `resolveListenHost` helper. Wire it into `PersistentPreRunE` so `listenAddress` is finalized before `startServer` reads it for `sc.Address`. +- `cmd/serve_test.go` — Add tests for `resolveListenHost` mirroring the style of `TestResolvePublicAddr`, covering the flag-changed-vs-env precedence matrix and the empty-env-string case. +- `README.md` — Flags table adds `PKGPROXY_HOST` to the `--host` row, a short note clarifies that listed env vars are interchangeable with their flags, and the two `podman run` examples (lines 18 and 23) drop `serve --host 0.0.0.0` in favor of `-e PKGPROXY_HOST=0.0.0.0`. +- `CHANGELOG.md` — One concise `[Unreleased]` entry. +- No changes to `.ko.yaml`. +- No changes to landing-page snippets (those describe client-side repo config, not server invocation). +- No changes to e2e tests; they invoke `serve` with explicit flags. diff --git a/openspec/changes/archive/2026-05-10-host-env-var/specs/listen-address-config/spec.md b/openspec/changes/archive/2026-05-10-host-env-var/specs/listen-address-config/spec.md new file mode 100644 index 0000000..ccae1a0 --- /dev/null +++ b/openspec/changes/archive/2026-05-10-host-env-var/specs/listen-address-config/spec.md @@ -0,0 +1,31 @@ +## ADDED Requirements + +### Requirement: Listen host is resolved from flag, then env var, then default +The `serve` subcommand SHALL resolve the listen host using the following ordered precedence, with each step producing the value used by the HTTP server's listen socket: + +1. The value of `--host` when the user explicitly passed the flag on the command line (detected via Cobra's `cmd.Flag("host").Changed` returning `true`). +2. The value of the `PKGPROXY_HOST` environment variable when it is set to a non-empty string. +3. The built-in default `localhost`. + +An empty `PKGPROXY_HOST` (set but empty, or unset) SHALL be treated as "no env-var input" and SHALL fall through to step 3. The listen port resolution is unaffected by this change and continues to come exclusively from the `--port` flag and its built-in default. + +#### Scenario: Explicit `--host` overrides everything +- **WHEN** the binary is started with `serve --host 192.168.10.4` and `PKGPROXY_HOST=10.0.0.1` is set +- **THEN** the HTTP server SHALL bind to `192.168.10.4:8080` + +#### Scenario: Explicit `--host localhost` is honored +- **WHEN** the binary is started with `serve --host localhost` and `PKGPROXY_HOST=0.0.0.0` is set +- **THEN** the HTTP server SHALL bind to `localhost:8080` +- **AND** the env var SHALL NOT override the explicit flag value, even though it equals the built-in default + +#### Scenario: `PKGPROXY_HOST` is used when the flag is absent +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST=0.0.0.0` is set +- **THEN** the HTTP server SHALL bind to `0.0.0.0:8080` + +#### Scenario: Empty `PKGPROXY_HOST` falls through to default +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST=` (set but empty) +- **THEN** the HTTP server SHALL bind to `localhost:8080` + +#### Scenario: Neither flag nor env var produces the built-in default +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST` is unset +- **THEN** the HTTP server SHALL bind to `localhost:8080` diff --git a/openspec/changes/archive/2026-05-10-host-env-var/tasks.md b/openspec/changes/archive/2026-05-10-host-env-var/tasks.md new file mode 100644 index 0000000..b0a3b21 --- /dev/null +++ b/openspec/changes/archive/2026-05-10-host-env-var/tasks.md @@ -0,0 +1,35 @@ +## 1. Resolver helper and wiring + +- [x] 1.1 Add `hostEnvVar = "PKGPROXY_HOST"` constant in `cmd/serve.go` near `publicHostEnvVar` +- [x] 1.2 Add `resolveListenHost(flagChanged bool, flagValue, envValue string) string` helper in `cmd/serve.go` implementing the flag → env → default precedence +- [x] 1.3 Update `PersistentPreRunE` in `newServeCommand()` to call `resolveListenHost(cmd.Flag("host").Changed, listenAddress, os.Getenv(hostEnvVar))` and assign the result back to `listenAddress` before calling `initConfig()` + +## 2. Unit tests + +- [x] 2.1 Add `TestResolveListenHost` to `cmd/serve_test.go` mirroring the table-driven style of `TestResolvePublicAddr` +- [x] 2.2 Cover scenarios: flag changed wins over env var, flag changed wins even when value equals default, env var used when flag unchanged, empty env var falls through to default, neither set returns default +- [x] 2.3 Run `go test ./cmd/... -run TestResolveListenHost` and confirm all subtests pass + +## 3. Documentation + +- [x] 3.1 In `README.md`, add `PKGPROXY_HOST` to the `Env Variable` column of the `--host` row in the flags table +- [x] 3.2 In `README.md`, add a short sentence near the flags table noting that any env var listed in the table may be set in the environment instead of passing the flag +- [x] 3.3 In `README.md`, update the two container-run examples (the `podman run …` snippets near the top under "Run the code") to use `-e PKGPROXY_HOST=0.0.0.0` and drop the `serve --host 0.0.0.0` arguments +- [x] 3.4 Add a concise (80–100 char) entry under `## [Unreleased]` in `CHANGELOG.md` describing the new env var + +## 4. Validation + +- [x] 4.1 Run `make ci-check` and confirm lint, govulncheck, and unit tests all pass +- [x] 4.2 Run `pre-commit run --all-files` and resolve any findings +- [x] 4.3 Run `make e2e` (or at minimum one distro, e.g. `make e2e DISTRO=fedora`) to confirm nothing in the e2e flow regressed + +## 5. Manual verification + +Verify the headline UX goal end-to-end. Run from a clean shell so leftover env vars don't influence results. + +- [x] 5.1 Build the binary: `make build` +- [x] 5.2 Default behavior (no flag, no env): `./bin/pkgproxy serve` in one shell, then in another run `ss -tlnp | grep 8080` (or `curl -sI http://127.0.0.1:8080/` and `curl -sI --connect-timeout 2 http://:8080/`). Expect a listener on `127.0.0.1:8080` only — connections from a non-loopback address must fail. +- [x] 5.3 Env var overrides default: `PKGPROXY_HOST=0.0.0.0 ./bin/pkgproxy serve`. Expect the listener to be on `0.0.0.0:8080` and a `curl` from a non-loopback address to succeed. +- [x] 5.4 Explicit flag wins over env: `PKGPROXY_HOST=0.0.0.0 ./bin/pkgproxy serve --host localhost`. Expect a listener on `127.0.0.1:8080` only (env var is ignored). +- [x] 5.5 Empty env var falls through: `PKGPROXY_HOST= ./bin/pkgproxy serve`. Expect the same listener as 5.2 (`127.0.0.1:8080` only). +- [x] 5.6 Container scenario (the original motivation): `make image-build`, then `podman run --rm -p 8080:8080 -e PKGPROXY_HOST=0.0.0.0 ko.local/pkgproxy:` (use the tag printed by `image-build`), and from the host run `curl -sI http://127.0.0.1:8080/`. Expect a `200 OK` for the landing page. Repeat the same `podman run` without `-e PKGPROXY_HOST=…` and confirm the curl now fails or hangs (server bound to container-local `localhost`, unreachable from the host). diff --git a/openspec/specs/listen-address-config/spec.md b/openspec/specs/listen-address-config/spec.md new file mode 100644 index 0000000..ccae1a0 --- /dev/null +++ b/openspec/specs/listen-address-config/spec.md @@ -0,0 +1,31 @@ +## ADDED Requirements + +### Requirement: Listen host is resolved from flag, then env var, then default +The `serve` subcommand SHALL resolve the listen host using the following ordered precedence, with each step producing the value used by the HTTP server's listen socket: + +1. The value of `--host` when the user explicitly passed the flag on the command line (detected via Cobra's `cmd.Flag("host").Changed` returning `true`). +2. The value of the `PKGPROXY_HOST` environment variable when it is set to a non-empty string. +3. The built-in default `localhost`. + +An empty `PKGPROXY_HOST` (set but empty, or unset) SHALL be treated as "no env-var input" and SHALL fall through to step 3. The listen port resolution is unaffected by this change and continues to come exclusively from the `--port` flag and its built-in default. + +#### Scenario: Explicit `--host` overrides everything +- **WHEN** the binary is started with `serve --host 192.168.10.4` and `PKGPROXY_HOST=10.0.0.1` is set +- **THEN** the HTTP server SHALL bind to `192.168.10.4:8080` + +#### Scenario: Explicit `--host localhost` is honored +- **WHEN** the binary is started with `serve --host localhost` and `PKGPROXY_HOST=0.0.0.0` is set +- **THEN** the HTTP server SHALL bind to `localhost:8080` +- **AND** the env var SHALL NOT override the explicit flag value, even though it equals the built-in default + +#### Scenario: `PKGPROXY_HOST` is used when the flag is absent +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST=0.0.0.0` is set +- **THEN** the HTTP server SHALL bind to `0.0.0.0:8080` + +#### Scenario: Empty `PKGPROXY_HOST` falls through to default +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST=` (set but empty) +- **THEN** the HTTP server SHALL bind to `localhost:8080` + +#### Scenario: Neither flag nor env var produces the built-in default +- **WHEN** the binary is started with `serve` (no `--host`) and `PKGPROXY_HOST` is unset +- **THEN** the HTTP server SHALL bind to `localhost:8080`