Skip to content

Conversation

korthout
Copy link
Member

@korthout korthout commented Aug 15, 2025

Description

This PR introduces a new module qa/archunit-test (io.camunda:camunda-archunit-tests) that can be used to define ArchUnit tests to cover the entire production code base.

It also adds an ArchUnit test to this module RequireKebabCaseInConditionalOnPropertyArchTest which helps ensure that we support both camelCase and kebab-case for config properties. The ConditionalOnProperty annotation requires the usage of kebab-case to support relaxed binding to properties.

To satisfy the new rule, I've also updated several usages of ConditionalOnProperty towards the following properties:

Before After
camunda.tasklist.webappEnabled camunda.tasklist.webapp-enabled
camunda.operate.webappEnabled camunda.operate.webapp-enabled
camunda.tasklist.importerEnabled camunda.tasklist.importer-enabled
camunda.operate.importerEnabled camunda.operate.importer-enabled
camunda.tasklist.featureFlag.processPublicEndpoints camunda.tasklist.feature-flag.process-public-endpoints

This change is backward compatible.

Note

This PR does not yet move over existing ArchUnit tests to the new module. I'll raise a discussion about this separately through #37010.

Checklist

Related issues

closes #36869

@github-actions github-actions bot added component/zeebe Related to the Zeebe component/team component/operate Related to the Operate component/team component/tasklist Related to the Tasklist component/team component/optimize Related to Optimize component/team labels Aug 15, 2025
@korthout korthout changed the base branch from main to korthout-36597-multi-tenancy-checks-enabled-kebab August 15, 2025 14:55
@korthout korthout force-pushed the korthout-36597-multi-tenancy-checks-enabled-kebab branch from 0334c69 to aaacddc Compare August 15, 2025 15:11
Base automatically changed from korthout-36597-multi-tenancy-checks-enabled-kebab to main August 18, 2025 09:31
@korthout korthout force-pushed the korthout-36869-condition-on-property-archunit-rule branch 3 times, most recently from 6f44a78 to 6da685a Compare August 20, 2025 14:10
@korthout korthout added backport stable/8.5 Backport a pull request to stable/8.5 backport stable/8.6 Backport a pull request to stable/8.6 backport stable/8.7 Backport a pull request to stable/8.7 backport stable/operate-8.5 labels Aug 20, 2025
Adds a new module that can be used to specify ArchUnit tests for the
orchestration cluster.

These tests have access to the entire production code through a
dependency on dist (io.camunda:camunda-zeebe).

The tests need to be run by surefire rather than failsafe to ensure
correct execution and access to loaded classes. With failsafe, tests are
executed but failures are not detected somehow. I did not understand why
this is. Note that the parent module (io.camunda:camunda-qa) attempts to
run all tests with failsafe rather than surefire so the archunit-tests's
pom explicitly re-configures this.

The choice to create a new module was made as it:
- allows to easily run these tests together
- depending on dist ensures it runs on all production code, including
  modules added later without needing to change dependencies
- helps others to find a fitting place for their ArchUnit tests
- groups together ArchUnit tests for easily gaining an overview

The downsides are:
- long feedback loop - I value this less than the long term ease of
  maintenance.
- doesn't run on non-prod code - No need to solve this now.

See: https://camunda.slack.com/archives/C08F5RD5X8B/p1755527617038109
The ConditionalOnProperty annotation requires kebab-case in its
attributes in order to correctly match them using relaxed binding to
spring loaded properties. In other words, to match to both my-example
and myExample, the @ConditionalOnProperty must refer to my-example. If
it refers to myExample, then it only works explicitly on myExample and
relaxed binding to my-example doesn't work.

To avoid this, we add an ArchUnit test that verifies that none of the
attributes of ConditionalOnProperty use camelCase. It does this simply
by checking for capitalized letters. If it fails, the error message
includes all the needed details, like the location of the forbidden
annotation, the specific attribute, and the failure.

To overcome the problem, simply switch to using kebab-case in the
attributes of ConditionalOnProperty.
ConditionalOnProperty requires specifying names of properties using
kebab-case.

This ensures that camunda.tasklist.webapp-enabled can be used rather
than requiring camunda.tasklist.webappEnabled explicitly.

Note that the docs document it as camunda.tasklist.webappEnabled, but
this change is backward compatible, as camunda.tasklist.webappEnabled
can still be matched due to relaxed binding. We decided to use
kebab-case for configs in the docs, but they have not been adjusted yet.
ConditionalOnProperty requires specifying names of properties using
kebab-case.

This ensures that camunda.operate.webapp-enabled can be used rather
than requiring camunda.operate.webappEnabled explicitly.

Note that the docs document it as camunda.operate.webappEnabled, but
this change is backward compatible, as camunda.operate.webappEnabled
can still be matched due to relaxed binding. We decided to use
kebab-case for configs in the docs, but they have not been adjusted yet.
ConditionalOnProperty requires specifying names of properties using
kebab-case.

This ensures that camunda.tasklist.importer-enabled can be used rather
than requiring camunda.tasklist.importerEnabled explicitly.

Note that the docs document it as camunda.tasklist.importerEnabled, but
this change is backward compatible, as camunda.tasklist.importerEnabled
can still be matched due to relaxed binding. We decided to use
kebab-case for configs in the docs, but they have not been adjusted yet.
ConditionalOnProperty requires specifying names of properties using
kebab-case.

This ensures that camunda.operate.importer-enabled can be used rather
than requiring camunda.operate.importerEnabled explicitly.

Note that the docs document it as camunda.operate.importerEnabled, but
this change is backward compatible, as camunda.operate.importerEnabled
can still be matched due to relaxed binding. We decided to use
kebab-case for configs in the docs, but they have not been adjusted yet.
ConditionalOnProperty requires specifying names of properties using
kebab-case.

This ensures that camunda.tasklist.feature-flag.process-public-endpoints
can be used rather than requiring
camunda.tasklist.featureFlag.processPublicEndpoints explicitly.
@korthout korthout force-pushed the korthout-36869-condition-on-property-archunit-rule branch from 6da685a to eba1343 Compare August 20, 2025 14:18
@korthout korthout marked this pull request as ready for review August 20, 2025 15:02
@korthout korthout requested a review from a team August 20, 2025 15:07
Copy link
Contributor

@mustafadagher mustafadagher left a comment

Choose a reason for hiding this comment

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

Thanks @korthout ! 🚀

👍 I like having the new module, and I hope it will be used more often to include similar tests like this one.

❓ Do we need to do anything extra to have tests in this module run in the CI? I assume not?

Without this, the qa modules are not used during `./mvnw test` or
`./mvnw verify`.
@korthout
Copy link
Member Author

❓ Do we need to do anything extra to have tests in this module run in the CI? I assume not?

@mustafadagher I expected it would run in General / [UT] Run Unit Tests but it isn't because it's not run on ./mvnw test at all. The reason is that the module is not configured fully. I have some ideas:

  • add it as a module to ./pom.xml
  • add it as a dependency to ./parent/pom.xml

I'll try out the first option and see if that works. If you have another, let me know.

@mustafadagher
Copy link
Contributor

@mustafadagher I expected it would run in General / [UT] Run Unit Tests but it isn't because it's not run on ./mvnw test at all. The reason is that the module is not configured fully. I have some ideas:

  • add it as a module to ./pom.xml
  • add it as a dependency to ./parent/pom.xml

I'll try out the first option and see if that works. If you have another, let me know.

@korthout in my other PR, that is based on your branch, I see them running and failing here, without a change to the root pom

@korthout
Copy link
Member Author

@korthout in my other PR, that is based on your branch, I see them running and failing here, without a change to the root pom

That workflow run checked out c73b402, which in turn has 3a39089 as a parent. So it already contained my fix. Note that PR workflows run against a merge between the PR head and base rather than on the PR head itself.

korthout and others added 2 commits August 21, 2025 08:14
The condition checks against uppercase characters only. This
means it accepts kebab-case and not camelCase or PascalCase.
Just staying not camelCase is thus not entirely correct.

We can easily resolve this by removing the mention of camelCase
here. Note that the validation already describes the actual
violation (no uppercase allowed).

I also took the chance to align both descriptions.
This change moves the RequireKebabCaseInConditionalOnPropertyArchTest to
the io.camunda package as it covers all those classes.

This makes it easier for other ArchTests to also align to the structure
of the codebase that they work with. The end goal is a module that
organizes its test in way where it's easy to see at a glance what code
is covered by these tests.
@korthout korthout force-pushed the korthout-36869-condition-on-property-archunit-rule branch from 7821ba6 to 42ce60f Compare August 21, 2025 06:15
@korthout korthout enabled auto-merge August 21, 2025 06:15
@korthout korthout added this pull request to the merge queue Aug 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 21, 2025
@korthout korthout added this pull request to the merge queue Aug 21, 2025
Merged via the queue into main with commit f197927 Aug 21, 2025
186 of 188 checks passed
@korthout korthout deleted the korthout-36869-condition-on-property-archunit-rule branch August 21, 2025 09:18
@backport-action
Copy link
Collaborator

Created backport PR for stable/8.5:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-36872-to-stable/8.5
git worktree add --checkout .worktree/backport-36872-to-stable/8.5 backport-36872-to-stable/8.5
cd .worktree/backport-36872-to-stable/8.5
git reset --hard HEAD^
git cherry-pick -x 43995116fc61966efa66a3acf1a68cb013b95ab4 d67a9f84a19353e7d3c5410cb6b309a8c1d446c5 d6f200e342211ccabb428950f0d13f5b292cf105 d8b567aace42680c722f73b9fcaffd10608eeefb feaf9335ea04da0a94bbb19ae0303c2295684cc6 b50ce94937798b416321ae169739a94f51267cf5 eba1343f4ce83b71e6d3043a0d029b11b86ab6ae 3a39089dd2e5b382b1f3b6d946411d74453ea02c 833b6b7e0abd1699089bc879fcb2d0bee7c5f3d8 42ce60f902b70c02de9c8d2d3eb4e0f66614bcc4
git push --force-with-lease

@backport-action
Copy link
Collaborator

Created backport PR for stable/operate-8.5:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-36872-to-stable/operate-8.5
git worktree add --checkout .worktree/backport-36872-to-stable/operate-8.5 backport-36872-to-stable/operate-8.5
cd .worktree/backport-36872-to-stable/operate-8.5
git reset --hard HEAD^
git cherry-pick -x 43995116fc61966efa66a3acf1a68cb013b95ab4 d67a9f84a19353e7d3c5410cb6b309a8c1d446c5 d6f200e342211ccabb428950f0d13f5b292cf105 d8b567aace42680c722f73b9fcaffd10608eeefb feaf9335ea04da0a94bbb19ae0303c2295684cc6 b50ce94937798b416321ae169739a94f51267cf5 eba1343f4ce83b71e6d3043a0d029b11b86ab6ae 3a39089dd2e5b382b1f3b6d946411d74453ea02c 833b6b7e0abd1699089bc879fcb2d0bee7c5f3d8 42ce60f902b70c02de9c8d2d3eb4e0f66614bcc4
git push --force-with-lease

@backport-action
Copy link
Collaborator

Created backport PR for stable/8.6:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-36872-to-stable/8.6
git worktree add --checkout .worktree/backport-36872-to-stable/8.6 backport-36872-to-stable/8.6
cd .worktree/backport-36872-to-stable/8.6
git reset --hard HEAD^
git cherry-pick -x 43995116fc61966efa66a3acf1a68cb013b95ab4 d67a9f84a19353e7d3c5410cb6b309a8c1d446c5 d6f200e342211ccabb428950f0d13f5b292cf105 d8b567aace42680c722f73b9fcaffd10608eeefb feaf9335ea04da0a94bbb19ae0303c2295684cc6 b50ce94937798b416321ae169739a94f51267cf5 eba1343f4ce83b71e6d3043a0d029b11b86ab6ae 3a39089dd2e5b382b1f3b6d946411d74453ea02c 833b6b7e0abd1699089bc879fcb2d0bee7c5f3d8 42ce60f902b70c02de9c8d2d3eb4e0f66614bcc4
git push --force-with-lease

@backport-action
Copy link
Collaborator

Created backport PR for stable/8.7:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-36872-to-stable/8.7
git worktree add --checkout .worktree/backport-36872-to-stable/8.7 backport-36872-to-stable/8.7
cd .worktree/backport-36872-to-stable/8.7
git reset --hard HEAD^
git cherry-pick -x 43995116fc61966efa66a3acf1a68cb013b95ab4 d67a9f84a19353e7d3c5410cb6b309a8c1d446c5 d6f200e342211ccabb428950f0d13f5b292cf105 d8b567aace42680c722f73b9fcaffd10608eeefb feaf9335ea04da0a94bbb19ae0303c2295684cc6 b50ce94937798b416321ae169739a94f51267cf5 eba1343f4ce83b71e6d3043a0d029b11b86ab6ae 3a39089dd2e5b382b1f3b6d946411d74453ea02c 833b6b7e0abd1699089bc879fcb2d0bee7c5f3d8 42ce60f902b70c02de9c8d2d3eb4e0f66614bcc4
git push --force-with-lease

github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2025
…ab-case in ConditionalOnProperty (#37052)

# Description
Backport of #36872 to `stable/8.7`.

relates to #36869
@cmur2
Copy link
Member

cmur2 commented Aug 25, 2025

@korthout I think this PR is causing the Maven release perform GHA to fail since it was merged: https://github.com/camunda/camunda/actions/runs/17143538365/job/48635366567

I assume that is because this PR adds qa to the top-level pom.xml and thus makes the Camunda QA module appear in the reactor for the release build - does that sound plausible? See my thread on Slack

@korthout
Copy link
Member Author

Yes. Covered in:

Will re-add running them in CI with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport stable/operate-8.5 backport stable/8.5 Backport a pull request to stable/8.5 backport stable/8.6 Backport a pull request to stable/8.6 backport stable/8.7 Backport a pull request to stable/8.7 component/operate Related to the Operate component/team component/optimize Related to Optimize component/team component/tasklist Related to the Tasklist component/team component/zeebe Related to the Zeebe component/team version:8.7.11
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add arch unit test to prevent config properties not supporting camelCase or kebab-case
4 participants