feat(manifests): honour the version argument, carry two Knative versions - #37
Merged
Conversation
getOperatorResources accepted a version and ignored it, returning whatever codegen emitted as the default -- the last version pulled. Every caller therefore got the same resources regardless of what it asked for, and the mismatch was invisible because logs still echoed the requested version. That is not theoretical. Adding a second Knative version to this package silently redirected the e2e suite at it: the suite pinned v1.15.0, reported installing v1.15.0, and applied v1.22.1. It then failed for eighteen minutes against a version nobody had chosen. A version now reads that version's vendored YAML. Read rather than embedded: carrying every version's docs in the generated objects would multiply the output by the number of versions, and the YAML already ships for getOperatorManifestPaths. Callers passing no version keep the generated objects, so nothing existing changes. With that fixed, Knative carries v1.15.0 and v1.22.1 again -- this repo's e2e installs the former, downstream deploys the latter, and each asks for the one it runs. Verified by reading back the webhook Deployment's version label: v1.15.0 -> 72 resources, webhook labelled 1.15.0 v1.22.1 -> 72 resources, webhook labelled 1.22.1 The plumbing was already in place -- setup.ts has always passed operator.version through; only the final accessor discarded it.
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.
getOperatorResources(id, version)accepted a version and ignored it, returning whatever codegen emitted as the default — the last version pulled.That's not theoretical. It's what cost us the 18-minute failures: adding a second Knative version silently redirected the e2e suite at it. The suite pinned v1.15.0, reported installing v1.15.0, and applied v1.22.1 — then failed against a version nobody had chosen, while every log line named the right one.
The fix
A version now reads that version's vendored YAML. Read rather than embedded — carrying every version's docs in the generated objects would multiply the output by the number of versions, and the YAML already ships for
getOperatorManifestPaths.Callers passing no version keep the generated objects, so nothing existing changes.
Verified by reading the resources back
The webhook Deployment's own version label — proof the selection reaches the resources, which the old code could not produce.
Knative carries both versions again
This repo's e2e installs v1.15.0; downstream deploys v1.22.1. Each asks for the one it runs, which is what
sources: Source[]was always for.The plumbing was already there —
setup.ts:151has always passedoperator.versionthrough. Only the final accessor discarded it.What this unblocks
Downstream can now consume this package for Knative instead of vendoring its own copy or fetching from GitHub releases at run time — the determinism goal this package exists for.
The real test is CI: the e2e pins v1.15.0 while the package carries both. If it comes back green at ~2.5 min, version selection holds end to end.