From 2bef2b00d0f42f678f53b4aca4ab53cafddbc463 Mon Sep 17 00:00:00 2001 From: Kamil Date: Wed, 16 Jul 2025 20:59:38 +0100 Subject: [PATCH 1/4] add .string() to filename Signed-off-by: Kamil --- pkg/capture/file/capture_filename_test.go | 20 ++++++++++++++++++++ pkg/capture/provider/network_capture_unix.go | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 pkg/capture/file/capture_filename_test.go diff --git a/pkg/capture/file/capture_filename_test.go b/pkg/capture/file/capture_filename_test.go new file mode 100644 index 0000000000..9f18a12357 --- /dev/null +++ b/pkg/capture/file/capture_filename_test.go @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +package file + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func CaptureFilenameFormat(t *testing.T) { + captureName := "capture-name" + nodeHostName := "node1" + timestamp := v1.NewTime(time.Date(2025, 7, 16, 12, 30, 0, 0, time.UTC)) + name := CaptureFilename{CaptureName: captureName, NodeHostname: nodeHostName, StartTimestamp: ×tamp} + assert.Equal(t, name.String(), "capture-name-node1-20250716123000UTC") +} diff --git a/pkg/capture/provider/network_capture_unix.go b/pkg/capture/provider/network_capture_unix.go index c6b966713d..4cabcc581b 100644 --- a/pkg/capture/provider/network_capture_unix.go +++ b/pkg/capture/provider/network_capture_unix.go @@ -111,7 +111,7 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(ctx context.Context, fil defer cancel() filename := file.CaptureFilename{CaptureName: ncp.CaptureName, NodeHostname: ncp.NodeHostName, StartTimestamp: ncp.StartTimestamp} - captureFileName := fmt.Sprintf("%s.pcap", filename) + captureFileName := fmt.Sprintf("%s.pcap", filename.String()) captureFilePath := filepath.Join(ncp.TmpCaptureDir, captureFileName) // Remove the folder in case it already exists to mislead the file size check. From 88edb44b268f9f9ce2a512b8d5aa3f3f41c2c160 Mon Sep 17 00:00:00 2001 From: Kamil Date: Wed, 16 Jul 2025 22:50:32 +0100 Subject: [PATCH 2/4] timestamp tests Signed-off-by: Kamil --- pkg/capture/file/timestamp.go | 7 +- pkg/capture/file/timestamp_test.go | 185 +++++++++++++++++++++++++++++ 2 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 pkg/capture/file/timestamp_test.go diff --git a/pkg/capture/file/timestamp.go b/pkg/capture/file/timestamp.go index 553dcafc01..946a0fc945 100644 --- a/pkg/capture/file/timestamp.go +++ b/pkg/capture/file/timestamp.go @@ -23,6 +23,11 @@ func StringToTime(timestamp string) (*metav1.Time, error) { } // Converts a metav1.Time to a string in the capture file name format +// Returns a zero time string if timestamp is nil +// Converts to UTC if other timezone is provided func TimeToString(timestamp *metav1.Time) string { - return timestamp.Format(captureFileNameTimestampFormat) + if timestamp == nil { + return (&metav1.Time{Time: time.Time{}}).Format(captureFileNameTimestampFormat) + } + return timestamp.UTC().Format(captureFileNameTimestampFormat) } diff --git a/pkg/capture/file/timestamp_test.go b/pkg/capture/file/timestamp_test.go new file mode 100644 index 0000000000..f6d5e99fdc --- /dev/null +++ b/pkg/capture/file/timestamp_test.go @@ -0,0 +1,185 @@ +package file + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNow(t *testing.T) { + before := time.Now().UTC().Truncate(time.Second) + result := Now() + after := time.Now().UTC().Truncate(time.Second) + + require.NotNil(t, result) + assert.GreaterOrEqual(t, result.Time, before) + assert.LessOrEqual(t, result.Time, after) + assert.Equal(t, 0, result.Time.Nanosecond()) // ensure timestamp is truncated +} + +func TestStringToTime(t *testing.T) { + tests := []struct { + name string + input string + expected *metav1.Time + wantError bool + }{ + { + name: "valid timestamp", + input: "20250101120000UTC", + expected: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)}, + wantError: false, + }, + { + name: "another valid timestamp", + input: "20251231235959UTC", + expected: &metav1.Time{Time: time.Date(2025, 12, 31, 23, 59, 59, 0, time.UTC)}, + wantError: false, + }, + { + name: "midnight timestamp", + input: "20250101000000UTC", + expected: &metav1.Time{Time: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)}, + wantError: false, + }, + { + name: "invalid format", + input: "2025-01-01 12:00:00", + expected: nil, + wantError: true, + }, + { + name: "empty string", + input: "", + expected: nil, + wantError: true, + }, + { + name: "invalid month", + input: "20251301120000UTC", + expected: nil, + wantError: true, + }, + { + name: "invalid day", + input: "20250132120000UTC", + expected: nil, + wantError: true, + }, + { + name: "invalid hour", + input: "20250101250000UTC", + expected: nil, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := StringToTime(tt.input) + if tt.wantError { + assert.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, tt.expected, result) + } else { + assert.NoError(t, err) + require.NotNil(t, result) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestTimeToString(t *testing.T) { + tests := []struct { + name string + input *metav1.Time + expected string + }{ + { + name: "valid time", + input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)}, + expected: "20250101120000UTC", + }, + { + name: "invalid month", + input: &metav1.Time{Time: time.Date(2025, 13, 1, 12, 0, 0, 0, time.UTC)}, + expected: "20260101120000UTC", + }, + { + name: "invalid day", + input: &metav1.Time{Time: time.Date(2025, 1, 32, 12, 0, 0, 0, time.UTC)}, + expected: "20250201120000UTC", + }, + { + name: "invalid hour", + input: &metav1.Time{Time: time.Date(2025, 1, 1, 25, 0, 0, 0, time.UTC)}, + expected: "20250102010000UTC", + }, + { + name: "invalid minutes", + input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 61, 0, 0, time.UTC)}, + expected: "20250101130100UTC", + }, + { + name: "invalid seconds", + input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 61, 0, time.UTC)}, + expected: "20250101120101UTC", + }, + { + name: "midnight", + input: &metav1.Time{Time: time.Date(2025, 12, 1, 0, 0, 0, 0, time.UTC)}, + expected: "20251201000000UTC", + }, + { + name: "end of day", + input: &metav1.Time{Time: time.Date(2025, 1, 1, 23, 59, 59, 0, time.UTC)}, + expected: "20250101235959UTC", + }, + { + name: "nil time pointer", + input: nil, // Should return a zero time string + expected: "00010101000000UTC", + }, + { + name: "zero time", + input: &metav1.Time{Time: time.Time{}}, // Same as nil case + expected: "00010101000000UTC", + }, + { + name: "time with different timezone", + input: &metav1.Time{Time: time.Date(2025, 6, 15, 14, 30, 45, 0, time.FixedZone("EST", -5*60*60))}, + expected: "20250615193045UTC", // Should be converted to UTC + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := TimeToString(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestTimeToStringAndBack(t *testing.T) { + originalTime := &metav1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)} + + timeString := TimeToString(originalTime) + parsedTime, err := StringToTime(timeString) + + require.NoError(t, err) + assert.True(t, originalTime.Time.Equal(parsedTime.Time)) +} + +func TestStringToTimeAndBack(t *testing.T) { + originalString := "20250101123000UTC" + + parsedTime, err := StringToTime(originalString) + require.NoError(t, err) + + timeString := TimeToString(parsedTime) + assert.Equal(t, originalString, timeString) +} From acb3d2c4b91dab19bf9fe0f7051fbb5c40e2d19f Mon Sep 17 00:00:00 2001 From: Kamil Date: Wed, 16 Jul 2025 22:59:37 +0100 Subject: [PATCH 3/4] filename tests Signed-off-by: Kamil --- pkg/capture/file/capture_filename_test.go | 76 +++++++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/pkg/capture/file/capture_filename_test.go b/pkg/capture/file/capture_filename_test.go index 9f18a12357..fc2327d4da 100644 --- a/pkg/capture/file/capture_filename_test.go +++ b/pkg/capture/file/capture_filename_test.go @@ -11,10 +11,74 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func CaptureFilenameFormat(t *testing.T) { - captureName := "capture-name" - nodeHostName := "node1" - timestamp := v1.NewTime(time.Date(2025, 7, 16, 12, 30, 0, 0, time.UTC)) - name := CaptureFilename{CaptureName: captureName, NodeHostname: nodeHostName, StartTimestamp: ×tamp} - assert.Equal(t, name.String(), "capture-name-node1-20250716123000UTC") +func TestCaptureFilenameFormat(t *testing.T) { + tests := []struct { + name string + captureName string + nodeHostname string + timestamp *v1.Time + expected string + }{ + { + name: "valid filename", + captureName: "capture-name", + nodeHostname: "node1", + timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)}, + expected: "capture-name-node1-20250101123000UTC", + }, + { + name: "different timezone", + captureName: "capture-name", + nodeHostname: "node1", + timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 8, 30, 0, 0, time.FixedZone("EDT", -4*60*60))}, // 8:30 EDT is 12:30 UTC + expected: "capture-name-node1-20250101123000UTC", + }, + { + name: "empty capture name", + captureName: "", + nodeHostname: "node1", + timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)}, + expected: "-node1-20250101123000UTC", + }, + { + name: "empty node name", + captureName: "capture-name", + nodeHostname: "", + timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)}, + expected: "capture-name--20250101123000UTC", + }, + { + name: "zero time", + captureName: "capture-name", + nodeHostname: "node1", + timestamp: &v1.Time{Time: time.Time{}}, + expected: "capture-name-node1-00010101000000UTC", + }, + { + name: "nil timestamp", + captureName: "capture-name", + nodeHostname: "node1", + timestamp: nil, + expected: "capture-name-node1-00010101000000UTC", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + filename := CaptureFilename{ + CaptureName: tt.captureName, + NodeHostname: tt.nodeHostname, + StartTimestamp: tt.timestamp, + } + + // .String() here relies on TimeToString(), which should handle nil timestamps gracefully + // and return a zero time string - this should not panic + var result string + assert.NotPanics(t, func() { + result = filename.String() + }, "CaptureFilename.String() should handle nil timestamp gracefully") + + assert.Equal(t, tt.expected, result) + }) + } } From 38bf3072eb8d1ce7d62134350988f76c89808c14 Mon Sep 17 00:00:00 2001 From: Kamil Date: Thu, 17 Jul 2025 09:55:28 +0100 Subject: [PATCH 4/4] linting Signed-off-by: Kamil --- pkg/capture/file/timestamp_test.go | 6 +++--- pkg/capture/provider/network_capture_unix.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/capture/file/timestamp_test.go b/pkg/capture/file/timestamp_test.go index f6d5e99fdc..21ad85a3bb 100644 --- a/pkg/capture/file/timestamp_test.go +++ b/pkg/capture/file/timestamp_test.go @@ -81,12 +81,12 @@ func TestStringToTime(t *testing.T) { t.Run(tt.name, func(t *testing.T) { result, err := StringToTime(tt.input) if tt.wantError { - assert.Error(t, err) + require.Error(t, err) assert.Nil(t, result) assert.Equal(t, tt.expected, result) } else { - assert.NoError(t, err) - require.NotNil(t, result) + require.NoError(t, err) + assert.NotNil(t, result) assert.Equal(t, tt.expected, result) } }) diff --git a/pkg/capture/provider/network_capture_unix.go b/pkg/capture/provider/network_capture_unix.go index 4cabcc581b..04081fba53 100644 --- a/pkg/capture/provider/network_capture_unix.go +++ b/pkg/capture/provider/network_capture_unix.go @@ -111,7 +111,7 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(ctx context.Context, fil defer cancel() filename := file.CaptureFilename{CaptureName: ncp.CaptureName, NodeHostname: ncp.NodeHostName, StartTimestamp: ncp.StartTimestamp} - captureFileName := fmt.Sprintf("%s.pcap", filename.String()) + captureFileName := filename.String() + ".pcap" captureFilePath := filepath.Join(ncp.TmpCaptureDir, captureFileName) // Remove the folder in case it already exists to mislead the file size check.