Refactor MirrorMaker2Connectors configuration setup

[tinaselenge]

Type of change

Select the type of your PR

• Refactoring

Description

Refactors MirrorMaker connector configuration setup so that only config providers used, instead of bash script.
Closes #10668

Checklist

Please go through this checklist and make sure all applicable tasks have been done

• Write tests
• Make sure all tests pass
• Update documentation
• Check RBAC rights for Kubernetes / OpenShift roles
• 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
• Supply screenshots for visual changes, such as Grafana dashboards

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

@tinaselenge Do we need any improvements to how the Pods are reloaded when the values change? The use of environment variables forced the rolling of the pods. So I'm not sure if we will miss it now or not.

[katheris]

@tinaselenge Do we need any improvements to how the Pods are reloaded when the values change? The use of environment variables forced the rolling of the pods. So I'm not sure if we will miss it now or not.

From what I understand reading the code I agree we need to consider what happens when the values change. The file and directory config providers only read the value when the connector or runtime starts as far as I understand. So today when the env var changes, the pod revision will have changed so we roll the Connect pods. But now there is nothing to notice the change in the file/directory that is used in the config provider.

Adding @mimaison to sanity check my understand of the Kafka FileConfigProvider and DirConfigProvider

[tinaselenge]

@scholzj @katheris very good points!

So this is my understanding how the current config set up works:

1. The MirrorMaker2 CR is updated/created
2. Based on the CR update, Assembly Operator creates the KafkaMirrorMaker2Cluster resource with the environment variables and volume mounts
   • An example of the environment variable KAFKA_MIRRORMAKER_2_OAUTH_CLIENT_SECRETS_CLUSTERS = "<clusterAlias>=<clientSecretName>/<secretKey>" (to store values for both source and target clusters, the value is prefixed with cluster alias)
3. Assembly Operator reconciles the pod set (so if env variable or volume mounted secret has changed, connect pods will be rolled)
4. When connect pod restarts, it creates a file /tmp/strimzi-mirrormaker2-connector.properties. This file contains:
   <clusterAlias>.oauth.client.secret=<the value is read from "/opt/kafka/mm2-oauth/<clusterAlias>/<clientSecretName>/<secretKey>" according to the env variable>
5. Assembly Operator reconciles MM2 connectors:
   • For each cluster, connector configuration is created using the config providers. For example, oauth.client.secret will be set to ${strimzifile:/tmp/strimzi-mirrormaker2-connector.properties:<clusterAlias>.oauth.client.secret

So we already use config providers to get the value of these secrets. The environment variables are set to the volume mount path of these secrets, rather than the actual secret values. So unless secret name or key has changed, the pods are not rolled as the environment variables would still be set to the same secret paths.

My understanding was that the pods are rolled because the actual data in the volume mounted secrets have changed, not because of the environment variables. This PR essentially removes the extra step of creating strimzi-mirrormaker2-connector.properties file to use for the config providers. Instead it will directly use the config providers to map to the volume mounted secret directories (${strimzidir:/opt/kafka/mm2-oauth/<clusterAlias>/<clientSecretName>:<secretKey>})

MirrorMaker2ST has some tests that updates the secret values and checks that MM2 pods have rolled such as testKMM2RollAfterSecretsCertsUpdateScramSha. There are tests for mirrormaker 2 deployment with oauth but it seems like we don't have tests that updates oauth secrets and checks that pods are rolled.

Please let me know, if I got anything wrong or what I described is not accurate.

[scholzj]

My understanding was that when the pods are rolled because the actual data in the volume mounted secrets have changed, not because of the environment variables.

Do they? That is pretty much what I asked about. In the current code when the KAFKA_MIRRORMAKER_2_OAUTH_CLIENT_SECRETS_CLUSTERS value changes from cluster-a=my-secret/my.key to cluster-b=my-secret/my-second.key, the Pods will be rolled because of the environment variable changed. So what I'm wondering about is:

• Do we care about these changes? (Maybe we don't, if we track changes to the actual value it maps to, we might not care that the key changed as long as its content is unchanged)
• If we care about these changes, do we have some other mechanism to roll the pods? E.g. because we have annotation tracking the configuration file etc.