Enable dynamic configuration of Metrics Reporter's allowlist

[im-konge]

Type of change

• Enhancement / new feature

Description

This PR adds prometheus.metrics.reporter.allowlist to the config-models, so we are able to change this configuration dynamically by configuring the allowList field inside the Kafka CR.
This configuration is forbidden in case that it is configured inside the spec.kafka.config, however we are able to do the dynamic update using the allowList - as the code goes through the configuration, checks if the particular config is inside config-model and if yes and it's not READ_ONLY, it performs the dynamic update.

So the only thing needed was to add this configuration to the config-models.
I tried also adding "custom-config-model" with special handling (commit d2c72f5), but FMPOV it was a lot of changes given the same result as updating the config-model-generator

Fixes #12400

Checklist

• Write tests
• Make sure all tests pass
• Update documentation
• Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.02%. Comparing base (1a0c0e1) to head (4f72050).

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #12576      +/-   ##
============================================
+ Coverage     75.00%   75.02%   +0.02%     
- Complexity     6434     6436       +2     
============================================
  Files           373      373              
  Lines         24881    24881              
  Branches       3205     3205              
============================================
+ Hits          18663    18668       +5     
+ Misses         4902     4899       -3     
+ Partials       1316     1314       -2     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[scholzj]

I wonder if having it in the models is the right way forward (and if yes, should we have some tests?). Do we expect it to differ per Kafka version? Or should we instead have for example a separate checks in the config diff instead?

[im-konge]

I wonder if having it in the models is the right way forward (and if yes, should we have some tests?). Do we expect it to differ per Kafka version? Or should we instead have for example a separate checks in the config diff instead?

As I mentioned in the description, I also added this custom config model, which looked to me to be too much complicated and "extra". I picked the way of having it in the config models as it is the simplest solution, the plugin is added to every Kafka version that particular Strimzi version supports. I was thinking about also how to handle more config options in the future and this was the best solution which came to my mind. Also, given the number of checks if the particular config is rollable (if it's not READ_ONLY or PER_BROKER etc.), this was the ideal solution.
But I'm open to other ideas.

[im-konge]

And yes, I wanted to add some UTs for the config-model-generator, but I wanted to know if you think this is the right way (or if someone else will come up with better idea)

[im-konge]

@strimzi/maintainers once you have time, it would be useful for me to see if there is any other path to pick, I had the custom config model path implemented, I was checking if we can change just code to handle it (however it was more changes and maybe it wasn't the best thing for the future) and then this. Thanks

[scholzj]

I'm not sure I would go with a custom JSON model. I wonder more if this can be handled when diffing the configuration directly.

[im-konge]

I'm not sure I would go with a custom JSON model. I wonder more if this can be handled when diffing the configuration directly.

That's what I tried - the difference (and the context if the Pod has older revision and should be rolled or should be just dynamically updated) is checked against the config model and yes, you can add some other checks or "workaround". I was just thinking about situations when you want to change the scope (PER_BROKER or CLUSTER_WIDE) for each configuration, having it handled in the code (like containing some map or something) is fine, but by adding it to the config model you will actually remove another set of code and the different branches of the code you will have to add there (which needs to be tested). In the config model we can just have tests that these custom configs are present and we can configure the scope.

Or do you think about different kind of path?