Message: parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00".

[jiridanek]

Describe the bug

Here's an example on using .sinceTime()

kubernetes-client/doc/CHEATSHEET.md

Lines 2342 to 2345 in 08e54a9

	- Only return logs after a specific date (RFC3339):
	```java
	client.pods().inNamespace("test").withName("foo").sinceTime("2020-09-10T12:53:30.154148788Z").getLog();
	```

When I use this with a timezoned timestamp with fabric8 kubernetes client 6.13.0, I get an exception from the remote:

Failure executing: GET at: https://api.crc.testing:6443/api/v1/namespaces/redhat-ods-operator/pods/rhods-operator-76bb96fb9d-qg2cs/log?pretty=false&sinceTime=2024-10-16T14:36:58+02:00. Message: parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00". Received status: Status(apiVersion=v1, code=400, details=null, kind=Status, message=parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00", metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=BadRequest, status=Failure, additionalProperties={}).
io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://api.crc.testing:6443/api/v1/namespaces/redhat-ods-operator/pods/rhods-operator-76bb96fb9d-qg2cs/log?pretty=false&sinceTime=2024-10-16T14:36:58+02:00. Message: parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00". Received status: Status(apiVersion=v1, code=400, details=null, kind=Status, message=parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00", metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=BadRequest, status=Failure, additionalProperties={}).
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.requestFailure(OperationSupport.java:660)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.requestFailure(OperationSupport.java:640)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.assertResponseCode(OperationSupport.java:589)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.handleRaw(OperationSupport.java:739)
	at io.fabric8.kubernetes.client.dsl.internal.OperationSupport.handleRawGet(OperationSupport.java:474)
	at io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl.doGetLog(PodOperationsImpl.java:133)
	at io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl.getLog(PodOperationsImpl.java:141)
	at io.fabric8.kubernetes.client.dsl.internal.core.v1.PodOperationsImpl.getLog(PodOperationsImpl.java:166)
	at io.fabric8.kubernetes.client.utils.internal.PodOperationUtil.getLog(PodOperationUtil.java:109)
	at io.fabric8.kubernetes.client.dsl.internal.apps.v1.ReplicaSetOperationsImpl.getLog(ReplicaSetOperationsImpl.java:86)
	at io.fabric8.kubernetes.client.dsl.internal.apps.v1.DeploymentOperationsImpl.getLog(DeploymentOperationsImpl.java:130)
	at io.fabric8.kubernetes.client.dsl.internal.apps.v1.RollableScalableResourceOperation.getLog(RollableScalableResourceOperation.java:89)

The problem appears to be that the + character in the timestamp is not urlencoded.

Fabric8 Kubernetes Client version

6.13.0

Steps to reproduce

client.pods().inNamespace("test").withName("foo").sinceTime("2024-10-16T14:36:58+02:00").getLog();

Expected behavior

I'd expect fabric8 to urlencode the timestamp.

Runtime

OpenShift

Kubernetes API Server version

other (please specify in additional context)

Environment

Linux, macOS

Fabric8 Kubernetes Client Logs

No response

Additional context

% oc version
Client Version: 4.17.0
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: 4.16.7
Kubernetes Version: v1.29.7+6abe8a1

[manusa]

Maybe encoding the parameter at this point fixes the issue:

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java

Line 146 in 25ecf84

sb.append("&sinceTime=").append(sinceTimestamp);

That whole method should be refactored to treat the URL correctly.
For the moment, could you try to see if by using URLUtils.encodeToUTF in that line solves the problem?

[jiridanek]

Looks like a legit fix

    @Test
    void testEncode() {
        String timestamp = "2024-10-16T14:36:58+02:00";
        String encoded = URLUtils.encodeToUTF(timestamp);
        System.out.println("encoded: " + encoded);
    }

prints encoded: 2024-10-16T14%3A36%3A58%2B02%3A00

and if I try the original url from error message, I get failure

oc get --raw 'https://api.crc.testing:6443/api/v1/namespaces/redhat-ods-operator/pods/rhods-operator-775fd676bf-x4q2b/log?pretty=false&sinceTime=2024-10-16T14:36:58+02:00'
Error from server (BadRequest): parsing time "2024-10-16T14:36:58 02:00" as "2006-01-02T15:04:05Z07:00": cannot parse " 02:00" as "Z07:00"

but after replacing the parameter with the encoded timestamp

oc get --raw 'https://api.crc.testing:6443/api/v1/namespaces/redhat-ods-operator/pods/rhods-operator-775fd676bf-x4q2b/log?pretty=false&sinceTime=2024-10-16T14%3A36%3A58%2B02%3A00'
{"level":"info","ts":"2024-10-17T10:25:30Z","logger":"setup","msg":"starting manager"}
{"level":"info","ts":"2024-10-17T10:25:30Z","logger":"controller-runtime.metrics","msg":"Starting metrics server"}
[...]

So I do like the general direction.

For now I fixed this by using Instant.now() instead of new Date() so I get the timestamp in UTC and don't have to deal with timezones in the timestamp at all. I believe relocating my computer to Greenwich or to the west of there would also work. My CI machines based in the US don't crash with this problem.

[manusa]

So I do like the general direction.

Sorry, it's unclear to me if the suggested change does fix the issue, from your comments my understanding is that it would but it's not 100% clear.

If so, we can proceed with an initial quick fix and maybe later further refactor the complete method to use an URL builder instead.

[jiridanek]

Sorry, it's unclear to me if the suggested change does fix the issue

I did not test the suggested change, I only did what I said I did, completely outside of fabric8 code. I did not know you're intending to actually be fixing it right now, I thought it's just random speculation on your part.

Right, I'll go figure out how to do the whole "build a snapshot version of fabric8 and run my project with that". It's like back in my younger days, this Java+Maven stuff, again.

[manusa]

Right, I'll go figure out how to do the whole "build a snapshot version of fabric8 and run my project with that". It's like back in my younger days, this Java+Maven stuff, again.

No, no, no worries. I mean, feel free to do it if you want to feel young again ;)

Just wanted to have a clear path to the fix once we can tackle the issue so there's no need to re-read everything again.

I did not test the suggested change, I only did what I said I did, completely outside of fabric8 code. I did not know you're intending to actually be fixing it right now, I thought it's just random speculation on your part.

I understand, didn't do this in the scope of the project but you actually executed the request. What is/was unclear to me is if the response actually contains what you requested i.e. are the logs returned within the specified time range.

If this is right, we can proceed with the suggested fix (not right away).

[jiridanek]

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.13.0:testCompile (default-testCompile) on project kubernetes-test: Fatal error compiling: java.lang.NoSuchMethodError: 'void io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinitionSpec.(java.util.List, io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceConversion, java.lang.String, io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinitionNames, java.lang.Boolean, java.lang.String, io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceSubresources, io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceValidation, java.lang.String, java.util.List)' -> [Help 1]

I'll figure it out, but yeah, feeling none the wiser than my younger self, dealing with stuff like that.

[jiridanek]

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java
index f22c63ad4..47253cfb3 100644
--- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java
+++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java
@@ -17,6 +17,7 @@ package io.fabric8.kubernetes.client.dsl.internal;
 
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.dsl.ExecListener;
+import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.URLUtils.URLBuilder;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
@@ -143,7 +144,7 @@ public class PodOperationContext {
     if (sinceSeconds != null) {
       sb.append("&sinceSeconds=").append(sinceSeconds);
     } else if (sinceTimestamp != null) {
-      sb.append("&sinceTime=").append(sinceTimestamp);
+      sb.append("&sinceTime=").append(URLUtils.encodeToUTF(sinceTimestamp));
     }
     if (tailingLines != null) {
       sb.append("&tailLines=").append(tailingLines);

This does work, but it mangles the timestamp a bit too much.

It goes from 2024-10-16T14:36:58+02:00 to 2024-10-16T14%3A36%3A58%2B02%3A00, which is correct, but it's encoding even characters that don't need to be encoded. Not encoding : is fine, and if it is encoded, the string becomes unreadable. Only + needs to be encoded, because unencoded + in url means (space character).

Thanks for your moral support with testing!

What I'm running (for my future reference) is

    @Test
    void testUpgradeOlm() throws IOException, InterruptedException {
        Date operatorLogCheckTimestamp = new Date();
        UpgradeUtils.deploymentLogIsErrorEmpty("redhat-ods-operator", "rhods-operator", operatorLogCheckTimestamp);
    }

[manusa]

It goes from 2024-10-16T14:36:58+02:00 to 2024-10-16T14%3A36%3A58%2B02%3A00, which is correct, but it's encoding even characters that don't need to be encoded. Not encoding : is fine, and if it is encoded, the string becomes unreadable. Only + needs to be encoded, because unencoded + in url means (space character).

I'm not sure what the util class is actually doing here, we can probably use something else instead (java.net) URLEncoder.

Otherwise, I don't think it's important how the URL looks in the end since this should be mostly transparent for the user (except for MockWebServer expectations, here there might be some problems)

Thanks for checking this out, much appreciated 🙏

[jiridanek]

Otherwise, I don't think it's important how the URL looks in the end since this should be mostly transparent for the user (except for MockWebServer expectations, here there might be some problems)

Right. I had to do mvn clean install -DskipTests because I had already failing tests on unmodified main, so I don't know about failing tests.