ADR-015: where a site's own data is allowed to live
ShopDB is a product several sites adopt, developed at one site against that site's live fleet, so every feature arrives carrying West Jefferson's hostnames, networks and share paths. A scan of plugins/ and shopdb/ returns 19 site literals, of which about 8 are a shipped default or a hardcode rather than a comment: a kiosk URL baked into a PowerShell payload a sister site cannot override, a WJ UNC path as the backup share default, and device.geaerospace.net shipped as a SETTING DEFAULT - inherited silently by anyone who adopts. The mechanisms already existed; the rule did not, so each value landed wherever was convenient. The rule: a setting with a NEUTRAL default first, a site-namespaced directory second, seed data third. Blank must be a working state. Fleet-wide vocabulary such as the gea-shopfloor-* imaging types is not site data and is out of scope - it already has a per-site override. Enforced by an addition to check-naming-and-style.sh, which CI already runs, in REPORT-ONLY mode. Making a 19-item backlog a hard gate the day it lands blocks every unrelated commit until someone clears it; SITE_LITERALS_ENFORCE=1 turns it into a gate once the listed hardcodes are done. The ADR carries that backlog, including that the display FQDN domain is defined three times in three files.
This commit is contained in:
@@ -24,6 +24,7 @@ Architecture decisions live in `docs/adr/`. Read those before making schema or c
|
|||||||
- ADR-012: GE-Enforce manifest ownership in shopdb - ACCEPTED
|
- ADR-012: GE-Enforce manifest ownership in shopdb - ACCEPTED
|
||||||
- ADR-013: Plugin catalog, curated shelf, and lean per-site builds - PROPOSED
|
- ADR-013: Plugin catalog, curated shelf, and lean per-site builds - PROPOSED
|
||||||
- ADR-014: Schema-lean per-site builds (retire cross-plugin FKs, prune not-installed plugin tables) - ACCEPTED
|
- ADR-014: Schema-lean per-site builds (retire cross-plugin FKs, prune not-installed plugin tables) - ACCEPTED
|
||||||
|
- ADR-015: Where a site's own data is allowed to live (setting with a neutral default, site-namespaced dir, or seed) - ACCEPTED
|
||||||
|
|
||||||
## Coding convention
|
## Coding convention
|
||||||
|
|
||||||
|
|||||||
117
docs/adr/ADR-015-site-specific-configuration.md
Normal file
117
docs/adr/ADR-015-site-specific-configuration.md
Normal file
@@ -0,0 +1,117 @@
|
|||||||
|
# ADR-015: Where a site's own data is allowed to live
|
||||||
|
|
||||||
|
- Status: ACCEPTED
|
||||||
|
- Date: 2026-08-11
|
||||||
|
- Deciders: cproudlock
|
||||||
|
- Relates to: ADR-004 (per-site instances, not multi-tenant), ADR-013 (plugin catalog + lean per-site builds), ADR-014 (schema-lean per-site builds)
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
ShopDB is built as a product several GE Aerospace sites can adopt (ADR-004:
|
||||||
|
each site runs its own instance). It is also developed at one site, West
|
||||||
|
Jefferson, against that site's live fleet. Every feature therefore arrives
|
||||||
|
carrying that site's hostnames, networks, share paths and vocabulary, and the
|
||||||
|
fastest way to ship any of them is to write the value where the code needs it.
|
||||||
|
|
||||||
|
That is not a hypothetical drift. A scan of `plugins/` and `shopdb/` for a site
|
||||||
|
host, a site FQDN or a site network returns 19 hits, of which roughly 8 are a
|
||||||
|
shipped default or a hardcode rather than a comment:
|
||||||
|
|
||||||
|
- `plugins/geenforce/seed_display_scope.py` bakes `https://tsgwp00525.wjs.
|
||||||
|
geaerospace.net/shopdb` and `device.geaerospace.net` into an inline PowerShell
|
||||||
|
payload. A sister site cannot override either without editing the source.
|
||||||
|
- `plugins/backups/services/registry.py` defaults the backup share root to a WJ
|
||||||
|
UNC path.
|
||||||
|
- `shopdb/core/api/settings.py` ships `device.geaerospace.net` and
|
||||||
|
`Printer-{ip}.printer.geaerospace.net` as SETTING DEFAULTS, so another site
|
||||||
|
inherits WJ's domain silently until someone notices.
|
||||||
|
- `shopdb/core/models/dashboarddefault.py` holds the same domain a second time
|
||||||
|
as a module constant, and `plugins/computers/api/routes.py` a third time as a
|
||||||
|
literal fallback.
|
||||||
|
|
||||||
|
The problem is not that site-specific values exist - they must. It is that the
|
||||||
|
repository already has three good mechanisms for them and no rule about which to
|
||||||
|
use, so each new value lands wherever was convenient. The result reads as drift
|
||||||
|
because it is unmanaged, not because any single choice was wrong.
|
||||||
|
|
||||||
|
Nothing here is a security matter: these are internal hostnames on an air-gapped
|
||||||
|
network, and `tests/test_docs_publishable.py` already gates what reaches the
|
||||||
|
public wiki. This is about whether a sister site can adopt the product without
|
||||||
|
reading our source.
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
A site's own data lives in one of three places, in this order of preference.
|
||||||
|
|
||||||
|
**1. A setting, with a NEUTRAL default.** Anything an operator could plausibly
|
||||||
|
change: hostnames, domains, share paths, networks, base URLs, label prefixes.
|
||||||
|
The default that ships must be blank or generic, never one site's value. Blank
|
||||||
|
must be a working state - code reads the setting and degrades sensibly, the way
|
||||||
|
`display_fqdn_domain` already treats blank as "use the hostname as-is". A
|
||||||
|
site-specific default is the failure mode this ADR exists to stop: it is
|
||||||
|
invisible, it looks deliberate, and it is wrong everywhere except here.
|
||||||
|
|
||||||
|
**2. A site-namespaced directory.** Anything that is genuinely one site's and
|
||||||
|
will never be another's, such as the West Jefferson import loader at
|
||||||
|
`scripts/site_imports/wjf/`. Never reached by another site's build.
|
||||||
|
|
||||||
|
**3. Seed data, not code.** Vocabularies and catalogs, loaded by `flask seed`
|
||||||
|
and editable per site afterwards.
|
||||||
|
|
||||||
|
Fleet-wide convention is NOT site-specific. The `gea-shopfloor-*` imaging types
|
||||||
|
are a GE fleet taxonomy, not one site's data, and they already carry a per-site
|
||||||
|
override through the `pctypemap_*` settings. They are out of scope here.
|
||||||
|
|
||||||
|
Comments and documentation may name a site where it explains WHY - "part markers
|
||||||
|
share a machine number at West Jefferson" is a fact a maintainer needs. Prefer
|
||||||
|
phrasing the rule generally and the example specifically.
|
||||||
|
|
||||||
|
## Enforcement
|
||||||
|
|
||||||
|
`scripts/check-naming-and-style.sh`, which already runs in CI, greps `plugins/`
|
||||||
|
and `shopdb/` for a site host, a site FQDN or a site network, excluding tests
|
||||||
|
and `site_imports/`.
|
||||||
|
|
||||||
|
It ships in REPORT-ONLY mode. It prints findings and does not fail the build,
|
||||||
|
because turning a 19-item backlog into a hard gate on the day it lands blocks
|
||||||
|
every unrelated commit until someone works through it. Set
|
||||||
|
`SITE_LITERALS_ENFORCE=1` to make it a gate; do that once the real hardcodes
|
||||||
|
below are cleared.
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
Positive:
|
||||||
|
|
||||||
|
- A sister site can adopt without reading our source for values it must change.
|
||||||
|
- The backlog is visible and counted rather than felt.
|
||||||
|
- New site-specifics are caught at commit time, in the same place the naming
|
||||||
|
rules are already enforced, so there is nothing new to remember.
|
||||||
|
|
||||||
|
Negative:
|
||||||
|
|
||||||
|
- Report-only means the rule is advisory until someone flips it, and an
|
||||||
|
advisory rule can be ignored.
|
||||||
|
- A neutral default is more code than a working one: every read needs a blank
|
||||||
|
case, which is a real cost paid at every call site.
|
||||||
|
- The pattern list is a denylist and will miss a site value it has not been
|
||||||
|
taught. It catches drift, not everything.
|
||||||
|
|
||||||
|
## Backlog
|
||||||
|
|
||||||
|
Real hardcodes and site-specific defaults, to clear before enforcing:
|
||||||
|
|
||||||
|
- `plugins/geenforce/seed_display_scope.py` - kiosk base URL, FQDN domain, and
|
||||||
|
the legacy-shortcut sweep pattern
|
||||||
|
- `plugins/backups/services/registry.py` - `DEFAULTSHAREROOT`
|
||||||
|
- `shopdb/core/api/settings.py` - `display_fqdn_domain` and the printer
|
||||||
|
hostname pattern defaults
|
||||||
|
- `shopdb/core/models/dashboarddefault.py` - `DEFAULT_DISPLAY_FQDN_DOMAIN`
|
||||||
|
- `plugins/computers/api/routes.py` - literal fallback duplicating that default
|
||||||
|
|
||||||
|
The domain appears three times in three files, which is its own defect: one
|
||||||
|
setting, one default, one read.
|
||||||
|
|
||||||
|
Cosmetic mentions in comments and UI placeholder text are acceptable under this
|
||||||
|
ADR and are not part of the backlog, with one exception worth fixing when
|
||||||
|
convenient: the GE-Enforce settings page shows WJ's real CIDRs as its
|
||||||
|
placeholder, which every site sees.
|
||||||
@@ -27,6 +27,7 @@ Each ADR captures a single architectural decision: the context, the decision its
|
|||||||
| [012](ADR-012-geenforce-manifest-ownership.md) | GE-Enforce manifest ownership in shopdb | ACCEPTED |
|
| [012](ADR-012-geenforce-manifest-ownership.md) | GE-Enforce manifest ownership in shopdb | ACCEPTED |
|
||||||
| [013](ADR-013-plugin-catalog-and-lean-builds.md) | Plugin catalog, curated shelf, and lean per-site builds | PROPOSED |
|
| [013](ADR-013-plugin-catalog-and-lean-builds.md) | Plugin catalog, curated shelf, and lean per-site builds | PROPOSED |
|
||||||
| [014](ADR-014-schema-lean-per-site.md) | Schema-lean per-site builds (retire cross-plugin FKs, prune not-installed plugin tables) | ACCEPTED |
|
| [014](ADR-014-schema-lean-per-site.md) | Schema-lean per-site builds (retire cross-plugin FKs, prune not-installed plugin tables) | ACCEPTED |
|
||||||
|
| [015](ADR-015-site-specific-configuration.md) | Where a site's own data is allowed to live (setting, site directory, or seed) | ACCEPTED |
|
||||||
|
|
||||||
## Authoring
|
## Authoring
|
||||||
|
|
||||||
|
|||||||
@@ -135,6 +135,41 @@ if [ -d plugins ]; then
|
|||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# ADR-015: one site's data does not belong in product code. A site host, a site
|
||||||
|
# FQDN or a site network in plugins/ or shopdb/ ships another site a value it
|
||||||
|
# cannot see and did not choose. Use a setting with a NEUTRAL default, a
|
||||||
|
# site-namespaced directory (scripts/site_imports/<site>/), or seed data.
|
||||||
|
#
|
||||||
|
# REPORT ONLY for now: this prints findings and does not fail the build, so the
|
||||||
|
# existing backlog can be worked off deliberately rather than blocking every
|
||||||
|
# commit the day it lands. Flip SITE_LITERALS_ENFORCE to 1 to make it a gate.
|
||||||
|
SITE_LITERALS_ENFORCE=${SITE_LITERALS_ENFORCE:-0}
|
||||||
|
|
||||||
|
echo "==> Checking for site-specific literals in product code (ADR-015)..."
|
||||||
|
# Deliberately narrow: a real site hostname, a site FQDN, or a site network.
|
||||||
|
# Fleet-wide vocabulary (gea-shopfloor-*) is NOT matched - it is overridable
|
||||||
|
# through the pctypemap settings and is not one site's data.
|
||||||
|
SITE_PATTERNS='tsgwp00525|\.geaerospace\.net|\bwjs\b|West Jefferson|10\.134\.48\.|10\.48\.249\.'
|
||||||
|
SITE_HITS=$(grep -rPn "$SITE_PATTERNS" "${EXCLUDES[@]}" \
|
||||||
|
--include='*.py' --include='*.vue' --include='*.js' \
|
||||||
|
plugins/ shopdb/ 2>/dev/null \
|
||||||
|
| grep -v '/tests\?/' \
|
||||||
|
| grep -v 'site_imports/' || true)
|
||||||
|
if [ -n "$SITE_HITS" ]; then
|
||||||
|
COUNT=$(echo "$SITE_HITS" | wc -l)
|
||||||
|
if [ "$SITE_LITERALS_ENFORCE" = "1" ]; then
|
||||||
|
echo "FAIL: $COUNT site-specific literal(s) in product code:"
|
||||||
|
echo "$SITE_HITS"
|
||||||
|
echo
|
||||||
|
VIOLATIONS=$((VIOLATIONS + 1))
|
||||||
|
else
|
||||||
|
echo " $COUNT site-specific literal(s) found (report only, not failing):"
|
||||||
|
echo "$SITE_HITS" | sed 's/^/ /'
|
||||||
|
echo " See docs/adr/ADR-015-site-specific-configuration.md"
|
||||||
|
echo
|
||||||
|
fi
|
||||||
|
fi
|
||||||
|
|
||||||
if [ "$VIOLATIONS" -gt 0 ]; then
|
if [ "$VIOLATIONS" -gt 0 ]; then
|
||||||
echo "=================================================="
|
echo "=================================================="
|
||||||
echo "$VIOLATIONS naming/style violation(s) found."
|
echo "$VIOLATIONS naming/style violation(s) found."
|
||||||
|
|||||||
Reference in New Issue
Block a user