Check client / agent versions over RPC#353
Conversation
8b2120c to
faac102
Compare
There was a problem hiding this comment.
I like the Interceptor mechanic and using the headers. Just the right tool for that purpose.
Which side should run the check?
The hard, authoritative check belongs to the agent. The agent is entity that must guard its own contract, and it's the only side that can actually reject. A client can decline to proceed, but only the agent can refuse to execute. So:
Generally, both, the client and the agent should include their version in the header
Agent (enforcement): The interceptor reads the client version from the request header and, on incompatibility, returns, e.g., connect.CodeFailedPrecondition together with helpful message before any module runs.
Client (advisory only): Read the agent version off the response header and, on mismatch, print a friendly warning ("agent is newer, please upgrade the client"). And maybe extended logic before v1.
Also check if the dutserver code needs to be adapted to reflect that behavior
db3db78 to
065e172
Compare
The dutagent build version is baked from git tags via -ldflags and advertised on every RPC response header. Clients compare it against their own version: minor mismatch warns, major errors, pre-release/dev diffs warn. Replaces the explicit Version RPC. Signed-off-by: llogen <christoph.lange@blindspot.software>
065e172 to
7b46f38
Compare
The ireturn linter's verdict on Broker.Start (which returns module.Session) is nondeterministic across golangci-lint builds and Go toolchains: on the official v2.6.1 binary used in CI it does not flag the return, so the //nolint:ireturn directive is reported as unused by nolintlint and fails the Lint job — while stricter local builds still flag it, so simply removing the directive would break local linting. Add Session to the ireturn allow list (matching the existing convention for intentionally-returned interfaces such as Module and Interceptor) and drop the now-redundant nolint directive. The result is deterministic: ireturn never flags module.Session, so no suppression is needed. Signed-off-by: Jens Topp <jens.topp@blindspot.software>
Verify dutctl client ↔ agent version compatibility
The dutagent now bakes its version from the git tag and advertises it on every RPC response. Clients compare it against their own version and react to the difference, so an incompatible pairing is caught immediately instead of failing later with a confusing protocol error.
Changes
-ldflags "-X .../buildinfo.buildVersion=<tag>".buildinfo.Semver()resolves it in order: injected build version → the module version fromdebug.ReadBuildInfo()(so a plaingo buildstill gets a real, git-derived version) → the release-please constant as a last-resort fallback.dutagent -vnow reflects this.pkg/versioncheckadds a connect server interceptor that stamps the agent's version onto theX-Dutctl-Versionheader of every RPC (unary and the streamingRun); the dutagent registers it.VersionRPC — superseded by the always-on header (dropped from the proto, generated code, handler, and tests).Compatibility policy
FailedPrecondition1.0.0-alpha.1vs1.0.0-alpha.5) → warningThe check runs agent→client: the agent advertises, the client enforces. The interceptors are exported so other dutctl clients can adopt them.
Testing
go test ./...,go vet ./..., andgolangci-lint runall pass.