Skip to content

Feature ETP-3459: Fixed local configurations and new make tasks#228

Open
Matias-Bernal wants to merge 4 commits intodevelopfrom
feature/ETP-3459
Open

Feature ETP-3459: Fixed local configurations and new make tasks#228
Matias-Bernal wants to merge 4 commits intodevelopfrom
feature/ETP-3459

Conversation

@Matias-Bernal
Copy link
Copy Markdown
Contributor

This pull request introduces comprehensive technical documentation for EtendoRX and the OBConnector module, and implements several improvements across the codebase to enhance error reporting, message handling, and authentication flexibility. The most important changes are grouped by theme below.

Documentation improvements:

  • Added docs/INDEX.md as a master technical documentation index for EtendoRX, including service catalog, platform/module documentation, diagrams, and ports reference.
  • Added docs/infrastructure.md detailing local development infrastructure for OBConnector, including Redpanda and Kafka stacks, service descriptions, topic management, and startup order.
  • Added docs/plans/2026-02-27-technical-documentation-design.md outlining the documentation structure, locations, and principles for both platform and module.

Kafka and async process enhancements:

  • Limited the number of stored executions in AsyncProcess to 50, preventing unbounded memory growth by trimming the oldest entries.
  • Truncated the params field in async process execution messages to 1000 characters to avoid oversized Kafka messages.
  • Added logging and filtering for malformed messages with null keys or values in AsyncProcessTopology, improving error visibility and robustness. [1] [2] [3]

Authentication flexibility:

  • Introduced the auth.disabled property in FilterContext to allow bypassing authentication for development or testing, setting default user context values when disabled.

Error reporting improvement:

  • Enhanced the BindedRestController to include entity name and ID in the "Record not found" error message for better debugging.

sebastianbarrozo and others added 4 commits February 28, 2026 00:13
- FilterContext: auth.disabled=true sets superuser context (userId=100)
  with restMethod/restUri to avoid NPE in DefaultFilters.
- DefaultFilters: null check for restMethod with actionable error message.
- BindedRestController: 404 error includes entity type and ID.
- AsyncProcessTopology: filter null key/value records before groupByKey.
- AsyncProcessController: relax body type to Map<String, Object>.
- Optional config server (spring.config.import=optional:configserver:)
  for auth, edge, webflux, asyncprocess modules.
- Local profile properties for DAS and asyncprocess (auth.disabled, ports).
- Makefile with orchestrated startup (up-local, up, down, status, logs),
  infra management (Redpanda/Kafka), build/test targets, mock receiver.
- Dev portal (portal/index.html) for service browser.
- build.gradle: pass system properties to bootRun tasks.
- Updated rxconfig templates (worker port 8102, obconnsrv template).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Split loadtest into loadtest.send (Kafka CDC) and loadtest.receive (HTTP POST)
- Add purge target to delete and auto-recreate all OBConnector Kafka topics
- Add mock target for external receiver testing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Platform docs cover architecture, getting started, Makefile reference (33 targets),
configuration (YAML/gradle/templates), and infrastructure (Docker Compose, ports).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Matias-Bernal Matias-Bernal requested review from RomanMagnoli and sebastianbarrozo and removed request for RomanMagnoli March 17, 2026 18:15
@sonarscanetendo
Copy link
Copy Markdown

Failed Quality Gate failed

  • E Security Rating on New Code (is worse than A)
  • B Reliability Rating on New Code (is worse than A)
  • 4 New Issues (is greater than 0)

Project ID: etendosoftware_etendo_rx_AYOKxNe4uJ79WHyLB97w

View in SonarQube

@PostMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.ACCEPTED)
public Map<String, String> index(@RequestBody Map<String, Map<String, ?>> bodyChanges,
public Map<String, String> index(@RequestBody Map<String, Object> bodyChanges,

Check notice

Code scanning / SnykCode

Spring Cross-Site Request Forgery (CSRF) Note

The {0} parameter is vulnerable to Cross Site Request Forgery (CSRF) attacks due to not using Spring Security. This could allow an attacker to execute requests on a user's behalf. Consider including Spring Security's CSRF protection within your application.
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Record not found");
throw new ResponseStatusException(HttpStatus.NOT_FOUND,
"Record not found: entity=" + repository.getClass().getSimpleName() + " id=" + id);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Avoid exposing internal system details like repository class names in the exception message. Consider using a more generic error message or, if needed, use a formal entity name instead of the Java class name.

        "Record not found for id=" + id);

Rationale: Exposing internal class names (repository.getClass().getSimpleName()) in error messages can leak information about the system's internal structure. While helpful for debugging, it is safer to provide a more generic message in a public-facing REST API.

RXCONFIG := $(ROOT)/rxconfig
PROPS := $(ROOT)/gradle.properties
JAVA_HOME ?= $(shell /usr/libexec/java_home -v 17 2>/dev/null || echo "$$HOME/Library/Java/JavaVirtualMachines/corretto-17.0.18/Contents/Home")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Avoid hardcoding machine-specific JDK paths. Use a more generic fallback or rely on the environment's JAVA_HOME variable.

JAVA_HOME   ?= $(shell /usr/libexec/java_home -v 17 2>/dev/null || echo "$$JAVA_HOME")

Rationale: The hardcoded path '$$HOME/Library/Java/JavaVirtualMachines/corretto-17.0.18/Contents/Home' is specific to a single developer's machine and macOS version. Linux users or macOS users with different JDK builds will encounter issues if JAVA_HOME is not already set.

plugins.withId('org.springframework.boot') {
tasks.named('bootRun') {
systemProperties System.properties
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider filtering the system properties to pass only those relevant to the application (e.g., using prefixes like 'spring.' or 'etendo.').

            systemProperties System.getProperties().findAll { it.key.toString().startsWith("spring.") || it.key.toString().startsWith("etendo.") }

Rationale: Passing all system properties via System.properties includes internal JVM properties (e.g., java.vm.version, user.dir, os.name) and environment-specific details that are generally not required by the Spring Boot application. Filtering these helps keep the child process environment clean and reduces the risk of accidental information exposure if the application logs its configuration.

.description(description)
.params(bodyChanges != null ? bodyChanges.toString() : "")
.params(rawParams.length() > 1000 ? rawParams.substring(0, 1000) + "..." : rawParams)
.time(new Date())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Adjust the substring length to 997 so that the total length, including the ellipsis, does not exceed 1000 characters. Additionally, consider using a named constant instead of the magic number 1000.

        .params(rawParams.length() > 1000 ? rawParams.substring(0, 997) + "..." : rawParams)

Rationale: The current implementation results in a 1003-character string when truncation occurs (1000 characters from substring + 3 for ellipsis). If the underlying database column has a maximum size of 1000, this will still trigger a 'Value too long' error. Truncating to 997 ensures the final string fits within 1000 characters.

// The bootstrap servers for Kafka.
@Value("${bootstrap_server:kafka:9092}")
@Value("${spring.kafka.bootstrap-servers:${bootstrap_server:localhost:29092}}")
private String bootstrapServer;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Ensure that 29092 is the intended default port for local development, as the standard Kafka port is 9092. If this aligns with your local docker-compose environment, it is correct.

@Value("${spring.kafka.bootstrap-servers:${bootstrap_server:localhost:9092}}")

Rationale: This change improves flexibility by supporting the standard Spring Boot Kafka property while maintaining backward compatibility with the legacy 'bootstrap_server' key. Using localhost:29092 as default is common in docker-compose setups.

properties.put(ConsumerConfig.ALLOW_AUTO_CREATE_TOPICS_CONFIG, "true");
properties.put(StreamsConfig.CACHE_MAX_BYTES_BUFFERING_CONFIG, "0");
properties.put(StreamsConfig.CACHE_MAX_BYTES_BUFFERING_CONFIG, "10485760");
properties.put(StreamsConfig.APPLICATION_SERVER_CONFIG, kafkaStreamsHostInfo);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using a more readable format for the byte value, such as a constant or a calculation (10 * 1024 * 1024), to clarify that it represents 10MB.

properties.put(StreamsConfig.CACHE_MAX_BYTES_BUFFERING_CONFIG, Long.toString(10 * 1024 * 1024L));

Rationale: Setting CACHE_MAX_BYTES_BUFFERING_CONFIG to 10MB (the Kafka default) instead of 0 improves performance by allowing record deduplication and reducing I/O operations. Using a calculation or a constant is more readable than the raw byte value.

executions.add(transactionClone);
while (executions.size() > MAX_EXECUTIONS) {
executions.pollLast();
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Verify the declaration of the 'executions' field. It should be changed to NavigableSet (or TreeSet) to support the pollLast() method.

((NavigableSet<AsyncProcessExecution>) executions).pollLast();

Rationale: The pollLast() method is available in the NavigableSet and Deque interfaces, but not in SortedSet or Set. Based on existing tests, the 'executions' field is likely declared as a SortedSet. If so, this call will cause a compilation error. Declaring the field as NavigableSet is the recommended approach to use this method while following interface-based programming.

bootRun {
debugOptions {
enabled = Boolean.parseBoolean((findProperty('debugEnabled') ?: enabled.get()) as String)
suspend = Boolean.parseBoolean((findProperty('debugSuspend') ?: suspend.get()) as String)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using the Gradle Provider API for a more idiomatic and type-safe way to handle property overrides.

        enabled.set(providers.gradleProperty('debugEnabled').map { it.toBoolean() }.getOrElse(enabled.get()))

Rationale: The Provider API (providers.gradleProperty) is the recommended way in modern Gradle (7+) to handle properties. It avoids manual string parsing and is more robust when dealing with lazy properties, aligning better with the reactive nature of Etendo RX modules.

suspend = Boolean.parseBoolean((findProperty('debugSuspend') ?: suspend.get()) as String)
server = Boolean.parseBoolean((findProperty('debugServer') ?: server.get()) as String)
port = Integer.valueOf((findProperty('debugPort') ?: port.get()) as String)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Apply the Provider API pattern to the port configuration as well for consistency.

        port.set(providers.gradleProperty('debugPort').map { it.toInteger() }.getOrElse(port.get()))

Rationale: Using the Provider API ensures better type safety and lazy evaluation, which is a best practice in modern Gradle build scripts.

@@ -0,0 +1,9 @@
server.port=8099
bootstrap_server=localhost:29092
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using the .yaml format instead of .properties for consistency with other EtendoRX services and common practices in the project.

modules_core/com.etendorx.asyncprocess/src/main/resources/application-local.yaml

Rationale: EtendoRX services typically use YAML for configuration files (e.g., application.yaml, das.yaml) to maintain consistency and take advantage of hierarchical structuring.

bootRun {
debugOptions {
enabled = Boolean.parseBoolean((findProperty('debugEnabled') ?: enabled.get()) as String)
suspend = Boolean.parseBoolean((findProperty('debugSuspend') ?: suspend.get()) as String)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using the .set() method for Gradle Property types to ensure consistent behavior across different Gradle versions.

enabled.set(Boolean.parseBoolean((findProperty('debugEnabled') ?: enabled.get()) as String))

Rationale: Modern Gradle tasks use Property types. While Groovy property assignment often works via setters, using .set() is more explicit and aligns with Gradle's best practices for lazy properties.

das.url=http://localhost:8092
classic.url=http://localhost:8080
token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMCIsImNsaWVudCI6IjAiLCJyb2xlIjoiMCIsIm9yZ2FuaXphdGlvbiI6IjAiLCJ3YXJlaG91c2UiOiI0RDQ1RkU0QzUxNTA0MTcwOTA0N0Y1MUQxMzlBMjFBQyIsImlhdCI6MTc3MzA5OTM4OX0.9Gm51jBvRquDvzr-M3UKJJz3unh6DgSKVpasoVcMmBmN4pyX4Y6yV-Eknt8--h2t6hR8nh4iw3wBVQWAU_sHiQ
admin.token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMjNDNTk1NzVCOUNGNDY3Qzk2MjA3NjBFQjI1NUIzODkiLCJyb2xlIjoiNDJEMEVFQjFDNjZGNDk3QTkwREQ1MjZEQzU5N0U2RjAiLCJvcmdhbml6YXRpb24iOiI3QkFCQTVGRjgwNDk0Q0FGQTU0REVCRDIyRUM0NkYwMSIsIndhcmVob3VzZSI6IjlDRjk4QTE4QkM3NTRCOTk5OThFNDIxRjkxQzVGRTEyIiwiaWF0IjoxNzczMDk5Mzg5fQ.QPNzs1Xr3xKYJmQz3WjOwyYIdyfkYzv3FJ4PrES2U3-j3PabIaN4SwHQ-5b2-lFAOQ5zV9-bzYjvDDykWxcWew
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Remove the hardcoded JWT token and use a placeholder or environment variable reference.

token=${AUTH_TOKEN:}

Rationale: Exposing hardcoded JWT tokens in the source code is a security vulnerability. These tokens grant authenticated access and should be managed via environment variables or secure configuration stores.

classic.url=http://localhost:8080
token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMCIsImNsaWVudCI6IjAiLCJyb2xlIjoiMCIsIm9yZ2FuaXphdGlvbiI6IjAiLCJ3YXJlaG91c2UiOiI0RDQ1RkU0QzUxNTA0MTcwOTA0N0Y1MUQxMzlBMjFBQyIsImlhdCI6MTc3MzA5OTM4OX0.9Gm51jBvRquDvzr-M3UKJJz3unh6DgSKVpasoVcMmBmN4pyX4Y6yV-Eknt8--h2t6hR8nh4iw3wBVQWAU_sHiQ
admin.token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMjNDNTk1NzVCOUNGNDY3Qzk2MjA3NjBFQjI1NUIzODkiLCJyb2xlIjoiNDJEMEVFQjFDNjZGNDk3QTkwREQ1MjZEQzU5N0U2RjAiLCJvcmdhbml6YXRpb24iOiI3QkFCQTVGRjgwNDk0Q0FGQTU0REVCRDIyRUM0NkYwMSIsIndhcmVob3VzZSI6IjlDRjk4QTE4QkM3NTRCOTk5OThFNDIxRjkxQzVGRTEyIiwiaWF0IjoxNzczMDk5Mzg5fQ.QPNzs1Xr3xKYJmQz3WjOwyYIdyfkYzv3FJ4PrES2U3-j3PabIaN4SwHQ-5b2-lFAOQ5zV9-bzYjvDDykWxcWew
private-key=-----BEGIN PRIVATE KEY-----MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQghw7e6vE9io6Y6HTHgHD6RZ7o74ZRHmfVpQIEVE4qo6ihRANCAARi7S13nL4F2GSFm4KhRiEV/TP/6K23be2/ZLJmh1tihweopvKjMTGnwPOP2n5xgSQo68LJP2FzwUzZ2kQNrTEh-----END PRIVATE KEY-----
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Remove the hardcoded admin token and use a placeholder or environment variable.

admin.token=${ADMIN_TOKEN:}

Rationale: The admin token provides privileged access to the system. Committing it to the repository allows anyone with access to the code to impersonate an administrator.

token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMCIsImNsaWVudCI6IjAiLCJyb2xlIjoiMCIsIm9yZ2FuaXphdGlvbiI6IjAiLCJ3YXJlaG91c2UiOiI0RDQ1RkU0QzUxNTA0MTcwOTA0N0Y1MUQxMzlBMjFBQyIsImlhdCI6MTc3MzA5OTM4OX0.9Gm51jBvRquDvzr-M3UKJJz3unh6DgSKVpasoVcMmBmN4pyX4Y6yV-Eknt8--h2t6hR8nh4iw3wBVQWAU_sHiQ
admin.token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMjNDNTk1NzVCOUNGNDY3Qzk2MjA3NjBFQjI1NUIzODkiLCJyb2xlIjoiNDJEMEVFQjFDNjZGNDk3QTkwREQ1MjZEQzU5N0U2RjAiLCJvcmdhbml6YXRpb24iOiI3QkFCQTVGRjgwNDk0Q0FGQTU0REVCRDIyRUM0NkYwMSIsIndhcmVob3VzZSI6IjlDRjk4QTE4QkM3NTRCOTk5OThFNDIxRjkxQzVGRTEyIiwiaWF0IjoxNzczMDk5Mzg5fQ.QPNzs1Xr3xKYJmQz3WjOwyYIdyfkYzv3FJ4PrES2U3-j3PabIaN4SwHQ-5b2-lFAOQ5zV9-bzYjvDDykWxcWew
private-key=-----BEGIN PRIVATE KEY-----MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQghw7e6vE9io6Y6HTHgHD6RZ7o74ZRHmfVpQIEVE4qo6ihRANCAARi7S13nL4F2GSFm4KhRiEV/TP/6K23be2/ZLJmh1tihweopvKjMTGnwPOP2n5xgSQo68LJP2FzwUzZ2kQNrTEh-----END PRIVATE KEY-----
public-key=MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEYu0td5y+BdhkhZuCoUYhFf0z/+itt23tv2SyZodbYocHqKbyozExp8Dzj9p+cYEkKOvCyT9hc8FM2dpEDa0xIQ==
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Remove the hardcoded private key. Use a reference to an externalized secret or an environment variable.

private-key=${PRIVATE_KEY:}

Rationale: Committing a private key is a critical security violation. Private keys must be kept secret and never stored in version control. If this key has been used in any environment, it must be rotated immediately.

protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
FilterChain filterChain) throws ServletException, IOException {
if (authDisabled) {
userContext.setClientId("0");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Restrict the authentication bypass logic to development environments only. Consider checking the active Spring profile (e.g., 'dev') and logging a high-severity warning when authentication is disabled.

if (authDisabled && isDevelopmentProfile()) { // where isDevelopmentProfile() checks for 'dev' profile

Rationale: Bypassing authentication entirely via a configuration flag is a major security risk. If accidentally enabled in a production environment, it allows unauthorized access with System Administrator privileges. Guarding this with environment-specific checks (like Spring profiles) is a standard security practice.

FilterChain filterChain) throws ServletException, IOException {
if (authDisabled) {
userContext.setClientId("0");
userContext.setOrganizationId("0");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Verify the scope of the 'userContext' bean. If it is not request-scoped, avoid modifying it directly. Instead, create a new instance or use a thread-local approach to ensure isolation between concurrent requests.

// Ensure userContext is request-scoped or local to the method

Rationale: Spring filters are singletons by default. If 'userContext' is an injected singleton bean or a field in the class, modifying its state directly within the filter method is not thread-safe. Concurrent requests will overwrite each other's client, organization, and user information, leading to data inconsistency and race conditions.

bootRun {
debugOptions {
enabled = Boolean.parseBoolean((findProperty('debugEnabled') ?: enabled.get()) as String)
suspend = Boolean.parseBoolean((findProperty('debugSuspend') ?: suspend.get()) as String)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Use Groovy's idiomatic property handling to simplify the conversions.

enabled = findProperty('debugEnabled')?.toString()?.toBoolean() ?: enabled.get()

Rationale: The current implementation performs redundant conversions (Casting to String and then using Boolean.parseBoolean). Groovy's toBoolean() on String combined with safe navigation is more idiomatic and readable.

suspend = Boolean.parseBoolean((findProperty('debugSuspend') ?: suspend.get()) as String)
server = Boolean.parseBoolean((findProperty('debugServer') ?: server.get()) as String)
port = Integer.valueOf((findProperty('debugPort') ?: port.get()) as String)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Use toInteger() for a more concise and idiomatic conversion.

port = findProperty('debugPort')?.toString()?.toInteger() ?: port.get()

Rationale: Using toInteger() on a String is more idiomatic in Groovy scripts than using Integer.valueOf() with explicit casting.

token:eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMjNDNTk1NzVCOUNGNDY3Qzk2MjA3NjBFQjI1NUIzODkiLCJyb2xlIjoiNDJEMEVFQjFDNjZGNDk3QTkwREQ1MjZEQzU5N0U2RjAiLCJvcmdhbml6YXRpb24iOiIwIiwid2FyZWhvdXNlIjoiNEQ0NUZFNEM1MTUwNDE3MDkwNDdGNTFEMTM5QTIxQUMiLCJpYXQiOjE3NDQ4OTYxOTB9.A3tNu2NZc7u-zXhSSbp0_E-S1qX0Z1rZALoCe5c1E1g4ahXNisk6K9w8-mbdb1ZjJmHx0OXkbRFxC-GMQnBEog
admin.token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMjNDNTk1NzVCOUNGNDY3Qzk2MjA3NjBFQjI1NUIzODkiLCJyb2xlIjoiNDJEMEVFQjFDNjZGNDk3QTkwREQ1MjZEQzU5N0U2RjAiLCJvcmdhbml6YXRpb24iOiI3QkFCQTVGRjgwNDk0Q0FGQTU0REVCRDIyRUM0NkYwMSIsIndhcmVob3VzZSI6IjlDRjk4QTE4QkM3NTRCOTk5OThFNDIxRjkxQzVGRTEyIiwiaWF0IjoxNzUxNjM1ODEzfQ.hYMT9R-s2Ot6qa1RxF8kzaudAHwthCN-m3OVuTVmNaHOBpBvwOzD0z4LfkOAGwZRvs09PP7en266koNMSJGi_Q
token:eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMCIsInJvbGUiOiIwIiwib3JnYW5pemF0aW9uIjoiMCIsIndhcmVob3VzZSI6IjRENDVGRTRDNTE1MDQxNzA5MDQ3RjUxRDEzOUEyMUFDIiwiaWF0IjoxNzczMTY5MjMzfQ.foUEPnfhkxstq84ndRAQeCqWBdnJ0wvQ_kyzP0-VJNMxYKiDvSMm8jpiflemzU96dOXb3TmgcBdoUEfakTWElQ
admin.token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMzkzNjNCMDkyMUJCNDI5M0I0ODM4Mzg0NDMyNUU4NEMiLCJyb2xlIjoiRTcxN0Y5MDJDNDRDNDU1NzkzNDYzNDUwNDk1RkYzNkIiLCJvcmdhbml6YXRpb24iOiIwIiwid2FyZWhvdXNlIjoiNUE2MUYzOEQ1N0YxNDdBM0EyOEU0QzYwNkI2NkRDNDciLCJpYXQiOjE3NzMxNjkzNDV9.MkSsjo-2sXFzO20EeIOogyqc_kr4_VmZ5rZN1b0Xdzx64CYHPb6yvznBblpNwrKsfIcFbFjESLHEiIZiwO2k8Q
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using the equals sign '=' as a separator for all properties in the file to maintain consistency.

token=eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzd3MiLCJhdWQiOiJzd3MiLCJ1c2VyIjoiMTAwIiwiY2xpZW50IjoiMCIsInJvbGUiOiIwIiwib3JnYW5pemF0aW9uIjoiMCIsIndhcmVob3VzZSI6IjRENDVGRTRDNTE1MDQxNzA5MDQ3RjUxRDEzOUEyMUFDIiwiaWF0IjoxNzczMTY5MjMzfQ.foUEPnfhkxstq84ndRAQeCqWBdnJ0wvQ_kyzP0-VJNMxYKiDvSMm8jpiflemzU96dOXb3TmgcBdoUEfakTWElQ

Rationale: The file currently mixes ':' and '=' as property separators (e.g., 'token:' vs 'admin.token='). While both are valid in .properties files, consistency improves maintainability and follows standard Java/Spring Boot conventions.


# Use local config files instead of Git when running in this workspace.
spring.profiles.active=native
spring.cloud.config.server.native.searchLocations=file:../../rxconfig
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Avoid hardcoding the active profile in the main configuration file of a core module. Consider using a profile-specific file (e.g., application-local.properties) or allowing it to be set via environment variables.

# spring.profiles.active=native

Rationale: Hardcoding spring.profiles.active=native in the base application.properties of a core module changes the default behavior for all deployments. While useful for local development, it forces users to explicitly override the profile in environments where a different configuration backend (like Git) is intended. It is generally better to set the active profile via environment variables or a local-only configuration file.


if (restMethod == null) {
log.error("[ addFilters ] restMethod is null — UserContext.restMethod was not set. "
+ "Check FilterContext auth-disabled block or token parsing. sql={}", sql);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Remove the SQL query from the error log message. If logging the query is necessary for debugging, it should be done at the TRACE or DEBUG level and ensuring no sensitive data is leaked.

      log.error("[ addFilters ] restMethod is null — UserContext.restMethod was not set. "
          + "Check FilterContext auth-disabled block or token parsing.");

Rationale: Logging raw SQL queries in error messages is a security risk as it can expose internal database structure and potentially sensitive data. In Etendo, internal system details should not be exposed in error logs at the ERROR level.

log.error("[ addFilters ] restMethod is null — UserContext.restMethod was not set. "
+ "Check FilterContext auth-disabled block or token parsing. sql={}", sql);
throw new IllegalArgumentException(
"restMethod is null in UserContext. Ensure FilterContext sets restMethod on the request.");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Replace 'IllegalArgumentException' with 'OBException'.

      throw new OBException(
          "restMethod is null in UserContext. Ensure FilterContext sets restMethod on the request.");

Rationale: Etendo framework standards require using OBException for error propagation within framework utilities. This ensures consistent exception handling, localization support, and logging throughout the Etendo ecosystem (including EtendoRX services).


# Disable auth for local development
auth.disabled=true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Warning

⚠️ Blocking Issue

Remove 'auth.disabled=true' from the committed properties file. If needed for local development, developers should define it in their local 'rxconfig' or via environment variables that are not tracked by Git.

# auth.disabled should be set via environment variables or a local non-committed file if needed.

Rationale: Disabling authentication is a security vulnerability. Committing this setting to the repository ensures it is included in builds and can be accidentally activated in non-local environments if the 'local' profile is triggered.

spring.datasource.url=jdbc:postgresql://localhost:5432/etendo
spring.datasource.username=tad
spring.datasource.password=tad
spring.jackson.serialization.FAIL_ON_EMPTY_BEANS=false
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using a placeholder for the password to allow customization via environment variables while keeping a default value for development.

spring.datasource.password=${DB_PASSWORD:tad}

Rationale: Hardcoding passwords in property files is a security risk. Using placeholders allows for local overrides without modifying the file.

@@ -0,0 +1,13 @@
server.port=8092
spring.datasource.url=jdbc:postgresql://localhost:5432/etendo
spring.datasource.username=tad
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using a placeholder for the database URL to allow local environment overrides.

spring.datasource.url=${DB_URL:jdbc:postgresql://localhost:5432/etendo}

Rationale: Hardcoding the connection URL makes the module less flexible for different local environments (e.g., using Docker or different DB names).

obconnector.url=http://localhost:8101
subapp.url=http://localhost:3000
classic.context.name=etendo_conn
etendorx.auth.url=http://localhost:8094 No newline at end of file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Verify if 'etendo_conn' is the intended default context name for the local environment.

classic.context.name=etendo

Rationale: The standard context path for Etendo Classic instances is typically '/etendo'. Using 'etendo_conn' as a default context name might lead to routing failures in standard local development environments unless the user has a specific setup.

@@ -0,0 +1,4 @@
obconnector.url=http://localhost:8101
subapp.url=http://localhost:3000
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider if this file should be committed to the repository or if it should be handled as a local override template.

# Move to application.yaml or exclude from Git

Rationale: Files named 'application-local.properties' are typically intended for personal developer overrides and are often excluded from version control via .gitignore. Committing this file to a core module resources folder bundles it into the final artifact. If these are meant to be shared defaults, 'application.yaml' or a '.template' file might be more appropriate.

document.querySelectorAll('.dot[data-port]').forEach(dot => {
const port = dot.dataset.port;
fetch(`http://localhost:${port}/`, { mode: 'no-cors', signal: AbortSignal.timeout(2000) })
.then(() => { dot.className = 'dot up'; })
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider using AbortController with setTimeout for the health check timeout to ensure better compatibility across a wider range of browser versions.

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 2000);
fetch(`http://localhost:${port}/`, { mode: 'no-cors', signal: controller.signal })
  .then(() => { 
    clearTimeout(timeoutId);
    dot.className = 'dot up'; 
  })
  .catch(() => { dot.className = 'dot unknown'; });

Rationale: AbortSignal.timeout is a relatively new API (Chrome 103+, Safari 16+, Node.js 17.3+). While likely supported in modern development environments, using AbortController with setTimeout provides broader compatibility for developers who might be using slightly older browser versions.

<p>Click a service on the left to open it here.</p>
</div>
<iframe id="frame" style="display:none"></iframe>
</main>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Be aware that some services might block being loaded in an iframe. Consider adding a fallback mechanism or an "Open in new tab" button for services that fail to load due to security headers.

<iframe id="frame" style="display:none"></iframe>
<!-- Consider adding a fallback 'Open in new tab' link if a service blocks iframe loading -->

Rationale: Many modern web services and tools (like Kafka Connect, Jaeger, or Spring Boot Actuator) may have security headers such as X-Frame-Options: DENY or Content-Security-Policy: frame-ancestors 'none'. These headers prevent the application from being loaded inside an iframe, which would break the core functionality of this portal.

uri: ${classic.url}
predicates:
- Method=GET, POST, DELETE, HEAD, PATCH
- Path=/${classic.context.name}/**
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Add the 'PUT' method to the 'classic_path_route' predicates.

            - Method=GET, PUT, POST, DELETE, HEAD, PATCH

Rationale: Etendo Classic and its REST services may require the PUT method for update operations. Adding it ensures compatibility with all classic features routed through the Edge.

- Path=/das/**
filters:
#- JwtAuthenticationFilter
- RewritePath=/das/(?<segment>.*), /$\{segment}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Consider enabling JwtAuthenticationFilter by default or adding a warning comment that security is disabled for this route.

            - JwtAuthenticationFilter

Rationale: Commenting out the JwtAuthenticationFilter disables security for the DAS and Subapp routes. While this might be intended for local development, it is safer to keep it enabled in templates to prevent accidental production deployments without authentication.

uri: ${obconnector.url}
predicates:
- Method=GET, POST, PUT, DELETE
- Path=/api/sync/**
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion

Include 'PATCH' and 'HEAD' methods in the 'obconnector' route predicates.

            - Method=GET, POST, PUT, DELETE, PATCH, HEAD

Rationale: API Connectors and modern RESTful services often utilize the PATCH method for partial updates and HEAD for existence checks. Adding these methods improves the robustness of the connector route.

Copy link
Copy Markdown
Collaborator

@etendobot etendobot left a comment

Choose a reason for hiding this comment

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

Changes requested by agent. Please resolve blocking issues.

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