Allow Access Operator deploy to a single Namespace or multiple Namespaces

[OwenCorrigan76]

By default, the Kafka Access Operator watches all namespaces. This PR allows deployment of a KafkaAccess resource in a defined namespace or namespaces, by using the Optional env STRIMZI_NAMESPACE.
For example:

          env:
            - name: STRIMZI_NAMESPACE
              value: "myproject, test"

This PR fixes:
#106

[im-konge]

Will this work when we configure to watch all (or any) Namespace? That means when you configure STRIMZI_NAMESPACE to *
I think you will need in that case o.watchingAllNamespaces().

[OwenCorrigan76]

Thanks for the comment @im-konge. That makes sense. Will implement this.

[im-konge]

FMPOV you should not have multiple files containing RoleBinding for each Namespace - similarly to the Cluster Operator (and what we have in docs) you can just have one and if needed, it can be created in every Namespace the operator will watch.

[im-konge]

Suggested change

	if (strimziNamespace != null && strimziNamespace.matches("\\*")) {
	if (strimziNamespace != null && strimziNamespace.equals("*")) {

I think this will be better than the regex stuff.

[im-konge]

Actually, as Jakub mentioned couple of times even before, this is the best thing:

Suggested change

	if (strimziNamespace != null && strimziNamespace.matches("\\*")) {
	if ("*".equals(strimziNamespace)) {

and you don't need the null check.

[im-konge]

I wonder if having something like this:

Suggested change

	String strimziNamespace = System.getenv("STRIMZI_NAMESPACE");
	String strimziNamespace = System.getenv().getOrDefault("STRIMZI_NAMESPACE", "*");

would be better.
TBH it would be better to have some configuration class that would check everything and return configuration for the operator, but I don't think it's scope of this PR.
That way you would keep the current behavior of the operator -> in case that you didn't configure this env variable, the operator will watch all Namespaces. Otherwise it will watch the set of Namespaces configured inside the STRIMZI_NAMESPACE env variable.

[im-konge]

So I think that this is not the correct way. What will happen when you pass the empty list of Namespaces into the settingNamespaces() method?
I would change the if-else here:

• check if the strimziNamespace is null or empty (the null shouldn't happen here, but it's better to be safe)
• then check if it's wildcard (you should cover also cases when you have space there - so using trim())
• I think you should cover case when you have wildcard and Namespace there (so something like - *,namespace-0)
• then you can split it by the ,
• if there is something wrong, you should throw exception and not pass there empty Set

You can check how it's done in cluster-operator: https://github.com/strimzi/strimzi-kafka-operator/blob/main/operator-common/src/main/java/io/strimzi/operator/common/config/ConfigParameterParser.java#L179-L195

[im-konge]

Wouldn't break be better than having another boolean value?
Also, if you would like to keep it, I would use different loop type - like while, which you can break and then fail inside it.

However what I saw as the cleanest solution is to use the edit:

        client.resources(KafkaAccess.class).resource(currentKafkaAccess).edit(ka -> {
            ka.getSpec().setSecretName(NEW_USER_PROVIDED_SECRET_NAME);
            return ka;
        });

or

        client.resources(KafkaAccess.class).resource(currentKafkaAccess).edit(ka -> new KafkaAccessBuilder(ka)
            .editOrNewSpec()
                .withSecretName(NEW_USER_PROVIDED_SECRET_NAME)
            .endSpec()
            .build()
        );

it should handle the conflict IIRC.