SecretEnricher : Use primitive `int` instead of `Integer` in for loop

[rohanKanojia]

Component

JKube Kit

Task description

Description

I see a boxed Integer type getting used as a local for loop scope variable:

https://github.com/eclipse/jkube/blob/3afc7bf1f8dbe4452826c5ec7ec58de5c9408d53/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/SecretEnricher.java#L85

Generally, it's better to rely on primitives unless we need to use these types in collections.

Expected Behavior

Integer is replaced with int

Acceptance Criteria

• [x] Integer is replaced with int in for loop
• [x] SecretEnricherTest passes after making this change

How to manually test my changes

Kubernetes

If you don't have a real Kubernetes cluster available (most probably), you can use Minikube or Kind to test with a local cluster.

• Minikube get started guide
• Kind (Requires Docker) quick start

OpenShift

If you don't have a real OpenShift cluster available (most probably), you can use Red Hat's developer Sandbox for Red Hat OpenShift. The only requirement is to have a Red Hat account.

Once you have your Sandbox environment, you'll need to download the oc tool from the cluster console. (Press the ? icon and from the context menu select Command line tools, you'll be redirected to https://$subdomain.openshiftapps.com/command-lines-tools where you'll be able to download the CLI for your platform)

[rohanKanojia]

On closer inspection, we can replace the loop using Java Stream API. Using Map doesn't seem to be relevant here. We should replace it with Set instead:

Set<String> secretNamesPresentInBuilder = builder.buildItems().stream()
            .filter(Secret.class::isInstance)
            .map(HasMetadata::getMetadata)
            .map(ObjectMeta::getName)
            .collect(Collectors.toSet());

then use secretNamesPresentInBuilder.contains here:

https://github.com/eclipse/jkube/blob/3afc7bf1f8dbe4452826c5ec7ec58de5c9408d53/jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/SecretEnricher.java#L126

[arsenalzp]

Hello, Could you please assign it to me?

[arsenalzp]

Should I write a test for this class, because test SecretEnricherTest doesn't exist?

[rohanKanojia]

@arsenalzp : This is an abstract class. Are there unit tests for classes extending this class?

[arsenalzp]

@arsenalzp : This is an abstract class. Are there unit tests for classes extending this class?

I saw a unit test for ConfigMap enricher, so it does make sense to do the same test for Secret one. It is just a proposal :)

[rohanKanojia]

@arsenalzp : I see DockerRegistrySecretEnricher which is extending this class. There is a test for DockerRegistrySecretEnricherTest. Could you please check if it's testing only Docker Registry Secret related logic or also base class.

Anyway, I don't think there is any harm in adding test for SecretEnricher. Please create a new issue for the proposal, I'll assign it to you.