Ignore user config provider aliases with conflict #11486
Conversation
Signed-off-by: MichaelMorris <michael.morris@est.tech>
There was a problem hiding this comment.
Hi @MichaelMorrisEst, sorry for the late review, and thanks for working on this. I left one comment for you to consider.
| LOGGER.warnCr(reconciliation, "config.provider " + alias + " ignored as it is not permitted. Not permitted aliases: " + strimziAliases); | ||
| userConfig.removeConfigOption("config.providers." + alias + ".class"); |
There was a problem hiding this comment.
A simple warning log can be easily missed. Also, the operator should never change the user-provided configuration, even if it's wrong or conflicting.
As suggested in the bug report, we can raise an InvalidConfigurationException, and let the reconciliation fail. With that, the error would be logged and surfaced in the Kafka resource status. Wdyt?
There was a problem hiding this comment.
Not sure if it is that simple, as we need to keep backwards compatibility in mind as well.
So what do we do today? IIRC, we just overwrite it? In which case warning while overwritting it is likely much better than failing the whole reconciliation.
There was a problem hiding this comment.
We don't overwrite. We keep both configurations and the user-provided one wins because it is applied last. Obviously, this make the operator fails if it is not compatible with what it expects (see the issue report). For this reason, I don't think there is a backwards compatibility issue raising an config exception. Of course, we can leave a release note about this change.
There was a problem hiding this comment.
Ok, I thought we just overwrote it. But if it is as you say, throwing the exception is good. Thanks for the clarification! 👍
Signed-off-by: MichaelMorris <michael.morris@est.tech>
Type of change
Description
Closes #11349. Handles the scenario where the user specifies config providers than conflict with the Strimzi defined config providers. When the situation arises, a warning is generated and the conflicting user config providers are ignored (as agreed at community call 29th May)
Checklist
Please go through this checklist and make sure all applicable tasks have been done