discriminator improvements

[csviri]

No description provided.

[metacosm]

Some documentation to explain what this does and why it's useful would be helpful for users.

[csviri]

added

[metacosm]

LGTM otherwise.

[csviri]

cc @shawkins

[shawkins]

With simple usage we'll have something like:

@ControllerConfiguration(
    dependents = {
        @Dependent(type = KeycloakServiceDependentResource.class, useEventSourceWithName = "serviceSource"),
        @Dependent(type = KeycloakDiscoveryServiceDependentResource.class, useEventSourceWithName = "serviceSource")
    })
...

Then

@KubernetesDependent(labelSelector = Constants.DEFAULT_LABELS_AS_STRING, resourceDiscriminator = KeycloakDiscoveryServiceDependentResource.NameResourceDiscriminator.class)
public class KeycloakDiscoveryServiceDependentResource extends CRUDKubernetesDependentResource<Service, Keycloak> {

    public static class NameResourceDiscriminator extends ResourceIDMatcherDiscriminator<Service, Keycloak>
        public NameResourceDiscriminator() {
            super("serviceSource", KeycloakDiscoveryServiceDependentResource::getName);
        } 
    }

   ...

Creating a constant for the event source name will help, but it looks like we've spread some knowledge in two places. Can we instead do:

@KubernetesDependent(labelSelector = Constants.DEFAULT_LABELS_AS_STRING)
public class KeycloakDiscoveryServiceDependentResource extends CRUDKubernetesDependentResource<Service, Keycloak> implements ResourceDiscriminatorInitializer<Service, Keycloak> {

    public void useEventSourceWithName(String name) {
        super.useEventSourceWithName(name);
        setResourceDiscriminator(new ResourceIDMatcherDiscriminator<>(name, KeycloakDiscoveryServiceDependentResource::getName));
    }
    ...

Or are methods like useEventSourceWithName not to be tampered with?

[csviri]

Wasn't really tested that way. TBH I don't like that much this annotation style definitions, basically because of these inflexibilities. But it is how it is, we can think to change appraoches for v5.

[shawkins]

In the keycloak case there is only a single Service event source, so eventSourceName doesn't seem required in all cases. Could it test for, or catch the exception, if there are multiple sources for the given type before resorting to the list?

[csviri]

makes sense, changed, pls check if this looks to you

[shawkins]

LGTM, this latest change helps side step the issue of passing around the eventSourceName.

[csviri]

LGTM, this latest change helps side step the issue of passing around the eventSourceName.

Originally tbh was thinking that since discriminators are just used with DR with "same type", that means not with bulk resources for example, there will be always a very small amount of them, but it's still true that it's better not loop though of those.