Skip to content

Commit 335c6db

Browse files
authored
fix: pcap file name formatting (#1749)
# Description Fix the bad formatting of the pcap file names when running `retina capture create`. Also adding testing for the utilities around how file names are created. ## Related Issue #1732 ## Checklist - [x] I have read the [contributing documentation](https://retina.sh/docs/Contributing/overview). - [x] I signed and signed-off the commits (`git commit -S -s ...`). See [this documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification) on signing commits. - [x] I have correctly attributed the author(s) of the code. - [x] I have tested the changes locally. - [x] I have followed the project's style guidelines. - [x] I have updated the documentation, if necessary. - [x] I have added tests, if applicable. ## Screenshots (if applicable) or Testing Completed <img width="450" height="47" alt="{D77C22CD-6FFA-44A0-B15B-B89E4542A2A2}" src="https://github.com/user-attachments/assets/306fa604-0162-49c8-bb07-0645df04ea84" /> The below shows a successful download of a packet capture which is then extracted. The final grep shows the file name of the `.pcap` which is in the same format as the `.tar.gz`. ``` ~/src/retina git:kamilp/capture-filename-format > k retina capture download --name pcap-filename -o ./downloads File to be downloaded: /host/mnt/retina/captures/pcap-filename-aks-agentpool-33289083-vmss000000-20250716220911UTC.tar.gz Creating download pod: pcap-filename-download-d49wc Obtaining file... Bytes retrieved: 189453 File written to: downloads/pcap-filename/pcap-filename-aks-agentpool-33289083-vmss000000-20250716220911UTC.tar.gz File to be downloaded: /host/mnt/retina/captures/pcap-filename-aks-agentpool-33289083-vmss000001-20250716220911UTC.tar.gz Creating download pod: pcap-filename-download-t86dl Obtaining file... Bytes retrieved: 277310 File written to: downloads/pcap-filename/pcap-filename-aks-agentpool-33289083-vmss000001-20250716220911UTC.tar.gz ~/src/retina git:kamilp/capture-filename-format* > cd ./downloads/ ~/src/retina/downloads git:kamilp/capture-filename-format* > mkdir extract ~/src/retina/downloads git:kamilp/capture-filename-format* > tar -xvf pcap-filename/pcap-filename-aks-agentpool-33289083-vmss000001-20250716220911UTC.tar.gz -C extract/ ~/src/retina/downloads git:kamilp/capture-filename-format* > cd extract/ ~/src/retina/downloads/extract git:kamilp/capture-filename-format* > ll | grep ".pcap" .rw-r--r-- kamilp 557 KB 2025-07-16 23:09:23 pcap-filename-aks-agentpool-33289083-vmss000001-20250716220911UTC.pcap --------- Signed-off-by: Kamil <kamil.prz@gmail.com>
1 parent 183c099 commit 335c6db

4 files changed

Lines changed: 276 additions & 2 deletions

File tree

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package file
5+
6+
import (
7+
"testing"
8+
"time"
9+
10+
"github.com/stretchr/testify/assert"
11+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
)
13+
14+
func TestCaptureFilenameFormat(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
captureName string
18+
nodeHostname string
19+
timestamp *v1.Time
20+
expected string
21+
}{
22+
{
23+
name: "valid filename",
24+
captureName: "capture-name",
25+
nodeHostname: "node1",
26+
timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)},
27+
expected: "capture-name-node1-20250101123000UTC",
28+
},
29+
{
30+
name: "different timezone",
31+
captureName: "capture-name",
32+
nodeHostname: "node1",
33+
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
34+
expected: "capture-name-node1-20250101123000UTC",
35+
},
36+
{
37+
name: "empty capture name",
38+
captureName: "",
39+
nodeHostname: "node1",
40+
timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)},
41+
expected: "-node1-20250101123000UTC",
42+
},
43+
{
44+
name: "empty node name",
45+
captureName: "capture-name",
46+
nodeHostname: "",
47+
timestamp: &v1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)},
48+
expected: "capture-name--20250101123000UTC",
49+
},
50+
{
51+
name: "zero time",
52+
captureName: "capture-name",
53+
nodeHostname: "node1",
54+
timestamp: &v1.Time{Time: time.Time{}},
55+
expected: "capture-name-node1-00010101000000UTC",
56+
},
57+
{
58+
name: "nil timestamp",
59+
captureName: "capture-name",
60+
nodeHostname: "node1",
61+
timestamp: nil,
62+
expected: "capture-name-node1-00010101000000UTC",
63+
},
64+
}
65+
66+
for _, tt := range tests {
67+
t.Run(tt.name, func(t *testing.T) {
68+
filename := CaptureFilename{
69+
CaptureName: tt.captureName,
70+
NodeHostname: tt.nodeHostname,
71+
StartTimestamp: tt.timestamp,
72+
}
73+
74+
// .String() here relies on TimeToString(), which should handle nil timestamps gracefully
75+
// and return a zero time string - this should not panic
76+
var result string
77+
assert.NotPanics(t, func() {
78+
result = filename.String()
79+
}, "CaptureFilename.String() should handle nil timestamp gracefully")
80+
81+
assert.Equal(t, tt.expected, result)
82+
})
83+
}
84+
}

pkg/capture/file/timestamp.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ func StringToTime(timestamp string) (*metav1.Time, error) {
2323
}
2424

2525
// Converts a metav1.Time to a string in the capture file name format
26+
// Returns a zero time string if timestamp is nil
27+
// Converts to UTC if other timezone is provided
2628
func TimeToString(timestamp *metav1.Time) string {
27-
return timestamp.Format(captureFileNameTimestampFormat)
29+
if timestamp == nil {
30+
return (&metav1.Time{Time: time.Time{}}).Format(captureFileNameTimestampFormat)
31+
}
32+
return timestamp.UTC().Format(captureFileNameTimestampFormat)
2833
}

pkg/capture/file/timestamp_test.go

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
package file
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
)
11+
12+
func TestNow(t *testing.T) {
13+
before := time.Now().UTC().Truncate(time.Second)
14+
result := Now()
15+
after := time.Now().UTC().Truncate(time.Second)
16+
17+
require.NotNil(t, result)
18+
assert.GreaterOrEqual(t, result.Time, before)
19+
assert.LessOrEqual(t, result.Time, after)
20+
assert.Equal(t, 0, result.Time.Nanosecond()) // ensure timestamp is truncated
21+
}
22+
23+
func TestStringToTime(t *testing.T) {
24+
tests := []struct {
25+
name string
26+
input string
27+
expected *metav1.Time
28+
wantError bool
29+
}{
30+
{
31+
name: "valid timestamp",
32+
input: "20250101120000UTC",
33+
expected: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)},
34+
wantError: false,
35+
},
36+
{
37+
name: "another valid timestamp",
38+
input: "20251231235959UTC",
39+
expected: &metav1.Time{Time: time.Date(2025, 12, 31, 23, 59, 59, 0, time.UTC)},
40+
wantError: false,
41+
},
42+
{
43+
name: "midnight timestamp",
44+
input: "20250101000000UTC",
45+
expected: &metav1.Time{Time: time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)},
46+
wantError: false,
47+
},
48+
{
49+
name: "invalid format",
50+
input: "2025-01-01 12:00:00",
51+
expected: nil,
52+
wantError: true,
53+
},
54+
{
55+
name: "empty string",
56+
input: "",
57+
expected: nil,
58+
wantError: true,
59+
},
60+
{
61+
name: "invalid month",
62+
input: "20251301120000UTC",
63+
expected: nil,
64+
wantError: true,
65+
},
66+
{
67+
name: "invalid day",
68+
input: "20250132120000UTC",
69+
expected: nil,
70+
wantError: true,
71+
},
72+
{
73+
name: "invalid hour",
74+
input: "20250101250000UTC",
75+
expected: nil,
76+
wantError: true,
77+
},
78+
}
79+
80+
for _, tt := range tests {
81+
t.Run(tt.name, func(t *testing.T) {
82+
result, err := StringToTime(tt.input)
83+
if tt.wantError {
84+
require.Error(t, err)
85+
assert.Nil(t, result)
86+
assert.Equal(t, tt.expected, result)
87+
} else {
88+
require.NoError(t, err)
89+
assert.NotNil(t, result)
90+
assert.Equal(t, tt.expected, result)
91+
}
92+
})
93+
}
94+
}
95+
96+
func TestTimeToString(t *testing.T) {
97+
tests := []struct {
98+
name string
99+
input *metav1.Time
100+
expected string
101+
}{
102+
{
103+
name: "valid time",
104+
input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)},
105+
expected: "20250101120000UTC",
106+
},
107+
{
108+
name: "invalid month",
109+
input: &metav1.Time{Time: time.Date(2025, 13, 1, 12, 0, 0, 0, time.UTC)},
110+
expected: "20260101120000UTC",
111+
},
112+
{
113+
name: "invalid day",
114+
input: &metav1.Time{Time: time.Date(2025, 1, 32, 12, 0, 0, 0, time.UTC)},
115+
expected: "20250201120000UTC",
116+
},
117+
{
118+
name: "invalid hour",
119+
input: &metav1.Time{Time: time.Date(2025, 1, 1, 25, 0, 0, 0, time.UTC)},
120+
expected: "20250102010000UTC",
121+
},
122+
{
123+
name: "invalid minutes",
124+
input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 61, 0, 0, time.UTC)},
125+
expected: "20250101130100UTC",
126+
},
127+
{
128+
name: "invalid seconds",
129+
input: &metav1.Time{Time: time.Date(2025, 1, 1, 12, 0, 61, 0, time.UTC)},
130+
expected: "20250101120101UTC",
131+
},
132+
{
133+
name: "midnight",
134+
input: &metav1.Time{Time: time.Date(2025, 12, 1, 0, 0, 0, 0, time.UTC)},
135+
expected: "20251201000000UTC",
136+
},
137+
{
138+
name: "end of day",
139+
input: &metav1.Time{Time: time.Date(2025, 1, 1, 23, 59, 59, 0, time.UTC)},
140+
expected: "20250101235959UTC",
141+
},
142+
{
143+
name: "nil time pointer",
144+
input: nil, // Should return a zero time string
145+
expected: "00010101000000UTC",
146+
},
147+
{
148+
name: "zero time",
149+
input: &metav1.Time{Time: time.Time{}}, // Same as nil case
150+
expected: "00010101000000UTC",
151+
},
152+
{
153+
name: "time with different timezone",
154+
input: &metav1.Time{Time: time.Date(2025, 6, 15, 14, 30, 45, 0, time.FixedZone("EST", -5*60*60))},
155+
expected: "20250615193045UTC", // Should be converted to UTC
156+
},
157+
}
158+
159+
for _, tt := range tests {
160+
t.Run(tt.name, func(t *testing.T) {
161+
result := TimeToString(tt.input)
162+
assert.Equal(t, tt.expected, result)
163+
})
164+
}
165+
}
166+
167+
func TestTimeToStringAndBack(t *testing.T) {
168+
originalTime := &metav1.Time{Time: time.Date(2025, 1, 1, 12, 30, 0, 0, time.UTC)}
169+
170+
timeString := TimeToString(originalTime)
171+
parsedTime, err := StringToTime(timeString)
172+
173+
require.NoError(t, err)
174+
assert.True(t, originalTime.Time.Equal(parsedTime.Time))
175+
}
176+
177+
func TestStringToTimeAndBack(t *testing.T) {
178+
originalString := "20250101123000UTC"
179+
180+
parsedTime, err := StringToTime(originalString)
181+
require.NoError(t, err)
182+
183+
timeString := TimeToString(parsedTime)
184+
assert.Equal(t, originalString, timeString)
185+
}

pkg/capture/provider/network_capture_unix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func (ncp *NetworkCaptureProvider) CaptureNetworkPacket(ctx context.Context, fil
111111
defer cancel()
112112

113113
filename := file.CaptureFilename{CaptureName: ncp.CaptureName, NodeHostname: ncp.NodeHostName, StartTimestamp: ncp.StartTimestamp}
114-
captureFileName := fmt.Sprintf("%s.pcap", filename)
114+
captureFileName := filename.String() + ".pcap"
115115
captureFilePath := filepath.Join(ncp.TmpCaptureDir, captureFileName)
116116

117117
// Remove the folder in case it already exists to mislead the file size check.

0 commit comments

Comments
 (0)