feat: fact disable mechanism (implemented) + packages fact design (ADR-0014/0015)#27
Merged
Conversation
…0015) ADR-0014: packages fact as source-namespaced lists of records — key on the install database (dpkg not deb, ips not pkg), name is a field not a key, per-source identity fields, one cheap read per source, system-global + collector context. CONTEXT: Package, Package source, Package record. openspec/add-packages-fact (validated --strict). ADR-0015: all facts on by default; disabling is opt-out and resolution-gated, via --disable / FACTS_DISABLE / facts.conf 'disable' (Facter 'blocklist' kept as compat alias). CONTEXT: Disabled fact. Both ADRs adversarially reviewed.
All facts on by default; disable via --disable / FACTS_DISABLE / facts.conf 'disable' (Facter 'blocklist' kept as compat alias). Resolution-gated: standalone-resolver facts skip work, multi-output categories resolve-then- prune. --no-block clears the set; disable-beats-query with a stderr diagnostic; FACTS_DISABLE reserved by resolved name. New capability fact-disable-controls + deltas on facts-cli-option-contract and facts-native-input-surface. Validated --strict.
…->disabled Add the facts-native 'disable' config key, which supersedes the Facter 'blocklist' (retained as compat alias). Rename the internal disabled-set concept for uniformity with the disable feature: Config.Blocklist->Disabled, EngineConfig.BlockedFacts->DisabledFacts, FilterBlockedFacts->FilterDisabledFacts, BlocklistedFacts*->DisabledFacts*, plan.blockedFacts->disabledFacts. Facter-compat surfaces unchanged: the 'blocklist' config key, --no-block, --list-block-groups/--list-cache-groups, LoadExternalFactsWithBlocklist. go build/vet/test ./... all green.
…d entries loadExternalEnvFacts skips any env var whose resolved fact name is 'disable' (FACTS_DISABLE / FACTSDISABLE / FACTER_DISABLE / FACTERDISABLE), so it feeds the disabled set instead of becoming an external fact. Add environmentDisabledFacts (facts-native FACTS_ wins over facter FACTER_) and a shared splitDisableList helper (reused by the --disable CLI flag next). Union into planDiscovery's disabled set is the following slice.
…ostic, group cleanup Complete the fact-disable mechanism (ADR-0015): - planDiscovery unions the disabled set from config 'disable', FACTS_DISABLE (env, gated by includeEnv), and --disable (EngineConfig.ExtraDisabled); the non-nil DisabledFacts override (--no-block) still wins. - --disable CLI flag (valued, repeatable, comma-split); --no-block clears it. - Resolution-gating: buildCoreFacts skips a single-output category resolver (networking, processors, memory, ssh, timezone, fips, augeas, xen) when disabled; multi-output (os/disks/uptime) and shared-probe (identity/dmi) stay eager + resolve-then-prune. CoreFacts memo keyed by disabled-set fingerprint. - Ambient-disable stderr diagnostic for env/config-disabled explicit queries (descendant + ancestor coverage); silent for --disable. - BuiltinFactGroups: drop ADR-0007 legacy flat names, keep structured roots. Reviewed by OpenCodeReview + Codex + a 3-lens adversarial pass; all findings applied (diagnostic edge cases, CoreFacts memo hardening, gating tests that assert probe non-invocation, selinux doc accuracy, help/man wording). Facter compat untouched (blocklist key, --no-block, --list-block-groups/ --list-cache-groups, LoadExternalFactsWithBlocklist). go build/vet/test ./... and openspec validate green.
… README + CHANGELOG Tick the implemented add-fact-disable-controls tasks (1.6 cache regression test left as a follow-up).
The internal/app integration suite runs full Run() discoveries; on the emulated DragonFly VM the package takes ~11m — validated passing in 6m24s on the nlab DragonFly guest with a 25m timeout — which exceeds go test's default 10m per-package limit. The new --disable integration tests were the last straw. The tests are correct; the emulated VMs just need more wall-clock (the job timeout-minutes is 45). Ubuntu 24.04 distro-facts failed separately on a Docker Hub registry timeout (infra flake), re-run by this push.
Track the BSD disks/partitions probe subprocess fan-out found while diagnosing the DragonFly CI timeout: skip md/vn/cd pseudo-devices and bound the DragonFly slice-target fan-out (660 disklabel spawns/discovery on a 127-md-device host). Proposal only; implemented in a follow-up PR. Validated --strict.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Two related pieces of work on one branch:
A. Fact disable mechanism — implemented. All facts are on by default; disable any fact or fact group via three unioned sources —
--disable a,b(CLI),FACTS_DISABLE=a,b(env), and thefacts.confdisablekey (the Facterblocklistkey stays as its compatibility alias). Disabling is resolution-gated: a disabled standalone-resolver fact (networking,processors,memory,ssh,timezone,fips,augeas,xen) is not resolved at all — its work is skipped, not just its output. Multi-output categories (os/disks/uptime) and shared-probe categories (identity/dmi) resolve then prune.--no-blockclears every disable; an env/config-disabled fact that is explicitly queried prints a one-line stderr diagnostic. Implements ADR-0015 /openspec/changes/add-fact-disable-controls.B.
packagesfact — design only. ADR-0014 +openspec/changes/add-packages-fact(validated) capture the design of a cross-platform installed-packages fact:packages.<source>as source-namespaced lists of records, keyed on the install database (dpkg,rpm,pkg,ips,nix, macOSreceipts/apps, Windowsregistry/appx, …). Implementation follows in a separate change; it rides on (A) —--disable packageswill skip collection entirely.Facter compatibility
Untouched: the
blocklistconfig key,--no-block,--list-block-groups/--list-cache-groups,LoadExternalFactsWithBlocklist.disable/FACTS_DISABLEare the facts-native spellings;blocklist/facter.confremain the compat tier.Review
The disable mechanism was reviewed by three independent passes — OpenCodeReview, Codex, and a 3-lens adversarial review. All three confirmed the fact output is correct (union, override-wins,
includeEnvsuppression, single-output gating, compat preserved). All findings were applied: ambient-diagnostic descendant/ancestor coverage,CoreFactsmemo keyed by disabled-set fingerprint, gating tests that assert the resolver's probe is not invoked (real work-avoidance),selinuxADR accuracy, and help/man wording.Design docs
docs/adr/0014-packages-fact-source-namespaced-record-lists.mddocs/adr/0015-facts-on-by-default-disable-is-opt-out-and-skips-resolution.mdCONTEXT.md—Package,Package source,Package record,Disabled factVerification
go build ./...·go vet ./...·go test ./...(all 9 packages) ·openspec validate add-fact-disable-controls --strict— all green.Follow-ups
add-fact-disable-controlstask 1.6 (dedicated "not served from cache" regression test) — behavior verified sound by review; the test itself is a follow-up.packagesfact (add-packages-fact).