Skip to content

Commit f8d219f

Browse files
Use atomic flare archive suffix
Co-authored-by: pducolin <45568537+pducolin@users.noreply.github.com>
1 parent f0c94d9 commit f8d219f

2 files changed

Lines changed: 13 additions & 61 deletions

File tree

comp/core/flare/helpers/builder.go

Lines changed: 12 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ package helpers
88

99
import (
1010
"context"
11-
"crypto/rand"
12-
"encoding/hex"
1311
"encoding/json"
1412
"errors"
1513
"fmt"
@@ -18,6 +16,7 @@ import (
1816
"regexp"
1917
"strings"
2018
"sync"
19+
"sync/atomic"
2120
"time"
2221

2322
"github.com/DataDog/datadog-agent/comp/core/flare/types"
@@ -30,10 +29,11 @@ import (
3029
)
3130

3231
const (
33-
filePerm = 0644
34-
ciJobIDEnvVar = "CI_JOB_ID"
32+
filePerm = 0644
3533
)
3634

35+
var archiveNameCounter atomic.Uint64
36+
3737
func newBuilder(root string, hostname string, localFlare bool, flareArgs types.FlareArgs) (*builder, error) {
3838
fb := &builder{
3939
tmpDir: root,
@@ -146,59 +146,24 @@ type builder struct {
146146
nonScrubbedFiles map[string]bool
147147
}
148148

149-
func getArchiveName() (string, error) {
150-
return getArchiveNameForTime(time.Now().UTC(), newArchiveNameID)
149+
func getArchiveName() string {
150+
return getArchiveNameForTime(time.Now().UTC(), newArchiveNameID())
151151
}
152152

153-
func getArchiveNameForTime(t time.Time, newID func() (string, error)) (string, error) {
153+
func getArchiveNameForTime(t time.Time, uniqueSuffix string) string {
154154
timeString := strings.ReplaceAll(t.Format(time.RFC3339), ":", "-")
155-
uniqueSuffix, err := newID()
156-
if err != nil {
157-
return "", fmt.Errorf("could not generate unique archive name suffix: %w", err)
158-
}
159155

160156
logLevel, err := log.GetLogLevel()
161157
logLevelString := ""
162158
if err == nil {
163159
logLevelString = "-" + logLevel.String()
164160
}
165161

166-
return fmt.Sprintf("datadog-agent-%s-%s%s.zip", timeString, uniqueSuffix, logLevelString), nil
167-
}
168-
169-
func newArchiveNameID() (string, error) {
170-
var randomBytes [8]byte
171-
if _, err := rand.Read(randomBytes[:]); err != nil {
172-
return "", err
173-
}
174-
return archiveNameID(os.Getenv(ciJobIDEnvVar), hex.EncodeToString(randomBytes[:])), nil
162+
return fmt.Sprintf("datadog-agent-%s-%s%s.zip", timeString, uniqueSuffix, logLevelString)
175163
}
176164

177-
func archiveNameID(ciJobID string, randomID string) string {
178-
ciJobID = sanitizeArchiveNamePart(ciJobID)
179-
if ciJobID == "" {
180-
return randomID
181-
}
182-
return fmt.Sprintf("job-%s-%s", ciJobID, randomID)
183-
}
184-
185-
func sanitizeArchiveNamePart(value string) string {
186-
var sanitized strings.Builder
187-
for _, r := range value {
188-
switch {
189-
case r >= 'a' && r <= 'z':
190-
sanitized.WriteRune(r)
191-
case r >= 'A' && r <= 'Z':
192-
sanitized.WriteRune(r)
193-
case r >= '0' && r <= '9':
194-
sanitized.WriteRune(r)
195-
case r == '-' || r == '_':
196-
sanitized.WriteRune(r)
197-
default:
198-
sanitized.WriteByte('-')
199-
}
200-
}
201-
return strings.Trim(sanitized.String(), "-")
165+
func newArchiveNameID() string {
166+
return fmt.Sprintf("%d-%d", os.Getpid(), archiveNameCounter.Add(1))
202167
}
203168

204169
func (fb *builder) Save() (string, error) {
@@ -243,17 +208,14 @@ func (fb *builder) Save() (string, error) {
243208
defer fb.Unlock()
244209
fb.isClosed = true
245210

246-
archiveName, err := getArchiveName()
247-
if err != nil {
248-
return "", err
249-
}
211+
archiveName := getArchiveName()
250212
archiveTmpPath := filepath.Join(fb.tmpDir, archiveName)
251213
archiveFinalPath := filepath.Join(os.TempDir(), archiveName)
252214

253215
// We first create the archive in our fb.tmpDir directory which is only readable by the current user (and
254216
// SYSTEM/ADMIN on Windows). Then we retrict the archive permissions before moving it to the system temporary
255217
// directory. This prevents other users from being able to read local flares.
256-
err = archive.Zip([]string{fb.flareDir}, archiveTmpPath)
218+
err := archive.Zip([]string{fb.flareDir}, archiveTmpPath)
257219
if err != nil {
258220
return "", err
259221
}

comp/core/flare/helpers/builder_test.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package helpers
88
import (
99
"context"
1010
"errors"
11-
"fmt"
1211
"os"
1312
"path/filepath"
1413
"testing"
@@ -128,23 +127,14 @@ func TestGetArchiveNameIsUniqueWithinSameSecond(t *testing.T) {
128127
names := make(map[string]struct{})
129128

130129
for range 100 {
131-
name, err := getArchiveNameForTime(timestamp, func() (string, error) {
132-
return fmt.Sprintf("unique-%d", len(names)), nil
133-
})
134-
require.NoError(t, err)
130+
name := getArchiveNameForTime(timestamp, newArchiveNameID())
135131
if _, found := names[name]; found {
136132
t.Fatalf("archive name %q was generated more than once", name)
137133
}
138134
names[name] = struct{}{}
139135
}
140136
}
141137

142-
func TestArchiveNameIDIncludesCIJobIDWhenAvailable(t *testing.T) {
143-
assert.Equal(t, "job-12345-abcdef", archiveNameID("12345", "abcdef"))
144-
assert.Equal(t, "job-123-45-abcdef", archiveNameID("123/45", "abcdef"))
145-
assert.Equal(t, "abcdef", archiveNameID("", "abcdef"))
146-
}
147-
148138
func TestAddFileFromFunc(t *testing.T) {
149139
fb := getNewBuilder(t)
150140
defer fb.clean()

0 commit comments

Comments
 (0)