createOrReplace on Secrets throws in case the resource already exists

[andreaTP]

Describe the bug

Version of the kubernetes-client: 5.11.2

Performing 2 createOrReplace on the same Secret cause the second call to fail with this error:

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://192.168.64.3:8443/api/v1/secrets/test. Message: the server could not find the requested resource. Received status: Status(apiVersion=v1, code=404, details=StatusDetails(causes=[], group=null, kind=null, name=null, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=the server could not find the requested resource, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=NotFound, status=Failure, additionalProperties={}).

Fabric8 Kubernetes Client version

next (development version)

Steps to reproduce

Running a Main like this would reproduce the issue:

public class Main {

    private static Secret getSecret() {
        return new SecretBuilder()
                .withNewMetadata()
                .withName("test")
                .withNamespace("default")
                .endMetadata()
                .build();
    }
    
    public static void main(String[] args) {
        Config config = new ConfigBuilder().withNamespace(null).build();
        KubernetesClient client = new DefaultKubernetesClient(config);

        client.secrets().createOrReplace(getSecret());
        client.secrets().createOrReplace(getSecret());
    }
    
}

Expected behavior

The resource is seamlessly replaced.

Runtime

minikube

Kubernetes API Server version

1.22.3@latest

Environment

macOS

Fabric8 Kubernetes Client Logs

io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://192.168.64.3:8443/api/v1/secrets/test. Message: the server could not find the requested resource. Received status: Status(apiVersion=v1, code=404, details=StatusDetails(causes=[], group=null, kind=null, name=null, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=the server could not find the requested resource, metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=NotFound, status=Failure, additionalProperties={}).
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.requestFailure (OperationSupport.java:683)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.requestFailure (OperationSupport.java:662)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.assertResponseCode (OperationSupport.java:613)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse (OperationSupport.java:556)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleResponse (OperationSupport.java:519)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleGet (OperationSupport.java:488)
    at io.fabric8.kubernetes.client.dsl.base.OperationSupport.handleGet (OperationSupport.java:458)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.handleGet (BaseOperation.java:696)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.getMandatory (BaseOperation.java:182)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.require (BaseOperation.java:163)
    at io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.requireFromServer (HasMetadataOperation.java:120)
    at io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.replace (HasMetadataOperation.java:187)
    at io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.replace (HasMetadataOperation.java:141)
    at io.fabric8.kubernetes.client.utils.CreateOrReplaceHelper.replace (CreateOrReplaceHelper.java:68)
    at io.fabric8.kubernetes.client.utils.CreateOrReplaceHelper.createOrReplace (CreateOrReplaceHelper.java:60)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.createOrReplace (BaseOperation.java:316)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.createOrReplace (BaseOperation.java:83)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.createOrReplace (BaseOperation.java:306)
    at io.fabric8.kubernetes.client.dsl.base.BaseOperation.createOrReplace (BaseOperation.java:83)
    at org.keycloak.Main.main (Main.java:27)
    at org.codehaus.mojo.exec.ExecJavaMojo$1.run (ExecJavaMojo.java:254)
    at java.lang.Thread.run (Thread.java:829)

Additional context

I suspect the issue sits in the replace method, but I haven't narrowed it down.

[shawkins]

The issue is with withNamespace(null) - that is actually like telling the config "there is no default"/"use any namespace". That is confusing the logic in HasMetadataOperation/OperationSupport that normally expects the config to supply a default namespace. If you just construct a client using new DefaultKubernetesClient(), or specify the default namespace withNamespace("default"), everything works as expected.

However I think this can also occur if something deletes the secret in question after the second call attempts the create, but before it calls replace - there's nothing really guarding against that concurrency issue. That ends up happening because you are using the original secret, not the one returned from the operations, so there's no resourceVersion set. The replace logic is trying to lookup the current resourceVersion.

[andreaTP]

@shawkins thanks for the answer!

I'm actually creating the secret with .withNamespace("default"), should I add .inNamespace("default") after client.secrets?

I think this behaviour is pretty misleading since other operations are performed successfully (e.g. get, delete, create) using the very same pattern.

Regarding the concurrency issue it's not kicking in here, you can even remove a createOrReplace from my example, run it twice and obtain the same result.

[shawkins]

I'm actually creating the secret with .withNamespace("default"), should I add .inNamespace("default") after client.secrets?

No, you don't need to. The client defaults to the default namespace, that will typically be "default". It's picking it up from the system kubernetes context.

I think this behaviour is pretty misleading since other operations are performed successfully (e.g. get, delete, create) using the very same pattern.

Right, I don't think it's intentional. Internally the client logic will set the config namespace to null for its own usage, but I don't think I've ever seen a case where the user sets the default namespace is null - thus you're hitting some unanticipated assumption. It can probably be tracked down if needed.

Regarding the concurrency issue it's not kicking in here, you can even remove a createOrReplace from my example, run it twice and obtain the same result.

I'm not saying that the concurrency issue applies here, just that it's possible in another case to hit the exception you are seeing and why.

[shawkins]

It can probably be tracked down if needed.

This is it -

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/OperationSupport.java

Line 156 in 7ef49cd

} else if (Utils.isNotNullOrEmpty(namespace)) {

The calling method to construct the url knows that the resource is supposed to be namespaced, but if the config/context namespace is null or empty, it will assume non-namespaced based upon the check on line 156.

@manusa @rohanKanojia is it an assumption there will always be a default namespace? Or can it be null and this other logic needs adjusted?

[andreaTP]

@shawkins great analysis, thanks for tracking this down!

[andreaTP]

I can add that the error doesn't occur for e.g. deployments

[shawkins]

I can add that the error doesn't occur for e.g. deployments

The dsl logic will be different depending on the resource type - for deployments there is rolling logic that is implemented for a replace.

To restate what is happening under the covers in your test shown above:

Config config = new ConfigBuilder().withNamespace(null).build();
KubernetesClient client = new DefaultKubernetesClient(config);
client.secrets()...

is like calling:

client.secrets().inAnyNamespace()...

Note what operations are available when you call client.secrets().inAnyNamespace() - those method calls will still work as expected using withNamespace(null). However anything else you can call that has access to a passed in item may not be accounting for the possibility of a null namespace.

I can address this with a narrow change to the require logic: master...shawkins:master

However all of the other operations would need similarly checked. If it's the case that we always expect the initial config passed into the client to specify a default namespace, I'd rather add a validation just for that.

[shawkins]

We'll address this in 6.0.0 - if the config has a null namespace, whether because there is not one on the context or the user calls withNamespace(null), the logic will assume an appropriate default - likely "default". In the event of a checkNamespace call failing, the logic should inform the user to use inNamespace or the original namespace was null, to set a better default on the Config.

[shawkins]

the logic will assume an appropriate default - likely "default"

The resolution will not assume "default", but throw a better exception message if a namespace cannot be determined.