Skip to content

Follow-ups from the #168 review loop (parity-harness breadth, version sources, env/handover hardening) #169

Description

@fentas

Follow-ups from the review loop on #168 (Go single-binary lo). Each item was raised by a reviewer, judged valid, and deliberately NOT folded into the PR — the reason is next to each one.

Parity harness tightening

  • hack/parity-orchestrate.sh ~L399 strips the whole kind create cluster argv line before diffing, so a Go regression in the cluster name or --image would pass. Narrow the normalisation to the volatile fields (temp config path).
  • hack/parity-operator.sh L97 ALLOW_ERR scrubs parse error|command not found from both stderrs; hack/parity-ops.sh L186 tolerates rc 2→1 for any Error: stderr. Both are broader than the divergence they were written for.
  • Why not in the PR: harness-only, and each needs a deliberate re-baseline of what the bash side emits.

Version sources disagree

  • .lok8s/VERSION = 0.1.0, latest tag v0.2.0, .bin/b.yaml pins v0.3.0 (placeholder for the first tag with lo-* assets), lo version prefers the VERSION file (internal/cli/cmd_version.go) over the ldflags stamp, lo --version shows the stamp. Docs examples use V=v0.3.0, which 404s until that tag exists.
  • Fix at tag time: bump .lok8s/VERSION, set the b.yaml pin to the real tag, and decide whether lo version should prefer the stamp once the binary is canonical.
  • Why not in the PR: owner call (VERSION is the frozen tree's file; the tag is not cut yet).

Environment passed to children

  • internal/execx/runner.go hands the full os.Environ() to every child (kubectl, docker, sops…), so SOPS_AGE_KEY etc. reach tools that do not need it. Bash did the same (exported env), so this is parity, but an allowlist per tool would be safer.

Handover

  • internal/kubehz/handover.go scp/ssh to the target node runs with StrictHostKeyChecking=no (inherited from bash). A known-hosts pin (the platform knows the node's host key) would close the MITM window while CA keys are in flight.
  • internal/kubehz/register.go ~L59: ids returned by the API are interpolated into URL paths without url.PathEscape.

Cosmetic

  • .lok8s/tilt/Tiltfile ~L574 comment still says lo hooks is "bash, bats-tested" (it is Go now). The frozen tree is not edited in feat: single-binary Go lo — full port with parity gates, goreleaser release, verified installer #168.
  • .lok8s/legacy/install/lo-up.min.tmpl L4 header says install/lo-up; it is embedded byte-for-byte in the published bundle, so it cannot change without rebuilding docs/public/lo-up.
  • The core profile's .lok8s/** glob now syncs .lok8s/legacy/install/** to consumers. Accepted under the move-never-delete rule; an exclude would need a b profile change.

🤖 Generated with Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions