-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Windows display name, clean-reinstall, macOS ensure-hosts, 3-OS service smoke CI #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| # A REAL, 3-OS end-to-end smoke test of `dig-node install`/`start`/`uninstall` against the | ||
| # actual OS service manager (Windows SCM / systemd / launchd) — the thing ci.yml's mocked | ||
| # `ServiceBackend` unit tests (crates/dig-node-service/src/service.rs) cannot prove: that | ||
| # `install` registers a service the OS actually starts and serves from, that a SECOND `install` | ||
| # cleanly recreates it instead of hitting Windows `CreateService` error 1073 (#494), and that | ||
| # `uninstall` actually removes the registration. | ||
| # | ||
| # Kept as its OWN workflow (not folded into ci.yml) because it performs REAL, mutating | ||
| # OS-service registration on every runner — a materially different (and slower/less hermetic) | ||
| # kind of check than the rest of the PR gate set. | ||
| name: Service smoke test | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - "crates/dig-node-service/src/service.rs" | ||
| - "crates/dig-node-service/src/config.rs" | ||
| - "crates/dig-node-service/src/hosts.rs" | ||
| - "packaging/**" | ||
| - ".github/workflows/service-smoke.yml" | ||
| push: | ||
| branches: | ||
| - main | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: ${{ github.ref }}-${{ github.workflow }}-${{ github.event_name }} | ||
| cancel-in-progress: true | ||
|
|
||
| env: | ||
| CARGO_TERM_COLOR: always | ||
|
|
||
| jobs: | ||
| smoke: | ||
| name: install/start/reinstall/uninstall (${{ matrix.os }}) | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [windows-latest, macos-14, ubuntu-latest] | ||
| runs-on: ${{ matrix.os }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Install Rust | ||
| uses: dtolnay/rust-toolchain@stable | ||
|
|
||
| - uses: Swatinem/rust-cache@v2 | ||
| with: | ||
| key: service-smoke | ||
|
|
||
| - name: Build dig-node (release) | ||
| run: cargo build --release --locked --bin dig-node | ||
|
|
||
| # `dig-node install` registers a USER-level systemd unit on Linux (no root needed), which | ||
| # needs a real user D-Bus session — the GH-hosted runner's non-interactive job shell does | ||
| # not start one implicitly. `enable-linger` + pointing at the per-uid runtime dir brings one | ||
| # up so `systemctl --user ...` (what service-manager shells out to) has a bus to talk to. | ||
| - name: (Linux) bring up a user systemd/D-Bus session | ||
| if: runner.os == 'Linux' | ||
| run: | | ||
| sudo loginctl enable-linger "$(whoami)" | ||
| echo "XDG_RUNTIME_DIR=/run/user/$(id -u)" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Resolve the built binary path | ||
| id: bin | ||
| shell: bash | ||
| run: | | ||
| if [ "${{ runner.os }}" = "Windows" ]; then | ||
| echo "path=target/release/dig-node.exe" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "path=target/release/dig-node" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Install (1st) — fresh registration | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" install --json | tee install-1.json | ||
| grep -q '"installed":true' install-1.json | ||
| grep -q '"reinstalled":false' install-1.json | ||
|
|
||
| - name: Windows — assert the SCM display name | ||
| if: runner.os == 'Windows' | ||
| shell: bash | ||
| run: | | ||
| sc.exe qc net.dignetwork.dig-node | tee sc-qc-1.txt | ||
| grep -Eq 'DISPLAY_NAME[[:space:]]*: DIG NETWORK: NODE' sc-qc-1.txt | ||
|
|
||
| - name: Start + poll for RUNNING | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" start --json || true | ||
| ok=0 | ||
| for _ in $(seq 1 20); do | ||
| if "${{ steps.bin.outputs.path }}" status --json | tee status-1.json | grep -q '"serving":true'; then | ||
| ok=1 | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| test "$ok" -eq 1 | ||
|
|
||
| - name: Install (2nd) — proves clean-reinstall avoids CreateService 1073 | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" install --json | tee install-2.json | ||
| grep -q '"installed":true' install-2.json | ||
| grep -q '"reinstalled":true' install-2.json | ||
|
|
||
| - name: Windows — re-assert the SCM display name after reinstall | ||
| if: runner.os == 'Windows' | ||
| shell: bash | ||
| run: | | ||
| sc.exe qc net.dignetwork.dig-node | tee sc-qc-2.txt | ||
| grep -Eq 'DISPLAY_NAME[[:space:]]*: DIG NETWORK: NODE' sc-qc-2.txt | ||
|
|
||
| - name: Start again + poll for RUNNING | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" start --json || true | ||
| ok=0 | ||
| for _ in $(seq 1 20); do | ||
| if "${{ steps.bin.outputs.path }}" status --json | tee status-2.json | grep -q '"serving":true'; then | ||
| ok=1 | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| test "$ok" -eq 1 | ||
|
|
||
| - name: Stop | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" stop --json | ||
|
|
||
| - name: Uninstall + assert the service is gone | ||
| shell: bash | ||
| run: | | ||
| "${{ steps.bin.outputs.path }}" uninstall --json | tee uninstall.json | ||
| grep -q '"installed":false' uninstall.json | ||
| if [ "${{ runner.os }}" = "Windows" ]; then | ||
| ! sc.exe query net.dignetwork.dig-node | ||
| elif [ "${{ runner.os }}" = "macOS" ]; then | ||
| ! launchctl print "gui/$(id -u)/net.dignetwork.dig-node" | ||
| else | ||
| ! systemctl --user cat net.dignetwork.dig-node.service | ||
| fi | ||
|
|
||
| - name: Upload diagnostics | ||
| if: always() | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: service-smoke-${{ matrix.os }} | ||
| path: | | ||
| install-1.json | ||
| install-2.json | ||
| status-1.json | ||
| status-2.json | ||
| uninstall.json | ||
| sc-qc-1.txt | ||
| sc-qc-2.txt | ||
| if-no-files-found: ignore | ||
Check warningCode scanning / CodeQL Workflow does not contain permissions Medium
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}
|
||
|
Comment on lines
+35
to
+163
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # Development Log — dig-node | ||
|
|
||
| High-signal realizations from debugging/development: non-obvious cross-system couplings, | ||
| sharp edges, and gotchas. Concise durable facts with context — NOT a change diary. See | ||
| `CLAUDE.md` §4.5 for the maintenance contract (a curator periodically re-verifies + prunes). | ||
|
|
||
| ## Bare-git dependency version pins unify across the WHOLE dependency graph, not per-manifest (#494) | ||
|
|
||
| A `git`-sourced Cargo dependency with NO `rev`/`branch`/`tag` is identified purely by its URL — | ||
| every crate in the build graph that declares `dig-constants = { git = "https://github.com/..." }` | ||
| with no ref resolves to the SAME package instance, whatever `version =` requirement each manifest | ||
| states individually. If two manifests state incompatible 0.x requirements against that one bare | ||
| source (e.g. `dig-node-core`'s transitive `dig-nat`/`dig-gossip`/`dig-dht`/`dig-onion` chain pins | ||
| `dig-constants = "0.2"`, i.e. `^0.2` = 0.2.x ONLY), cargo cannot resolve a single version | ||
| satisfying both — bumping one manifest's bare-git requirement to `"0.3"` breaks the whole graph | ||
| until every bare-git consumer moves together. | ||
|
|
||
| **The escape hatch:** an EXPLICIT `rev =` pin is a cargo-DISTINCT source from a bare git dep, | ||
| even at the identical commit — so a crate that needs a newer version of a NOT-yet-crates.io'd | ||
| dependency can `rev`-pin its OWN copy without forcing every other bare-git consumer forward. This | ||
| is exactly how `dig-node-service` picked up `dig_constants::DIG_NODE_PORT` (added in 0.3.0) | ||
| without waiting on `dig-nat`/`dig-gossip`/`dig-dht`/`dig-onion` to move off their `^0.2` pin: it | ||
| added `dig-constants = { version = "0.3", git = "...", rev = "<v0.3.0 commit>" }`, giving the | ||
| crate its own 0.3.0 instance living alongside the graph's existing 0.2.1 (bare-git) and 0.1.0 | ||
| (crates.io registry) instances. Safe whenever the only thing crossing the boundary between the | ||
| two instances is a plain value type (here, a `u16` constant) — never safe if a type from one | ||
| instance needs to be passed to/from code built against the other. | ||
|
|
||
| ## The dig-installer's `install` → `start` sequencing constrains what `dig-node install` may do | ||
|
|
||
| `dig-installer`'s `register_dig_node` step calls `dig-node install` and then, when configured to | ||
| start it (the default), a SEPARATE `dig-node start` — and treats a `start` FAILURE as fatal for | ||
| that installer step (unlike the tolerant treatment of an `install` failure). This means | ||
| `dig-node install` must NEVER auto-start the service itself: if it did, the installer's follow-up | ||
| `start` would hit "service already running" (Windows SCM 1056, or a systemd/launchd | ||
| no-op-or-error depending on backend) and could flip the installer's REPORTED `installed` status | ||
| to `false` even though the service is actually up and running fine. `dig-dns`'s equivalent | ||
| `reinstall()` DOES auto-start at the end of its clean-reinstall — that pattern was deliberately | ||
| NOT mirrored here for this reason. Any future change to `dig-node install`'s start behavior must | ||
| also update `dig-installer`'s `register_dig_node`/`install_service` in the SAME unit of work. | ||
|
|
||
| ## `service-manager`'s systemd backend registers under `to_script_name()`, not `to_qualified_name()` (#494) | ||
|
|
||
| `ServiceLabel` has TWO different string renderings — `to_qualified_name()` (`{qualifier}. | ||
| {organization}.{application}`, e.g. `net.dignetwork.dig-node`) and `to_script_name()` | ||
| (`{organization}-{application}`, e.g. `dignetwork-dig-node` — the qualifier is DROPPED | ||
| entirely). `service-manager` 0.7's Windows (`sc.rs`) and launchd (`launchd.rs`) backends both | ||
| register the service under `to_qualified_name()`, but its **systemd** backend (`systemd.rs`) | ||
| names the actual unit file from `to_script_name()` instead — a real, silent divergence with no | ||
| compile-time signal. Any code that probes "is this service registered?" by shelling out | ||
| directly (`service-manager` itself exposes no such query) MUST use the SAME name the relevant | ||
| backend actually registered under, per-platform — using `to_qualified_name()` uniformly makes | ||
| the probe always report "not found" on Linux, invisibly. This was caught only by a REAL 3-OS | ||
| `service-smoke` CI run (mocked-backend unit tests, being backend-agnostic by design, cannot | ||
| catch it) — a second `dig-node install` on `ubuntu-latest` reported `reinstalled:false` because | ||
| `is_installed()` never saw the service it had just registered. | ||
|
|
||
| ## Windows `sc create` always names the display the same as the service id | ||
|
|
||
| `service-manager` 0.7's `sc.rs` backend hardcodes the Windows SCM display name to the service id | ||
| at `sc create` time — there is no `ServiceInstallCtx` field for it. The only way to set a | ||
| friendly display name is a POST-create `sc config <id> displayname= "<name>"` follow-up (and, | ||
| per #494, a `sc qc <id>` read-back to actually confirm it took, rather than trusting the `sc | ||
| config` exit code). `service-manager`'s systemd/launchd backends have no display-name-equivalent | ||
| override either — for systemd the closest analog is `Description=`, generated from the label with | ||
| no override field; for launchd there is no such key at all. The NATIVE `.deb`/`.pkg`/`.msi` | ||
| packages (`packaging/`) sidestep this entirely by shipping their own static unit | ||
| file/plist/WiX-`ServiceInstall` with the friendly name baked in — only the bare `dig-node install` | ||
| CLI path (not via a native package) needs the `sc config`/`sc qc` dance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.