davsclaus commented on code in PR #27409:
URL: https://github.com/apache/camel/pull/27409#discussion_r4206866082
##########
core/camel-main/src/main/java/org/apache/camel/main/BaseMainSupport.java:
##########
@@ -3020,7 +3020,7 @@ protected void autoConfigurationFromProperties(
// load properties from ENV (override existing)
if
(mainConfigurationProperties.isAutoConfigurationEnvironmentVariablesEnabled()) {
Map<String, String> env = MainHelper
- .filterEnvVariables(new String[] { "camel.component.",
"camel.dataformat.", "camel.language." });
+ .filterEnvVariables(new String[] { "CAMEL_COMPONENT_",
"CAMEL_DATAFORMAT_", "CAMEL_LANGUAGE_" });
Review Comment:
This line is the actual bug 1 fix, and nothing tests it yet. Reverting it
back to `"camel.component."` keeps every test green, because `MainHelperTest`
calls `filterEnvVariables(new String[] { "CAMEL_COMPONENT_" })` itself. An
end-to-end test that sets `CAMEL_COMPONENT_SEDA_QUEUE_SIZE` the way
`MainPropertyPlaceholderWithEnvTest` sets its ENV, starts `Main`, and asserts
the seda component's `queueSize` would stop this from regressing silently
again, as it did after CAMEL-16345.
##########
core/camel-main/src/main/java/org/apache/camel/main/MainHelper.java:
##########
@@ -163,17 +164,20 @@ public void addComponentEnvVariables(Map<String, String>
env, Properties propert
Set<String> toRemove = new HashSet<>();
env.forEach((k, v) -> {
if (custom) {
+ if (!k.startsWith("CAMEL_COMPONENT_")) {
Review Comment:
The prefix guard fixes the `CAMEL_LANGUAGE_FOO_BAR` →
`camel.component.oo.bar` problem, but no test covers it. A `MainHelperTest`
case that puts a leftover `CAMEL_LANGUAGE_FOO_BAR` and
`CAMEL_DATAFORMAT_FOO_BAR` in `env`, runs the three custom passes, and asserts
`camel.language.foo.bar` / `camel.dataformat.foo.bar` (and no
`camel.component.*`) would guard it. The same applies to the language branch
keeping camelCase (`CAMEL_LANGUAGE_EXCHANGEPROPERTY_TRIM` →
`camel.language.exchangeProperty.trim`), which currently has no test.
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -3700,6 +3700,48 @@ When `camel.opentelemetry.*` properties are configured
but OpenTelemetry is disa
the `camel.opentelemetry2.*` or `camel.telemetryDev.*` properties are now
used. Previously they were ignored whenever
any `camel.opentelemetry.*` property was configured.
+=== camel-main - `CAMEL_COMPONENT_*`, `CAMEL_DATAFORMAT_*`, `CAMEL_LANGUAGE_*`
environment variables now take effect
+
+Two bugs in `camel-main`'s environment-variable processing were fixed and are
now active.
+
+**Bug 1 — variables were silently ignored since Camel 3.9 (CAMEL-16345)**
+
+`filterEnvVariables` uppercases the variable names before comparing them
against the supplied prefixes, so the prefixes
+themselves must be uppercase. Before this fix the code passed lowercase
prefixes (`camel.component.`, etc.), causing
+every match to fail silently. This has been corrected: `CAMEL_COMPONENT_*`,
`CAMEL_DATAFORMAT_*` and `CAMEL_LANGUAGE_*`
+variables are now actually read.
+
+If your deployments intentionally set such variables but relied on them being
ignored (for example to pass secrets that
+should not reach the component), use
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false` to opt out, or
remove the variables.
+
+Variables that refer to a name not present on the classpath (for example
`CAMEL_COMPONENT_KAFKA_BROKERS` in an
+application that does not include camel-kafka) will fail the startup with an
`IllegalArgumentException`. There is no
+per-variable opt-out: rename the variable, remove it, or set
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`.
+
+Secrets passed as component environment variables (for example
`CAMEL_COMPONENT_AWS2_S3_SECRET_KEY`) will be seen by
+the `prod`-profile security check as plain-text secrets and will fail the
startup. The
+`camel.component.aws2-s3.secret-key={{env:CAMEL_COMPONENT_AWS2_S3_SECRET_KEY}}`
placeholder form does not help —
+`filterEnvVariables` still picks up the original variable and binds its value
directly. To pass a secret without
+triggering the security check, either rename the environment variable to a
name that does not match the
+`CAMEL_COMPONENT_*` pattern, configure the value through a vault, or disable
auto-configuration entirely with
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`.
Review Comment:
Renaming alone leaves the option unset: the renamed variable also has to be
referenced with a placeholder.
```suggestion
triggering the security check, either rename the environment variable to a
name that does not match the
`CAMEL_COMPONENT_*` pattern and reference it with a placeholder (for example
`camel.component.aws2-s3.secret-key={{env:MY_AWS_SECRET_KEY}}`), configure
the value through a vault, or disable
auto-configuration from environment variables with
`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`
(which also turns off the `CAMEL_MAIN_*` variables).
```
##########
core/camel-main/src/main/java/org/apache/camel/main/MainHelper.java:
##########
@@ -59,24 +60,24 @@ public final class MainHelper {
private final String version;
private final StopWatch stopWatch;
- private final Set<String> componentEnvNames = new HashSet<>();
- private final Set<String> dataformatEnvNames = new HashSet<>();
- private final Set<String> languageEnvNames = new HashSet<>();
+ private final Map<String, String> componentEnvNames = new HashMap<>();
+ private final Map<String, String> dataformatEnvNames = new HashMap<>();
+ private final Map<String, String> languageEnvNames = new HashMap<>();
public MainHelper() {
stopWatch = new StopWatch();
try {
InputStream is =
MainHelper.class.getResourceAsStream("/org/apache/camel/main/components.properties");
- loadLines(is, componentEnvNames, s -> "CAMEL_COMPONENT_" +
s.toUpperCase(Locale.US).replace('-', '_'));
+ loadLines(is, componentEnvNames, s -> "CAMEL_COMPONENT_" +
s.toUpperCase(Locale.US).replace('-', '_'), s -> s);
Review Comment:
Nit: `valueFunc` is `s -> s` at all three call sites, so `loadLines` could
take only the key function and do `map.put(keyFunc.apply(line), line)`.
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -3700,6 +3700,48 @@ When `camel.opentelemetry.*` properties are configured
but OpenTelemetry is disa
the `camel.opentelemetry2.*` or `camel.telemetryDev.*` properties are now
used. Previously they were ignored whenever
any `camel.opentelemetry.*` property was configured.
+=== camel-main - `CAMEL_COMPONENT_*`, `CAMEL_DATAFORMAT_*`, `CAMEL_LANGUAGE_*`
environment variables now take effect
+
+Two bugs in `camel-main`'s environment-variable processing were fixed and are
now active.
+
+**Bug 1 — variables were silently ignored since Camel 3.9 (CAMEL-16345)**
+
+`filterEnvVariables` uppercases the variable names before comparing them
against the supplied prefixes, so the prefixes
+themselves must be uppercase. Before this fix the code passed lowercase
prefixes (`camel.component.`, etc.), causing
+every match to fail silently. This has been corrected: `CAMEL_COMPONENT_*`,
`CAMEL_DATAFORMAT_*` and `CAMEL_LANGUAGE_*`
+variables are now actually read.
+
+If your deployments intentionally set such variables but relied on them being
ignored (for example to pass secrets that
+should not reach the component), use
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false` to opt out, or
remove the variables.
+
+Variables that refer to a name not present on the classpath (for example
`CAMEL_COMPONENT_KAFKA_BROKERS` in an
+application that does not include camel-kafka) will fail the startup with an
`IllegalArgumentException`. There is no
+per-variable opt-out: rename the variable, remove it, or set
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`.
+
+Secrets passed as component environment variables (for example
`CAMEL_COMPONENT_AWS2_S3_SECRET_KEY`) will be seen by
+the `prod`-profile security check as plain-text secrets and will fail the
startup. The
+`camel.component.aws2-s3.secret-key={{env:CAMEL_COMPONENT_AWS2_S3_SECRET_KEY}}`
placeholder form does not help —
+`filterEnvVariables` still picks up the original variable and binds its value
directly. To pass a secret without
+triggering the security check, either rename the environment variable to a
name that does not match the
+`CAMEL_COMPONENT_*` pattern, configure the value through a vault, or disable
auto-configuration entirely with
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`.
+
+**Bug 2 — ambiguous name resolution**
+
+When the catalog contained component/dataformat/language pairs whose names
were prefixes of each other (such as `netty`
+/ `netty-http`, `js` / `jsonpath`, `avro` / `avroJackson`), the variable was
randomly mapped to either name.
+A longest-match strategy now always selects the most specific name.
+
+**Known limitation — `rest` / `rest-api`**
+
+`CAMEL_COMPONENT_REST_API_DOC=api.json` is matched to
`camel.component.rest-api.doc` (the longer match), not to
+`camel.component.rest.apiDoc`. Use `CAMEL_COMPONENT_REST_APIDOC=api.json`
instead (option names are matched
+case-insensitively). Note that using
`camel.component.rest.apiDoc={{env:CAMEL_COMPONENT_REST_API_DOC}}` does not work
+as a workaround — `filterEnvVariables` still matches the original variable and
binds it to `rest-api.doc` in addition
+to the placeholder.
Review Comment:
The same limitation applies to custom-named instances whose name starts with
a catalog name: with an extra kafka component registered as `kafka-dr`,
`CAMEL_COMPONENT_KAFKA_DR_BROKERS` matches `kafka` and becomes
`camel.component.kafka.dr-brokers`, an unknown option that fails the startup.
It is worth a sentence here. Configuring such instances through
`application.properties`
(`camel.component.kafka-dr.brokers={{env:KAFKA_DR_BROKERS}}`) is the workaround.
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -3700,6 +3700,48 @@ When `camel.opentelemetry.*` properties are configured
but OpenTelemetry is disa
the `camel.opentelemetry2.*` or `camel.telemetryDev.*` properties are now
used. Previously they were ignored whenever
any `camel.opentelemetry.*` property was configured.
+=== camel-main - `CAMEL_COMPONENT_*`, `CAMEL_DATAFORMAT_*`, `CAMEL_LANGUAGE_*`
environment variables now take effect
+
+Two bugs in `camel-main`'s environment-variable processing were fixed and are
now active.
+
+**Bug 1 — variables were silently ignored since Camel 3.9 (CAMEL-16345)**
+
+`filterEnvVariables` uppercases the variable names before comparing them
against the supplied prefixes, so the prefixes
+themselves must be uppercase. Before this fix the code passed lowercase
prefixes (`camel.component.`, etc.), causing
+every match to fail silently. This has been corrected: `CAMEL_COMPONENT_*`,
`CAMEL_DATAFORMAT_*` and `CAMEL_LANGUAGE_*`
+variables are now actually read.
+
+If your deployments intentionally set such variables but relied on them being
ignored (for example to pass secrets that
+should not reach the component), use
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false` to opt out, or
remove the variables.
+
+Variables that refer to a name not present on the classpath (for example
`CAMEL_COMPONENT_KAFKA_BROKERS` in an
+application that does not include camel-kafka) will fail the startup with an
`IllegalArgumentException`. There is no
+per-variable opt-out: rename the variable, remove it, or set
+`camel.main.autoConfigurationEnvironmentVariablesEnabled=false`.
Review Comment:
Two gaps in this paragraph:
- `camel.main.autoConfigurationEnvironmentVariablesEnabled=false` is not a
targeted opt-out. It also turns off `CAMEL_MAIN_*` and the other ENV-based
configuration, which `BaseMainSupport` gates on the same flag. Users who
configure Camel through `CAMEL_MAIN_*` in containers need to know that before
they reach for it, here and in the two other places it is recommended.
- An **empty** variable fails too (`CAMEL_COMPONENT_SEDA_QUEUE_SIZE=` → *...
because value is empty* from `validateOptionAndValue`), and empty variables are
common (`X=${UNSET}` in compose, `value: ""` in a pod spec). It should be
listed next to the unknown-name case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
