Skip to content

Render a declared limit through the manifest read path (#78) - #82

Merged
sehkone merged 1 commit into
mainfrom
sehkone/issue-78
Aug 29, 2026
Merged

Render a declared limit through the manifest read path (#78)#82
sehkone merged 1 commit into
mainfrom
sehkone/issue-78

Conversation

@sehkone

@sehkone sehkone commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

The bump to manifest format version 5 was cut so a producer's manifest could carry limit_nofile, but nothing asserted that end. The wire form was exercised for the two refusals and for the floor version that emits no directive; the positive case reached the renderer only from a hand-built template, so the read path this bump exists for was never driven end to end.

This adds a_manifest_at_the_current_version_renders_a_declared_descriptor_limit in src/manifest.rs: a manifest at MANIFEST_FORMAT_VERSION declaring limit_nofile parses, the decoded spec carries the value, and the renderer emits LimitNOFILE=8000 on the line after RestartSec=, asserted against the full unit text. It mirrors the test the previous format bump left behind for the manager endpoint.

No production code changes.

Part of #78

Not addressed

Everything else the issue asked for — the limit_nofile field on UnitTemplate, the two ModuleSpecError variants rejecting Some(0) and Some(u64::MAX), the LimitNOFILE= emission between RestartSec= and the sandbox booleans, the producer/ceiling version bump with the floor held still, the rewritten check_format_version doc comment, and the rest of the test plan — already landed on main in #80, which closed the issue. This branch carries only the one read-path assertion that work left uncovered, so nothing in the issue is outstanding once it merges.

Test plan

  • cargo fmt -- --check --config group_imports=StdExternalCrate is clean
  • cargo clippy --all-targets -- -D warnings and cargo clippy --all-targets --features test-support -- -D warnings are clean
  • cargo test and cargo test --features test-support pass
  • A manifest at MANIFEST_FORMAT_VERSION declaring limit_nofile parses, and the decoded UnitTemplate carries the value
  • That decoded spec renders LimitNOFILE=8000 in [Service] on the line after RestartSec= and before the [Install] section
  • The existing floor-version test still asserts a manifest at MIN_MANIFEST_FORMAT_VERSION renders no Limit line
  • The byte-for-byte golden tests for templates that set no limit pass unchanged

The bump to manifest format version 5 was cut so a producer's manifest
could carry `limit_nofile`, but nothing asserted that end: the wire form
was exercised for the two refusals and for the floor version that emits
no directive, while the positive case reached the renderer only from a
hand-built template. A spec decoded from a producer's bytes now renders
`LimitNOFILE=` too, mirroring the test the previous bump left behind.

Part of #78
@sehkone

sehkone commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

[Reviewer Round 1]

Approved — no findings.

The new test in src/manifest.rs genuinely exercises the missing producer-wire-form path: it parses a v5 manifest containing limit_nofile, confirms the decoded field, then asserts the complete rendered unit including LimitNOFILE=8000.

It complements the existing renderer placement and floor-compatibility tests without broadening scope. PR #82’s Part of #78, ## Not addressed, and test plan accurately reflect that #80 already closed the implementation issue.

@sehkone

sehkone commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

[Review Verdict Round 1: APPROVED]

@sehkone
sehkone merged commit 452eb68 into main Aug 29, 2026
4 checks passed
@sehkone
sehkone deleted the sehkone/issue-78 branch August 29, 2026 08:10
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.

1 participant