refactor(e2e): extract shared kat-server backends - #5987
Closed
ppeble wants to merge 1 commit into
Closed
Conversation
Every fixture under test/e2e/fixtures/ was carrying its own copy of the same handful of kat-server Deployment+Service shapes, which was most of the duplication in manifests.yaml across the suite. Add test/e2e/backends/ with one file per distinct shape (http-echo, target, grpc-echo, rls) and repoint fixtures at it with an extra apply step in chainsaw-test.yaml. Chainsaw resolves apply.file relative to the test's own directory, so a relative path can reach outside fixtures/<name>/ into backends/ and still land in that test's ephemeral namespace. A shared file's Deployment/Service both use a fixed name, since apply has no per-fixture parameter the way a helm chart would. http-basic, gzip-content-type, gzip-minimum and server-name share http-echo because none of their probes assert a specific backend name that would collide with it; extauth-http and ratelimit-grpc pull in target (and ratelimit-grpc also rls) for the one backend each duplicated verbatim, keeping their auth/rls-specific pieces and all CRDs in their own manifests.yaml; grpc-basic pulls in grpc-echo. header-routing, gzip-unsupported-content-type and tcpmapping-basic keep their backend Deployment+Service inline: each asserts a backend name that only it uses (or, for header-routing, needs two distinctly-named backends of the same shape at once), so extracting them would just move the duplication rather than remove it. Net line count under test/e2e/ drops from 2184 to 2043 despite adding four new files, and the fixtures/*/manifests.yaml total drops from 835 to 584 lines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Phil Peble <phil@peble.net>
|
✔️ e1eabc1 - Conventional commits check succeeded. |
Member
Author
|
Closing this. Not pursuing the shared-backend refactor right now; the duplication it removes is not blocking anything. The Chainsaw behaviour it established is worth recording for anyone who picks this up later: |
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
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.
Description
Every fixture under
test/e2e/fixtures/was carrying its own copy of the same handful of kat-serverDeployment+Serviceshapes, which accounted for most of the duplication across the suite'smanifests.yamlfiles.This adds
test/e2e/backends/with one file per distinct shape and repoints fixtures at it with an extraapplystep. Chainsaw resolvesapply.filerelative to the directory containing thechainsaw-test.yamlthat references it rather than the process working directory, so a relative path can climb out offixtures/<name>/intobackends/and the resources still land in that test's own ephemeral namespace.No intended change in coverage. This is groundwork: roughly 11 more fixtures are queued behind it, and without this each one would carry another ~40 lines of copied backend YAML.
Shapes added
backends/http-echo.yamlhttp-echohttp-basic,gzip-content-type,gzip-minimum,server-namebackends/target.yamltargetextauth-http,ratelimit-grpcbackends/grpc-echo.yamlgrpc-echogrpc-basicbackends/rls.yamlrlsratelimit-grpcEach fixture keeps its own CRDs (
Mapping,Module,AuthService,RateLimitService, and so on) in its ownmanifests.yaml. Only the backendDeployment+Servicemoves out.What deliberately stayed inline
A shared file's
DeploymentandServiceuse a fixedmetadata.name, becauseapplyhas no equivalent of a helm value to parameterize it per fixture. A fixture can therefore only pull in a shared backend if its probe either does not assert on.json.backendor asserts exactly the name the shared file uses.Three fixtures keep their backends inline for that reason:
header-routingneeds two distinctly-named backends of the same shape at once (hr-target1,hr-target2).gzip-unsupported-content-typeassertsgzip-unsup-echo, a name only it uses.tcpmapping-basicassertstcp-echo, likewise.Extracting those would move the duplication rather than remove it.
Line counts
fixtures/*/manifests.yamlbackends/test/e2e/The tree shrinks by 95 lines despite four new files and a new README section.
Testing
Ran locally against a k3d cluster, per fixture and then all together:
http-basic,gzip-content-type,gzip-minimum,server-name,grpc-basic,extauth-http,ratelimit-grpc. All pass.header-routing,tcpmapping-basic,gzip-unsupported-content-type. All pass.make e2e/run: all 10 fixtures pass together.Checklist
test/e2e/README.mddocumentsbackends/, when a fixture can use it, and when it cannot