feat(npm): add npm.publish_inactivity_timeout part 1/2 (upload-inactivity-deadline plan: 4/4)

What

Adds the npm.publish_inactivity_timeout configuration key: schema, loader, cross-field validation, example config and reference row. Part 1 of 2 of the plan's step 4.

Nothing reads the key on this MR. The handler arm that reads it is part 2, so the two review separately: this one is the configuration contract, that one is the request behavior.

ADR-004 asks for this inactivity timeout by name and cites Slowloris.

Stack, in merge order

Step MR State
1, a re-arming reader in internal/server !2345 (merged) merged
2, the OCI blob upload arm !2346 (merged) merged
3, the Maven artifact PUT arm !2351 (merged) merged
4 part 1, this configuration key !2459 (merged) (this MR) open
4 part 2, the npm publish arm !2460 (merged) open

Merge order: this MR first. Part 2 reads config.NpmConfig.PublishInactivityTimeout, so it does not compile without this one. Guardrail 25 asks for that order to be stated where a reader will look, and !2460 (merged)'s description names it too.

What the key does

It bounds the gap between two reads of a publish envelope body rather than the transfer as a whole, so a client that keeps sending is not cut while an idle one is. npm.publish_read_timeout stays the absolute ceiling.

The loader's three behaviors, each with a test:

  • An explicitly set value above the ceiling is refused at startup, naming both fields.
  • An omitted key takes the compiled-in 5m default and is then clamped down to a lower ceiling, so a deployment that only lowers publish_read_timeout still boots rather than failing validation over a key it never wrote.
  • A value equal to the ceiling is accepted and leaves the bound inert, because every arm part 2 makes then resolves to the ceiling.

The clamp logs nothing, and that silence is deliberate: the identical clamps at internal/config/container.go:120 and internal/config/maven.go:99 are silent too. It cannot hide a misconfiguration, because an explicitly set value above the ceiling is refused by NpmConfig.validate rather than clamped. The clamp fires only on a key nobody wrote, and it leaves the window equal to publish_read_timeout rather than unbounded, so part 2's arm still cuts a stalled publish, at the read deadline instead of earlier.

The field pairs features.field_presence = EXPLICIT with (buf.validate.field).string.min_len = 1, the same pattern container and maven already use. That lets the wire boundary separate three cases: absent defaults, explicitly blank is refused naming the field, and a set value is parsed and then checked against the cross-field invariant in NpmConfig.validate.

It is the only field in NpmConfig carrying a protovalidate constraint; checked against all 14 others.

Claims that only hold before part 2 lands

The key is inert here, so the three surfaces that describe it are written as a condition and its event rather than asserting behavior that does not exist yet: they say the value is parsed, clamped and validated and no read deadline moves, and what happens once the publish handler arms the wrap. Each therefore reads correctly on both sides of part 2's merge.

Part 2 rewrites those three passages to their settled wording. That is 52 lines of re-touched text and the only overlap between the two parts.

Files

File Reviewable LOC
internal/config/npm_test.go 183
internal/config/npm.go 47
proto/artifactregistry/config/v1/config.proto 33
config.example.yaml 21
docs/dev/configuration-reference.md 3
internal/config/testdata/npm_happy_path.yaml 1

288 reviewable LOC, comfortably under the 500-LOC guideline, plus gen/artifactregistry/config/v1/config.pb.go (45 lines, regenerated rather than hand-edited). That file is in the diff because buf:generate-check diffs gen/, so an unlisted regeneration is a red pipeline rather than a tidy omission.

Guardrail 19 is satisfied: the proto, the loader, config.example.yaml and the configuration reference all move together, and the new reference row names no spec, plan or sprint identifier.

Testing

internal/config/npm_test.go:

  • TestLoad_Npm_ExplicitEmptyStringRejected — the min_len = 1 boundary.
  • TestLoad_Npm_PublishInactivityAboveReadTimeoutInvariant — the cross-field refusal, in both directions and at equality.
  • TestLoad_Npm_DefaultedInactivityYieldsToALowerCeiling — the one cross-field case validate must not reach, on a key that was never written.
  • TestLoad_Npm_ConfigExampleDefault — the value this file ships.
  • TestLoad_Npm_InvalidDurationFields — every npm duration field crossed with {unparseable, zero, negative}, matching TestLoad_Container_InvalidDurationFields. The zero and negative branches are what pin parseDurationField's positivity arm from the npm side, and that arm is what keeps a non-positive window out of part 2's handler, where server.NewDeadlineReader panics on one.

internal/config/testdata/npm_happy_path.yaml carries the key at 6m, distinct from the 5m default, so the happy-path test reads a parsed value rather than a defaulted one.

No conformance run applies and no e2e scenario changes: this MR adds no route and moves no protocol behavior. Part 2 carries the e2e.npm.publish.stalled-publish-cut scenario.

Related to #1230 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading