Treat a missing k8s config scope as an empty one - #7452
Open
pditommaso wants to merge 1 commit into
Open
Conversation
A pipeline that selects the Kubernetes executor without declaring a `k8s` block aborts with `Cannot invoke "java.util.Map.get(Object)" because "opts" is null`. `K8sExecutor.getK8sConfig` passes `session.config.k8s` straight to `K8sConfig(Map)`, and an absent scope navigates to null, which the constructor dereferences on its first line. Every `k8s` option has a default or is legitimately unset, so a config with no `k8s` block is a valid configuration, not an error -- the no-arg constructor already states as much by delegating to an empty map, and `K8sDriverLauncher` guards the same call with an `instanceof Map` check. Only the executor path does not, which is why the failure needs no unusual setup to reproduce: an executor declaration and nothing else. Fixed in the constructor rather than at the call site, so the contract holds for every caller instead of being re-established at each one. Both specs fail with the NPE when the fix is reverted. Assisted-by: Claude Opus 5 (1M context) via Claude Code Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
✅ Deploy Preview for nextflow-docs canceled.
|
Member
|
The established pattern for config classes is to guard at the call site (see for example |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A pipeline that selects the Kubernetes executor without declaring a
k8sblock aborts before running anything:K8sExecutor.getK8sConfigpassessession.config.k8sstraight toK8sConfig(Map); an absent scope navigates tonull, and the constructor dereferences it on its first line.Why this is a bug, not a missing requirement
Every
k8soption either has a default or is legitimately unset, so a config with nok8sblock is a valid configuration. The codebase already says so in two places:K8sConfig()(no-arg) delegates tothis(Collections.emptyMap());K8sDriverLauncher:286guards the same construction withconfig.k8s instanceof Map ? new K8sConfig(config.k8s as Map) : new K8sConfig().The executor path is the one caller that does neither.
Fix
Tolerate the null in the constructor rather than at the call site, so the contract holds for every caller instead of being re-established at each one:
Tests
Two specs, one per layer:
K8sConfigTest—new K8sConfig(null)behaves as the no-arg form, and defaults that do not come from the map (clientRefreshInterval) still apply.K8sExecutorTest—getK8sConfig()with a session whose config has nok8skey, which is the reported path.Both fail with the original NPE when the fix is reverted. Full
nf-k8ssuite: 216 tests, 0 failures.🤖 Generated with Claude Code