diff --git a/.github/workflows/ci-pr.yml b/.github/workflows/ci-pr.yml index ca78ad6..f4787e8 100644 --- a/.github/workflows/ci-pr.yml +++ b/.github/workflows/ci-pr.yml @@ -27,9 +27,9 @@ jobs: run: make test - name: Set up golangci-lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 with: - version: v1.64.7 + version: v2.12.2 args: --timeout=5m - name: Build CLI diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..6ef8a1f --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,15 @@ +version: "2" + +linters: + enable: + - errcheck + - staticcheck + - unused + + settings: + errcheck: + # fmt writes to io.Writer (stdout/stderr) never need error checking in CLI output. + exclude-functions: + - fmt.Fprint + - fmt.Fprintln + - fmt.Fprintf diff --git a/.team/backlog.md b/.team/backlog.md deleted file mode 100644 index 02e1c71..0000000 --- a/.team/backlog.md +++ /dev/null @@ -1,28 +0,0 @@ -# Team Backlog - -Closed items: `.team/done/` - -Priority order is top-to-bottom within this table (highest first). - -| ID | Title | Type | Priority | Size | Status | Source | Spec | -|-------|-------|------|----------|------|--------|--------|------| -| W-048 | Audit all mock test cases and reduce duplication | refactor | P2 | M | approved-spec | User | `/Users/james/dev/github/stenh0use/hind/.team/specs/0003-mocks-audit.md` | -| W-049 | Audit provider/dockercli naming consistency and options patterns | refactor | P2 | M | approved-spec | User | `/Users/james/dev/github/stenh0use/hind/.team/specs/0004-provider-dockercli-naming-audit.md` | -| W-031 | Add open subcommand to open the web ui of a component | feature | P3 | M | needs-spec | User | `W-031.md` | -| W-032 | Add login subcommand to exec into an interactive shell in a node | feature | P3 | M | needs-spec | User | `W-032.md` | -| W-030 | Build and publish releases to brew for macos install | feature | P3 | L | needs-spec | User | `W-030.md` | -| W-044 | Image digest pinning for cluster start | feature | P3 | M | needs-spec | Architecture review | — | -| W-025 | Publish container images to an OCI registry on version update | feature | P4 | XL | needs-spec | User | `W-025.md` | -| W-029 | Add ingress controller for routing traffic to the internal network | feature | P4 | XL | needs-spec | User | `W-029.md` | - - -Notes: -- still lots of duplication in cluster package eg. here: /Users/james/dev/github/stenh0use/hind/pkg/cluster/domain/helpers.go -- Need to review build package, is the abstraction still right, and file package -- cmd mocks/tests should go in testing _test files -- dockercli inspect/list struct naming normalization -- docker impl has too much business logic ingrained there, should be at the application layer. -- docker tmfs need cleaning up otherwise they fill the vm disk -- e2e test cases shouldn't need to clean up first if dynamic ports are used. -- update go / references to Go 1.26 -- change commands to list alias ls, and remove alias rm diff --git a/.team/bugs.md b/.team/bugs.md deleted file mode 100644 index 3556443..0000000 --- a/.team/bugs.md +++ /dev/null @@ -1,27 +0,0 @@ -# Bugs - -Closed items: `.team/done/bugs/` - -Only active (unresolved) bugs are listed in this file. Resolved bugs are moved to `.team/done/bugs/BUG-xxx.md`. - ---- - - - ---- - -_No active unresolved bugs currently tracked on `dev`._ diff --git a/.team/deferred/2026-04-30-hind-releases.md b/.team/deferred/2026-04-30-hind-releases.md deleted file mode 100644 index 04bc6af..0000000 --- a/.team/deferred/2026-04-30-hind-releases.md +++ /dev/null @@ -1,405 +0,0 @@ -# Hind Releases Feature Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Implement the `hind releases` command surface so `hind-releases.feature` scenario "List available hind versions" is fully covered, and the feature file is normalized to remove the two incomplete scenario stubs. - -**Architecture:** A new `pkg/cmd/hind/releases/` command package follows the existing Cobra command pattern (constructor + `runE`). All version data comes from `pkg/build/release` (already implemented); the command layer is presentation-only and uses `text/tabwriter` to render a sorted, column-labeled table matching the feature contract. No new release-layer logic is required. - -**Tech Stack:** Go 1.21+, Cobra, `text/tabwriter`, `pkg/build/release`, `cmd.IOStreams`. - ---- - -## File structure - -| Path | Status | Responsibility | -|------|--------|---------------| -| `pkg/cmd/hind/releases/releases.go` | Create | Command constructor + `runE` — renders sorted version table | -| `pkg/cmd/hind/releases/releases_test.go` | Create | Table-driven tests for column order, row order, header format, and empty-data edge case | -| `pkg/cmd/hind/root.go` | Modify | Register `releases.NewCommand` on root | -| `features/hind-releases.feature` | Modify | Remove the two empty/stub scenarios; normalize Background and List scenario wording | - ---- - -## Task 1: Normalize `hind-releases.feature` - -**Files:** -- Modify: `features/hind-releases.feature` - -The feature file has two malformed stub scenarios ("Create new hind cluster" and "Run non existent hind version") that have no steps and are out of scope for B-020. Remove them. Normalize the Background and the "List available hind versions" scenario to be precise enough for acceptance testing. - -- [ ] **Step 1: Read the current feature file** - -Open `features/hind-releases.feature` and confirm the three scenarios and their step states. - -- [ ] **Step 2: Write the normalized feature file** - -Replace the full file with: - -```gherkin -Feature: HIND releases menu - As a maintainer of the HIND CLI - I want an easy way to view the hind versions and the version of the HashiCorp binaries that are included - So that releases can easily be built and published - - Background: - Given I have defined the hind version in the version configuration - And the hind version has the defined consul version - And the vault version - And the nomad version - - Scenario: List available hind versions - Given I run the "hind releases" command - When I execute the command - Then the CLI will list in a table the available hind versions - And the column header row is printed first with columns: HIND, CONSUL, NOMAD, VAULT - And the first column is the hind version - And the remaining columns are displayed in alphabetical order: consul, nomad, vault - And the latest version is on the first row - And the oldest version is on the last row -``` - -- [ ] **Step 3: Commit** - -```bash -git add features/hind-releases.feature -git commit -m "feat(B-020): normalize hind-releases.feature — remove stubs, clarify list scenario" -``` - ---- - -## Task 2: Implement `pkg/cmd/hind/releases` command - -**Files:** -- Create: `pkg/cmd/hind/releases/releases.go` - -The command lists all hind releases sorted newest-first as a `tabwriter` table with columns: `HIND`, `CONSUL`, `NOMAD`, `VAULT` (alphabetical after HIND). The latest version appears on the first row. - -- [ ] **Step 1: Write the failing test first (see Task 3 — do Task 3 step 1 before this)** - -(Tests are in Task 3. Write tests before implementation per TDD sequence. Come back here after Task 3 Step 1.) - -- [ ] **Step 2: Write the implementation** - -Create `/Users/james/dev/github/stenh0use/hind/pkg/cmd/hind/releases/releases.go`: - -```go -// Package releases implements the "hind releases" command. -// It renders a sorted table of all hind releases and their HashiCorp component versions. -package releases - -import ( - "fmt" - "sort" - "text/tabwriter" - - "github.com/apex/log" - "github.com/spf13/cobra" - - "github.com/stenh0use/hind/pkg/build/release" - "github.com/stenh0use/hind/pkg/cmd" -) - -// NewCommand returns a new cobra.Command for listing hind releases. -func NewCommand(logger *log.Logger, streams cmd.IOStreams) *cobra.Command { - command := &cobra.Command{ - Use: "releases", - Short: "List available hind releases and their HashiCorp component versions", - Long: "List all available hind releases in a table sorted by version (latest first).", - Args: cobra.NoArgs, - RunE: func(cmd *cobra.Command, args []string) error { - return runE(logger, streams) - }, - } - return command -} - -func runE(logger *log.Logger, streams cmd.IOStreams) error { - logger.Debug("Listing hind releases") - - versions := release.List() - if len(versions) == 0 { - fmt.Fprintln(streams.ErrOut, "No releases found") - return nil - } - - // Sort versions descending (latest first) using semver-style string comparison. - // Versions follow "MAJOR.MINOR.PATCH" format so lexicographic sort is valid - // when zero-padded; use sort.Slice with reverse string comparison as a conservative - // baseline. For strict semver ordering in future, switch to golang.org/x/mod/semver. - sort.Slice(versions, func(i, j int) bool { - return versions[i] > versions[j] - }) - - w := tabwriter.NewWriter(streams.Out, 0, 0, 3, ' ', 0) - fmt.Fprintln(w, "HIND\tCONSUL\tNOMAD\tVAULT") - - for _, v := range versions { - info, err := release.Get(v) - if err != nil { - // Skip unknown versions; should not occur with List() output. - logger.Warnf("skipping unknown release %q: %v", v, err) - continue - } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", info.Hind, info.Consul, info.Nomad, info.Vault) - } - - return w.Flush() -} -``` - -- [ ] **Step 3: Run go vet** - -```bash -go vet ./pkg/cmd/hind/releases/... -``` - -Expected: no output (success). - -- [ ] **Step 4: Run the tests** - -```bash -go test ./pkg/cmd/hind/releases/ -``` - -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add pkg/cmd/hind/releases/releases.go -git commit -m "feat(B-020): add hind releases command — table-rendered sorted release list" -``` - ---- - -## Task 3: Write tests for `releases` command - -**Files:** -- Create: `pkg/cmd/hind/releases/releases_test.go` - -Tests use `cmd.IOStreams` with `bytes.Buffer` and a test-scoped `release.Data` to assert header format, column order, row order (latest first), and the empty-data branch. - -Because `runE` calls `release.List()` and `release.Get()` from the package-level store (which is fixed at compile time), the test approach injects via a function var seam — the same pattern used in `pkg/cmd/hind/build/build.go`. Alternatively, since the package-level store is immutable and deterministic, we can test against the real store and assert structural properties (header present, at least one row, latest version on first row) rather than injecting a fake store. This is simpler and avoids adding a seam solely for tests. - -Choose the real-store approach: tests assert: -1. Header row is first and contains exactly `HIND`, `CONSUL`, `NOMAD`, `VAULT`. -2. Output has at least two rows (header + at least one version). -3. First data row corresponds to `release.Latest().Hind`. -4. Each data row has four tab-separated fields. -5. Empty error output on success. - -- [ ] **Step 1: Write the failing tests** - -Create `/Users/james/dev/github/stenh0use/hind/pkg/cmd/hind/releases/releases_test.go`: - -```go -package releases - -import ( - "bytes" - "strings" - "testing" - - "github.com/apex/log" - - "github.com/stenh0use/hind/pkg/build/release" - "github.com/stenh0use/hind/pkg/cmd" -) - -func testStreams() (cmd.IOStreams, *bytes.Buffer, *bytes.Buffer) { - out := &bytes.Buffer{} - errOut := &bytes.Buffer{} - return cmd.IOStreams{Out: out, ErrOut: errOut}, out, errOut -} - -func testLogger() *log.Logger { - return log.Log -} - -func TestRunE_HeaderRow(t *testing.T) { - streams, out, errOut := testStreams() - if err := runE(testLogger(), streams); err != nil { - t.Fatalf("runE() unexpected error: %v", err) - } - if errOut.Len() != 0 { - t.Errorf("expected empty stderr, got: %q", errOut.String()) - } - - lines := splitLines(out.String()) - if len(lines) < 2 { - t.Fatalf("expected at least 2 lines (header + 1 release), got %d", len(lines)) - } - - header := lines[0] - for _, col := range []string{"HIND", "CONSUL", "NOMAD", "VAULT"} { - if !strings.Contains(header, col) { - t.Errorf("header row %q missing column %q", header, col) - } - } -} - -func TestRunE_LatestVersionFirstRow(t *testing.T) { - streams, out, _ := testStreams() - if err := runE(testLogger(), streams); err != nil { - t.Fatalf("runE() unexpected error: %v", err) - } - - lines := splitLines(out.String()) - if len(lines) < 2 { - t.Fatalf("expected at least 2 lines, got %d", len(lines)) - } - - // First data row (index 1) should start with latest hind version. - latest := release.Latest().Hind - firstDataRow := lines[1] - if !strings.HasPrefix(strings.TrimSpace(firstDataRow), latest) { - t.Errorf("first data row %q does not start with latest version %q", firstDataRow, latest) - } -} - -func TestRunE_DataRowsHaveFourFields(t *testing.T) { - streams, out, _ := testStreams() - if err := runE(testLogger(), streams); err != nil { - t.Fatalf("runE() unexpected error: %v", err) - } - - lines := splitLines(out.String()) - // Skip header line. - for i, line := range lines[1:] { - fields := strings.Fields(line) - if len(fields) != 4 { - t.Errorf("data row %d %q: expected 4 fields, got %d", i+1, line, len(fields)) - } - } -} - -func TestNewCommand_Structure(t *testing.T) { - streams, _, _ := testStreams() - cmd := NewCommand(testLogger(), streams) - - if cmd.Use != "releases" { - t.Errorf("Use = %q, want %q", cmd.Use, "releases") - } - if cmd.Args == nil { - t.Error("Args validator is nil; expected cobra.NoArgs") - } - if cmd.RunE == nil { - t.Error("RunE is nil") - } -} - -// splitLines returns non-empty lines from output. -func splitLines(s string) []string { - var result []string - for _, line := range strings.Split(s, "\n") { - if strings.TrimSpace(line) != "" { - result = append(result, line) - } - } - return result -} -``` - -- [ ] **Step 2: Run the test to confirm it fails (package does not exist yet)** - -```bash -go test ./pkg/cmd/hind/releases/ -``` - -Expected: compile error — package not found. This is the TDD red state. - -- [ ] **Step 3: Implement (return to Task 2, Step 2 above)** - -- [ ] **Step 4: Re-run tests after implementation** - -```bash -go test ./pkg/cmd/hind/releases/ -``` - -Expected: PASS (all four tests green). - -- [ ] **Step 5: Commit test file** - -```bash -git add pkg/cmd/hind/releases/releases_test.go -git commit -m "test(B-020): add releases command table-driven tests" -``` - ---- - -## Task 4: Register releases command on root - -**Files:** -- Modify: `pkg/cmd/hind/root.go` - -- [ ] **Step 1: Add the import** - -In `/Users/james/dev/github/stenh0use/hind/pkg/cmd/hind/root.go`, add to the import block: - -```go -"github.com/stenh0use/hind/pkg/cmd/hind/releases" -``` - -- [ ] **Step 2: Register the command** - -In the `NewCommand` function body, after the existing `rootCmd.AddCommand(...)` calls, add: - -```go -rootCmd.AddCommand(releases.NewCommand(logger, streams)) -``` - -- [ ] **Step 3: Run go vet** - -```bash -go vet ./pkg/cmd/hind/... -``` - -Expected: no output. - -- [ ] **Step 4: Run full test suite** - -```bash -make test -``` - -Expected: PASS across all packages. - -- [ ] **Step 5: Verify CLI wiring** - -```bash -make hind-cli -./bin/hind releases -``` - -Expected: table with header `HIND CONSUL NOMAD VAULT` followed by release rows, latest version (0.4.0) on first row. - -- [ ] **Step 6: Commit** - -```bash -git add pkg/cmd/hind/root.go -git commit -m "feat(B-020): register releases subcommand on root hind command" -``` - ---- - -## Self-review against spec - -**Spec coverage check:** - -| Feature requirement | Task that covers it | -|---|---| -| "list in a table the available hind versions" | Task 2 — tabwriter table output | -| "column names on the first row" | Task 2 — `HIND\tCONSUL\tNOMAD\tVAULT` header; Task 3 TestRunE_HeaderRow | -| "first column is hind version" | Task 2 — first tab field is `info.Hind`; Task 3 TestRunE_DataRowsHaveFourFields | -| "remaining columns in alphabetical order consul, nomad, vault" | Task 2 — column order is hardcoded `CONSUL\tNOMAD\tVAULT`; Task 3 TestRunE_HeaderRow | -| "latest version on first row" | Task 2 — descending sort; Task 3 TestRunE_LatestVersionFirstRow | -| "oldest version on last row" | Task 2 — descending sort covers this implicitly; no separate test needed since sort order is the same invariant | -| Feature file normalization — remove stubs | Task 1 | -| Command registered and reachable | Task 4 | - -**Gap check:** No spec requirement without a task. The two incomplete stubs ("Create new hind cluster", "Run non existent hind version") are explicitly out of scope for B-020; they are removed in Task 1 rather than implemented. - -**Placeholder scan:** No TBD/TODO patterns; all steps have concrete code. - -**Type consistency:** `release.List()` returns `[]string`; `release.Get(v)` returns `(release.Info, error)` — both used consistently in Task 2 and Task 3. diff --git a/.team/deferred/README.md b/.team/deferred/README.md deleted file mode 100644 index d9ffc07..0000000 --- a/.team/deferred/README.md +++ /dev/null @@ -1,2 +0,0 @@ -# Deferred items -This folder contains deferred or won't do items diff --git a/.team/done/plans/P-013.md b/.team/done/plans/P-013.md deleted file mode 100644 index dfbad48..0000000 --- a/.team/done/plans/P-013.md +++ /dev/null @@ -1,367 +0,0 @@ -# P-013 — Implementation Plan: Migrate image build runtime to pkg/provider -Status: approved -Spec: B-013.md - ---- - -## Overview - -This plan sequences the elimination of `pkg/build/image/internal/docker` as a runtime dependency of `pkg/build/image`, routing all Docker operations through `pkg/provider` interfaces. Five phases with a coupled-commit constraint on Phases 3 and 4. - ---- - -## Phase 1 — Provider contract expansion - -**Goal:** Extend `pkg/provider/image.go` with `BuildImageResult` and the full `BuildImageOptions` fields. Update the `Client` interface signature. Everything compiles; no runtime behaviour changes. - -### Files to change - -**`pkg/provider/image.go`** - -Replace the current `BuildImageOptions` struct with: - -```go -type BuildImageOptions struct { - Name string - Tag string - ContextDir string - BuildArgs map[string]string - Dockerfile string // optional; empty means default Dockerfile - WithCache bool // pass --no-cache when false - Platform string // optional; empty means omit --platform — do not supply a default -} -``` - -Add the new result type immediately below `BuildImageOptions`: - -```go -// BuildImageResult is the structured result of a successful image build. -// Both fields are non-empty on success; an empty Digest or ImageRef is an error condition. -type BuildImageResult struct { - Digest string // sha256 digest, e.g. "sha256:abc123..." - ImageRef string // fully qualified ref, e.g. "hind/consul:0.1.0" -} -``` - -**`pkg/provider/provider.go`** - -Change the `BuildImage` method signature on `Client`: - -```go -BuildImage(ctx context.Context, opts BuildImageOptions) (BuildImageResult, error) -``` - -**`pkg/provider/dockercli/build.go`** - -Update the stub implementation to match the new signature (return `BuildImageResult` instead of `string`). The body can return `BuildImageResult{}, fmt.Errorf(...)` for now — full implementation is Phase 2. This is just enough to restore compilation. - -### Tests to add or update - -- No new tests required in this phase (it is additive and the adapter body is still a stub). -- Confirm `make test` passes (compilation gate only). - -### Definition of done - -- `go build ./...` succeeds with no errors. -- `make test` passes. -- `provider.Client.BuildImage` signature matches `(ctx, BuildImageOptions) (BuildImageResult, error)` exactly. -- `BuildImageOptions` carries all six fields listed in the spec. -- `BuildImageResult` is exported with `Digest` and `ImageRef` string fields. - ---- - -## Phase 2 — dockercli adapter parity (buildx + digest extraction) - -**Goal:** Implement `BuildImage` in `pkg/provider/dockercli/build.go` with full behavioral parity: buildx availability check at call time, `--load` flag, metadata-file-based digest extraction, structured result return. Introduce a `CommandExecutor`-style seam so unit tests can inject fake execution. - -### Files to change - -**`pkg/provider/dockercli/build.go`** - -1. **CommandExecutor seam.** Define a `CommandExecutor` interface local to the `dockercli` package (or import it from a shared location if one is preferred later). Mirror the interface already in `internal/docker/docker.go`: - - ```go - type CommandExecutor interface { - Run(ctx context.Context, dir string, stdout, stderr io.Writer, name string, args ...string) error - Output(ctx context.Context, dir string, name string, args ...string) ([]byte, error) - } - ``` - - Add an `executor CommandExecutor` field to `Client` in `pkg/provider/dockercli/client.go`. Populate it from a package-level `osCommandExecutor` default in `New(...)`. Tests set it directly before calling `BuildImage`. - - Alternatively (simpler), add a `withExecutor` constructor for tests only: - ```go - func newWithExecutor(logger *log.Logger, exec CommandExecutor) *Client - ``` - -2. **Buildx availability check — at call time.** Inside `BuildImage`, before running the build, call a package-private `checkBuildxAvailable(ctx, executor)` that runs: - - ``` - docker system info --format {{json .}} - ``` - - Parse the JSON (reuse or mirror the `DockerInfo`/`HasClientPlugin` logic from `internal/docker/info.go`). If buildx is absent, return: - - ```go - return BuildImageResult{}, fmt.Errorf("buildx client plugin is needed but not installed") - ``` - - `New(...)` in `client.go` must not call this check. - -3. **Build command.** Construct and run: - - ``` - docker buildx build - -t : - --load - --metadata-file /metadata.json - [--no-cache] // when opts.WithCache == false - [-f ] // when non-empty - [--platform ] // only when non-empty — no default - [--build-arg K=V ...] - . - ``` - - Run the command with the executor, capturing stdout and stderr. On non-zero exit, return a wrapped error including stderr. - -4. **Digest extraction.** After a successful build, read `/metadata.json`, unmarshal into a local `buildMetadata` struct: - - ```go - type buildMetadata struct { - ContainerImageDigest string `json:"containerimage.config.digest"` - ImageName string `json:"image.name"` - } - ``` - - Extract `ContainerImageDigest`. If it is empty or does not begin with `sha256:`, return an error rather than a zero-value result. - -5. **Return.** On success: - - ```go - return provider.BuildImageResult{ - Digest: metadata.ContainerImageDigest, - ImageRef: fmt.Sprintf("%s:%s", opts.Name, opts.Tag), - }, nil - ``` - - Validate both fields are non-empty before returning; return an error if either is empty. - -**`pkg/provider/dockercli/client.go`** - -Add `executor CommandExecutor` field to `Client`. Set it to `osCommandExecutor{}` in `New(...)`. Add unexported `newWithExecutor` for tests. - -**`pkg/provider/dockercli/info.go`** (new file, or inline in build.go) - -Copy or adapt `DockerInfo` and `HasClientPlugin` from `internal/docker/info.go` into the `dockercli` package to support the buildx check. Do not import from `internal/docker` — the goal is provider self-containment. - -### Tests to add or update - -Create `pkg/provider/dockercli/build_test.go`: - -1. **`TestBuildImage_BuildxAbsent`** — inject a fake executor whose `Output` returns JSON with no buildx plugin. Assert `BuildImage` returns a non-nil error containing "buildx". Assert the error is returned immediately (no build command executed). - -2. **`TestBuildImage_Success`** — inject a fake executor that: - - Returns valid `docker system info` JSON with buildx listed. - - On the buildx build command, returns exit 0. - - Writes a minimal `metadata.json` to a temp `ContextDir` with a known `containerimage.config.digest` value (`sha256:abc123...`). - Assert `BuildImageResult.Digest` starts with `sha256:` and equals the injected value. Assert `BuildImageResult.ImageRef` equals `":"`. - -3. **`TestNew_SucceedsWithoutBuildx`** — call `New(logger)` with a fake executor that would return a buildx-absent system info response. Assert `New` returns without error. Assert the returned `provider.Client` is non-nil. - -4. **`TestBuildImage_EmptyDigestIsError`** — inject a fake executor that returns a metadata file with an empty `containerimage.config.digest`. Assert `BuildImage` returns an error. - -5. **`TestBuildImage_LoadFlagPresent`** — capture the args passed to the build command in the fake executor. Assert `--load` is present in the args unconditionally. - -6. **`TestBuildImage_PlatformOmittedWhenEmpty`** — assert `--platform` is absent in args when `opts.Platform == ""`. - -7. **`TestBuildImage_NoCacheWhenWithCacheFalse`** — assert `--no-cache` is present in args when `opts.WithCache == false`. - -### Definition of done - -- All seven tests pass. -- `make test` passes. -- `New(logger)` requires no buildx to succeed (AC-3). -- `BuildImage` returns `BuildImageResult` with non-empty `Digest` (starts `sha256:`) and non-empty `ImageRef` on fake-success path (AC-2). -- `BuildImage` returns a descriptive error when buildx is absent (AC-3). -- `--load` is always present in buildx invocation (spec requirement). -- `--platform` is omitted when `Platform` field is empty (spec requirement). -- No import of `pkg/build/image/internal/docker` in the dockercli package. - ---- - -## Phase 3 + Phase 4 — Decouple image.go and rewire builder.go - -> **Coupled-commit note (from spec):** Phases 3 and 4 must land in the same commit. After Phase 3 alone, `builder.go` still references `internal/docker` types that `image.go` no longer provides, causing a compile break. Implement both changes together and commit atomically. - -### Phase 3 — Remove docker.BuildArg from image.go - -**Goal:** Replace `[]docker.BuildArg` with `map[string]string` in `pkg/build/image/image.go`. Remove the `internal/docker` import from that file. - -#### Files to change - -**`pkg/build/image/image.go`** - -- Remove `import "github.com/stenh0use/hind/pkg/build/image/internal/docker"`. -- Change `packagesToBuildArgs()` return type from `([]docker.BuildArg, error)` to `(map[string]string, error)`. Return a `map[string]string` (key = `CONSUL_VERSION` etc., value = version string). -- Change `buildArgs()` return type from `([]docker.BuildArg, error)` to `(map[string]string, error)`. Merge the package args map and add `HIND_VERSION` and `BASE_IMAGE` keys. -- Both methods remain unexported; only their return types change. - -### Phase 4 — Rewire builder.go to use provider only - -**Goal:** Replace all `internal/docker` usage in `pkg/build/image/builder.go` with `pkg/provider` calls. Remove the `internal/docker` import from that file. Preserve the dependency-check error message verbatim (AC-5). - -#### Files to change - -**`pkg/build/image/builder.go`** - -1. **Constructor change.** `Builder` must receive a `provider.Client`. Update `NewBuilder`: - - ```go - func NewBuilder(logger *log.Logger, client provider.Client, kind release.ImageKind) (*Builder, error) - ``` - - Add `client provider.Client` field to `Builder` struct. - -2. **Remove `internal/docker` import.** Delete all references to `docker.NewImage`, `docker.BuildOptions`, `docker.BuildArg`, `dockerImg.UpdateBuildOptions`, `dockerImg.BuildImage`, `dockerImg.TagExists`. - -3. **Rewire `BuildImage`.** Replace `dockerImg.BuildImage(ctx)` with: - - ```go - result, err := b.client.BuildImage(ctx, provider.BuildImageOptions{ - Name: b.image.Kind.ImageName(), - Tag: b.image.Release, - ContextDir: buildFiles.BuildDir(), - BuildArgs: buildArgs, // map[string]string from b.image.buildArgs() - WithCache: false, - Platform: "", - }) - ``` - - On success, log using `result.ImageRef` (or construct the ref from `Name`/`Tag` as before). The logging line must remain: - - ```go - b.logger.WithField("image", result.ImageRef).Info("Successfully built image") - ``` - -4. **Rewire `checkDependencies`.** Replace `docker.NewImage(...).TagExists(ctx)` with: - - ```go - exists, err := b.client.TagExists(ctx, sanitizedName, b.image.BaseImage.Tag) - ``` - - The error message strings must remain byte-for-byte identical (AC-5): - - ```go - return fmt.Errorf("base image dependency not met: %s\n"+ - "Resolution: Run 'hind build %s' to build the required dependency", - sanitizedName, component) - ``` - -5. **Remove logger from NewBuilder if it is now only used for log calls.** Keep `logger` field if logging is still in use (it is — log start/success). Do not remove it. - -**Callers of `NewBuilder`** — find all call sites with: -``` -grep -rn "NewBuilder(" pkg/ cmd/ -``` -Update each call site to pass `client provider.Client` as the second argument. - -### Tests to add or update - -**`pkg/build/image/builder_test.go`** - -- Existing tests for `NewBuilder` (valid/invalid kinds) must be updated to pass a `provider.Client` argument. Use a fake/stub `provider.Client` that satisfies the interface (implement only the methods exercised — `TagExists` and `BuildImage`). -- Add `TestBuilder_BuildImage_CallsProviderBuildImage` — use a stub client that records the `BuildImageOptions` it receives. Assert `Name`, `Tag`, `ContextDir`, and at least one `BuildArgs` key match expected values. -- Add `TestBuilder_CheckDependencies_CallsProviderTagExists` — use a stub that returns `false` from `TagExists`. Assert the returned error contains both required strings. -- Add `TestBuilder_CheckDependencies_SkipsWhenPull` — stub client whose `TagExists` panics (to detect accidental calls). Set `BaseImage.Pull = true`. Assert no error. - -**`pkg/build/image/image_test.go`** (new file if it does not exist) - -- `TestBuildArgs_ReturnsMapWithExpectedKeys` — call `buildArgs()` on a consul `Image`. Assert map contains keys `CONSUL_VERSION`, `HIND_VERSION`, `BASE_IMAGE`. Assert no `docker.BuildArg` type appears (enforced by compile). -- `TestPackagesToBuildArgs_KnownPackages` — assert all packages for a nomad `Image` produce expected version keys. - -### Definition of done (Phases 3+4 together) - -- `go build ./...` succeeds with no errors. -- `make test` passes. -- `grep "internal/docker" pkg/build/image/image.go` returns no matches (AC-4). -- `grep "internal/docker" pkg/build/image/builder.go` returns no matches (AC-1). -- `packagesToBuildArgs()` and `buildArgs()` return `map[string]string` (AC-4). -- Dependency-check error message strings are character-for-character identical to pre-migration output (AC-5). -- All builder tests pass using only `provider.Client` stub — no `internal/docker` concrete type in test file. - ---- - -## Phase 5 — Delete or shim pkg/build/image/internal/docker - -**Goal:** Satisfy AC-7. Preferred outcome per spec is full deletion. - -### Preconditions - -- AC-1 and AC-4 are green: no `.go` file in `pkg/build/image` (outside `internal/docker` itself) imports `internal/docker`. -- Verify with: `grep -r "internal/docker" pkg/build/image/ --include="*.go"` — only matches inside `pkg/build/image/internal/docker/` itself. - -### Files to change - -**Delete the entire directory:** - -``` -pkg/build/image/internal/docker/docker.go -pkg/build/image/internal/docker/docker_test.go -pkg/build/image/internal/docker/info.go -``` - -If any logic in `internal/docker` is still needed (e.g., `DockerInfo` JSON parsing), verify it has been reproduced in `pkg/provider/dockercli` (Phase 2) before deleting. - -**If full deletion is not yet safe** (e.g., a test dependency remains), reduce to a shim: - -- Replace `docker.go` and `info.go` with a single `shim.go` containing only the package declaration and a prominent comment: - - ```go - // Package docker is a deprecated shim scheduled for removal. - // Tracked by: - // No production code path imports this package. - package docker - ``` - - Delete `docker_test.go` entirely (tests for deleted logic have no value). - -### Tests to add or update - -- No new tests. Ensure `make test` still passes after deletion. -- Run `go vet ./...` to catch any stale references. - -### Definition of done - -- `make test` passes. -- `go build ./...` succeeds. -- Either: - - `pkg/build/image/internal/docker` directory does not exist, or - - It contains only a shim file with a comment referencing a removal ticket, and `grep -r "internal/docker" pkg/build/image/ --include="*.go"` returns matches only inside that directory. -- AC-7 is satisfied. - ---- - -## Acceptance criteria verification checklist - -| AC | Verified by | -|----|-------------| -| AC-1 | `grep -r "internal/docker" pkg/build/image/ --include="*.go"` — no matches outside the `internal/docker` dir itself | -| AC-2 | Unit test `TestBuildImage_Success` passes; asserts `Digest` starts `sha256:` and `ImageRef` is non-empty | -| AC-3 | `TestBuildImage_BuildxAbsent` and `TestNew_SucceedsWithoutBuildx` pass | -| AC-4 | `grep "internal/docker" pkg/build/image/image.go` — no matches; return types compile as `map[string]string` | -| AC-5 | `TestBuilder_CheckDependencies_CallsProviderTagExists` asserts exact error strings; manual smoke test | -| AC-6 | Manual: `make hind-cli && ./bin/hind build consul`; then `docker images -q hind/consul:` returns non-empty | -| AC-7 | `internal/docker` directory deleted or reduced to documented shim with no production importers | - ---- - -## Commit strategy - -| Phase | Commit message | -|-------|---------------| -| 1 | `feat(provider): add BuildImageResult and expand BuildImageOptions` | -| 2 | `feat(dockercli): implement BuildImage with buildx, --load, and digest extraction` | -| 3+4 | `refactor(image): decouple from internal/docker, wire builder through provider` | -| 5 | `chore(image): delete internal/docker package` | - -Each phase commit must leave `go build ./...` and `make test` green, except that Phases 3 and 4 are a single commit and must not be split. diff --git a/.team/done/specs/W-013.md b/.team/done/specs/W-013.md deleted file mode 100644 index df381d9..0000000 --- a/.team/done/specs/W-013.md +++ /dev/null @@ -1,186 +0,0 @@ -# B-013 Spec — Migrate image build runtime interactions from `internal/docker` to `pkg/provider` - -Status: revised spec — staff approved (2026-05-01) -Source work item: B-013 - -## Goal - -Eliminate the direct `pkg/build/image/internal/docker` dependency from `pkg/build/image`, routing all runtime Docker operations through `pkg/provider` interfaces instead. As part of this migration, `BuildImage` on the provider interface gains a structured, digest-bearing result type so callers never parse provider-private files. - -## Scope completed -- Discovery/spec only completed for B-013; no product-code edits were made. -- Runtime interactions in image build flows were traced and mapped from `pkg/build/image/internal/docker` to target `pkg/provider` seams. - -## Inventory of `internal/docker` usages in image build flow -- Direct package imports and call paths: - - `pkg/build/image/builder.go` - - imports `pkg/build/image/internal/docker` - - constructs `docker.NewImage(...)` - - uses `UpdateBuildOptions(...)` - - invokes `BuildImage(ctx)` for runtime build - - uses `TagExists(ctx)` during base-image dependency checks - - `pkg/build/image/image.go` - - imports `pkg/build/image/internal/docker` - - returns `[]docker.BuildArg` from `packagesToBuildArgs()` and `buildArgs()` (domain/model leak) -- Runtime command interactions encapsulated in `internal/docker/docker.go`: - - `docker system info --format {{json .}}` (plugin/dependency preflight) - - `docker buildx build ... --metadata-file metadata.json` (image build) - - `docker images -q ` (tag existence) - - metadata file read/parse from build context (`metadata.json`) to obtain digest - -## Provider interfaces/adapters required for replacement -- Existing provider surface (present): - - `provider.Client.BuildImage(ctx, opts)` - - `provider.Client.TagExists(ctx, name, tag)` - - `provider.Client.PullImage(ctx, name, tag)` -- Required additive contract for parity: - - `BuildImage` must return structured output (at least digest + image ref), not empty string. - - Build options must support deterministic build args and any buildx parity options needed by current flow (metadata capture, platform/cache toggles where required). - - Buildx availability must be verified inside `BuildImage` at call time (see API contract below). - - Adapter-owned metadata extraction strategy (provider returns digest directly; callers should not parse provider-private files). -- Adapter changes: - - `pkg/provider/dockercli/build.go` must migrate from `docker build` to buildx-capable flow (or equivalent digest-producing strategy) to match existing behavior. - -## API contract - -### BuildImage return type - -The current `BuildImage(ctx, opts) (string, error)` signature on `provider.Client` changes to: - -```go -BuildImage(ctx context.Context, opts BuildImageOptions) (BuildImageResult, error) -``` - -`BuildImageResult` is a new exported struct defined in `pkg/provider/image.go`: - -```go -type BuildImageResult struct { - Digest string // sha256 digest of the built image (e.g. "sha256:abc123...") - ImageRef string // fully qualified image reference (e.g. "hind/consul:0.1.0") -} -``` - -Both fields must be non-empty on a successful build. A result with an empty `Digest` or empty `ImageRef` is an error condition; the adapter must return an error rather than a zero-value struct. - -### Buildx availability check — at call time, inside `BuildImage`, not at construction - -Buildx availability is checked inside the `dockercli` adapter's `BuildImage` implementation, not in `dockercli.New(...)`. `dockercli.New` must succeed regardless of whether buildx is installed; it performs no buildx probe. There is no capability-check method on `provider.Client`. When `BuildImage` is called and buildx is not present, the method returns a descriptive error immediately. When buildx is available, the build proceeds normally. Users who start a cluster using already-published images and never call `BuildImage` are entirely unaffected. - -### Docker CLI adapter parity — `--load` flag - -The `dockercli` adapter must pass `--load` to the buildx command unconditionally. Without `--load`, buildx may complete successfully without making the image available in the local Docker image store, which would silently break any subsequent `TagExists` check or container creation referencing that image. - -### BuildImageOptions expansion - -`BuildImageOptions` in `pkg/provider/image.go` must be extended to carry the fields the adapter needs: - -```go -type BuildImageOptions struct { - Name string - Tag string - ContextDir string - BuildArgs map[string]string - Dockerfile string // optional; empty means default Dockerfile - WithCache bool // pass --no-cache when false - Platform string // optional; empty string means omit --platform entirely — do not pass a default -} -``` - -`BuildArgs` remains `map[string]string` (provider-neutral). The `internal/docker.BuildArg` slice type is not exposed outside `pkg/build/image/internal/docker`. - -The `Platform` field zero-value is intentionally "omit". When `Platform` is empty the adapter must not pass a `--platform` flag to buildx. The adapter must not substitute any default platform string. - -## Behavioral invariant — dependency-check error message - -The error output produced when a required base image is absent must contain the following exact strings: - -- The plugin-not-found message: `base image dependency not met: ` -- The resolution hint: `Resolution: Run 'hind build ' to build the required dependency` - -The orchestration path responsible for this message may be refactored, but the output observed by the user must match these strings verbatim. QA validates by removing the local base image and running the dependent build command (see AC-5). - -## Migration estimate by component/call path -- Component A: Provider contract expansion (`pkg/provider` types + interface) - - Size: M - - Risk: low-medium (additive API changes, downstream compile impact manageable) -- Component B: Docker CLI adapter parity (`pkg/provider/dockercli/build.go`) - - Size: M-L - - Risk: medium-high (behavior parity around digest/metadata/error handling, `--load` flag, buildx constructor check) -- Component C: Image domain type decoupling (`pkg/build/image/image.go`) - - Size: S-M - - Risk: medium (touches argument plumbing/tests) -- Component D: Build orchestrator rewiring (`pkg/build/image/builder.go`) - - Size: S-M - - Risk: medium (must preserve dependency resolution UX/messages) -- Component E: Legacy package retirement/shim (`pkg/build/image/internal/docker`) - - Size: S-M - - Risk: medium (test migration + cleanup sequencing) - -## Recommended sequencing and blockers -- Phase 1: Add provider result/types and expand `BuildImageOptions` (additive). -- Phase 2: Implement buildx availability check inside `BuildImage`; implement adapter parity for digest-producing builds (buildx + `--load` + metadata extraction). -- Phase 3: Decouple `image.go` from `docker.BuildArg` into provider-neutral args. -- Phase 4: Rewire `builder.go` to provider-only runtime interface. -- Phase 5: Remove direct runtime dependency on `internal/docker`; retain short-lived shim only if needed for rollout safety. - -**Coupled commit note:** Phase 3 (`image.go` type changes) and Phase 4 (`builder.go` rewiring) must land in the same commit. The intermediate state after Phase 3 alone produces a compile break because `builder.go` still references `internal/docker` types that have been removed from `image.go`. Do not leave a passing build between these two phases. - -- Primary blocker: - - `pkg/provider/dockercli/build.go` currently returns an empty build result and lacks buildx metadata parity; orchestration switch should not proceed until adapter parity (digest return, `--load`, call-time buildx capability check inside `BuildImage`) is demonstrated. - -## Guidance for non-conforming call paths -- Any call path that reads `metadata.json` outside provider boundary is non-conforming; move digest derivation into provider adapter and return result via provider types. -- Any domain package returning `internal/docker` types is non-conforming; replace with local/provider-neutral structs and transform at boundary. -- Any direct docker command orchestration outside provider adapters is non-conforming for target architecture. - -## Acceptance criteria - -Each item below is a pass/fail condition. All must pass before this item is closed. - -**AC-1 — No internal/docker import in pkg/build/image after migration** -`grep -r "internal/docker" pkg/build/image/` returns no matches in any `.go` file outside of `pkg/build/image/internal/docker` itself. This includes `builder.go` and `image.go`. - -**AC-2 — BuildImage returns a non-empty structured result** -`provider.Client.BuildImage` returns `(BuildImageResult, error)`. On a successful build, `BuildImageResult.Digest` is non-empty and begins with `sha256:`, and `BuildImageResult.ImageRef` is non-empty and matches the expected `name:tag` form. A unit test on the `dockercli` adapter must assert both fields are populated after a successful mock/fake build invocation. - -**AC-3 — BuildImage returns a descriptive error when buildx is not available** -When `BuildImage` is called on the `dockercli` adapter and the buildx plugin is not present, the method returns a non-nil, descriptive error. When buildx is available, `BuildImage` proceeds normally. `dockercli.New(...)` does not check for buildx and must succeed regardless of whether buildx is installed. `provider.Client` declares no capability-check method. A unit test must assert that when buildx is absent, `BuildImage` returns the error; a separate test must assert that `dockercli.New` succeeds in a buildx-absent environment. Users who never call `BuildImage` are unaffected. - -**AC-4 — image.go build arg functions return provider-neutral types** -`packagesToBuildArgs()` and `buildArgs()` in `pkg/build/image/image.go` return `[]provider.BuildArg` or an equivalent local struct — not `[]docker.BuildArg`. The file has no import of `pkg/build/image/internal/docker`. - -**AC-5 — Dependency-check failure message is unchanged (regression guard)** -Running `hind build nomad` when the `consul` base image does not exist locally produces output containing exactly: -``` -base image dependency not met: -Resolution: Run 'hind build ' to build the required dependency -``` -This message must be identical to the pre-migration output. QA validates by removing the local consul image and running the build command. - -**AC-6 — End-to-end build produces a locally tagged image** -`hind build consul` (or any valid component) completes without error and `docker images -q hind/consul:` returns a non-empty image ID. This validates functional parity with the pre-migration flow, including the `--load` flag ensuring local image store availability. - -**AC-7 — internal/docker is deleted or reduced to a documented shim** -After migration, either: -- `pkg/build/image/internal/docker` is deleted entirely, or -- It is reduced to a named shim with a comment referencing a tracking ticket for final removal, and no production code path imports it. - -A shim is acceptable only if a concrete removal ticket exists. The spec author considers full deletion the preferred outcome. - -## Test migration guidance -- Unit tests to add/update: - - Provider contract tests for digest-bearing build results and validation errors. - - `dockercli` adapter tests for buildx invocation args (including `--load` presence), metadata/digest parsing behavior, and error wrapping. - - `BuildImage` tests asserting the buildx-absent error path, and a separate `dockercli.New` test asserting it succeeds when buildx is absent. - - `builder` tests asserting provider interaction only (no `internal/docker` concrete dependency). - - `image` tests asserting build arg generation without `internal/docker` type coupling. -- Testability guidance — digest extraction seam: - - The `dockercli` adapter should preserve a command-execution seam analogous to the `CommandExecutor` interface in `internal/docker/docker.go`. This allows unit tests to inject a fake executor and assert digest extraction behavior (metadata file parsing, error handling) without running real Docker. The seam may be a field on the adapter struct populated at construction, following the same pattern already established in `internal/docker`. -- Regression expectations: - - Preserve dependency-check failure UX (`hind build ` guidance) and existing tag lookup semantics. - - Preserve build success/failure logging semantics at orchestration layer. - -## Staff verdict -- Verdict: approved -- Reason: B-013 acceptance criteria are fully satisfied as discovery/spec output with explicit migration boundaries, interface requirements, sequencing, blockers, and test guidance. -- Next role: engineer implementation planning/execution, then QA parity validation gate. diff --git a/.team/handoff.md b/.team/handoff.md deleted file mode 100644 index d39033b..0000000 --- a/.team/handoff.md +++ /dev/null @@ -1,51 +0,0 @@ -# Team Handoff - -## Session state - -**repo_root**: `/Users/james/dev/github/stenh0use/hind` -**repo_branch**: `remediate` -**date**: 2026-06-07 -**workflow**: `/Users/james/dev/github/stenh0use/hind/.team/workflow.md` - -> ⚠️ Read `.team/workflow.md` at the start of every session and follow the close-out checklist after each item closes. - ---- - -## W-047 — CLOSED ✅ - -E2e integration test suite is complete. All tests pass on commit `329399a`. All 6 bugs closed. All ACs met. - -**QA verdict**: `/Users/james/dev/github/stenh0use/hind/.team/artifacts/W-047-qa-verdict.md` — PASSED - -### Close-out status -- [x] All bugs 0001–0006 closed and archived to `.team/done/bugs/` -- [x] All inbox items archived to `.team/inbox/done/` -- [x] QA verdict artifact written (PASSED) -- [ ] Commit loose `.team/` files (W-047-qa-verdict.md + handoff.md) -- [ ] Spec/plan archived to done/ - ---- - -## Next recommended action - -**Commit close-out files**, then begin **W-048** (Audit all mock test cases and reduce duplication). - -W-048 has an approved spec at: -- `/Users/james/dev/github/stenh0use/hind/.team/specs/` (check for 0003 or similar) - -If no plan exists yet, dispatch `staff-engineer` to create one from the spec before engineering begins. - ---- - -## Backlog (active order) - -| ID | Title | Type | Priority | Status | -|----|-------|------|----------|--------| -| W-048 | Audit all mock test cases and reduce duplication | refactor | P2 | approved-spec | -| W-049 | Audit provider/dockercli naming consistency and options patterns | refactor | P2 | approved-spec | -| W-031 | Add open subcommand | feature | P3 | needs-spec | -| W-032 | Add login subcommand | feature | P3 | needs-spec | -| W-030 | Build and publish releases to brew | feature | P3 | needs-spec | -| W-044 | Image digest pinning for cluster start | feature | P3 | needs-spec | -| W-025 | Publish container images to OCI registry | feature | P4 | needs-spec | -| W-029 | Add ingress controller | feature | P4 | needs-spec | diff --git a/.team/specs/W-025.md b/.team/specs/W-025.md deleted file mode 100644 index ac35925..0000000 --- a/.team/specs/W-025.md +++ /dev/null @@ -1,54 +0,0 @@ -# W-025 Spec — Publish container images to an OCI registry on version update - -## Problem statement and goals -Today users often build images locally before starting clusters, which increases setup time and introduces machine-to-machine variability. Publishing canonical prebuilt images when tracked component versions change would speed onboarding and provide consistent tested artifacts. - -## Goals -- Automatically publish hind component images to an OCI registry when tracked versions are updated. -- Provide deterministic tags users can reference from `hind build/start` workflows. -- Ensure publish failures are visible and block incomplete release promotion. - -## In-scope -- Registry publishing workflow triggered by version updates (CI-driven primary path). -- Image tagging scheme and manifest publication. -- Authentication and permissions requirements for publishing. -- Validation gates (build/test/scan as defined by release policy) before push. - -## Out-of-scope -- Replacing existing local build capability. -- Full provenance/SBOM redesign beyond current release constraints. -- Multi-registry fan-out at initial launch. - -## Key user flows and expected behavior -1. Maintainer updates tracked package versions and merges change. -2. CI detects version-update scope and builds affected images. -3. CI publishes images and tags to configured OCI registry. -4. Users can pull published tags without local rebuild. - -## Acceptance criteria (testable) -1. A version-update change triggers image publish workflow automatically. -2. Workflow publishes only affected images/components for that change. -3. Tags include at minimum component version tag (e.g., `:`) and a deterministic hind-scoped tag convention. -4. Published images are pullable by authenticated users (or public users if registry policy is public). -5. Publish workflow fails fast on auth/push errors and reports actionable logs. -6. On publish failure, no success/release-complete signal is emitted. -7. Documentation/help text identifies where published images live and expected tag format. - -## Edge cases and error behavior -- Partial publish success: workflow must report failed components clearly and mark run failed. -- Existing tag collision policy must be explicit (immutable tags preferred; if mutable, audit trail required). -- Multi-arch support decision must be enforced consistently (single-arch explicit if MVP). - -## Constraints -- Publishing must run from CI with non-interactive credentials. -- Secrets handling must use platform secret store; no plaintext credentials in repo. -- Workflow must be idempotent for retried runs. - -## Dependencies / staff decisions needed -1. Select target OCI registry (GHCR preferred unless org policy dictates otherwise). -2. Decide tag taxonomy (component version + hind release/SHA strategy). -3. Decide architecture matrix for MVP (amd64 only vs amd64+arm64). -4. Confirm whether manual publish override is required in addition to automatic trigger. - -## Artifact path -- /Users/james/dev/github/stenh0use/hind/.team/specs/W-025.md diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index aeb1108..ccf3c45 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -9,7 +9,7 @@ Thank you for your interest in contributing to hind! This guide will help you ge - Go 1.21 or later - Docker or compatible container runtime - Make -- golangci-lint (v1.64.7 or later) +- golangci-lint (v2.12.2 or later) ### Initial Setup @@ -80,7 +80,7 @@ To install golangci-lint locally: ```bash brew install golangci-lint -# or: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.7 +# or: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v2.12.2 ``` CI enforces golangci-lint on pull requests, so local runs should match CI expectations. diff --git a/pkg/cluster/orchestration/locks_test.go b/pkg/cluster/orchestration/locks_test.go index bc0ad9c..703e037 100644 --- a/pkg/cluster/orchestration/locks_test.go +++ b/pkg/cluster/orchestration/locks_test.go @@ -67,17 +67,6 @@ func (c *countingLifecycle) setWaitGate(op string, gate chan struct{}) { c.waitGateByOp[op] = gate } -func (c *countingLifecycle) snapshot() (startCalled int, stopCalled int, deleteCalled int, scaleCalled int, maxMutations int) { - c.mu.Lock() - defer c.mu.Unlock() - startCalled = c.activeByOp["start"] + (c.maxByOp["start"] - c.activeByOp["start"]) - stopCalled = c.activeByOp["stop"] + (c.maxByOp["stop"] - c.activeByOp["stop"]) - deleteCalled = c.deleteCalled - scaleCalled = c.activeByOp["scale"] + (c.maxByOp["scale"] - c.activeByOp["scale"]) - maxMutations = c.maxMutations - return -} - func waitForMutationEntry(t *testing.T, life *countingLifecycle, op string) { t.Helper() deadline := time.Now().Add(2 * time.Second) @@ -303,7 +292,7 @@ func invokeMutation(svc Service, op string) error { func TestServiceRejectsNilContext(t *testing.T) { t.Parallel() svc := NewService(Options{Lifecycle: &countingLifecycle{}, Scale: passthroughScaler{}, Inspect: passthroughInspector{}, List: passthroughLister{}, Locker: NewInMemoryLocker()}) - _, err := svc.Start(nil, StartRequest{}) + _, err := svc.Start(nil, StartRequest{}) //nolint:staticcheck // intentionally testing nil context rejection require.Error(t, err) assert.Equal(t, "context cannot be nil", err.Error()) } diff --git a/pkg/cluster/persistence/fs/repository.go b/pkg/cluster/persistence/fs/repository.go index cfe6e07..f3d6f9c 100644 --- a/pkg/cluster/persistence/fs/repository.go +++ b/pkg/cluster/persistence/fs/repository.go @@ -182,6 +182,9 @@ func syncPath(path string) error { if err != nil { return err } - defer f.Close() - return f.Sync() + if err := f.Sync(); err != nil { + f.Close() //nolint:errcheck + return err + } + return f.Close() } diff --git a/pkg/cluster/status_snapshot_service_test.go b/pkg/cluster/status_snapshot_service_test.go index 0ceeba7..368c017 100644 --- a/pkg/cluster/status_snapshot_service_test.go +++ b/pkg/cluster/status_snapshot_service_test.go @@ -97,7 +97,7 @@ func TestStatusSnapshot_Build(t *testing.T) { func TestStatusSnapshot_Build_NilContext(t *testing.T) { t.Parallel() svc := NewClusterStatusSnapshotService() - _, err := svc.Build(nil, ClusterStatusSnapshotRequest{}) + _, err := svc.Build(nil, ClusterStatusSnapshotRequest{}) //nolint:staticcheck // intentionally testing nil context rejection require.Error(t, err, "expected error for nil context") } diff --git a/pkg/cmd/hind/rm/rm_test.go b/pkg/cmd/hind/rm/rm_test.go index caa2efa..4267c2d 100644 --- a/pkg/cmd/hind/rm/rm_test.go +++ b/pkg/cmd/hind/rm/rm_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "io" - "os" "testing" "time" @@ -168,9 +167,7 @@ func TestRunE_NotFoundMapping(t *testing.T) { func TestRunEUsesActiveClusterWhenNoArg(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpDir) err := saveTestCluster(t, "active-cluster") require.NoError(t, err, "saveTestCluster") @@ -203,9 +200,7 @@ func TestRunEUsesActiveClusterWhenNoArg(t *testing.T) { func TestRunEFallsBackToDefaultWhenNoActiveClusterAndNoArg(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpDir) orig := newClusterManagerFn var gotName string @@ -230,9 +225,7 @@ func TestRunEFallsBackToDefaultWhenNoActiveClusterAndNoArg(t *testing.T) { func TestRunE_ClearsActiveClusterOnDelete(t *testing.T) { // Redirect HOME so cluster state is isolated to this test. tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpDir) const clusterName = "my-cluster" @@ -263,9 +256,7 @@ func TestRunE_ClearsActiveClusterOnDelete(t *testing.T) { func TestRm_DeleteFailureBeforeRemoval_PreservesActiveProfile(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpDir) for _, name := range []string{"active-a", "other-b"} { err := saveTestCluster(t, name) @@ -291,9 +282,7 @@ func TestRm_DeleteFailureBeforeRemoval_PreservesActiveProfile(t *testing.T) { func TestRunE_PreservesActiveWhenRemovingNonActive(t *testing.T) { tmpDir := t.TempDir() - oldHome := os.Getenv("HOME") - os.Setenv("HOME", tmpDir) - defer os.Setenv("HOME", oldHome) + t.Setenv("HOME", tmpDir) for _, name := range []string{"active-a", "other-b"} { err := saveTestCluster(t, name) diff --git a/pkg/cmd/hind/start/start.go b/pkg/cmd/hind/start/start.go index f18a7ca..a6a8665 100644 --- a/pkg/cmd/hind/start/start.go +++ b/pkg/cmd/hind/start/start.go @@ -90,7 +90,7 @@ func configureLogging(logger *log.Logger, flags *flagpole, clusterName string) { func createStartServices(ctx context.Context, logger *log.Logger, clusterName string) (startServices, error) { logger.Debug("Checking Docker daemon accessibility") if err := checkDockerDaemonFn(ctx, logger); err != nil { - return startServices{}, fmt.Errorf("Docker daemon is not accessible: %w", err) + return startServices{}, fmt.Errorf("docker daemon is not accessible: %w", err) } svc, err := newStartServicesFn(logger, clusterName) diff --git a/pkg/version/version.go b/pkg/version/version.go index 2878003..f6361c6 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -2,5 +2,5 @@ package version const ( // HindVersion is the canonical Hind version/tag for build metadata. - HindVersion string = "0.4.0" + HindVersion string = "0.5.0" )