Skip to content

Reworking proxy configuration#96

Draft
tombentley wants to merge 13 commits intokroxylicious:mainfrom
tombentley:configuration-redesign
Draft

Reworking proxy configuration#96
tombentley wants to merge 13 commits intokroxylicious:mainfrom
tombentley:configuration-redesign

Conversation

@tombentley
Copy link
Copy Markdown
Member

This proposal covers a significant reworking of the Kroxylicious proxy's configuration system.

Comment thread proposals/015-config2-multi-file-plugin-configuration.md
Comment thread proposals/015-config2-multi-file-plugin-configuration.md Outdated
Comment thread proposals/015-config2-multi-file-plugin-configuration.md
Comment thread proposals/015-config2-multi-file-plugin-configuration.md
Comment thread proposals/015-config2-multi-file-plugin-configuration.md
Comment on lines +574 to +580
For the filesystem-based snapshot, passwords are stored in a `passwords.yaml` file in the
configuration directory. This file maps resource names to their passwords:

```yaml
# passwords.yaml
my-keystore: changeit
```
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit weak. It precludes the possibility of different secret content coming from different trust zones, because someone has to assemble a single file where all the passwords are in the clear.
It would be better if this allows people to add their key store and their password each as its own file. That way FS permissions could be used so that Bob can add his keystore, but not decrypt Alice's keystore when he does so.

Comment thread proposals/015-config2-multi-file-plugin-configuration.md Outdated
Comment on lines +588 to +591
The keystore format need not be user-specified.
Common formats have distinctive magic bytes (JKS: `0xFEEDFEED`, PKCS12: ASN.1 `0x30`, PEM: `-----BEGIN`).
The deserialiser probes the bytes and selects the appropriate format.
PKCS12 is the default keystore type since Java 9; JKS is legacy.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we support file extensions other than just .data then maybe the file extension could also be used. It's not clear whether the consumer of the bytes gets passed just the file content (bytes) or can also see the extension.

Are the magic bytes and trial deserialization hard coded to just the common stores? Should probably explain in a bit more detail how this might work for other keystore types, like EJKS.

Comment thread proposals/015-config2-multi-file-plugin-configuration.md
format:

```
kroxylicious migrate-config -i legacy-config.yaml -o output-dir/
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to bolt keywords onto the existing kroxylicious app that's the entry point to running the proxy? It could be the top of a slippery slope, where we add utility functionality which ends up bloating the container image for the proxy, and including functionality which we might not want to be available to the proxy.

As an alternative, we could have a separate kroxyctl which had this functionality (and other similar code that's not needed for the proxy process).

Initial draft proposing a Kubernetes-inspired multi-file configuration
system with explicit plugin references, dependency graph validation,
JSON Schema validation, and a migration tool.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
The five numbered motivations now match the original problem statement:
per-plugin schemas, uniformity, dependency understanding for dynamic
reloading, individual plugin identity for control plane, and principled
config versioning.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Plugin instance files must include a `name` field matching the filename.
This makes files self-describing for debugging and non-filesystem sources.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Explain why HasPluginReferences exists (explicit > introspection),
why PluginReference is not a breaking change (config versioning),
and why we enforce referential integrity unlike Kubernetes.
Clarify which types are public API (kroxylicious-api) vs internal.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
This reverts commit 83938a7.

Signed-off-by: Tom Bentley <tbentley@redhat.com>
Plugin instance files must use FQCNs for the type field, making them
self-describing without depending on which plugins are loadable.
JSON Schema enum constraints provide editor code completion.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
…serialisation

Plugin references in YAML are bare instance name strings. The config type
constructs PluginReference internally via HasPluginReferences, combining
the statically-known interface type with the instance name from YAML.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
…onfig2 proposal

Describe the full Snapshot interface (metadata, bytes, passwords, generation
numbers) once in the Snapshot abstraction section instead of partially describing
it and then re-describing it in Non-YAML resources. Add @ResourceType annotation,
TLS plugin interfaces, and filesystem layout for binary resources.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
- Document version dispatch via instanceof rather than passing version string
- Mandate K8s CRD-compatible JSON Schema dialect for plugin schemas
- Require schemas for versioned plugins
- Add HasPluginReferences validation rule for versioned config types
- Note that unreachable plugins are not an error
- Expand @stateless with YAML example and lifecycle detail
- Document ResolvedPluginRegistry as public API with full code context
- Add dynamic reloading integration subsection
- Fix dangling "these resources" reference in non-YAML section
- Allow flexible sidecar file extensions (.p12, .jks) instead of .data
- Add ANTLR line number caveat for inline resources
- Add ResourceSerializer/ResourceDeserializer interface declarations
- Document @ResourceType placement and package
- Add trust zone limitation and K8s/Vault password sourcing detail
- Note ResourceDeserializer receives bytes only, not file extension
- Add concrete VaultKmsConfigV1 TLS material example
- Add open question on migration tool packaging

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Tom Bentley <tbentley@redhat.com>
@tombentley tombentley force-pushed the configuration-redesign branch from 8e0e721 to e7f8b3b Compare April 16, 2026 02:54
@k-wall
Copy link
Copy Markdown
Member

k-wall commented Apr 23, 2026

Discussed at Community Call 23rd April. Tom is looking for feedback.

@k-wall k-wall added the triaged label Apr 23, 2026
@tombentley tombentley moved this to Want to Do in Release 0.21.0 Apr 24, 2026
@tombentley tombentley self-assigned this Apr 24, 2026

The configuration model is tree-like. If two filters need the same KMS
then each must embed a copy of the KMS configuration. There is no way to define a plugin
instance once and reference it from multiple consumers.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAML does have anchors and aliases

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but that's more of a syntax level thing. The API we have for the config file (Jackson) would essentially duplicate those JsonNode or mapped POJOs, so we would see two objects which were .equals, but not ==. You can use @JsonIdentityInfo to make the references first class. But if you're seriously proposing that as an alternative then I think you should elaborate how you'd expect it to work.

Similarly, external tools (editors, config linters, CI validators, Kubernetes operators)
cannot inspect or validate plugin configurations (see [2436](https://github.com/kroxylicious/kroxylicious/issues/2436)),
and there is no declarative way to validate plugin configurations before
the proxy starts (somewhat related: [3267](https://github.com/kroxylicious/kroxylicious/issues/3267#issuecomment-4042031788)).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no declarative way to validate plugin configurations before the proxy starts

that one aspect is solvable without redesigning the config model. Tools such as httpd and squid have config-check command line parameter.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're commenting on part of one sentence, and not really accounting for the full context in the paragraph as a whole. Sure, we can provide a tool. But that won't be built into anyone's editor, and won't provide anything like code completion, and doesn't address the documentation part.

Plugin developers can ship a JSON Schema for each config version at:

```
META-INF/kroxylicious/schemas/{PluginSimpleName}/{version}.schema.yaml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

puzzled why you chose simple name, when you made the case for FQCN above. Why the discrepancy?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude chose that and I was too tired to argue. I agree with you.


Discovery of schemas by external tooling depends on plugin developers publishing the schema so that it can be discovered by tools.
One way to do that is to publish the schema on a public webserver, allowing end users to make use of JSON Schema's `$schema` keyword.
Editors often support downloading such schemas.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must confess, I've actually rarely done this myself. Maybe I'm an outlier? Is this something others commonly do?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might find, if you're using intellij for instance, that you're getting the benefits of schemas published to schemastore without even knowing it. It is, admittedly, more painful to configure an unpublished schema manually.

Personally I find the code completion and documentation part of using schemas at least as useful as the validation part.

As a plugin evolves, the plugin implementation either accepts the YAML or it does not.
If a newer version of a plugin removes support for a configuration property the user will likely be confused, because the configuration used to work — but the plugin has no effective way to communicate that the user is speaking the old language.
Explicit version strings (following Kubernetes conventions: v1alpha1, v1beta1, v1) would give plugin developers a principled mechanism for config schema evolution, including support for multiple concurrent versions during migration periods.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One place I see the configuration schemas being very useful is validating the documentation. As our documentation gets larger, keeping the configuration snippets accurate will become increasing hard. Tool support will help greatly.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I said on line 52.

}
```

### Snapshot abstraction
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to self: haven't followed this properly eyt

5. Writes `proxy.yaml` by passing through the raw YAML (stripping `filterDefinitions`, replacing
`micrometer` with instance name references).

**Open question:** whether the migration tool should be a subcommand of the proxy entrypoint or
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for the separate CLI

encrypt.yaml # metadata + YAML config (no sidecar needed)
```

For a plugin instance named `foo`, the metadata is always in `foo.yaml`. If a sidecar data file exists, it must be the only other file in the directory whose name without extension is `foo` (e.g. `foo.p12`, `foo.jks`, `foo.pem`). Any extension is permitted (except `.yaml` which is taken), which allows filesystem tools that depend on extensions to work naturally. The runtime validates that at most one sidecar file exists per plugin instance.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about plugins that need more than one data file? Limiting to a single file seems quite restrictive.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is restrictive to some extent. Maybe we should make it more flexible, but can you provide some more concrete use cases where it would be a benefit?

For text-based resources like ACL rules, YAML multi-line syntax (`|`) is simplest.
The versioned config type can use a `String` field instead of a file path:

```yaml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the example here is not yaml

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is. What makes you say that?

* @return the password as a char array, or {@code null} if no password is configured
*/
@Nullable
char[] resourcePassword(String resourceName);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resources like keystores have a store password and key password, which maybe distinct.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but look at the direction of travel: JKS is not recommented for new applications (since Java 9).
What became the new default, PKCS#12, in its Java implementation, more or less expects the passwords to be the same. Quoting gemini, the "best practice":

If you are using PKCS12 in Java, you should keep the store password and the key password identical. If you provide a different password for the key, many non-Java tools (and even some older Java application servers like Tomcat or JBoss) may fail to read the private key because they expect the passwords to match. If you need distinct security layers, it is better to use file-system-level permissions rather than relying on different PKCS12 passwords.

@k-wall
Copy link
Copy Markdown
Member

k-wall commented Apr 27, 2026

I see the attractions of the proposals - the modularity, re-use, schema validation are all desirable features. But this already looks like a significant piece of work and it'll take significant effort. There's bound to be issues that crop up during the implementation. That's all opportunity cost where we don't get to work on features (that are more directly applicable to use-cases). Do we really need it? Components like Httpd and Tomcat seem to have done fine without something analogous.

I'm floating around +0.5

Copy link
Copy Markdown
Member Author

@tombentley tombentley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look @k-wall.

I see the attractions of the proposals - the modularity, re-use, schema validation are all desirable features. But this already looks like a significant piece of work and it'll take significant effort. There's bound to be issues that crop up during the implementation. That's all opportunity cost where we don't get to work on features (that are more directly applicable to use-cases). Do we really need it? Components like Httpd and Tomcat seem to have done fine without something analogous.

One thing you've not pointed to there is the configuration versioning and schema evolution part. Our conversations about Kroxylicious 1.0 have landed on that as an important thing to resolve. This proposal addresses that, but you're saying we don't need it, or at least are unwilling to commit to the effort needed to implement this proposal. So I'm wondering whether you see a conflict in holding both those positions, or whether you think we can solve them in some other way, or just that 1.0 is not something we need.


The configuration model is tree-like. If two filters need the same KMS
then each must embed a copy of the KMS configuration. There is no way to define a plugin
instance once and reference it from multiple consumers.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but that's more of a syntax level thing. The API we have for the config file (Jackson) would essentially duplicate those JsonNode or mapped POJOs, so we would see two objects which were .equals, but not ==. You can use @JsonIdentityInfo to make the references first class. But if you're seriously proposing that as an alternative then I think you should elaborate how you'd expect it to work.

Similarly, external tools (editors, config linters, CI validators, Kubernetes operators)
cannot inspect or validate plugin configurations (see [2436](https://github.com/kroxylicious/kroxylicious/issues/2436)),
and there is no declarative way to validate plugin configurations before
the proxy starts (somewhat related: [3267](https://github.com/kroxylicious/kroxylicious/issues/3267#issuecomment-4042031788)).
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're commenting on part of one sentence, and not really accounting for the full context in the paragraph as a whole. Sure, we can provide a tool. But that won't be built into anyone's editor, and won't provide anything like code completion, and doesn't address the documentation part.

As a plugin evolves, the plugin implementation either accepts the YAML or it does not.
If a newer version of a plugin removes support for a configuration property the user will likely be confused, because the configuration used to work — but the plugin has no effective way to communicate that the user is speaking the old language.
Explicit version strings (following Kubernetes conventions: v1alpha1, v1beta1, v1) would give plugin developers a principled mechanism for config schema evolution, including support for multiple concurrent versions during migration periods.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I said on line 52.

Plugin developers can ship a JSON Schema for each config version at:

```
META-INF/kroxylicious/schemas/{PluginSimpleName}/{version}.schema.yaml
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude chose that and I was too tired to argue. I agree with you.


Discovery of schemas by external tooling depends on plugin developers publishing the schema so that it can be discovered by tools.
One way to do that is to publish the schema on a public webserver, allowing end users to make use of JSON Schema's `$schema` keyword.
Editors often support downloading such schemas.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might find, if you're using intellij for instance, that you're getting the benefits of schemas published to schemastore without even knowing it. It is, admittedly, more painful to configure an unpublished schema manually.

Personally I find the code completion and documentation part of using schemas at least as useful as the validation part.

encrypt.yaml # metadata + YAML config (no sidecar needed)
```

For a plugin instance named `foo`, the metadata is always in `foo.yaml`. If a sidecar data file exists, it must be the only other file in the directory whose name without extension is `foo` (e.g. `foo.p12`, `foo.jks`, `foo.pem`). Any extension is permitted (except `.yaml` which is taken), which allows filesystem tools that depend on extensions to work naturally. The runtime validates that at most one sidecar file exists per plugin instance.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is restrictive to some extent. Maybe we should make it more flexible, but can you provide some more concrete use cases where it would be a benefit?

For text-based resources like ACL rules, YAML multi-line syntax (`|`) is simplest.
The versioned config type can use a `String` field instead of a file path:

```yaml
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is. What makes you say that?

* @return the password as a char array, or {@code null} if no password is configured
*/
@Nullable
char[] resourcePassword(String resourceName);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, but look at the direction of travel: JKS is not recommented for new applications (since Java 9).
What became the new default, PKCS#12, in its Java implementation, more or less expects the passwords to be the same. Quoting gemini, the "best practice":

If you are using PKCS12 in Java, you should keep the store password and the key password identical. If you provide a different password for the key, many non-Java tools (and even some older Java application servers like Tomcat or JBoss) may fail to read the private key because they expect the passwords to match. If you need distinct security layers, it is better to use file-system-level permissions rather than relying on different PKCS12 passwords.

k-wall added a commit to k-wall/design that referenced this pull request Apr 28, 2026
Add PRs kroxylicious#96, kroxylicious#98, kroxylicious#99, kroxylicious#100, kroxylicious#101, and kroxylicious#103 which were opened
after the initial script was created.

Note: PR kroxylicious#100 is already correctly named (100-sidecar-injection-webhook.md).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
k-wall added a commit to k-wall/design that referenced this pull request Apr 28, 2026
This change simplifies the proposal numbering system by using PR numbers
as proposal identifiers, eliminating number collisions and removing the
need for a separate allocation process.

Changes:
- Simplified proposals/README.md to focus on author workflow
  - Removed index tables (directory listing serves as the index)
  - Streamlined instructions for creating and renaming proposals
- Updated proposal template with workflow instructions
  - Require PR number in title format: # <PR#> - <Title>
  - Moved workflow instructions into comment block
- Added GitHub workflow to automatically check proposal numbering
  - Validates both filename and title format
  - Updates PR description when proposal files don't match PR number
  - Provides exact commands to fix naming issues
  - Removes warning once corrected
  - Handles both added and renamed files
  - Runs on all PRs (ready for mandatory status check)
- Added notification script for existing open PRs
  - After merge, run notify-open-prs.sh to ask authors to rebase
  - Workflow will automatically guide them through renaming
  - Updated with all current open proposal PRs (kroxylicious#70, kroxylicious#82, kroxylicious#83, kroxylicious#85,
    kroxylicious#88, kroxylicious#93, kroxylicious#94, kroxylicious#96, kroxylicious#98, kroxylicious#99, kroxylicious#100, kroxylicious#101, kroxylicious#103)

Proposals 001-019 retain their original numbers.

Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Keith Wall <kwall@apache.org>
k-wall added a commit that referenced this pull request Apr 28, 2026
This change simplifies the proposal numbering system by using PR numbers
as proposal identifiers, eliminating number collisions and removing the
need for a separate allocation process.

Changes:
- Simplified proposals/README.md to focus on author workflow
  - Removed index tables (directory listing serves as the index)
  - Streamlined instructions for creating and renaming proposals
- Updated proposal template with workflow instructions
  - Require PR number in title format: # <PR#> - <Title>
  - Moved workflow instructions into comment block
- Added GitHub workflow to automatically check proposal numbering
  - Validates both filename and title format
  - Updates PR description when proposal files don't match PR number
  - Provides exact commands to fix naming issues
  - Removes warning once corrected
  - Handles both added and renamed files
  - Runs on all PRs (ready for mandatory status check)
- Added notification script for existing open PRs
  - After merge, run notify-open-prs.sh to ask authors to rebase
  - Workflow will automatically guide them through renaming
  - Updated with all current open proposal PRs (#70, #82, #83, #85,
    #88, #93, #94, #96, #98, #99, #100, #101, #103)

Proposals 001-019 retain their original numbers.

Assisted-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Signed-off-by: Keith Wall <kwall@apache.org>
@k-wall
Copy link
Copy Markdown
Member

k-wall commented Apr 28, 2026

Hi! We've updated the proposal numbering system to use PR numbers as proposal identifiers.

Action required: Please rebase your PR on main.

Once you rebase, you'll need to rename your proposal file and update the title:

git mv proposals/015-config2-multi-file-plugin-configuration.md proposals/096-config2-multi-file-plugin-configuration.md
# Update title: remove any old number prefix and add PR number
sed -i.bak '0,/^# /{s/^# \([0-9]\{3\}\|xxx\|nnn\|000\) - /# 96 - /; t; s/^# /# 96 - /}' proposals/096-config2-multi-file-plugin-configuration.md && rm proposals/096-config2-multi-file-plugin-configuration.md.bak
git add proposals/096-config2-multi-file-plugin-configuration.md
git commit -m "Rename proposal to use PR number"
git push

The GitHub workflow will automatically check your proposal file naming after you push and update this PR description if any corrections are still needed.

See proposals/README.md for the updated workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Want to Do

Development

Successfully merging this pull request may close these issues.

2 participants