diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..b315b5c --- /dev/null +++ b/.gitattributes @@ -0,0 +1,8 @@ +# Everything in this repository is consumed on Linux: deploy/bootstrap.sh is embedded +# verbatim by bootstrap_asset.go and piped into `bash -s`, the Dockerfiles and entrypoints +# are read inside the images, and the operator ships YAML. Without eol=lf a Windows +# checkout under core.autocrlf=true hands all of them CRs. +* text=auto eol=lf + +# The gradle wrappers' Windows launchers are the one thing that wants CRLF. +*.bat text eol=crlf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..e2ddc1f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,94 @@ +# Runs the checks on pull requests and on main. +# +# release.yml already runs `go vet` and `go test`, but only once a vX.Y.Z tag exists — by then +# a red change is on the release path and the only remedy is a new tag. This is the same gate +# moved to where it can still stop something, plus the panel suite, which nothing ran at all: +# the release goes through the Dockerfile, and the Dockerfile runs `npm run build`, never +# `npm test`. +# +# The push trigger is limited to main rather than every branch, for the reason release.yml is +# not repeated here: a branch with an open PR would otherwise run the whole suite twice per +# push, once for refs/heads/ and once for refs/pull/N/merge. Those are different +# concurrency groups, so neither cancels the other, and this repository is private and billed +# for both. A branch with no PR open yet is the one case that loses coverage, and opening the +# PR is what asks for the answer. +name: ci + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + go: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - run: go vet ./... + - run: go test ./... + + shell: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # bootstrap.sh is the only thing that ever runs on a fresh host, and nothing here can + # run it — it wants root, a package manager and k3s. Syntax plus the extracted-block + # checks in bootstrap_test.sh is the coverage that is reachable without a machine. + # Each file is parsed by the interpreter its own shebang names. A blanket `sh -n` is + # wrong and not obviously so: on a developer machine `sh` is usually bash and passes + # everything, while the runner's `sh` is dash and rejects bootstrap.sh at the first of + # its arrays. Honouring the shebang is what makes local and CI agree. + - name: Check shell syntax + run: | + for f in $(git ls-files '*.sh'); do + case "$(head -1 "$f")" in + *bash) bash -n "$f" || exit 1 ;; + *) sh -n "$f" || exit 1 ;; + esac + done + + - run: sh deploy/bootstrap_test.sh + + panel: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + # The Dockerfile's `FROM node:` is the only place the panel's Node version is + # declared — there is no .nvmrc and no engines field. Reading it here rather than + # repeating the number keeps CI testing the version a release is actually built on; + # a second copy would drift silently the first time the image is bumped. + - name: Read the panel's Node version from the Dockerfile + id: node + run: | + version="$(sed -n 's/^FROM.*node:\([0-9][0-9]*\)-.*/\1/p' Dockerfile | head -1)" + [ -n "$version" ] || { echo "Dockerfile has no 'FROM ... node:' line"; exit 1; } + echo "version=${version}" >> "$GITHUB_OUTPUT" + + - uses: actions/setup-node@v4 + with: + node-version: ${{ steps.node.outputs.version }} + cache: npm + cache-dependency-path: panel/package-lock.json + + - run: npm ci + working-directory: panel + + - run: npm test + working-directory: panel + + - run: npm run typecheck + working-directory: panel diff --git a/bootstrap_asset_test.go b/bootstrap_asset_test.go index b54c803..78914f8 100644 --- a/bootstrap_asset_test.go +++ b/bootstrap_asset_test.go @@ -73,9 +73,15 @@ func TestLobbyLuckPermsWiringIsConsistent(t *testing.T) { // default it can inherit — and nothing else in the install would notice it missing. The failure // surfaces only when a legacy player joins, on a host that installed cleanly. func TestBootstrapPinsViaBlockConnectionsOff(t *testing.T) { - // go:embed takes the working tree verbatim, and this repository pins no eol attribute, so - // a Windows checkout embeds CRLF. Only the assertion spanning a line break below cares. - script := strings.ReplaceAll(BootstrapScript(), "\r\n", "\n") + // go:embed takes the working tree verbatim, so the eol attribute is what keeps a Windows + // checkout from compiling CRs into the installer. Assert it rather than normalizing them + // away: the only assertion that would otherwise notice is the one spanning a line break + // below, and it would report a missing pin instead of the line endings. + script := BootstrapScript() + if strings.Contains(script, "\r\n") { + t.Fatal("embedded bootstrap.sh has CRLF line endings; .gitattributes pins *.sh to LF " + + "and this script is piped into `bash -s` on a Linux host") + } const key = "serverside-blockconnections" if !strings.Contains(script, key+": false") { diff --git a/cmd/felis/nano.go b/cmd/felis/nano.go index c01c3c5..a1c7a76 100644 --- a/cmd/felis/nano.go +++ b/cmd/felis/nano.go @@ -13,8 +13,16 @@ package main // off-loopback with -listen; see the flag below for why that is an explicit opt-in. // // This is the no-database delivery of the identical brain `felis api` mounts through its -// route table (internal/api.HasJoinedHandler). `felis setup --nano` / the bootstrap nano -// choice install this as the runtime service; here it just serves. +// route table (internal/api.HasJoinedHandler). The bootstrap installer's nano choice — its +// `[2] Felis-nano` prompt, or FELIS_INSTALL_MODE=nano — installs this as the +// felis-nano.service unit; here it just serves. +// +// There is deliberately no `felis setup --nano`. setup re-images the host through the full +// bootstrap TUI and carries no install-mode parameter anywhere (nothing in Go reads or sets +// FELIS_INSTALL_MODE), so such a flag would either re-run the installer — which is what +// pointing at the installer already does — or tear a full install down into a nano one, +// which is an uninstall, not a flag. This is the same reasoning that makes setup's --dev +// refuse and name the installer rather than pretend to choose a channel. import ( "context" diff --git a/cmd/felis/version.go b/cmd/felis/version.go index f3e983c..7654327 100644 --- a/cmd/felis/version.go +++ b/cmd/felis/version.go @@ -1,66 +1,66 @@ -package main - -import ( - "fmt" - "io" - "runtime" - "runtime/debug" -) - -// version is the build stamp injected at link time via -// -// -ldflags "-X main.version=v1.2.3" (release channel: the tag verbatim) -// -ldflags "-X main.version=v1.2.3+g1a2b3c4" (dev channel: tag + build metadata) -// -// deploy/bootstrap.sh computes it per install channel (FELIS_VERSION_BOOTSTRAP): -// the release channel stamps the resolved tag verbatim (v1.2.3), the dev channel -// stamps "+g". It stays "dev" for an un-stamped local -// `go build`, where ReadBuildInfo below still surfaces the vcs revision. -// -// NOT `git describe`, for two reasons that both bite. Its "--g" form -// puts the distance in the PRERELEASE field, which sorts BELOW the bare tag, so a -// dev build ahead of v1.2.3 would compare as older than v1.2.3 and `felis update` -// would propose "upgrading" onto the release it already contains — hence "+", which -// is build metadata and ignored for ordering. And bootstrap's primary clone is -// --depth 1, which carries no tags, so describe would fall back to a bare SHA that -// updates.Parse rejects outright. -var version = "dev" - -// cmdVersion prints the build stamp. It takes no flags and never touches the -// cluster, so it is safe to run as any user (unlike setup/breakGlass). -func cmdVersion(args []string, stdout, stderr io.Writer) int { - fmt.Fprintf(stdout, "felis %s\n", resolvedVersion()) - fmt.Fprintf(stdout, " go: %s %s/%s\n", runtime.Version(), runtime.GOOS, runtime.GOARCH) - if rev, ok := vcsRevision(); ok { - fmt.Fprintf(stdout, " revision: %s\n", rev) - } - return 0 -} - -// resolvedVersion prefers the ldflag stamp, then the module version recorded by -// `go install`, and only reports "unknown" when neither is present. -func resolvedVersion() string { - if version != "" { - return version - } - if bi, ok := debug.ReadBuildInfo(); ok && bi.Main.Version != "" { - return bi.Main.Version - } - return "unknown" -} - -// vcsRevision returns the git commit the binary was built from when the build -// carried VCS stamping (local `go build` in a checkout; the docker build strips -// .git, so there the ldflag version carries the identity instead). -func vcsRevision() (string, bool) { - bi, ok := debug.ReadBuildInfo() - if !ok { - return "", false - } - for _, s := range bi.Settings { - if s.Key == "vcs.revision" && s.Value != "" { - return s.Value, true - } - } - return "", false -} +package main + +import ( + "fmt" + "io" + "runtime" + "runtime/debug" +) + +// version is the build stamp injected at link time via +// +// -ldflags "-X main.version=v1.2.3" (release channel: the tag verbatim) +// -ldflags "-X main.version=v1.2.3+g1a2b3c4" (dev channel: tag + build metadata) +// +// deploy/bootstrap.sh computes it per install channel (FELIS_VERSION_BOOTSTRAP): +// the release channel stamps the resolved tag verbatim (v1.2.3), the dev channel +// stamps "+g". It stays "dev" for an un-stamped local +// `go build`, where ReadBuildInfo below still surfaces the vcs revision. +// +// NOT `git describe`, for two reasons that both bite. Its "--g" form +// puts the distance in the PRERELEASE field, which sorts BELOW the bare tag, so a +// dev build ahead of v1.2.3 would compare as older than v1.2.3 and `felis update` +// would propose "upgrading" onto the release it already contains — hence "+", which +// is build metadata and ignored for ordering. And bootstrap's primary clone is +// --depth 1, which carries no tags, so describe would fall back to a bare SHA that +// updates.Parse rejects outright. +var version = "dev" + +// cmdVersion prints the build stamp. It takes no flags and never touches the +// cluster, so it is safe to run as any user (unlike setup/breakGlass). +func cmdVersion(args []string, stdout, stderr io.Writer) int { + fmt.Fprintf(stdout, "felis %s\n", resolvedVersion()) + fmt.Fprintf(stdout, " go: %s %s/%s\n", runtime.Version(), runtime.GOOS, runtime.GOARCH) + if rev, ok := vcsRevision(); ok { + fmt.Fprintf(stdout, " revision: %s\n", rev) + } + return 0 +} + +// resolvedVersion prefers the ldflag stamp, then the module version recorded by +// `go install`, and only reports "unknown" when neither is present. +func resolvedVersion() string { + if version != "" { + return version + } + if bi, ok := debug.ReadBuildInfo(); ok && bi.Main.Version != "" { + return bi.Main.Version + } + return "unknown" +} + +// vcsRevision returns the git commit the binary was built from when the build +// carried VCS stamping (local `go build` in a checkout; the docker build strips +// .git, so there the ldflag version carries the identity instead). +func vcsRevision() (string, bool) { + bi, ok := debug.ReadBuildInfo() + if !ok { + return "", false + } + for _, s := range bi.Settings { + if s.Key == "vcs.revision" && s.Value != "" { + return s.Value, true + } + } + return "", false +} diff --git a/deploy/bootstrap.sh b/deploy/bootstrap.sh index 9bd4a50..adc3ec1 100644 --- a/deploy/bootstrap.sh +++ b/deploy/bootstrap.sh @@ -30,6 +30,14 @@ # FELIS_INSTALL_MODE full|nano — skip the prompt (default: ask on a tty, else full) # FELIS_NANO_LISTEN listen addr for `felis nano` (default: 127.0.0.1:8081 — loopback # only; set a private-network IP to serve an off-host proxy) +# FELIS_LEGACY_FORWARDING_SERVERS comma-separated backends that receive their identity +# through the handshake address instead of modern forwarding +# (default: legacy18). Read once at Velocity start, so changing it +# means re-running this script and restarting the proxy. +# FELIS_VELOCITY_FORK_JAR path to a Felis-Legacy Velocity fork build to install as the +# proxy instead of the stock download (default: unset, stock). +# FELIS_VELOCITY_FORK_JAR_SHA256 expected sha256 of that jar. REQUIRED whenever the jar +# above is set; the install refuses on a mismatch. # FELIS_GO_VERSION Go toolchain used to build the nano binary (default: 1.26.4) # FELIS_REPO_URL git URL to build from (raw script mode only) # FELIS_VERSION_BOOTSTRAP release|dev — which version to install (default: release). @@ -95,6 +103,11 @@ INSTALL_MODE="${FELIS_INSTALL_MODE:-}" # own players stop getting in. Same-host Velocity reaches 127.0.0.1 fine; a proxy on # another machine must opt in explicitly with FELIS_NANO_LISTEN=:8081. FELIS_NANO_LISTEN="${FELIS_NANO_LISTEN:-127.0.0.1:8081}" +# Backends that take their forwarded identity through the handshake address instead of +# proxy-wide modern forwarding. See write_velocity_service for why a protocol-47 backend +# needs this. Overridable because adding a second 1.8 backend otherwise means editing this +# script; it is still a restart-time list, not one that follows the CRs. +FELIS_LEGACY_FORWARDING_SERVERS="${FELIS_LEGACY_FORWARDING_SERVERS:-legacy18}" FELIS_GO_VERSION="${FELIS_GO_VERSION:-1.26.4}" PKG_LOCK_TIMEOUT="${PKG_LOCK_TIMEOUT:-${APT_LOCK_TIMEOUT:-900}}" APT_LOCK_TIMEOUT="${APT_LOCK_TIMEOUT:-$PKG_LOCK_TIMEOUT}" @@ -123,9 +136,20 @@ FELIS_VELOCITY_VERSION="${FELIS_VELOCITY_VERSION:-3.5.1}" # # Opt-in because it is unmeasured where it counts: FL-008's probe runs offline-mode # against a stub, and this jar would carry every real Mojang session on the server. -# The build lives in Felis-Legacy and is not byte-reproducible, so there is no digest -# to pin here — the jar is trusted because that probe certified the build. FELIS_VELOCITY_FORK_JAR="${FELIS_VELOCITY_FORK_JAR:-}" +# Expected sha256 of that jar, REQUIRED whenever it is set. Case and internal spaces are +# ignored, so whatever sha256sum, Get-FileHash or certutil printed can be pasted as-is. +# No digest is hardcoded here: +# the build lives in Felis-Legacy and has never been reproduced on a second machine, so +# any constant this script carried would pin one machine's output rather than the fork. +# +# So this is not a supply-chain signature and does not pretend to be one — an operator +# who can write the jar can write this value too. What it does buy: a path is not an +# identity, and every re-run of this script re-checks it. A truncated copy, a stale build +# left at the same path, or the two-patch jar where the three-patch one was meant all +# change the digest and stop the install. Naming the digest once is what turns "whatever +# is at that path today" into one specific build. +FELIS_VELOCITY_FORK_JAR_SHA256="${FELIS_VELOCITY_FORK_JAR_SHA256:-}" # Temurin 25: Velocity 3.5 needs 21+, and 25 is also what a future Velocity 4 requires, # so the runtime does not have to move again when the pin does. Distro JDK packaging is # a lottery across four package managers — a tarball is one code path everywhere (same @@ -1462,12 +1486,31 @@ install_jre() { install_velocity() { install_jre - local url tmp + local url tmp have want prepare_velocity_layout if [ -n "$FELIS_VELOCITY_FORK_JAR" ]; then [ -f "$FELIS_VELOCITY_FORK_JAR" ] \ || die "FELIS_VELOCITY_FORK_JAR is not a readable file: ${FELIS_VELOCITY_FORK_JAR}" - log "installing the Felis-Legacy Velocity fork from ${FELIS_VELOCITY_FORK_JAR}" + # Hash stdin, never the path — same reason as install_via_plugins: sha256sum escapes its + # output line for a filename carrying a backslash or a newline, and the leading "\" that + # adds would fail every comparison below. + have="$(sha256sum <"$FELIS_VELOCITY_FORK_JAR" | cut -d' ' -f1)" + # Refuse rather than warn. This jar is the proxy every player connects through, and a + # warning in an install log is not a gate. The digest is printed so the first run after + # a deliberate rebuild is one copy-paste, not an investigation. + [ -n "$FELIS_VELOCITY_FORK_JAR_SHA256" ] || die \ + "FELIS_VELOCITY_FORK_JAR_SHA256 is required whenever FELIS_VELOCITY_FORK_JAR is set. + The jar at that path hashes to ${have}. + Check that against the build you meant to install, then re-run with + FELIS_VELOCITY_FORK_JAR_SHA256=${have}" + # Normalise the operator's digest before comparing. sha256sum prints lowercase, but the + # build host is often Windows, where Get-FileHash prints uppercase and certutil has + # shipped both with and without spaces between the bytes. All three name the same jar, + # so comparing raw would refuse two of the three spellings and word it as tampering. + want="$(printf '%s' "$FELIS_VELOCITY_FORK_JAR_SHA256" | tr -d '[:space:]' | tr 'A-Z' 'a-z')" + [ "$have" = "$want" ] || die \ + "FELIS_VELOCITY_FORK_JAR checksum mismatch: got ${have}, expected ${want}" + log "installing the Felis-Legacy Velocity fork from ${FELIS_VELOCITY_FORK_JAR} (sha256 ${have})" atomic_install_file "$FELIS_VELOCITY_FORK_JAR" "${VELOCITY_DIR}/velocity.jar" 0644 root root else log "resolving the newest Velocity ${FELIS_VELOCITY_VERSION} build" @@ -1591,9 +1634,20 @@ install_velocity_service() { # behind ViaVersion, which strips modern forwarding's login-plugin-message when it down-translates # the proxy->backend pipeline to protocol 47; only the handshake field survives Via. The Felis # fork reads this list from -Dfelis.legacy-forwarding.servers and forwards those servers legacy; - # every other backend keeps modern+secret untouched. v1 hardcodes the one legacy backend; the - # upgrade path is to have the operator render this list from the MinecraftServer CRs. - local legacy_forwarding_servers="legacy18" + # every other backend keeps modern+secret untouched. + # + # The list is a JVM system property, so it is fixed for the life of the proxy process and a + # change needs a Velocity restart. FELIS_LEGACY_FORWARDING_SERVERS makes that reachable + # without editing this script, which is as far as a startup property can go. Having it follow + # the MinecraftServer CRs instead is a larger change: the forwarding decision lives in the + # fork's patch to Velocity core, not in the Felis plugin, so core would need to read state the + # plugin owns and refreshes. + # + # The -D below is double-quoted in ExecStart on purpose. The fork trims each element, so it + # accepts "legacy18, legacy112", but systemd splits ExecStart on whitespace before java ever + # sees it -- unquoted, that spelling would hand java a stray "legacy112" argument and the unit + # would not start. Quoting keeps the whole property one argv item. + local legacy_forwarding_servers="${FELIS_LEGACY_FORWARDING_SERVERS}" cat > "$VELOCITY_SERVICE" < in:"; echo "$3"; fails=$((fails + 1)) ;; + esac +} + +# --- the FELIS_VELOCITY_FORK_JAR digest gate ------------------------------------------- +# This jar becomes the proxy every player connects through, so the interesting cases are the +# two refusals, not the happy path. + +gate="$(awk '/have="\$\(sha256sum <"\$FELIS_VELOCITY_FORK_JAR"/,/^ log "installing the Felis-Legacy/' "$BS")" +[ -n "$gate" ] || { echo "FAIL: no fork-jar digest gate found in $BS"; exit 1; } +# awk runs an unmatched end pattern to EOF, which would quietly pipe the rest of bootstrap.sh +# into the `sh -c` below. The emptiness check above only catches a broken start pattern. +[ "$(printf '%s\n' "$gate" | wc -l)" -lt 40 ] \ + || { echo "FAIL: the extracted block is not the gate -- did its last line move?"; exit 1; } + +jar="$(mktemp)" +trap 'rm -f "$jar"' EXIT +printf 'stand-in for a fork build\n' > "$jar" +want="$(sha256sum <"$jar" | cut -d' ' -f1)" + +run_gate() { # digest + FELIS_VELOCITY_FORK_JAR="$jar" FELIS_VELOCITY_FORK_JAR_SHA256="$1" sh -c ' + die() { printf "DIE: %s\n" "$*"; exit 1; } + log() { printf "LOG: %s\n" "$*"; } + '"$gate" +} + +out="$(run_gate '')" +expect "fork jar without a digest is refused" "DIE: FELIS_VELOCITY_FORK_JAR_SHA256 is required" "$out" +expect "the refusal names the jar's real digest" "$want" "$out" + +out="$(run_gate 'deadbeef')" +expect "a mismatched digest is refused" "checksum mismatch: got ${want}, expected deadbeef" "$out" + +out="$(run_gate "$want")" +expect "the matching digest installs" "LOG: installing the Felis-Legacy Velocity fork" "$out" +expect "the install line records the digest" "(sha256 ${want})" "$out" + +# The build host is usually Windows, where nothing prints the digest the way sha256sum does: +# Get-FileHash returns uppercase and certutil has shipped the bytes space-separated. Feeding +# the gate its own output can never catch that -- these two cases are the operator's paste. +out="$(run_gate "$(printf '%s' "$want" | tr 'a-z' 'A-Z')")" +expect "an uppercase digest is the same digest" "LOG: installing the Felis-Legacy Velocity fork" "$out" + +out="$(run_gate "$(printf '%s' "$want" | sed 's/../& /g')")" +expect "a space-separated digest is the same digest" "LOG: installing the Felis-Legacy Velocity fork" "$out" + +# --------------------------------------------------------------------------------------- +if [ "$fails" -eq 0 ]; then + echo "ALL PASS" +else + echo "$fails FAILED" +fi +exit "$fails" diff --git a/deploy/crd/felis.lolicon.best_minecraftservers.yaml b/deploy/crd/felis.lolicon.best_minecraftservers.yaml index e51642b..f58883f 100644 --- a/deploy/crd/felis.lolicon.best_minecraftservers.yaml +++ b/deploy/crd/felis.lolicon.best_minecraftservers.yaml @@ -269,10 +269,6 @@ spec: storage: description: Storage configures the world PVC. properties: - retainOnDelete: - description: RetainOnDelete keeps the PVC when the MinecraftServer - is deleted. - type: boolean size: description: Size is the requested PVC capacity (e.g. "10Gi"). type: string diff --git a/docs/deferred-seams.md b/docs/deferred-seams.md new file mode 100644 index 0000000..7c53a6b --- /dev/null +++ b/docs/deferred-seams.md @@ -0,0 +1,113 @@ +# Deferred integration seams + +`INTEGRATION-ONLY` and `KNOWN-LIMITATION` are grep-able markers in the Go source. +This file is the index of what each one currently means, so that reading the +unfinished face of the system does not require re-deriving it from 34 comment +sites. + +It exists for two reasons. The markers do not all mean the same thing — four +distinct states share them, and "declared, nothing implements it" reads exactly +like "implemented, but its I/O cannot be exercised from this repo". And a marker +outlives the condition it describes: two of them were stale when this index was +first assembled, both claiming as future work something that had already shipped. + +The pattern here is the one `internal/updater/doc.go` already uses for its own +package — state the verification boundary in buckets, so a green test suite is not +mistaken for a finished integration. This file is the same idea across the whole +tree. + +## The marker collides with a different vocabulary in troubleshooting.md + +`docs/troubleshooting.md` uses `[INTEGRATION-ONLY]` for something else, defined in +its own opening at `:19`: the symptom is produced by the kubelet, kaniko or a live +handshake, so it cannot be reproduced from the repository. Those twelve marks say +where a failure comes from. They are not unfinished work and are not indexed below. +A grep across `*.md` and `*.go` returns both sets; only the Go ones are seams. + +## Declared, nothing implements it + +- `internal/updates/seams.go:32` — `Notifier`. `internal/mail` sends OTP over SMTP, + but nothing adapts it to this interface and no in-game channel exists. `felis + update` passes nil deliberately: a human typing the command is the notification. +- `internal/updates/seams.go:43` — `Applier`. Nothing applies an update anywhere. A + nil applier is not silent — `Run` records `errNoApplier` against every planned + apply, so a mis-scheduled apply is loud rather than lost. +- `internal/updater/gatherer_integration.go:22` — the two current-version seams + `NewSysGatherer` leaves nil, for the in-cluster path: the k8s read of the + control-plane Deployment image, and the Velocity jar inspection. Both are answered + on the host path (see "Built" below), so this gap is specific to a caller that has + a cluster client instead of the node. +- `internal/api/handlers_updates.go:21,28` — the maintenance window persists and the + API serves it, but the in-cluster CronJob that would hand a real window to a runner + does not exist. `felis update` runs with a zero window, under which every + `Scheduled` component degrades to a notify, so no path can currently claim an + apply is under way. +- `internal/submit/blobstore.go:40` — the uploads PVC is mounted into felis-api but + not into the Kaniko build Pod, so a submitted context is durable at the derived + location without yet being readable by the build that consumes it. + +## Built; only its I/O is unverifiable from this repo + +Code exists and is unit-tested against fakes. What is missing is a host, a cluster +or a real upstream account to run it against — not an implementation. + +- `cmd/felis/tui_edge_apply.go:246,274,295` — the `nft` edge fence, its idempotent + teardown, and the cloudflared invocation. +- `internal/cfsetup/runner.go:18` and `internal/cfsetup/cfsetup.go:329` — the real + Cloudflare Tunnel and Access API calls; `internal/cfsetup/cfsetup_test.go:11` + drives the whole flow through a fake. +- `internal/api/console.go:39`, `internal/api/logstream.go:236`, + `internal/api/logstream.go:306`, `internal/fileedit/k8sjobs.go:45` — each needs a + live cluster (RCON, `pods/log` follow, a Job). +- `internal/api/handlers_access.go:170,490` — parsing real vanilla and LuckPerms + command output. + +## Deliberately accepted, not scheduled to close + +These are decisions, not backlog. Each names the condition under which it would be +worth revisiting. + +- `internal/api/pgrepo.go:281` — the quota check and `ClaimServer` are two statements + (audit #4 TOCTOU). Closeable only against a real Postgres. +- `internal/api/api.go:671` — `cooldownLimiter` is process-local, so across N api + replicas a caller could draw up to N OTP codes per window. The intra-replica burst + is closed; cross-replica bounding needs a shared store, out of scope for a + single-replica install. +- `internal/submit/submit.go:436` and `internal/submit/submit_test.go:351` — a + post-CAS `Approve` + failure leaves a row indistinguishable from the benign case, so `Approve` returns a + distinct error naming the running build rather than allowing a blind re-drive that + would double-push. The alternative ordering is worse. +- `cmd/felis/tui_edge_apply.go:246`, second marker on the same site — the fence + targets nftables. On a firewalld host a reload can flush the standalone table; + firewalld-native coordination is not handled. A missing `nft` binary fails loud + rather than leaving the port open. +- `internal/api/handlers_account.go:169` — the reclaim "start fresh vs inherit" + choice is CODE-ONLY on the Java/Velocity side; the link-status endpoint reports + link completion only and does not surface it. + +## Wired since the marker was written + +- `internal/api/handlers_email_otp.go:54,223,227` and `internal/api/api.go:84` — + SMTP shipped on + 2026-07-20 (`internal/mail`, wired at `cmd/felis/api.go:264`). The nil-`Mailer` + branch that logs the code server-side is a runtime fallback for an install with no + `[smtp]` section, not an unbuilt feature. The comments are accurate; the reading + "Felis cannot send mail" is not. +- `internal/config/config.go:117` — was stale. It described the upload transport as a + deferred integration after both backends had shipped (`LocalContextStore`, + `S3ContextStore`, selected in `cmd/felis/api.go` by the shape of the configured + base). Corrected in the change that added this file; what remains deferred is only + Kaniko's read, indexed above. +- `internal/updater/doc.go:44` — was stale. Its REMAINING INTEGRATION bullet listed + the `felis update` CLI and the off-cluster Velocity jar read, both of which exist + (`cmd/felis/update.go`, `internal/updater/gatherer_host.go`). Corrected in the same + change; the two nil seams it also names are real and remain above. + +## Recorded outside the code + +- `deploy/limbo/README.md:139` — no NetworkPolicy locks the minecraft-namespace + egress or the control-namespace ingress today, which is why the login pod reaches + `felis-api-internal:8081`. This is a conditional obligation rather than a seam: if + a future deployment adds either lock, it must also open that path. Spec v4.1 §21 + asks for those policies; `cmd/felis/manifests.go` renders the game-port one. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 6f63538..c9fe44a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -20,8 +20,8 @@ graded for how far the in-repo Go test suite proves the behaviour: containerd, Postgres, or the network, not by Felis Go code; you will see it in `kubectl describe` / pod logs, never in `MinecraftServer.status`. - **[INERT]** — the configuration field exists in the CRD but no controller - reads it. Tuning it does nothing. These are the highest-value traps and are - collected in §10. + reads it. Tuning it does nothing. §12 lists the one field this still applies + to, alongside the fields that *are* read and the condition each depends on. The operator never invents the parent domain; routing identity is `spec.subdomain` under the deployment zone. Examples below use @@ -32,14 +32,21 @@ concrete host. ## 1. Server is stuck in `Starting` and never becomes `Running` -`MinecraftServer.status.phase` stays `Starting`. The single most important fact: -**the operator has no start timeout.** A start that never succeeds loops in -`Starting`, requeued every 5s, **forever** — it is never auto-escalated to -`Failed`. [GO-TESTED: `TestReconcileRunning_RconProbeFailureStaysStarting` -asserts the phase stays `Starting`.] Do **not** reach for -`spec.startup.timeoutSeconds` / `spec.startup.readinessTimeoutSeconds` — both are -[INERT] (§10). A hung start must be diagnosed from pod state, not from -`MinecraftServer.status`. +`MinecraftServer.status.phase` stays `Starting`. A start that never succeeds is +requeued every 5s until one of the two startup budgets expires, then escalated +to `Failed` — `StartupTimeout` if the pod never passed TCP readiness, +`ReadinessTimeout` if the RCON probe never succeeded. Both default to **300s** +when `spec.startup.timeoutSeconds` / `spec.startup.readinessTimeoutSeconds` are +unset or `0`, and both are measured from `status.startRequestedAt`. +[GO-TESTED: `TestReconcileRunning_StartupTimeoutConvertsToFailed`, +`TestReconcileRunning_ReadinessTimeoutConvertsToFailed`.] + +So `Starting` seen *once* is normal and +`TestReconcileRunning_RconProbeFailureStaysStarting` asserts exactly that — a +single failed probe must not flap the phase. `Starting` seen for longer than the +budget means the reconcile loop is not running at all; check the operator's own +logs before tuning anything. Either way the underlying cause is diagnosed from +pod state, not from `MinecraftServer.status`. First, read the condition reason: @@ -111,8 +118,10 @@ message is the verbatim dial error: port yet, RCON is disabled in `server.properties`, or `spec.rcon.port` (default 25575) is wrong. [INTEGRATION-ONLY for the live handshake.] -The per-probe timeout is a fixed 5s in code — it is **not** derived from -`spec.startup.readinessTimeoutSeconds` ([INERT]). +The per-probe timeout is a fixed 5s in code (`prober.go:45`, shortened further if +the reconcile context has a nearer deadline). It is **not** derived from +`spec.startup.readinessTimeoutSeconds`, which is the deadline for the whole +start, not for one probe — see §12. --- @@ -463,14 +472,35 @@ store paths are [INTEGRATION-ONLY]. ## 11. Idle auto-stop never fires; player count always shows 0 -**Idle auto-stop is entirely unimplemented in the operator.** `spec.idle.*` -([INERT], §12) is read by no controller, and no idle controller is registered. -An empty `Running` server stays `Running`. +Both are implemented, and both hang off the same switch: **`spec.rcon.enabled`**. +Check it first. -Relatedly, `status.players.online` is **permanently 0**: the only writer of -`status.players` zeroes it on stop, and the RCON prober only dials + closes — it -never runs `list`. Any panel reading `status.players.online` will always show -empty. Do not build alerting on it, and do not expect "empty server" automation. +```sh +kubectl get minecraftserver -o jsonpath='{.spec.rcon.enabled}' +``` + +The player tally is a by-product of the RCON readiness probe — `prober.go:63` +runs `list` on the same connection that just authenticated, and `parseListReply` +extracts the tally from `There are (\d+) of a max of (\d+) players online`. With +RCON disabled the probe never runs, `players` keeps its zero value, and +`markRunningReady` (`reconciler.go:413`) writes that zero into +`status.players.online`. So a permanent 0 means "never sampled", not "nobody +online". + +Idle auto-stop (`reconciler.go:175`) reads that same tally, which is why it +carries the RCON condition explicitly: + +```go +if server.Spec.Rcon.Enabled && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 { +``` + +The comment above it says why: with RCON off the zero tally "would read as +'empty' and use to stop a server full of people". So the guard is deliberate — +enabling `spec.idle.*` without RCON is a no-op by design, not a missing feature. + +Both fields set and still nothing happens? Then the probe is failing rather than +disabled: the server would be stuck in `Starting` with `RconNotReachable` +(`reconciler.go:156`), which is §1's symptom, not this one. Note the reaper's `last_active_at` (§10) is a *different* subsystem (Postgres business layer, bumped by join events) — it keeps worlds alive against the @@ -478,22 +508,21 @@ reaper, but it does **not** auto-stop empty running servers. --- -## 12. Inert configuration fields (highest-value traps) +## 12. A configuration field seems to be ignored -These CRD fields exist and validate, but **no controller reads them.** Setting -them has **no effect**. Verified by grep: each appears only in the type -definition and its deepcopy, never in a controller. +Every field below is read by a controller. What varies is the condition that +decides whether setting it does anything. | Field | What you might expect | Reality | |---|---|---| -| `spec.startup.timeoutSeconds` | Start budget before `Failed` | **[INERT]** — no Starting→Failed timeout exists; a hung start loops forever (§1) | -| `spec.startup.readinessTimeoutSeconds` | First-probe budget | **[INERT]** — probe timeout is a fixed 5s in code | -| `spec.idle.autoStopEnabled` | Auto-stop empty servers | **[INERT]** — idle auto-stop unimplemented (§11) | -| `spec.idle.emptySecondsBeforeStop` | Empty grace period | **[INERT]** | -| `spec.storage.retainOnDelete` | Keep/drop PVC on delete | **[INERT]** — world PVCs **always** survive server deletion; only the reaper ever deletes a world PVC (§13) | +| `spec.startup.timeoutSeconds` | Start budget before `Failed` | Read by `startupTimedOut` (`reconciler.go:479`), called at `:126`. `0` or unset falls back to **300s**, then `markFailed("StartupTimeout")` | +| `spec.startup.readinessTimeoutSeconds` | First-probe budget | Read by `readinessTimedOut` (`reconciler.go:490`), called at `:157`. `0` or unset falls back to **300s**, then `markFailed("ReadinessTimeout")`. Not to be confused with the prober's own 5s dial timeout (`prober.go:45`) | +| `spec.idle.autoStopEnabled` | Auto-stop empty servers | Read at `reconciler.go:175` — but gated on `spec.rcon.enabled`, since the player tally comes from the RCON probe (§11) | +| `spec.idle.emptySecondsBeforeStop` | Empty grace period | Same branch. Must be `> 0`; the guard treats `0` as "off", not "stop immediately" | -If a runbook tells someone to "tune `startup.timeoutSeconds`" for a hung start, -it is wrong — use `kubectl describe pod` (§1a) instead. +Both startup budgets are measured from the same `status.startRequestedAt`, so +`readinessTimeoutSeconds` is not a budget *after* pod readiness — it is a +deadline for the whole start, applied on the RCON-probe branch. --- @@ -503,16 +532,29 @@ This is expected. The world PVC is a StatefulSet `VolumeClaimTemplate`. There is **no `persistentVolumeClaimRetentionPolicy` and no finalizer** anywhere in the operator. Deleting the `MinecraftServer` garbage-collects the StatefulSet, but StatefulSet deletion does **not** cascade to its template PVCs, and nothing else -cleans them up. So the world PVC **always survives** server deletion, regardless -of `spec.storage.retainOnDelete` ([INERT], §12). The **only** code that deletes a -world PVC is the reaper, and only after a verified backup (§10). To reclaim a -world PVC manually: +cleans them up. So the world PVC **always survives** server deletion. The +**only** code that deletes a world PVC is the reaper, and only after a verified +backup (§10). To reclaim a world PVC manually: ``` kubectl get pvc -l app.kubernetes.io/name= kubectl delete pvc # irreversible — the world is gone ``` +`spec.storage.retainOnDelete` sat in the CRD and reached no controller. Spec +v4.1 §5 asks for it — 「删除:finalizer 清 Service/STS/ConfigMap,PVC 按 +`retainOnDelete`」 — and neither half was ever built: there is no finalizer, and +nothing read the field. It was removed rather than implemented, which is a +deliberate departure from that line, recorded here because the spec is a frozen +document and still says otherwise. + +The reasoning is that implementing it buys a second path that deletes a world — +one that skips the reaper's verified-backup check — in order to restore a +finalizer whose other listed duties (Service, StatefulSet, ConfigMap) +ownerReference GC already performs. A CR still carrying the field keeps working: +the API server prunes the unknown key on its next write, and nothing above +changes, because retention was never conditional in the first place. + --- ## 14. Metrics for diagnosis (spec §23) diff --git a/internal/apis/felis/v1alpha1/minecraftserver_types.go b/internal/apis/felis/v1alpha1/minecraftserver_types.go index d175428..0c80348 100644 --- a/internal/apis/felis/v1alpha1/minecraftserver_types.go +++ b/internal/apis/felis/v1alpha1/minecraftserver_types.go @@ -192,8 +192,6 @@ type StorageSpec struct { Size string `json:"size,omitempty"` // StorageClassName selects the StorageClass; empty uses the default. StorageClassName string `json:"storageClassName,omitempty"` - // RetainOnDelete keeps the PVC when the MinecraftServer is deleted. - RetainOnDelete bool `json:"retainOnDelete,omitempty"` } // LifecycleSpec tunes graceful shutdown (spec §7). The operator injects a diff --git a/internal/config/config.go b/internal/config/config.go index cd8daaa..d3769ef 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -118,9 +118,12 @@ type RegistryConfig struct { // modpack's Kaniko build context is pinned. It belongs to the §16 build // subsystem's input domain (the build-context store), introduced by the // user-directed modpack approval lane (see internal/submit package doc). The - // lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; the upload - // transport that places the blob there is a separate, deferred integration - // (INTEGRATION-ONLY). It is kept distinct from [archive] on purpose — a world + // lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; both transports + // that place the blob there now ship (submit.LocalContextStore for a local path, + // submit.S3ContextStore for an s3:// base, selected in cmd/felis by the shape of + // this value). What stays deferred is the far end — Kaniko reading that context + // from inside the build Pod (INTEGRATION-ONLY, see submit/blobstore.go). It is + // kept distinct from [archive] on purpose — a world // archive (§19 WorldArchiver) and a build context (§16) are different artifacts // with different lifecycles, so the two must not share a store binding. UserUploadsContext string `toml:"user_uploads_context"` diff --git a/internal/updater/doc.go b/internal/updater/doc.go index 1452d3d..df4db98 100644 --- a/internal/updater/doc.go +++ b/internal/updater/doc.go @@ -41,11 +41,15 @@ // read and a CLI read agree instead of the image masquerading as a prerelease. The // CLI seam (execRunner) is wired for real; only its exec I/O is un-verified here. // -// - REMAINING INTEGRATION (genuinely I/O-bound — needs a node/cluster/mailbox): the -// two current-version PRODUCING seams the gatherer still lacks — the k8s read of the -// control-plane Deployment's image (felis-api) and the off-cluster Velocity jar -// inspection, both left nil so those components surface a gather error rather than a -// wrong version — plus the concrete Notifier (SMTP + in-game) and Applier -// (control-plane image bump, cloudflared swap), the `felis update` CLI + CronJob -// entry point, and the runtime append of the live Pinned Minecraft fleet. +// - REMAINING INTEGRATION (genuinely I/O-bound — needs a cluster/mailbox): the two +// current-version PRODUCING seams NewSysGatherer still leaves nil — the k8s read of +// the control-plane Deployment's image (felis-api) and the Velocity jar inspection — +// so an IN-CLUSTER caller surfaces a gather error rather than a wrong version. The +// ON-HOST caller has both: NewHostGatherer (gatherer_host.go) answers felis-api from +// the running binary's build stamp and Velocity from the installed jar's manifest, +// and that is what the built `felis update` CLI runs on. Still absent: the concrete +// Notifier (SMTP + in-game) and Applier (control-plane image bump, cloudflared swap) +// — the CLI passes nil for both on purpose, so it reports and never applies — the +// in-cluster CronJob entry point, and the runtime append of the live Pinned +// Minecraft fleet. package updater diff --git a/plugins/README.md b/plugins/README.md index 905cfd5..291dc82 100644 --- a/plugins/README.md +++ b/plugins/README.md @@ -1,12 +1,16 @@ # Felis server-side plugins -These are the in-cluster and edge plugins for Felis. Every module **except the -lobby** ships the in-game first leg of the §10 account-link flow: a player who is already online +These are the in-cluster and edge plugins for Felis. The Velocity proxy and the three loader +mods ship the in-game first leg of the §10 account-link flow: a player who is already online (so Mojang has verified their UUID) runs `/link`; the plugin asks felis-api to mint a one-time code for that UUID and shows it in chat. The player then enters the code on the web console → **Account** page (the second leg), which binds the code to their logged-in account. The web side is already built. +The **limbo** module reaches the same felis-api endpoint without a command: it is the login +gate, so it mints the code on join for anyone not yet linked and holds them until they redeem +it. The **paper** lobby ships neither — see below. + The **Velocity** module additionally carries the §11 domain-autostart routing loop — recognizing each server's subdomain, registering backends dynamically, waking a sleeping target and holding the player until it is ready. It is a full @@ -19,14 +23,22 @@ The **Paper** module is different in kind: it is the §12 lobby UI face. It ship to Velocity, which is the only side that ever talks to felis-api. See **[Lobby menu](#lobby-menu-§12)** below. -| Module | Platform | Target | Jar | -| ------------------ | --------------------------- | ----------------------------------- | ---------------------------- | -| `velocity/` | Velocity proxy plugin | velocity-api 3.3.0-SNAPSHOT | `felis-velocity-0.2.0.jar` | -| `fabric/` | Fabric server mod | MC 1.20.1 / fabric-loader 0.16.x | `felis-fabric-0.1.0.jar` | -| `forge/` | Forge server mod | MC 1.20.1 / Forge 47.3.0 | `felis-forge-0.1.0.jar` | -| `neoforge/` | NeoForge server mod | MC 1.20.4 / NeoForge 20.4.251 | `felis-neoforge-0.1.0.jar` | -| `paper/` | Paper server plugin (lobby) | paper-api 1.21.4-R0.1-SNAPSHOT | `felis-paper-0.1.0.jar` | -| `shared/` | *(not built on its own)* | — | source compiled into each | +| Module | Platform | Target | Jar | Built by the installer | +| ------------------ | --------------------------- | ----------------------------------- | ---------------------------- | ---------------------- | +| `velocity/` | Velocity proxy plugin | velocity-api 3.3.0-SNAPSHOT | `felis-velocity-0.1.0.jar` | yes | +| `limbo/` | LOOHP/Limbo plugin (login) | Limbo API / Java 17 bytecode | `felis-limbo-0.1.0.jar` | yes | +| `paper/` | Paper server plugin (lobby) | paper-api 1.21.4-R0.1-SNAPSHOT | `felis-paper-0.1.0.jar` | yes | +| `fabric/` | Fabric server mod | MC 1.20.1 / fabric-loader 0.16.x | `felis-fabric-0.1.0.jar` | no | +| `forge/` | Forge server mod | MC 1.20.1 / Forge 47.3.0 | `felis-forge-0.1.0.jar` | no | +| `neoforge/` | NeoForge server mod | MC 1.20.4 / NeoForge 20.4.251 | `felis-neoforge-0.1.0.jar` | no | +| `shared/` | *(not built on its own)* | — | source compiled into each | source only | + +"Built by the installer" is what `deploy/bootstrap.sh` produces, and it is the same set +`bootstrap_asset.go` embeds into the felis binary for the TUI install path, which has no source +checkout to build from. **The three loader mods are not in that set** — a finished install has +no `felis-fabric`/`felis-forge`/`felis-neoforge` jar anywhere. They build from this checkout with +the commands under [Building](#building) and are deployed by hand; the account-link flow they +carry works, but nothing installs them for you. ## Architecture @@ -176,6 +188,7 @@ preference): | Module | Gradle | Why | | ----------- | ----------- | --------------------------------------------------------------- | | `velocity` | 9.5.1 (system) | plain `java` plugin — no loader Gradle plugin | +| `limbo` | 9.5.1 (system), **JDK 21 toolchain** | plain `java` plugin; current LOOHP/Limbo releases ship class-file major 65, so the compiler JDK must be ≥ 21 to read them. It emits `release 17` bytecode, so the jar still loads on any Limbo running Java 17+ | | `fabric` | 8.8 (wrapper) | loom 1.7.4 uses `Problems.forNamespace`, removed in Gradle 9 | | `forge` | 8.8 (wrapper) | ForgeGradle 6 is Gradle-8-only | | `neoforge` | 8.14 (wrapper) | NeoGradle 7.1.38 requires Gradle API ≥ 8.14 | @@ -185,18 +198,20 @@ preference): # Velocity — system Gradle is fine gradle -p plugins/velocity build -# Paper — system Gradle too, but it compiles on a Java-21 toolchain (see table) +# Paper and limbo — system Gradle too, but both compile on a Java-21 toolchain (see table) gradle -p plugins/paper build +gradle -p plugins/limbo build -# Fabric / Forge / NeoForge — use the per-module wrapper +# Fabric / Forge / NeoForge — use the per-module wrapper. Nothing installs these; the jar you +# want is the one this produces. plugins/fabric/gradlew -p plugins/fabric build plugins/forge/gradlew -p plugins/forge build plugins/neoforge/gradlew -p plugins/neoforge build ``` -Requires JDK 17 — **except `paper`, which needs a Java-21 toolchain available to -Gradle** (paper-api 1.21.4 is a Java-21 artifact; the rest of the suite is Java -17). The first build of each mod downloads and remaps/decompiles Minecraft, so it +Requires JDK 17 — **except `paper` and `limbo`, which need a Java-21 toolchain available to +Gradle** (paper-api 1.21.4 is a Java-21 artifact and the Limbo API is compiled to major 65; the +rest of the suite is Java 17). The first build of each mod downloads and remaps/decompiles Minecraft, so it takes a few minutes; subsequent builds are fast. Jars land in each module's `build/libs/`.