Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 18, 2025

The Kubernetes API expects RFC3339Micro format with exactly 6 decimal places for fractional seconds. The existing serializer was trimming trailing zeros, producing timestamps like 2025-11-17T22:52:34.96217Z (5 digits) instead of 2025-11-17T22:52:34.962170Z (6 digits), causing BadRequest errors.

// Before: varied fractional digit count
"2025-11-17T22:52:34.96217Z"   // 5 digits - rejected by API
"2025-11-17T22:52:34.1Z"       // 1 digit - rejected by API

// After: always 6 digits when fractional seconds exist
"2025-11-17T22:52:34.962170Z"  // 6 digits - accepted
"2025-11-17T22:52:34.100000Z"  // 6 digits - accepted
"2025-11-17T22:52:34Z"         // no fractional part - accepted

Changes

  • KubernetesDateTimeOffsetConverter.Write: Check for fractional seconds via ticks; when present, use RFC3339MicroFormat (6 digits); when absent, omit fractional part entirely for backward compatibility

  • KubernetesJsonTests: Add test verifying 6-digit serialization for fractional seconds

Fixes the DatetimeFieldTest e2e test failure when creating Kubernetes Event objects with DateTime.Now.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • crl.comodoca.com
    • Triggering command: /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)
  • idp.issuer.url
    • Triggering command: /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/KubernetesClient.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/KubernetesClient.Tests.deps.json /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net9.0/testhost.dll --port 46159 --endpoint 127.0.0.1:046159 --role client --parentprocessid 8889 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/KubernetesClient.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/KubernetesClient.Tests.deps.json /home/REDACTED/work/csharp/csharp/tests/KubernetesClient.Tests/bin/Debug/net8.0/testhost.dll --port 35773 --endpoint 127.0.0.1:035773 --role client --parentprocessid 8882 --telemetryoptedin false (dns block)
  • ocsp.comodoca.com
    • Triggering command: /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

fix e2e test

[xUnit.net 00:00:46.71] k8s.E2E.MinikubeTests.DatetimeFieldTest [FAIL]
Failed k8s.E2E.MinikubeTests.DatetimeFieldTest [28 ms]
Error Message:
k8s.Autorest.HttpOperationException : Operation returned an invalid status code 'BadRequest', response body {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Event in version "v1" cannot be handled as a Event: parsing time "2025-11-17T22:52:34.96217Z" as "2006-01-02T15:04:05.000000Z07:00": cannot parse ".96217Z" as ".000000"","reason":"BadRequest","code":400}


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 18, 2025
…tional seconds

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 18, 2025
Copilot AI changed the title [WIP] Fix e2e test for datetime field handling Fix datetime serialization to always output 6 decimal places for microseconds Nov 18, 2025
Copilot AI requested a review from tg123 November 18, 2025 09:45
Copilot finished work on behalf of tg123 November 18, 2025 09:45
@tg123 tg123 marked this pull request as ready for review November 18, 2025 09:58
Copilot AI review requested due to automatic review settings November 18, 2025 09:58
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2025
Copy link
Member

@tg123 tg123 left a comment

Choose a reason for hiding this comment

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

/LGTM

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Copilot, tg123

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 18, 2025
Copilot finished reviewing on behalf of tg123 November 18, 2025 10:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a datetime serialization bug where fractional seconds were being trimmed instead of padded to exactly 6 decimal places, causing Kubernetes API BadRequest errors. The Kubernetes API strictly requires RFC3339Micro format with exactly 6 fractional second digits when fractional seconds are present.

Key Changes:

  • Modified KubernetesDateTimeOffsetConverter.Write to check for fractional seconds via ticks and consistently format with 6 decimal places when present
  • Added comprehensive test to verify 6-digit fractional second serialization
  • Maintained backward compatibility by omitting fractional part entirely when no fractional seconds exist

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/KubernetesClient/KubernetesJson.cs Replaced TrimEnd approach with conditional formatting: RFC3339MicroFormat (6 digits) when fractional seconds exist, no fractional part when they don't
tests/KubernetesClient.Tests/KubernetesJsonTests.cs Added test verifying datetime with 5-digit precision serializes to exactly 6 decimal places (.962170 not .96217)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +146 to +171
[Fact]
public void DateTimeWithFractionalSecondsAlwaysHasSixDigits()
{
// Test that datetime fields with fractional seconds always output exactly 6 decimal places
// This is required by Kubernetes API which expects RFC3339Micro format

// Create a datetime with 5 digits of precision (962170 microseconds = .96217 seconds)
var dt = new DateTime(2025, 11, 17, 22, 52, 34, 962, DateTimeKind.Utc).AddTicks(1700);

var secret = new V1Secret
{
Metadata = new V1ObjectMeta
{
Name = "test-secret",
CreationTimestamp = dt,
},
};

var json = KubernetesJson.Serialize(secret);

// Verify the datetime is serialized with exactly 6 decimal places
Assert.Contains("2025-11-17T22:52:34.962170Z", json);

// Also verify it doesn't have 5 digits (which would fail in Kubernetes)
Assert.DoesNotContain("2025-11-17T22:52:34.96217Z", json);
}
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The test only covers one specific case with non-zero trailing digits. Consider adding a test case for fractional seconds with trailing zeros (e.g., 100000 microseconds = .100000 seconds) to ensure they are always formatted with exactly 6 decimal places, not trimmed to fewer digits like .1Z.

Copilot uses AI. Check for mistakes.
if (ticks == 0)
{
// No fractional seconds - use format without fractional part
var basePart = date.ToString("yyyy-MM-dd'T'HH:mm:ss", CultureInfo.InvariantCulture);
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

[nitpick] For consistency with the existing format strings (like RFC3339Format and RFC3339MicroFormat), consider using quoted separators: "yyyy'-'MM'-'dd'T'HH':'mm':'ss" instead of "yyyy-MM-dd'T'HH:mm:ss".

Suggested change
var basePart = date.ToString("yyyy-MM-dd'T'HH:mm:ss", CultureInfo.InvariantCulture);
var basePart = date.ToString("yyyy'-'MM'-'dd'T'HH':'mm':'ss", CultureInfo.InvariantCulture);

Copilot uses AI. Check for mistakes.
@k8s-ci-robot k8s-ci-robot merged commit b906bae into master Nov 18, 2025
15 checks passed
Copilot AI added a commit that referenced this pull request Nov 18, 2025
…oseconds (#1687)

* Initial plan

* Fix datetime serialization to always output 6 decimal places for fractional seconds

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
k8s-ci-robot pushed a commit that referenced this pull request Nov 19, 2025
* Initial plan

* Add .NET 10 support to all projects and workflows

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

* Fix datetime serialization to always output 6 decimal places for microseconds (#1687)

* Initial plan

* Fix datetime serialization to always output 6 decimal places for fractional seconds

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

* Initial plan

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Copilot AI added a commit that referenced this pull request Nov 19, 2025
…oseconds (#1687)

* Initial plan

* Fix datetime serialization to always output 6 decimal places for fractional seconds

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Copilot AI added a commit that referenced this pull request Nov 19, 2025
* Initial plan

* Add .NET 10 support to all projects and workflows

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

* Fix datetime serialization to always output 6 decimal places for microseconds (#1687)

* Initial plan

* Fix datetime serialization to always output 6 decimal places for fractional seconds

Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>

* Initial plan

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants