Add cloud logging service feature by Yavor16 · Pull Request #1847 · cloudfoundry/multiapps-controller

Yavor16 · 2026-05-28T06:51:24Z

No description provided.

# Conflicts: # multiapps-controller-process/src/main/resources/org/cloudfoundry/multiapps/controller/process/backup-existing-app.bpmn # multiapps-controller-process/src/main/resources/org/cloudfoundry/multiapps/controller/process/deploy-app.bpmn # multiapps-controller-process/src/main/resources/org/cloudfoundry/multiapps/controller/process/stop-dependent-modules.bpmn # multiapps-controller-process/src/main/resources/org/cloudfoundry/multiapps/controller/process/undeploy-app.bpmn

sonarqubecloud · 2026-06-01T10:57:30Z

Quality Gate failed

Failed conditions
1 Security Hotspot
78.9% Coverage on New Code (required ≥ 80%)
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

IvanBorislavovDimitrov · 2026-06-04T11:24:54Z

+
+    @Override
+    protected String getStepErrorMessage(ProcessContext context) {
+        return "Well, failed! Deal with it!";


:O why so rude?

IvanBorislavovDimitrov · 2026-06-04T11:37:50Z

+import org.cloudfoundry.multiapps.controller.core.auditlogging.model.ConfigurationChangeActions;
+import org.cloudfoundry.multiapps.controller.persistence.model.LoggingConfiguration;
+
+public class CloudLoggingServiceConfigurationAuditLog {


Because SAP Cloud logging service is an internal proprietary service it should not be exposed in the open source. The whole logic related to the service must be move in the internal project.

Move every SAP related thing to the internal project and only leave the what is actually open source in this PR

IvanBorislavovDimitrov · 2026-06-04T12:09:36Z

+        Map<String, String> identifiers = new HashMap<>();
+        identifiers.put(MTA_ID_PROPERTY_NAME, loggingConfiguration.getMtaId());
+        identifiers.put(NAMESPACE_PROPERTY_NAME, loggingConfiguration.getNamespace());
+        auditLoggingFacade.logDataAccessAuditLog(new AuditLogConfiguration(username,


why buildIdentifiers is not used in this method?

IvanBorislavovDimitrov · 2026-06-04T12:10:13Z

+                                                                           identifiers));
+    }
+
+    public void logListLoggingConfigurations(String username, String spaceId) {


Is this really used?

IvanBorislavovDimitrov · 2026-06-04T12:12:31Z

+        identifiers.put(NAMESPACE_PROPERTY_NAME, loggingConfiguration.getNamespace());
+        identifiers.put(TARGET_SPACE_PROPERTY_NAME, loggingConfiguration.getTargetSpace());
+        identifiers.put(TARGET_ORG_PROPERTY_NAME, loggingConfiguration.getTargetOrg());
+        identifiers.put(SERVICE_INSTANCE_NAME_PROPERTY_NAME, loggingConfiguration.getServiceInstanceName());
+        identifiers.put(SERVICE_KEY_NAME_PROPERTY_NAME, loggingConfiguration.getServiceKeyName());


Can we use the IDs of target space and service instance?

IvanBorislavovDimitrov · 2026-06-04T13:27:58Z

+        return logsMap;
+    }
+
+    private void getMessagesToLog(String log, Map<LogLevel, List<LogLogLog>> logsMap) {


return a Map of the logs instead of in-parameters

IvanBorislavovDimitrov · 2026-06-04T13:28:38Z

+            InputStream serverCaStream = getCredentialInputStream(loggingConfiguration.getServerCa());
+            InputStream clientCertStream = getCredentialInputStream(loggingConfiguration.getClientCert());
+            InputStream clientKeyStream = getCredentialInputStream(loggingConfiguration.getClientKey());


are these streams closed?

IvanBorislavovDimitrov · 2026-06-04T13:29:10Z

+                                                 .build();
+    }
+
+    public record LogLogLog(String log, LocalDateTime dateTime) {


What is this Log :D

IvanBorislavovDimitrov · 2026-06-04T13:30:31Z

+        }
+    }
+
+    private boolean areCloudLoggingParametersValid(String serviceInstanceName, String serviceKeyName) {


inverted naming?

IvanBorislavovDimitrov · 2026-06-04T13:31:10Z

+        String credential = (String) credentials.get(credentialsName);
+
+        if (credential == null) {
+            throw new IllegalArgumentException("Missing required " + credentialsName + " credential for SAP Cloud Logging export");


Use SLException instead.

IvanBorislavovDimitrov

Additional comments on OperationLogsExporter not yet covered in the earlier review.

IvanBorislavovDimitrov · 2026-06-04T13:48:59Z

+    }
+
+    private boolean hasRequestFailed(ResponseEntity<Void> response) {
+        if (response == null) {


null response is treated as success here. .toBodilessEntity().block() can return null (empty Mono, cancellation, certain WebClient error paths). Treating that as a non-failure means a silently-dropped batch is reported as sent. Either treat null as failure, or document precisely under which condition it can be null and still be safe.

IvanBorislavovDimitrov · 2026-06-04T13:49:03Z

+
+        for (ExternalOperationLogEntry entry : externalLogEntries) {
+            String entryJson = JsonUtil.toJson(entry);
+            int entrySize = entryJson.getBytes().length;


entryJson.getBytes().length uses the JVM default charset. The body that actually goes on the wire is UTF-8 (Jackson default). On a non-UTF-8 default platform you'll under- or over-estimate the batch size and either oversize a request or split too aggressively. Use entryJson.getBytes(StandardCharsets.UTF_8).length.

IvanBorislavovDimitrov · 2026-06-04T13:49:06Z

+
+            String cleanedMessage = extractMessage(message);
+            String level = logLevels.get(levelIndex);
+            LocalDateTime date = dateLevels.get(levelIndex);


logLevels.get(levelIndex) / dateLevels.get(levelIndex) have no bounds check. The split regex and the level/date regexes walk the same input independently, so a single malformed line desynchronises them and you get an IndexOutOfBoundsException. Not fail-safe — even with isFailSafe = true the exception is unchecked and escapes.

IvanBorislavovDimitrov · 2026-06-04T13:49:08Z

+
+    private List<LocalDateTime> getLogDate(String log) {
+        Matcher matcher = MESSAGE_LOG_DATE_PATTERN.matcher(log);
+        List<LocalDateTime> logLevels = new ArrayList<>();


Variable is named logLevels but holds LocalDateTime. Rename to logDates.

IvanBorislavovDimitrov · 2026-06-04T13:49:10Z

+    private String extractMessage(String message) {
+        String trimmed = message.substring(message.indexOf("]") + 1)
+                                .trim();
+        return trimmed.substring(0, trimmed.length() - 1);


trimmed.substring(0, trimmed.length() - 1) assumes the message ends with a delimiter. On an empty trimmed this throws StringIndexOutOfBoundsException; on a message that doesn't actually end in the expected # we silently drop the last real character. Guard the length, or use a regex that captures the body explicitly.

IvanBorislavovDimitrov · 2026-06-04T13:49:14Z

+            webClient = clientCache.get(loggingConfiguration.getOperationId());
+        }
+
+        LOGGER.debug(MessageFormat.format(Messages.CREATING_WEBCLIENT_WITH_MTLS_CONFIGURATION_FOR_ENDPOINT_1,


This debug line fires on every call — including cache hits — but the message reads "creating WebClient with mTLS configuration…", which is wrong on a hit. Move it inside the if branch above so it only logs when a client is actually created.

IvanBorislavovDimitrov · 2026-06-04T13:49:17Z

+    }
+
+    private void logErrorOrThrowExceptionBasedOnFailSafe(LoggingConfiguration loggingConfiguration, String message) {
+        if (loggingConfiguration.isFailSafe()) {


loggingConfiguration.isFailSafe() returns @Nullable Boolean; auto-unboxing here will NPE if the field is null. Either default it at the model layer (e.g. @Value.Default) or null-check at the call site. The same trap exists in ExternalLoggingServiceConfigurationsCalculator#getServiceKeyWithLoggingConfiguration.

Yavor16 force-pushed the export-logs-to-logging-service-squash branch from 20788df to 7220532 Compare May 28, 2026 06:57

fix rebase problems

bb7fc04

Yavor16 force-pushed the export-logs-to-logging-service-squash branch from c1abde6 to bb7fc04 Compare June 1, 2026 10:51

IvanBorislavovDimitrov reviewed Jun 4, 2026

View reviewed changes

Conversation

Yavor16 commented May 28, 2026

Uh oh!

sonarqubecloud Bot commented Jun 1, 2026

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IvanBorislavovDimitrov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants