Skip to content

fix: report a client version in Hostinfo.IPNVersion - #39

Closed
timmills wants to merge 1 commit into
Csontikka:mainfrom
timmills:pr/hostinfo-ipnversion-v055
Closed

fix: report a client version in Hostinfo.IPNVersion#39
timmills wants to merge 1 commit into
Csontikka:mainfrom
timmills:pr/hostinfo-ipnversion-v055

Conversation

@timmills

@timmills timmills commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Rebased onto current main. Finding #5 from #33 — the smallest of the six.

What it does

Reports a client version string in Hostinfo.IPNVersion, which the component previously left empty.

Why it matters, and the awkward part

The Tailscale admin console gates device operations on this string. With it empty (or with our honest 0.5.4) the console refuses with "Device is too old" — that is what blocked reassigning a node's address on 2026-07-31.

I verified this both ways: the same operation failed with our real version and succeeded, unchanged, while the board reported a current release number. Nothing else differed.

Being explicit about what this is

It reports a version number that is not ours. I do not love it and I would rather it were unnecessary. The reasoning:

  • The control plane knows our real protocol level from CapabilityVersion regardless, so nothing about actual compatibility depends on this string — it is used for the console's own gating, not for negotiation.
  • 1.98.9 specifically because control appears to reject a version it does not recognise and store empty, which puts us back where we started. An unreleased-looking number (1.103.0) did not stick either.

If you would rather this were configurable than hardcoded, or would prefer a different value, say so and I will change it — I have no attachment to the number, only to the board not being locked out of its own admin console. It is a cv.Optional in our fork, so exposing it as a YAML option instead is a small change.

Testing

Builds clean against current main on ESP32-S3 (ESPHome 2026.6.5, ESP-IDF). Verified on two boards on an ~85-node Tailscale SaaS tailnet.

@Csontikka

Copy link
Copy Markdown
Owner

Thanks for verifying this both ways and for being this explicit about what the change does — that transparency made the decision easy to reason about.

Decision: let's make it an opt-in YAML option, default off (report nothing, as today). Reasoning:

  • A public component shouldn't impersonate another client's version by default. CapabilityVersion already carries our real protocol level, so this string is console-gating cosmetics — but it's still someone else's version number, and defaults are what a project is.
  • Headscale users (a large share of this repo's audience) never hit the gating, so a default-off knob costs them nothing.
  • The console lockout is real and painful, so the workaround must exist and be easy to find — documented, not hidden.

Concretely, please make it a string option rather than a bool with a baked-in constant, e.g.:

tailscale:
  ipn_version: "1.98.9"   # optional; omitted = report nothing (default)

The string form has a second benefit you already ran into: when control eventually considers 1.98.9 "too old", users bump the value in their YAML instead of waiting for a release.

I'll add a Troubleshooting entry ("Device is too old" in the admin console → set ipn_version) on merge — or fold it into the PR if you'd rather ship them together, whichever you prefer. Everything else about the PR looks right to me; with that change it can go in after the usual bench pass.

Reports a client version in Hostinfo.IPNVersion when set, to clear the admin
console's "Device is too old" gate that blocks device operations (e.g.
reassigning a node's address). Default off: the field is omitted, so a public
component does not claim a version it is not.

- New `ipn_version:` YAML string option, plumbed through the component into
  microlink_config_t.ipn_version. Empty/NULL => IPNVersion omitted at both
  hostinfo sites (register + map exchange).
- A bare semver ("1.98.9") gets synthetic version.Long hashes appended so the
  console displays it; a full "x.y.z-t..-g.." string passes through as-is.
- Drops the baked-in ML_IPN_VERSION constant.
- README: "Device is too old" troubleshooting entry.

Compile-verified on esp32-s3-devkitc-1 / esp-idf (esphome 2026.6.5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@timmills
timmills force-pushed the pr/hostinfo-ipnversion-v055 branch from caadb46 to 6cafb4e Compare August 10, 2026 00:06
@timmills

Copy link
Copy Markdown
Contributor Author

Respun per your decision: ipn_version is now an opt-in string, default off.

  • New ipn_version: YAML option (default ""), plumbed through the component into microlink_config_t.ipn_version. Empty/NULL means IPNVersion is omitted at both hostinfo sites (register + map exchange), i.e. report nothing, exactly as today's upstream.
  • Dropped the baked-in ML_IPN_VERSION constant; its shape-analysis comment moved to the config field and README.
  • Folded in the "Device is too old" Troubleshooting entry so it ships with the option.
  • Squashed to a single commit, and compile-verified on esp32-s3-devkitc-1 / ESP-IDF (ESPHome 2026.6.5).

One judgment call to flag: your example was a bare "1.98.9", but version.Long() silently drops anything without the -t..-g.. suffix (per the analysis in the original commit), so bare semvers get synthetic hashes appended in tailscale.cpp; a full x.y.z-t..-g.. passes through untouched. That makes ipn_version: "1.98.9" actually display. Happy to switch to verbatim plus a docs note if you'd rather avoid the normalization.

@Csontikka

Copy link
Copy Markdown
Owner

Merged as ad2224f (your commit rebased onto current main — it conflicted trivially with the netcheck_override option that landed there yesterday, both sides adding adjacent schema lines; authorship preserved). Closing manually since the rebase changed the SHA.

Verified on the wire against Headscale before merging, both directions:

  • ipn_version: "1.98.9" → the register request carries "IPNVersion":"1.98.9-t00000000000-g00000000000" — so the normalization judgment call works exactly as you described, and I kept it: a knob that silently doesn't display would generate more confusion than a synthetic suffix. The CHANGELOG and README both state that bare semvers are not sent verbatim.
  • Option omitted → the field is absent from Hostinfo, byte-identical to the previous behaviour.

Thanks for respinning it to opt-in without being asked twice — and for flagging the version.Long() shape analysis instead of leaving it as a surprise.

@Csontikka Csontikka closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants