fix: handling empty yaml config file with a comment only#3301
fix: handling empty yaml config file with a comment only#3301csviri wants to merge 6 commits intooperator-framework:mainfrom
Conversation
csviri
changed the title
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
csviri
force-pushed
the
improve-yaml-comment
branch
from
f929694 to
67a70eb
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves YamlConfigProvider behavior when the YAML config file is effectively empty (empty content or comment-only), and adds regression tests to cover those cases.
Changes:
- Add tests asserting empty/comment-only YAML files produce no config values.
- Adjust YAML loading to return an empty config map when Jackson reports empty input.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| operator-framework/src/test/java/io/javaoperatorsdk/operator/config/loader/provider/YamlConfigProviderTest.java | Adds coverage for empty and comment-only YAML config files. |
| operator-framework/src/main/java/io/javaoperatorsdk/operator/config/loader/provider/YamlConfigProvider.java | Adds handling for Jackson “no content” parsing cases by returning an empty map. |
| } catch (com.fasterxml.jackson.databind.exc.MismatchedInputException e) { | ||
| log.warn("{} contains no configuration data", path); | ||
| return Map.of(); |
There was a problem hiding this comment.
Catching MismatchedInputException broadly will also swallow real YAML/type errors (e.g., a top-level YAML sequence instead of a mapping) and treat them as "no configuration data", changing prior behavior where such cases failed fast. Please narrow this handling to the true empty/comment-only case (e.g., detect empty content via a JsonParser and nextToken()==null, or otherwise check for the specific "no content" condition) so malformed/non-map configs still surface as errors.
There was a problem hiding this comment.
I agree with this comment. You cannot be sure the file is empty.
There was a problem hiding this comment.
good point, improved this.
| } catch (com.fasterxml.jackson.databind.exc.MismatchedInputException e) { | ||
| log.warn("{} contains no configuration data", path); | ||
| return Map.of(); |
There was a problem hiding this comment.
I agree with this comment. You cannot be sure the file is empty.
…onfig/loader/provider/YamlConfigProviderTest.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…onfig/loader/provider/YamlConfigProvider.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Attila Mészáros a_meszaros@apple.com