Complete Groovy to Java 17 migration and modernize with Lombok and Records

[DerDaehne]

Overview

This pull request completes the migration of the remaining core application components from Groovy to Java 17. The changes focus on reducing boilerplate code and improving
readability by implementing Lombok, Java 17 Records, and modern Java syntax features.

---

Key Changes

1. Boilerplate Reduction with Lombok

• Lombok Integration: Configured the Lombok dependency and annotation processor inside pom.xml.
• Config.java Refactoring: Applied @Getter and @Setter to the outer class and all 28 nested static schema classes. Standard trivial getters and setters were removed, resulting
  in a net reduction of 1,352 lines of code (~63% reduction). All custom validation logic and constructors initializing helm defaults were preserved.
• Credentials.java and ScmCentralSchema.java: Refactored with @Getter, @Setter, and @tostring(exclude = "password") to eliminate more than 160 lines of boilerplate while
  ensuring sensitive passwords are never written to application logs.

2. Adoption of Java 17 Records

• Role.java and Permission.java: Converted to native Records to enforce immutability and replace repetitive class structures. Compact constructors were implemented for input
  validation.
• FreeMarker Compatibility: Kept DockerImageParser.Image as a standard class but annotated it with Lombok @Getter instead of converting it to a Record. This avoids template
  parsing issues caused by the custom AllowListFreemarkerObjectWrapper requiring standard JavaBean getter methods (i.e. getRegistry()).
• Test Updates: Updated property-style accesses in Groovy Spock tests (ScmManagerProviderTest.groovy) to use record-style method accessors (e.g. p.groupPermission()).

3. Modern Java 17 Features

• Pattern Matching for instanceof: Modernized K8sClient.java inside extractPhase(HasMetadata) to eliminate manual casting when checking for Pod instances.
• Switch Expressions: Refactored visibility and access level mapping in GitlabProvider.java using switch expressions with arrow syntax (->) and multi-label cases (MAINTAIN,
  ADMIN ->).
• Text Blocks: Converted inline Groovy script strings in PrometheusConfigurator.java into Java 17 Text Blocks ("""), removing raw newline string concatenations.

4. Test Suite and Build Validation

• Helm Version Fix: Corrected a version mismatch in GitopsPlaygroundCliTest.groovy by raising the expected Helm version to 3.11.10 to match the default configuration in
  ScmTenantSchema.java.
• Test Executions: All 527 unit and integration tests are passing.

[mdroll]

Supressing warnings without comment

[mdroll]

Need a bunch of rework here and there. Not explicitly looked on general sonar hints.
Still we need to test also the others profiles. And also test on openshift / OKD.

[mdroll]

Newer used

[mdroll]

This method is suited for to be in a own method

[mdroll]

Next pitch switch to more modern API-Way:

Stream.of(
clusterResourcesRootDir(),
clusterResourcesAppsDir(),
clusterResourcesArgoCdDir(),
clusterResourcesApplicationsDir(),
clusterResourcesProjectsDir()
).forEach(this::createDirectorySafely);

[mdroll]

Never used

[mdroll]

Never used

[mdroll]

Never used

[mdroll]

What is this?

[mdroll]

Supressing here
Lombok may can be added here so a few setter/getter methods are not needed

[FelixWende99]

This @SuppressWarnings("java:S3776") is applied to the entire class, which hides complexity warnings for all methods. Consider narrowing it to only the specific methods that truly need it (e.g. applyYaml), or reduce complexity there instead. A class-level suppression makes it easy to miss new problems added later.

[FelixWende99]

This delete-then-create pattern is non-atomic: if the delete succeeds but the subsequent create fails, the secret is left absent. The createImagePullSecret method uses createOr(NonDeletingOperation::update) which is idempotent in a single API call. Consider using the same pattern here:
secretsClient.resource(secret).createOr(NonDeletingOperation::update);

[FelixWende99]

Catching Exception broadly here can silently swallow real connectivity or authorization errors (e.g. 403 Forbidden would be treated as "namespace does not exist", causing createNamespace to attempt creation and fail with a confusing error). Consider catching only KubernetesClientException and rethrowing for non-404 status codes.

[FelixWende99]

The dockerConfig JSON string is assembled by hand via string concatenation. A username or password containing " would break the JSON silently. Use a proper serializer instead:
Map<String, Object> dockerConfigMap = Map.of( "auths", Map.of(host, Map.of("username", user, "password", password, "auth", auth))); String dockerConfig = Serialization.asJson(dockerConfigMap);

[FelixWende99]

The two convenience overloads delete(resource) and delete(resource, namespace) both delegate to this varargs method with an empty array, which immediately throws IllegalArgumentException("Missing selectors"). This makes those overloads permanently broken. Either remove the empty-array guard for "delete all" semantics, or remove/document the no-selector overloads.

[FelixWende99]

Code review of the Groovy-to-Java migration across the 20 biggest classes. Co-Authored by Copilot

[FelixWende99]

Undocumented fallthrough to STRATEGIC_MERGE — An unrecognised type string silently falls through to STRATEGIC_MERGE. Consider throwing IllegalArgumentException so misconfigurations surface immediately rather than silently using the wrong patch type.

[FelixWende99]

Inconsistent not-found contract — getCustomResourceClient throws KubernetesApiResourceNotFoundException when the match is empty, but findApiResourceViaDiscovery returns an empty map. Callers must know which convention applies. Consider making both throw, or both return Optional<Map<...>>.

[FelixWende99]

app bean used after its originating ApplicationContext is closed — app is fetched from the first context (line 83), which is then closed on line 97. app is reused on line 122 from the new context but the same object reference from the old closed context is passed to runHook. Fetch app from the second context after register() for consistency, or document why cross-context reuse is safe here.

[FelixWende99]

Potential NPE when scmManager is null — newConfig.getScm().getScmManager().getUrl() is called unconditionally. In GitLab-only mode, GitHandler.validate() explicitly sets scmManager to null. Add a null guard:

if (newConfig.getScm().getScmManager() != null && hasText(newConfig.getScm().getScmManager().getUrl())) {

[FelixWende99]

MalformedURLException wrapped as UncheckedIOException — MalformedURLException is not an I/O error. Wrapping it as UncheckedIOException misrepresents the failure to callers and catch blocks. Use IllegalArgumentException (consistent with how line 328 handles the same exception type in isUrlSetAndValid).

[FelixWende99]

URI.create(url) throws IllegalArgumentException on empty or invalid URL — getHost() and getProtocol() call URI.create(url) unconditionally. If url is empty (its default value is ""), URI.create("").getHost() returns null rather than throwing, but callers expecting a non-null host may behave unexpectedly. A blank-url guard or Optional return type would make the contract explicit.

[FelixWende99]

listDirectories method has no return value and unclear purpose — This method (lines 196–202) calls getAllFilesFromDirectoryWithEnding(parentDir, "") which matches all files (not just directories), then logs each path at DEBUG. The method name says "list directories" but lists all files. Either fix the predicate to filter for directories only, or rename/document the method's actual behaviour.

[FelixWende99]

Freemarker getStaticModels() failure wrapped as RuntimeException — The catch block on line 200 wraps a Exception as RuntimeException. This is the same pattern flagged elsewhere in the codebase. DefaultObjectWrapperBuilder.build() does not declare checked exceptions; if it throws, it's a programming error (misconfiguration), not an expected runtime failure. Let it propagate unchecked or use a more specific message.

[FelixWende99]

new FileSystemUtils() bypasses DI — prepareHelmValues() calls new FileSystemUtils() directly instead of using a constructor-injected instance. This makes the class harder to test and breaks Micronaut's singleton scope for FileSystemUtils. Add FileSystemUtils fileSystemUtils as a constructor parameter and use it here.

[FelixWende99]

waitForScmmAvailable swallows InterruptedException in the timeout loop — Line 209 catches InterruptedException, restores the interrupt flag with Thread.currentThread().interrupt(), but then continues the loop rather than returning or throwing. This means an interrupted thread will busy-spin until the timeout expires instead of stopping promptly. Add a return or break after Thread.currentThread().interrupt().

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.