-
Notifications
You must be signed in to change notification settings - Fork 236
feat: configuration adapters #3177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Changes from all commits
9651b52
fe1be63
252ecd4
390172c
a7a7161
90bfe0c
16480cb
1f96fc5
27bc53c
8f5279a
8489013
94aba48
baf1201
1640d8a
9a39eda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.api.config.loader; | ||
|
|
||
| import java.util.function.BiConsumer; | ||
|
|
||
| /** | ||
| * Associates a configuration key and its expected type with the setter that should be called on an | ||
| * overrider when the {@link ConfigProvider} returns a value for that key. | ||
| * | ||
| * @param <O> the overrider type (e.g. {@code ConfigurationServiceOverrider}) | ||
| * @param <T> the value type expected for this key | ||
| */ | ||
| public class ConfigBinding<O, T> { | ||
|
|
||
| private final String key; | ||
| private final Class<T> type; | ||
| private final BiConsumer<O, T> setter; | ||
|
|
||
| public ConfigBinding(String key, Class<T> type, BiConsumer<O, T> setter) { | ||
| this.key = key; | ||
| this.type = type; | ||
| this.setter = setter; | ||
| } | ||
|
|
||
| public String key() { | ||
| return key; | ||
| } | ||
|
|
||
| public Class<T> type() { | ||
| return type; | ||
| } | ||
|
|
||
| public BiConsumer<O, T> setter() { | ||
| return setter; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,250 @@ | ||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||
| * Copyright Java Operator SDK Authors | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||||||||||||||||||||||
| * you may not use this file except in compliance with the License. | ||||||||||||||||||||||||||
| * You may obtain a copy of the License at | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||||||||||||||||||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||||||||||||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||||||||||||||||
| * See the License for the specific language governing permissions and | ||||||||||||||||||||||||||
| * limitations under the License. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| package io.javaoperatorsdk.operator.api.config.loader; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import java.time.Duration; | ||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||
| import java.util.function.Consumer; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import io.fabric8.kubernetes.api.model.HasMetadata; | ||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; | ||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; | ||||||||||||||||||||||||||
| import io.javaoperatorsdk.operator.processing.retry.GenericRetry; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public class ConfigLoader { | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private static final ConfigLoader DEFAULT = new ConfigLoader(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public static ConfigLoader getDefault() { | ||||||||||||||||||||||||||
| return DEFAULT; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public static final String DEFAULT_OPERATOR_KEY_PREFIX = "josdk."; | ||||||||||||||||||||||||||
| public static final String DEFAULT_CONTROLLER_KEY_PREFIX = "josdk.controller."; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Key prefix for controller-level properties. The controller name is inserted between this prefix | ||||||||||||||||||||||||||
| * and the property name, e.g. {@code josdk.controller.my-controller.finalizer}. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| private final String controllerKeyPrefix; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private final String operatorKeyPrefix; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| // Operator-level (ConfigurationServiceOverrider) bindings | ||||||||||||||||||||||||||
| // Only scalar / value types that a key-value ConfigProvider can supply are | ||||||||||||||||||||||||||
| // included. Complex objects (KubernetesClient, ExecutorService, …) must be | ||||||||||||||||||||||||||
| // configured programmatically and are intentionally omitted. | ||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| static final List<ConfigBinding<ConfigurationServiceOverrider, ?>> OPERATOR_BINDINGS = | ||||||||||||||||||||||||||
| List.of( | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "check-crd", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::checkingCRDAndValidateLocalModel), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "reconciliation.termination-timeout", | ||||||||||||||||||||||||||
| Duration.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withReconciliationTerminationTimeout), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "reconciliation.concurrent-threads", | ||||||||||||||||||||||||||
| Integer.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withConcurrentReconciliationThreads), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "workflow.executor-threads", | ||||||||||||||||||||||||||
| Integer.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withConcurrentWorkflowExecutorThreads), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "close-client-on-stop", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withCloseClientOnStop), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "informer.stop-on-error-during-startup", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withStopOnInformerErrorDuringStartup), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "informer.cache-sync-timeout", | ||||||||||||||||||||||||||
| Duration.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withCacheSyncTimeout), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "dependent-resources.ssa-based-create-update-match", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withSSABasedCreateUpdateMatchForDependentResources), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "use-ssa-to-patch-primary-resource", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withUseSSAToPatchPrimaryResource), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "clone-secondary-resources-when-getting-from-cache", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ConfigurationServiceOverrider::withCloneSecondaryResourcesWhenGettingFromCache)); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| // Controller-level retry property suffixes | ||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| static final String RETRY_MAX_ATTEMPTS_SUFFIX = "retry.max-attempts"; | ||||||||||||||||||||||||||
| static final String RETRY_INITIAL_INTERVAL_SUFFIX = "retry.initial-interval"; | ||||||||||||||||||||||||||
| static final String RETRY_INTERVAL_MULTIPLIER_SUFFIX = "retry.interval-multiplier"; | ||||||||||||||||||||||||||
| static final String RETRY_MAX_INTERVAL_SUFFIX = "retry.max-interval"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| // Controller-level (ControllerConfigurationOverrider) bindings | ||||||||||||||||||||||||||
| // The key used at runtime is built as: | ||||||||||||||||||||||||||
| // CONTROLLER_KEY_PREFIX + controllerName + "." + <suffix> | ||||||||||||||||||||||||||
| // --------------------------------------------------------------------------- | ||||||||||||||||||||||||||
| static final List<ConfigBinding<ControllerConfigurationOverrider<?>, ?>> CONTROLLER_BINDINGS = | ||||||||||||||||||||||||||
| List.of( | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "finalizer", String.class, ControllerConfigurationOverrider::withFinalizer), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "generation-aware", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ControllerConfigurationOverrider::withGenerationAware), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "label-selector", String.class, ControllerConfigurationOverrider::withLabelSelector), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "max-reconciliation-interval", | ||||||||||||||||||||||||||
| Duration.class, | ||||||||||||||||||||||||||
| ControllerConfigurationOverrider::withReconciliationMaxInterval), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "field-manager", String.class, ControllerConfigurationOverrider::withFieldManager), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "trigger-reconciler-on-all-events", | ||||||||||||||||||||||||||
| Boolean.class, | ||||||||||||||||||||||||||
| ControllerConfigurationOverrider::withTriggerReconcilerOnAllEvents), | ||||||||||||||||||||||||||
| new ConfigBinding<>( | ||||||||||||||||||||||||||
| "informer-list-limit", | ||||||||||||||||||||||||||
| Long.class, | ||||||||||||||||||||||||||
| ControllerConfigurationOverrider::withInformerListLimit)); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| private final ConfigProvider configProvider; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ConfigLoader() { | ||||||||||||||||||||||||||
| this(new DefaultConfigProvider(), DEFAULT_CONTROLLER_KEY_PREFIX, DEFAULT_OPERATOR_KEY_PREFIX); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ConfigLoader(ConfigProvider configProvider) { | ||||||||||||||||||||||||||
| this(configProvider, DEFAULT_CONTROLLER_KEY_PREFIX, DEFAULT_OPERATOR_KEY_PREFIX); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public ConfigLoader( | ||||||||||||||||||||||||||
| ConfigProvider configProvider, String controllerKeyPrefix, String operatorKeyPrefix) { | ||||||||||||||||||||||||||
| this.configProvider = configProvider; | ||||||||||||||||||||||||||
| this.controllerKeyPrefix = controllerKeyPrefix; | ||||||||||||||||||||||||||
| this.operatorKeyPrefix = operatorKeyPrefix; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Returns a {@link Consumer} that applies every operator-level property found in the {@link | ||||||||||||||||||||||||||
| * ConfigProvider} to the given {@link ConfigurationServiceOverrider}. Returns no-op consumer when | ||||||||||||||||||||||||||
| * no binding has a matching value, preserving the previous behavior. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
|
Comment on lines
151
to
155
|
||||||||||||||||||||||||||
| public Consumer<ConfigurationServiceOverrider> applyConfigs() { | ||||||||||||||||||||||||||
| return buildConsumer(OPERATOR_BINDINGS, operatorKeyPrefix); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Returns a {@link Consumer} that applies every controller-level property found in the {@link | ||||||||||||||||||||||||||
| * ConfigProvider} to the given {@link ControllerConfigurationOverrider}. The keys are looked up | ||||||||||||||||||||||||||
| * as {@code josdk.controller.<controllerName>.<property>}. Returns {@code null} when no binding | ||||||||||||||||||||||||||
| * has a matching value. | ||||||||||||||||||||||||||
|
Comment on lines
+163
to
+164
|
||||||||||||||||||||||||||
| * as {@code josdk.controller.<controllerName>.<property>}. Returns {@code null} when no binding | |
| * has a matching value. | |
| * as {@code josdk.controller.<controllerName>.<property>}. This method never returns | |
| * {@code null}; if no binding has a matching value, it returns a no-op consumer. |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JavaDoc states that this method "Returns {@code null} when no binding has a matching value", but the implementation always returns a non-null Consumer. When no bindings match, buildConsumer returns a no-op consumer (o -> {}), not null. The JavaDoc should be updated to reflect that this method always returns a non-null Consumer, which may be a no-op if no configuration values are found.
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method does not validate that controllerName is not null. If null is passed, a NullPointerException will be thrown at line 169 when concatenating strings. Consider adding a null check with an appropriate exception message, such as: Objects.requireNonNull(controllerName, "controllerName must not be null").
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unchecked double-cast here is a maintenance hazard and the comment mentions covariance, but BiConsumer is invariant in its type parameters. A safer approach is to adjust ConfigBinding to accept BiConsumer<? super O, ? super T> and/or define CONTROLLER_BINDINGS with a type that doesn’t require casting (e.g., keep it raw/erased in one place and only cast the final Consumer once), minimizing the scope of unchecked operations.
| // Cast is safe: the setter BiConsumer<ControllerConfigurationOverrider<?>, T> is covariant in | |
| // its first parameter for our usage – we only ever call it with | |
| // ControllerConfigurationOverrider<R>. | |
| List<ConfigBinding<ControllerConfigurationOverrider<R>, ?>> bindings = | |
| (List<ConfigBinding<ControllerConfigurationOverrider<R>, ?>>) (List<?>) CONTROLLER_BINDINGS; | |
| Consumer<ControllerConfigurationOverrider<R>> consumer = buildConsumer(bindings, prefix); | |
| // Build the consumer from the shared controller bindings using a raw list to avoid | |
| // propagating unchecked generic casts. We then narrow it in a single place below. | |
| Consumer<?> rawConsumer = buildConsumer((List) CONTROLLER_BINDINGS, prefix); | |
| Consumer<ControllerConfigurationOverrider<R>> consumer = | |
| (Consumer<ControllerConfigurationOverrider<R>>) rawConsumer; |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retry override behavior is new but isn’t covered by tests yet. Add unit tests that set one or more retry keys and assert that the returned controller consumer calls withRetry(...) and that the resulting GenericRetry instance reflects only the configured overrides (including Double parsing for retry.interval-multiplier).
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retry configuration logic (buildRetryConsumer method) lacks test coverage. Consider adding tests that verify retry properties (retry.max-attempts, retry.initial-interval, retry.interval-multiplier, retry.max-interval) are correctly loaded from the ConfigProvider and applied to the ControllerConfigurationOverrider. This would ensure the retry configuration works as expected and prevent regressions.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * Copyright Java Operator SDK Authors | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.javaoperatorsdk.operator.api.config.loader; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface ConfigProvider { | ||
|
|
||
| /** | ||
| * Returns the value associated with {@code key}, converted to {@code type}, or an empty {@link | ||
| * Optional} if the key is not set. | ||
| * | ||
| * @param key the dot-separated configuration key, e.g. {@code josdk.cache.sync.timeout} | ||
| * @param type the expected type of the value; supported types depend on the implementation | ||
| * @param <T> the value type | ||
| * @return an {@link Optional} containing the typed value, or empty if the key is absent | ||
| * @throws IllegalArgumentException if {@code type} is not supported by the implementation | ||
| */ | ||
| <T> Optional<T> getValue(String key, Class<T> type); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The retry-related keys don’t document units/semantics for interval values (e.g., are the
Longvalues milliseconds? seconds?). Add Javadoc near these suffix constants (or onbuildRetryConsumer) that clearly states expected units and acceptable ranges; alternatively consider usingDurationfor interval properties (and converting to whateverGenericRetryexpects) for consistency with other duration configs.