Skip to content

refactor: commit the vendor OpenAPI specs instead of fetching them - #114

Merged
ShocOne merged 2 commits into
mainfrom
refactor/commit-the-vendor-openapi-specs
Aug 25, 2026
Merged

refactor: commit the vendor OpenAPI specs instead of fetching them#114
ShocOne merged 2 commits into
mainfrom
refactor/commit-the-vendor-openapi-specs

Conversation

@ShocOne

@ShocOne ShocOne commented Aug 25, 2026

Copy link
Copy Markdown
Member

Stacked on #113, which is stacked on #112. Merge in order and each retargets cleanly. Happy to squash #113 into this one if you'd rather review a single change — this deletes most of what #113 renames.

The documents are test input. Fetching them at test time bought nothing and cost the build.

thousandeyes.upstreamUrl serves whatever is current, so the pin went stale every time Cisco published — and it went stale with no push behind it, which is why Actions showed main green while a fresh checkout was red. That has now happened three times: 7.0.99 → 7.0.100 (#92), → 7.0.102 (#112), and the 7.0.99 pin before it.

What replaces it

The document is committed at internal/vendor_openapi_specs/testdata/thousandeyes.yaml and embedded:

//go:embed testdata/thousandeyes.yaml
var thousandEyes []byte

func ThousandEyes() []byte { return thousandEyes }

Every machine reads the same bytes, offline, and a vendor's release schedule cannot reach the build.

Nothing outside tests imports this package, so the embed never enters the shipped tfpfgen binary.

What that deletes

All of it existed only to make fetching safe:

Gone Was for
vendor_openapi_specs.lock.json recording what the bytes should be
SHA-256 verification catching a document that moved under the pin
cache.go — atomic publication, staleness gates, race handling not re-downloading 13 MB per package
fetch.go — mirror-then-upstream fallback surviving an upstream that serves "current"
offline-vs-REQUIRED skip split not failing on a plane, while not skipping to green in CI
refresh.goCheckUpstream, RewriteLock rewriting a pin once it went stale

~2,400 lines including tests. The package is now 21 lines and one test.

This also removes the trap I flagged earlier: fetch() returns the first source that answers 200 and verifyAgainstPin() checks the hash afterwards, with no fallback — so populating mirrorUrl without rewriting it in lockstep with sha256 would have made a pin permanently unfetchable and blamed our own mirror. That whole class is gone rather than defended against.

Keeping the one property worth keeping

The pin's real value was noticing when the document changed. That moves to the consumer:

const (
    version   = "7.0.102"
    pathCount = 208
)

Replace the document and this fails, deliberately, instead of passing quietly against whatever the new one contains.

Two judgement calls

  • GitHub's document is dropped, not committed. No test ever parsed it — the only references were a naming test's unrelated string literal and the package's own test asserting the lock listed it. Committing 12.9 MB would have bought nothing. Say the word and I'll add it as a second exercised document.
  • The size rule now exempts testdata/, as the 800-line and vendor-name rules already do. A committed vendor document is test input, not a binary; the rule still catches a build artefact at the repo root, which is what it was for.

Verification

No network, so nothing here depends on Cisco being up:

$ go test -race -covermode=atomic ./internal/...   all pass
$ bash scripts/coverage_gate.sh    91.3% total, vendor_openapi_specs 100.0%
$ bash scripts/repo_hygiene_gate.sh   pass
$ golangci-lint run                0 issues
$ gofmt -l .                       clean
$ grep -rn 'net/http' internal/vendor_openapi_specs/   (nothing)

🤖 Generated with Claude Code

ShocOne and others added 2 commits August 25, 2026 11:04
The documents are test input. Fetching them at test time bought nothing and
cost the build: the ThousandEyes URL serves whatever is current, so the pin
went stale whenever Cisco published, and main went red with no push behind it
— twice before this, and once more this week.

The document is now committed and embedded. Tests read the same bytes on every
machine, offline, and a vendor's release schedule cannot reach them.

Deletes the machinery that existed only to make fetching safe: the lock, the
SHA-256 verification, the cache with its atomic publication and staleness
gates, the mirror fallback, the offline-versus-required skip split, and the
pin-rewriting. Roughly 2,400 lines, tests included.

The version and path count move into the consumer that asserts them, so
replacing the document fails a test rather than passing quietly against
whatever the new one contains.

GitHub's document is dropped rather than committed: no test ever parsed it,
so committing 12.9 MB would have bought nothing either.

The size rule exempts testdata/, as the file-length and vendor-name rules
already do — a committed vendor document is test input, not a binary.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ments

internal/vendor_openapi_specs names ThousandEyes in non-test source, because
that is what the accessor returns. The rule exists so a general toolkit does
not ship one vendor's constants as defaults, and a test document is not a
default — the same reason testdata/ was already exempt.

The exemption is one directory: a vendor name anywhere else still fails.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ShocOne
ShocOne deleted the branch main August 25, 2026 10:43
@ShocOne ShocOne closed this Aug 25, 2026
@ShocOne ShocOne reopened this Aug 25, 2026
@ShocOne
ShocOne changed the base branch from refactor/name-the-vendor-openapi-specs to main August 25, 2026 10:44
@ShocOne
ShocOne merged commit fa1f6d5 into main Aug 25, 2026
4 checks passed
@ShocOne
ShocOne deleted the refactor/commit-the-vendor-openapi-specs branch August 25, 2026 10:48
ShocOne added a commit that referenced this pull request Aug 25, 2026
Extends the table through #114 and stops describing the fetch-and-pin scheme
in the two rows that named it, since #114 removed it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant