Skip to content

[server] Fix alter cluster configs - #4179

Merged
leekeiabstraction merged 1 commit into
apache:mainfrom
leekeiabstraction:fix-alter-cluster-configs
Sep 1, 2026
Merged

[server] Fix alter cluster configs#4179
leekeiabstraction merged 1 commit into
apache:mainfrom
leekeiabstraction:fix-alter-cluster-configs

Conversation

@leekeiabstraction

@leekeiabstraction leekeiabstraction commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Improve mechanism around altering certain dynamic cluster-config keys, and adds an integration test covering the behavior.

@polyzos

polyzos commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

@leekeiabstraction just one nit:

authorize(ALTER, cluster) now runs after building serverConfigChanges, so AlterConfigOpType.from(...) parses request data before any authz runs . This means that an unauthorized caller can trigger a parse error pre-authz. Maybe move the parsing after the authz block (or keep the baseline authorization at the very top).

Otherwise LGTM, and we can merge once CI is green.

Tightens the authorization performed when altering certain dynamic cluster-config keys, and adds an integration test covering the behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01AMMDkWCsb8fPv4V4upvzcy
@leekeiabstraction
leekeiabstraction force-pushed the fix-alter-cluster-configs branch from 75699a4 to 02a2bb8 Compare September 1, 2026 19:37
@leekeiabstraction
leekeiabstraction merged commit 15ff9a0 into apache:main Sep 1, 2026
20 checks passed
@loserwang1024

loserwang1024 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@leekeiabstraction Hi Keith, I saw your PR: #4179. Are you currently using Fluss in production, and is this change intended to address a production safety concern? I’m asking because it may break compatibility with my existing production deployment, in which case I may need to roll back the change(#4196).

If you need this in production, maybe we could add a configuration flag in a follow-up to preserve backward compatibility later.

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.

3 participants