Skip to content

fix: (java-generator) Use timezone format compatible with Kubernetes#7117

Merged
manusa merged 1 commit intofabric8io:mainfrom
matteriben:iss_7116
Jun 9, 2025
Merged

fix: (java-generator) Use timezone format compatible with Kubernetes#7117
manusa merged 1 commit intofabric8io:mainfrom
matteriben:iss_7116

Conversation

@matteriben
Copy link
Copy Markdown
Contributor

@matteriben matteriben commented May 31, 2025

Description

fix: (java-generator) Use timezone format compatible with Kubernetes
Fixes #7116

Example error message:

Invalid value: "2025-05-30T23:04:44Zulu": startTime in body must be of type date-time

See also:
https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@matteriben matteriben marked this pull request as draft May 31, 2025 00:56
Signed-off-by: Matt Riben <matt.riben@hashgraph.com>
@matteriben matteriben marked this pull request as ready for review May 31, 2025 01:09
@sonarqubecloud
Copy link
Copy Markdown

@manusa manusa added this to the 7.4.0 milestone Jun 9, 2025 — with automated-tasks
Copy link
Copy Markdown
Member

@manusa manusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx!

@manusa manusa merged commit d09d207 into fabric8io:main Jun 9, 2025
18 checks passed
@andreaTP
Copy link
Copy Markdown
Member

andreaTP commented Jun 9, 2025

Sorry for being late at the party here, date time has been especially challenging when it comes to date-time, the original fix was done in #5384
I distinctly remember choosing different formats for writing(stricter) than for reading, and allowing more use cases reported on SO.

In this case, I'd be a bit on the fence accepting a PR without an exhaustive set of examples to cover the change.
cc. @manusa

public static final String DEFAULT_SER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssVV";
public static final String DEFAULT_DESER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ss[XXX][VV]";
public static final String DEFAULT_SER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssXXX";
public static final String DEFAULT_DESER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssXXX";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matteriben here you are effectively restricting the date-time formats accepted by the deserializer, any specific reason for doing it?

Copy link
Copy Markdown
Contributor Author

@matteriben matteriben Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreaTP I'm sorry for the delayed response. Yes, that is right. This change does restrict date-time formats accepted by the deserializer. However, as far as I understand the formats no longer accepted (for example, 'Europe/Paris') are also not supported by the kubernetes api server.

https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html

  Symbol  Meaning                     Presentation      Examples
  ------  -------                     ------------      -------
   V       time-zone ID                zone-id           America/Los_Angeles; Z; -08:30
   X       zone-offset 'Z' for zero    offset-X          Z; -08; -0830; -08:30; -083015; -08:30:15;

ZoneId: This outputs the time-zone ID, such as 'Europe/Paris'. If the count of letters is two, then the time-zone ID is output. Any other count of letters throws IllegalArgumentException.

Offset X and x: This formats the offset based on the number of pattern letters. One letter outputs just the hour, such as '+01', unless the minute is non-zero in which case the minute is also output, such as '+0130'. Two letters outputs the hour and minute, without a colon, such as '+0130'. Three letters outputs the hour and minute, with a colon, such as '+01:30'. Four letters outputs the hour and minute and optional second, without a colon, such as '+013015'. Five letters outputs the hour and minute and optional second, with a colon, such as '+01:30:15'. Six or more letters throws IllegalArgumentException. Pattern letter 'X' (upper case) will output 'Z' when the offset to be output would be zero, whereas pattern letter 'x' (lower case) will output '+00', '+0000', or '+00:00'.

// RFC 3339 - from: https://swagger.io/docs/specification/data-models/data-types/
public static final String DEFAULT_SER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssVV";
public static final String DEFAULT_DESER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ss[XXX][VV]";
public static final String DEFAULT_SER_DATETIME_FORMAT = "yyyy-MM-dd'T'HH:mm:ssXXX";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the VV format should be "stricter" (emit a more stable output) than XXX across different Java/JDK versions IIRC.

Copy link
Copy Markdown
Contributor Author

@matteriben matteriben Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreaTP I'm sorry for the delayed response. As far as I understand 'VV' will output a string like 'Europe/Paris' or 'Zulu' (which kubernetes api server does not accept as date-time). As for 'XXX' as far as I understand it should output "the hour and minute, with a colon, such as '+01:30'.", or "'Z' when the offset to be output would be zero".

Indeed, this is what I see with jshell (21.0.6):

jshell> java.time.format.DateTimeFormatter.ofPattern("XXX").format(java.time.ZonedDateTime.now())
$1 ==> "-05:00"

jshell> java.time.format.DateTimeFormatter.ofPattern("XXX").format(java.time.ZonedDateTime.now(java.time.ZoneOffset.UTC))
$2 ==> "Z"

As for the behavior across different Java/JDK versions, I found JDK-8066806. This seems to be a parsing bug, and I'm not sure if this bug was what you were referring to? I don't really understand what was fixed ('-8:00' is not valid, '-08:00' is valid). I think this bug could potentially cause inconsistent behavior with older builds of Java 8. It's not clear to me if parsing with "[XXX][VV]" instead of "XXX" would be an effective workaround for this bug.

@matteriben
Copy link
Copy Markdown
Contributor Author

Sorry for being late at the party here, date time has been especially challenging when it comes to date-time, the original fix was done in #5384 I distinctly remember choosing different formats for writing(stricter) than for reading, and allowing more use cases reported on SO.

In this case, I'd be a bit on the fence accepting a PR without an exhaustive set of examples to cover the change. cc. @manusa

@andreaTP I'm also sorry for the delayed response here, I agree with you that this could benefit from more exhaustive test coverage. In particular the output of the serializer should be considered valid by the kubernetes api server and the deserializer should be able to handle anything that the kubernetes api server considers valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

java-generator type=string format=date-time serializes with format that is not compatible with k8s

4 participants