finos / finos/architecture-as-code
Validation: how should the document loader route references when validating with -c (CalmHub URL)?
- Dominant language
- TypeScript
- Stars
- 399
- Forks
- 138
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 37
Description
## Question / design decision (split from PR #2805)
PR #2805 (validation engine redesign) adds strict control validation, which loads each control's
`requirement-url` and `config-url` document and validates the config against the requirement schema.
This surfaced a **document-loader routing** question when validating with `-c `, captured
here for a maintainer steer before we bake a rule into the loader.
### The failing case
Smoke test `validate against a CalmHub-hosted pattern URL passes`
(`cli/smoke/generate-validate.smoke.spec.ts`) runs, in effect:
```
calm validate -p -a -c http://localhost:8080
```
The architecture's controls reference requirement schemas hosted on a **different** host, e.g.
`https://calm.finos.org/getting-started/controls/permitted-connection.requirement.json`.
`CalmHubDocumentLoader` is registered for `http:`, `https:` and `calm:`
(`CALM_HUB_PROTOS`, `shared/src/document-loader/document-loader.ts`). With `-c` set, it claims that
external `https://calm.finos.org/...` reference, **discards the host**, and fetches only the path
against the configured hub base URL (`http://localhost:8080`). The hub does not have that document, so
it fails **fatally**, and `MultiStrategyDocumentLoader` stops the chain before
`DirectUrlDocumentLoader` can fetch it from the real host:
```
Loader CalmHubDocumentLoader failed fatally loading document:
https://calm.finos.org/getting-started/controls/permitted-connection.requirement.json
"Could not load requirement schema ...: Failed to load document from CALMHub:
https://calm.finos.org/getting-started/controls/permitted-connection.requirement.json"
```
Without `-c`, there is no CalmHub loader in the chain, so the same references resolve correctly via
`DirectUrlDocumentLoader`. So the failure is specific to: **validate with `-c` against an architecture
that references documents on another host.**
### Candidate fix (host-scoping rules)
Scope the CalmHub loader to its own host:
1. `calm:` references are always owned by the CalmHub loader (unchanged).
2. `http(s)` references whose host **equals** the configured hub host are owned by the CalmHub loader.
3. `http(s)` references whose host **differs** are recoverable and fall through to
`DirectUrlDocumentLoader` (host allow-list, no auth).
This makes the failing test pass while keeping hub-hosted pattern and document URLs working. It loses
no behaviour that currently works: today an other-host `http(s)` reference is fetched (path-only)
against the `-c` hub, which either 404s fatally or silently returns the wrong document if that path
happens to exist on the `-c` hub.
### The question
Do we want to adopt these routing rules as the loader contract?
- Is host-scoping (rules 1 to 3) the policy we want, or should the CalmHub loader behave differently,
for example treat `-c` as a deliberate global redirect so **all** `http(s)` references resolve from
the configured hub (an offline or air-gapped mirror model)?
- Under host-scoping, a reference to a **different, authenticated** CalmHub than `-c` is unsupported:
it falls through to the direct loader, which sends no auth. Is that acceptable as a follow-up, or do
we need per-host authentication as part of this work?
Happy to apply the host-scoping change in #2805 once we agree the rules; until then that one smoke
test stays red.
### Additional context
Split from PR #2805 to keep that PR focused. Sibling follow-ups: #2821 (cycle detection) and #2822
(de-duplicate a shared invalid sub-architecture's findings).
Contributor guide
Research direction
Start by running the failing smoke test in cli/smoke/generate-validate.smoke.spec.ts and reading shared/src/document-loader/document-loader.ts, including CalmHubDocumentLoader, DirectUrlDocumentLoader, and MultiStrategyDocumentLoader. Review PR #2805 and the routing question; done requires an agreed loader contract and a passing smoke test for validation with -c and references hosted on another host.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- cli, testing, tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100