feat: system test framework proposal - #111
Conversation
|
I like the ideas you are expressing in the proposal. I think it is the right way to go. |
henryZrncik
left a comment
There was a problem hiding this comment.
Looks Great to me.
I think most of the proposed solution suits perfectly. I think to to gret extent current situation of System tests addressed mainly testing assuming operator and subsequently using resource manager mostly on test level instead of inside function calls. Because there is also mention of different enviroments (minikube, ocp etc) we need to also address the need to make http calls and client calls regardless of acessing local cluster or remote, but this represents minimal changes that are to be part of this. 👍
|
|
||
| Four categories of system test have fundamentally different concerns: | ||
|
|
||
| - **Feature tests** — does record encryption work? Does authorisation enforce ACL rules? These tests care only that a correctly-configured proxy exists and is serving traffic. They must not care how the proxy was deployed. They have no Kubernetes dependency. |
There was a problem hiding this comment.
Where do transport tests fit into this hierarchy? Upstream/Downstream TLS, certificate auth etc.
There was a problem hiding this comment.
I think the transport level is hopefully just a feature test. I can see it might spill into the builders a bit. I've tweaked the definition a little to clarify things.
|
|
||
| **`CrdProxyFixture`**: takes an `Installer` as a constructor dependency. The installer puts the operator into the cluster; the fixture applies Kroxylicious CRDs (`KafkaProxy`, `VirtualKafkaCluster`, `KafkaProtocolFilter`) via Server-Side Apply, then waits for observable convergence signals — the controller has reconciled the resources and the Deployment has reached stable state with updated replicas ready and serving. The fixture knows the CRD schema and the convergence protocol, not the operator's internals. | ||
|
|
||
| **`ManifestProxyFixture`**: translates `ProxyScenario` into a proxy configuration file and a Kubernetes Deployment, applies them, then waits for the Deployment to reach stable state. No operator or installer required. |
There was a problem hiding this comment.
Is this opinionated about ingress?
There was a problem hiding this comment.
The intent is for tests to be agnostic of ingress. The test calls proxy.bootstrap() and gets the right address — it doesn't know or care how that was set up.
There is real tension here though: the proxy's advertised addresses have to match the ingress configuration, so the fixture implementation needs to solve that coupling. That's non-trivial — but it's an implementation concern, not a test-facing one. As long as ProxyScenario and ProxyHandle keep the test abstracted from the mechanics of config generation, we're in the best place we can be.
|
|
||
| Four fixture implementations cover the deployment mechanisms: | ||
|
|
||
| **`CrdProxyFixture`**: takes an `Installer` as a constructor dependency. The installer puts the operator into the cluster; the fixture applies Kroxylicious CRDs (`KafkaProxy`, `VirtualKafkaCluster`, `KafkaProtocolFilter`) via Server-Side Apply, then waits for observable convergence signals — the controller has reconciled the resources and the Deployment has reached stable state with updated replicas ready and serving. The fixture knows the CRD schema and the convergence protocol, not the operator's internals. |
There was a problem hiding this comment.
So is CrdFixture converting from ProxyScenario/RecordEncryptionFilterSpec objects etc into CRDs?
There was a problem hiding this comment.
Yes. Each ProxyFixture implementation translates the same ProxyScenario into whatever the deployment mechanism needs. CrdProxyFixture generates CRD resources (KafkaProxy, VirtualKafkaCluster, KafkaProtocolFilter). ManifestProxyFixture generates a proxy configuration file and a Kubernetes Deployment. StandaloneProxyFixture generates a configuration file and starts a local process. The test provides the same ProxyScenario regardless — the fixture decides what to do with it.
| void createTopic(String topic, String bootstrap, int partitions, int replicas); | ||
|
|
||
| // Richer operations — enabled by in-process drivers | ||
| void produceInTransaction(String topic, String bootstrap, List<ProducerRecord> records); |
There was a problem hiding this comment.
I'm not sure this API is sufficiently rich to model use cases like sendOffsetsToTransaction.
There was a problem hiding this comment.
Agreed — the API as shown isn't rich enough for things like sendOffsetsToTransaction. The current high-level gestures (produce, consume, create topic) map to what the CLI-based clients can do today, and that's the starting point. For system tests, the goal is high-level gestures, not covering every possible Kafka operation — that's for lower-level tests.
I'm 100% on board that the CLI isn't good enough long-term, but what the right answer looks like depends heavily on what the FFI work throws up and what we can sensibly offer from that. I'd rather let that inform the API shape than lock it in now.
| A plain Java value object describing what configuration the proxy should have. No knowledge of namespaces, CRD templates, or deployment mechanism. | ||
|
|
||
| ```java | ||
| ProxyScenario scenario = ProxyScenario.builder() |
There was a problem hiding this comment.
Is the intention here to have this builder compose the Proxy's own existing configuration types? I'm concerned about introducing another configuration model into the proxy (core config model + crds + a new one).
There was a problem hiding this comment.
The intent is for ProxyScenario to be a thin, purpose-built DSL for system tests — not a general configuration model. It describes high-level intent: "a proxy with filter X and upstream Y" without caring whether that manifests as a YAML blob in a config file or a set of CRD resources.
The bet is that there are enough common high-level actions to make this worthwhile. For example, a record encryption test might say:
withFilter(new RecordEncryptionSpec(Kms.VAULT, Set.of("topicA", "topicB")))The test doesn't care about KEK naming patterns, key rotation config, or how the KMS is connected — it just says "these topics get encrypted" and asserts that other topics don't. KMS provisioning and configuration sits in the installer layer, not the test. That's the level of gesture the DSL aims at.
The tension is real — this is the hardest seam to extract cleanly, and there are good questions to work through: where does RecordEncryptionSpec come from? How do test authors discover available gestures? How does this work for BYO filters? I've seen purpose-built test DSLs work very well in the past but they're expensive to build and iterate on. Answering those questions now feels like over-specifying — this proposal is trying to answer the bigger-picture question of how we structure system tests and make downstream packages easier to validate. The DSL shape will need its own iteration once the overall structure lands.
There was a problem hiding this comment.
starting with passing around jackson JsonObject blobs or HashMaps for filter config would be no worse than what's implemented. Agree that we can decide on that separately. (should keep in mind this is something we can iterate on rapidly as opposed to public API)
There was a problem hiding this comment.
Agreed — JsonObject blobs give us a good starting point rather than trying to spec out the DSL now. That's still my ultimate goal: a test author expressing "these topics get encrypted with this KMS" without caring about filter config shape. But iterating toward that from a working blob-based foundation is a lot more tractable than speccing it up front.
|
|
||
| Introduce a layered abstraction for system tests that separates test intent from deployment mechanism, and organise tests into modules by what they cover: feature behaviour, operator reconciliation, webhook behaviour, and installation validation. | ||
|
|
||
| A `ProxyScenario` describes the desired proxy configuration in deployment-agnostic terms; a `ProxyFixture` translates that into running infrastructure and blocks until convergence; the resulting `ProxyHandle` is a token of convergence that gates all subsequent interaction. An `Installer` — the primary downstream extension point — handles getting the operator, CRDs, and RBAC into the cluster independently of how proxies are deployed. |
There was a problem hiding this comment.
What's actually providing the upstream kafka and how's it configured? I think ideally we'd like this to be pluggable too. It could be an InVM or TestContainer cluster coming from the test extension, a cluster stood up by Strimzi, a Cloud Kafka Service (Confluent).
There was a problem hiding this comment.
Addressed in 9d9b831 — introduced KafkaClusterFixture and KafkaClusterHandle as the pluggable seam for upstream cluster provisioning.
robobario
left a comment
There was a problem hiding this comment.
Thanks @SamBarker sounds like a nice separation of concerns,the main thing I'd like added is some details about resource teardown, who's responsible, when/where is it executed?
|
|
||
| ### `ProxyFixture` Implementations | ||
|
|
||
| Tests never instantiate fixtures or installers — the JUnit extension reads system properties (`-Dfixture`, `-Dinstaller`) and composes them. The composition is extension-internal; the test sees only an injected fixture. |
There was a problem hiding this comment.
worth having an env var style and precedence rules? env vars may drop more easily into things like github CI, where system props give you more control within an environment.
So we could have KROXYLICIOUS_TEST_FIXTURE and KROXYLICIOUS_TEST_INSTALLER env vars but prefer the system props. I guess a namespacing on the system props could also be worthwhile.
There was a problem hiding this comment.
Good suggestion — the whole point of fixture selection is to make automation easy, and env vars compose more naturally with CI pipelines than JVM flags. Less idiomatic Java, but the right tradeoff here.
On reflection though, env vars alone might be sufficient: they work in Maven via Surefire <environmentVariables>, they work in Gradle, and they work in whatever a downstream distributor's CI looks like. System properties are Java convention, but they don't buy anything that env vars don't already cover. Inclined to go env vars only and keep the API surface simpler — KROXYLICIOUS_TEST_FIXTURE, KROXYLICIOUS_TEST_INSTALLER, KROXYLICIOUS_TEST_KAFKA_CLUSTER. Thoughts?
There was a problem hiding this comment.
I am down for using environment variables here: all our STs use this approach so KROXYLICIOUS_TEST_FIXTURE etc. seems as good way to go for me
|
|
||
| Introduce a layered abstraction for system tests that separates test intent from deployment mechanism, and organise tests into modules by what they cover: feature behaviour, operator reconciliation, webhook behaviour, and installation validation. | ||
|
|
||
| A `ProxyScenario` describes the desired proxy configuration in deployment-agnostic terms; a `ProxyFixture` translates that into running infrastructure and blocks until convergence; the resulting `ProxyHandle` is a token of convergence that gates all subsequent interaction. A `KafkaClusterFixture` provisions the upstream Kafka cluster and returns a `KafkaClusterHandle` — the same handle-based convergence pattern, making cluster provisioning pluggable across implementations (TestContainers, Strimzi, in-VM). An `Installer` — the primary downstream extension point — handles getting project components (operator, webhook, CRDs, RBAC) into the cluster independently of how proxies are deployed. |
There was a problem hiding this comment.
might be a bit nitpicky but the scenario name might be a bit overloaded since we're in a testing context, I read it and thing of an entire given/when/then BDD style test. but other options are also a bit crusty in their own ways ProxyConfig, ProxyDefinition, ProxyDescriptor
There was a problem hiding this comment.
Fair point — and on reflection the asymmetry is a design smell I hadn't noticed. OperatorFixture and WebhookFixture have no *Scenario siblings because their "input" is the raw surface they're testing directly (CRDs, pod specs). ProxyScenario isn't one of a family — it's a one-off concept, and the name implies siblings that don't exist and shouldn't.
ProxyDefinition works better: it reads as "the definition of what the proxy should be" without the BDD connotation, and it's a natural extension point — we can start with JSON blobs and evolve toward a richer DSL without changing the type name or the fixture API.
| A plain Java value object describing what configuration the proxy should have. No knowledge of namespaces, CRD templates, or deployment mechanism. | ||
|
|
||
| ```java | ||
| ProxyScenario scenario = ProxyScenario.builder() |
There was a problem hiding this comment.
starting with passing around jackson JsonObject blobs or HashMaps for filter config would be no worse than what's implemented. Agree that we can decide on that separately. (should keep in mind this is something we can iterate on rapidly as opposed to public API)
|
|
||
| Environment variables work uniformly across Maven (via Surefire `<environmentVariables>`), Gradle, and any CI tooling a downstream distributor uses. The extension composes them: it instantiates the installer, passes it to the fixture constructor, and manages the lifecycle. When `KROXYLICIOUS_TEST_INSTALLER` is not set, the fixture uses its default (manifest-based for fixtures that require an installer). Standalone and manifest fixtures do not take an installer. When `KROXYLICIOUS_TEST_KAFKA_CLUSTER` is not set, the extension uses a sensible default for the fixture type. | ||
|
|
||
| ### `KafkaClient` Abstraction |
There was a problem hiding this comment.
This section looks a bit light compared to the rest, but I agree in principal that it makes sense to abstract like this along the client/environment lines. Do you want to shuffle it to a new proposal or take it all the way here, or we could vote on the principle since this is system tests and not public API? I assume we'd need to define environment variables for picking the client driver and environment and so on?
Should probably check, do you imagine this TCK idea being public API with the same strictures about modifications, or could we operate on a best-effort compatibility here?
Introduces layered abstractions (ProxyScenario, FilterSpec, ProxyFixture, ProxyHandle) that separate test intent from deployment mechanism, enabling deployment-agnostic feature tests and test-first development. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
OperatorCapability now models the operator's externally observable state via generation-based reconciliation observation rather than the checksum annotation. Tests that assert on specific operator mechanisms (e.g. checksum change detection) observe resource state directly. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
OperatorCapability handles convergence and deployment agnosticism. Tests that assert on specific resource state (e.g. checksum annotations) use an injected KubernetesClient to observe resources directly, keeping the two concerns separate. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Three distinct test categories become separate modules with different compile-time dependencies: systemtest-feature (no K8s dependency), systemtest-operator (K8s client and CRD types), and systemtest-installer (one test per install method). All three are TCK-consumable. Installer is a public interface — the primary downstream extension point. CrdProxyFixture replaces OperatorProxyFixture to reflect that the fixture uses the CRD API, not the operator's internals. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Fixture/installer composition is extension-internal, driven by system properties. Installer tests use a single smoke test — the test does not vary between installers, CI provides the matrix. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
- Summary now mentions all four test categories and the Installer - "Three categories/modules" → "Four" throughout - Add systemtest-webhook to TCK module list with description - Feature test example: remove namespace parameter, use kafkaClient - Tags section: clarify module boundaries as primary separation - Rejected alternatives: @operator tag "ensures present", not "selects fixture" - Webhook section: rewrite as two-module story (installer + behaviour) - Affected projects: add systemtest-webhook module - Fix grammar: "An CrdProxyFixture" → "A CrdProxyFixture" Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Replace "delegated to QE" with community-neutral language and frame setup cost as a perceived barrier rather than a statement of fact. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
The same separation of concerns that motivates the fixture model applies to how tests interact with Kafka. Three independent axes: test intent (what), client driver (which implementation), and execution environment (where). The KafkaClient interface shows a richer target shape including transactions and consumer group management, enabled by in-process drivers (Java client, librdkafka/Sarama via FFI). CLI drivers implement produce/consume only. Produce and consume are the starting point; richer operations are the target. Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Feature tests care about proxy behaviour under a given configuration, not just happy-path "correctly-configured" scenarios. Installer is the abstraction over installation mechanism (Helm, OLM, manifests) for project components generally (operator, webhook, CRDs, RBAC), not just the operator. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
Reshape the fixture model so each test module has a fixture type matched to what it exercises: ProxyFixture for feature tests, OperatorFixture for operator reconciliation tests, WebhookFixture for admission webhook tests. Installer gains a Component enum so fixtures request only what they need (OPERATOR, WEBHOOK). Move test modules under a systemtest/ parent for physical grouping. Extract the Installer interface into a public API module for downstream extensibility. Specific installer implementations (Helm, OLM, manifests) are recognised as likely but out of scope. Tags shift from component requirements to runtime environment skip conditions (e.g. skip when no Kubernetes cluster is available). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
…ning Upstream Kafka provisioning was an ambient assumption — clusterName appeared in examples without explaining where it comes from. KafkaClusterFixture and KafkaClusterHandle follow the same handle-based convergence pattern as ProxyFixture/ProxyHandle, making cluster provisioning explicit and pluggable across implementations (in-VM, TestContainers, Strimzi, managed services). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
…ection to env vars, clarify reconfigure/waitForRestart semantics - Rename ProxyScenario -> ProxyDefinition throughout: the name implied a family of *Scenario siblings (OperatorScenario, WebhookScenario) that don't exist and shouldn't. ProxyDefinition reads as a one-off type and is a natural extension point for evolving from JSON blobs to a richer DSL. - Switch fixture/installer/client selection from system properties to environment variables (KROXYLICIOUS_TEST_FIXTURE, KROXYLICIOUS_TEST_INSTALLER, KROXYLICIOUS_TEST_KAFKA_CLUSTER, KROXYLICIOUS_TEST_CLIENT_DRIVER, KROXYLICIOUS_TEST_CLIENT_LOCATION). Env vars work uniformly across Maven (via Surefire), Gradle, and any CI tooling downstream distributors use. System properties are Java convention but add no practical value here. - Clarify that reconfigure() follows the same convergence contract as apply() — it blocks until the proxy has converged to the new config and returns a new ProxyHandle. The old handle should not be used after reconfigure(). - Expand waitForRestart() description: it is a high-level gesture for tests where the restart itself is the scenario under test (e.g. asserting clients reconnect seamlessly), not a convergence gate for config changes — that is reconfigure()'s job. Signed-off-by: Sam Barker <sam@quadrocket.co.uk>
841b41e to
f025fe5
Compare
Summary
ProxyScenario) from deployment mechanism (ProxyFixture) with convergence gating (ProxyHandle)systemtest-feature,systemtest-operator,systemtest-webhook,systemtest-installerInstalleras the primary downstream extension point — downstream varies by installation method, not proxy deploymentTest plan
🤖 Generated with Claude Code