Skip to content

Conversation

smiklosovic
Copy link
Contributor

@smiklosovic smiklosovic commented Sep 16, 2025

Implementation of CEP-55

This patch is rather easy to go through. A lot of files were changed just because of renaming.

For reviewers, I would just focus on implementation and not bothering with tests, docs and all sauce around that. Tests are very easy to follow as well.

# Guardrail to warn or fail when setting / altering a password.
# Supported character sets are (both upper and lower-case): English, Cyrillic and modern Cyrillic, Czech, German, Polish.
# Password is invalid if all characters are from non-supported character set. If a password is otherwise valid,
# but it contains characters from unsupported language, these characters contribute only to password length rule.
# All digits and all following special characters are supported too: !"#$%&()*+,-./:;<=>?@[\]^_`{|}~
#password_validator:
#password_policy:
Copy link
Contributor Author

@smiklosovic smiklosovic Sep 16, 2025

Choose a reason for hiding this comment

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

it is better if we rename this now as this has not appeared in any release yet and it would be just too late for that otherwise. I think this better reflects what this does as it holds both generator as well as validator under one "policy". Same case for role name.

@@ -2290,6 +2290,92 @@ drop_compact_storage_enabled: false
# maximum_replication_factor_warn_threshold: -1
# maximum_replication_factor_fail_threshold: -1

#role_name_policy:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

both policies were missing, in cassandra_latest.yaml.

@@ -0,0 +1,4 @@
= Role name validation and generation
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be finished upon merge.

Copy link

@worryg0d worryg0d left a comment

Choose a reason for hiding this comment

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

Hello.

I’ve spent some time going through the code in this patch. Since I’m new to the community and not yet deeply familiar with the C* codebase, I focused mainly on spotting things like code duplications, redundant conditions, typos, and log message corrections. Hoping that this will free more experienced and more familiar with the Cassandra codebase reviewers from such things.

I also spotted this (1) misuse of ValueValidator.class for the logger of the ValueGenerator class. Since this patch already touches that class, it might be a good place to fix it as well.

https://github.com/apache/cassandra/blob/b63ff3b003c68f94cc29223d54816c020a491684/src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java#L38

@smiklosovic smiklosovic force-pushed the CASSANDRA-20897 branch 4 times, most recently from dd7410e to 075793e Compare September 22, 2025 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants