finos / finos/architecture-as-code

Validation: how should the document loader route references when validating with -c (CalmHub URL)?

Open
#2,827 0 comments 0 reactions 0 assignees View on GitHub
validator
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.