Skip to content

Commit 3bfff1a

Browse files
committed
refactor record.go, address linting items
Signed-off-by: Eric Pickard <piceri@github.com>
1 parent a48254e commit 3bfff1a

8 files changed

Lines changed: 101 additions & 94 deletions

File tree

internal/controller/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ type ttlCache interface {
4848
}
4949

5050
type deploymentRecordPoster interface {
51-
PostOne(ctx context.Context, record *deploymentrecord.DeploymentRecord) error
52-
PostCluster(ctx context.Context, records []*deploymentrecord.DeploymentRecord, cluster string) ([]byte, error)
51+
PostOne(ctx context.Context, record *deploymentrecord.Record) error
52+
PostCluster(ctx context.Context, records []*deploymentrecord.Record, cluster string) ([]byte, error)
5353
}
5454

5555
type podMetadataAggregator interface {

internal/controller/controller_integration_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,23 @@ import (
2626

2727
type mockRecordPoster struct {
2828
mu sync.Mutex
29-
records []*deploymentrecord.DeploymentRecord
29+
records []*deploymentrecord.Record
3030
err error // to simulate failures
3131
}
3232

33-
func (m *mockRecordPoster) PostOne(_ context.Context, record *deploymentrecord.DeploymentRecord) error {
33+
func (m *mockRecordPoster) PostOne(_ context.Context, record *deploymentrecord.Record) error {
3434
m.mu.Lock()
3535
defer m.mu.Unlock()
3636
m.records = append(m.records, record)
3737
return m.err
3838
}
3939

40-
func (m *mockRecordPoster) PostCluster(_ context.Context, _ []*deploymentrecord.DeploymentRecord, _ string) ([]byte, error) {
40+
func (m *mockRecordPoster) PostCluster(_ context.Context, _ []*deploymentrecord.Record, _ string) ([]byte, error) {
4141
return nil, nil
4242
}
4343

4444
// Helper that allows tests to read captured records safely.
45-
func (m *mockRecordPoster) getRecords() []*deploymentrecord.DeploymentRecord {
45+
func (m *mockRecordPoster) getRecords() []*deploymentrecord.Record {
4646
m.mu.Lock()
4747
defer m.mu.Unlock()
4848
return slices.Clone(m.records)

internal/controller/controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ type mockPoster struct {
3333
clusterErr error
3434
}
3535

36-
func (m *mockPoster) PostOne(_ context.Context, _ *deploymentrecord.DeploymentRecord) error {
36+
func (m *mockPoster) PostOne(_ context.Context, _ *deploymentrecord.Record) error {
3737
m.mu.Lock()
3838
defer m.mu.Unlock()
3939
m.calls++
4040
return m.lastErr
4141
}
4242

43-
func (m *mockPoster) PostCluster(_ context.Context, records []*deploymentrecord.DeploymentRecord, _ string) ([]byte, error) {
43+
func (m *mockPoster) PostCluster(_ context.Context, records []*deploymentrecord.Record, _ string) ([]byte, error) {
4444
m.mu.Lock()
4545
defer m.mu.Unlock()
4646
m.clusterCalls++

internal/controller/reporting.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (c *Controller) processEvent(ctx context.Context, event PodEvent) error {
117117
return lastErr
118118
}
119119

120-
func (c *Controller) processSyncEvents(ctx context.Context, syncClusterPods []interface{}) error {
120+
func (c *Controller) processSyncEvents(ctx context.Context, syncClusterPods []any) error {
121121
syncRecords := c.makeSyncRecords(ctx, syncClusterPods)
122122
if len(syncRecords) == 0 {
123123
slog.Info("No sync records to post")
@@ -139,7 +139,7 @@ func (c *Controller) processSyncEvents(ctx context.Context, syncClusterPods []in
139139
)
140140
return fmt.Errorf("failed to post sync cluster records: %w", err)
141141
}
142-
var deploymentRecords deploymentrecord.DeploymentRecordsClusterResp
142+
var deploymentRecords deploymentrecord.RecordsClusterResp
143143
err = json.Unmarshal(respBody, &deploymentRecords)
144144
if err != nil {
145145
slog.Error("Failed to unmarshall response",
@@ -157,9 +157,9 @@ func (c *Controller) processSyncEvents(ctx context.Context, syncClusterPods []in
157157
return nil
158158
}
159159

160-
func (c *Controller) makeSyncRecords(ctx context.Context, syncClusterPods []interface{}) []*deploymentrecord.DeploymentRecord {
160+
func (c *Controller) makeSyncRecords(ctx context.Context, syncClusterPods []any) []*deploymentrecord.Record {
161161
seenSyncRecords := make(map[string]bool)
162-
var syncRecords []*deploymentrecord.DeploymentRecord
162+
var syncRecords []*deploymentrecord.Record
163163
for _, p := range syncClusterPods {
164164
pod, ok := p.(*corev1.Pod)
165165
if !ok {
@@ -219,7 +219,7 @@ func (c *Controller) makeSyncRecords(ctx context.Context, syncClusterPods []inte
219219
return syncRecords
220220
}
221221

222-
func (c *Controller) fillCaches(deploymentRecords deploymentrecord.DeploymentRecordsClusterResp) {
222+
func (c *Controller) fillCaches(deploymentRecords deploymentrecord.RecordsClusterResp) {
223223
slog.Info("Filling caches after posting sync cluster records")
224224
// Fill observedDeployments cache with successful digests
225225
for _, r := range deploymentRecords.DeploymentRecords {
@@ -348,7 +348,7 @@ func (c *Controller) recordContainer(ctx context.Context, pod *corev1.Pod, conta
348348
return nil
349349
}
350350

351-
func (c *Controller) buildRecord(pod *corev1.Pod, container corev1.Container, eventType, workloadName string, aggPodMetadata *metadata.AggregatePodMetadata) *deploymentrecord.DeploymentRecord {
351+
func (c *Controller) buildRecord(pod *corev1.Pod, container corev1.Container, eventType, workloadName string, aggPodMetadata *metadata.AggregatePodMetadata) *deploymentrecord.Record {
352352
dn := getARDeploymentName(pod, container, c.cfg.Template, workloadName)
353353
digest := getContainerDigest(pod, container.Name)
354354

internal/controller/reporting_test.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package controller
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"testing"
89
"time"
@@ -120,7 +121,7 @@ func TestProcessSyncEvents_EmptyPodList(t *testing.T) {
120121
poster := &mockPoster{}
121122
ctrl := newTestController(poster)
122123

123-
err := ctrl.processSyncEvents(context.Background(), []interface{}{})
124+
err := ctrl.processSyncEvents(context.Background(), []any{})
124125
require.NoError(t, err)
125126
assert.Equal(t, 0, poster.getPostClusterCalls(), "PostCluster should not be called for empty pod list")
126127
}
@@ -130,28 +131,28 @@ func TestProcessSyncEvents_HappyPath(t *testing.T) {
130131
digest := "sha256:abc123"
131132
unknownDigest := "sha256:notfound999"
132133
unauthorizedDigest := "sha256:unauthorized999"
133-
clusterResp := deploymentrecord.DeploymentRecordsClusterResp{
134+
clusterResp := deploymentrecord.RecordsClusterResp{
134135
TotalCount: 1,
135-
DeploymentRecords: []*deploymentrecord.DeploymentRecordResp{{
136-
DeploymentRecord: deploymentrecord.DeploymentRecord{
137-
DeploymentRecordBase: deploymentrecord.DeploymentRecordBase{
136+
DeploymentRecords: []*deploymentrecord.RecordResp{{
137+
Record: deploymentrecord.Record{
138+
BaseRecord: deploymentrecord.BaseRecord{
138139
DeploymentName: "default/test-deploy/app",
139140
Digest: digest,
140141
},
141142
},
142143
}},
143-
Errors: []*deploymentrecord.DeploymentRecordErrorResp{
144+
Errors: []*deploymentrecord.RecordErrorResp{
144145
{
145-
DeploymentRecord: deploymentrecord.DeploymentRecord{
146-
DeploymentRecordBase: deploymentrecord.DeploymentRecordBase{
146+
Record: deploymentrecord.Record{
147+
BaseRecord: deploymentrecord.BaseRecord{
147148
Digest: unknownDigest,
148149
},
149150
},
150151
Cause: "not_found",
151152
},
152153
{
153-
DeploymentRecord: deploymentrecord.DeploymentRecord{
154-
DeploymentRecordBase: deploymentrecord.DeploymentRecordBase{
154+
Record: deploymentrecord.Record{
155+
BaseRecord: deploymentrecord.BaseRecord{
155156
Digest: unauthorizedDigest,
156157
},
157158
},
@@ -166,10 +167,10 @@ func TestProcessSyncEvents_HappyPath(t *testing.T) {
166167
ctrl := newTestController(poster)
167168
ctrl.workloadResolver = &mockResolver{name: "test-deploy"}
168169

169-
err = ctrl.processSyncEvents(context.Background(), []interface{}{
170-
makePod("app", "test-deploy-abc123", digest, "ReplicaSet"),
171-
makePod("unknown", "test-deploy-abc123", unknownDigest, "ReplicaSet"),
172-
makePod("unauthorized", "test-deploy-abc123", unauthorizedDigest, "ReplicaSet"),
170+
err = ctrl.processSyncEvents(context.Background(), []any{
171+
makeTestPod("app", "test-deploy-abc123", digest, "ReplicaSet"),
172+
makeTestPod("unknown", "test-deploy-abc456", unknownDigest, "ReplicaSet"),
173+
makeTestPod("unauthorized", "test-deploy-abc789", unauthorizedDigest, "Job"),
173174
})
174175
require.NoError(t, err)
175176
assert.Equal(t, 1, poster.getPostClusterCalls(), "PostCluster should be called once")
@@ -196,9 +197,9 @@ func TestProcessSyncEvents_DedupeContainers(t *testing.T) {
196197
ctrl := newTestController(poster)
197198
ctrl.workloadResolver = &mockResolver{name: "test-deploy"}
198199

199-
pod := makePod("app", "test-deploy-abc123", digest, "ReplicaSet")
200+
pod := makeTestPod("app", "test-deploy-abc123", digest, "ReplicaSet")
200201

201-
err := ctrl.processSyncEvents(context.Background(), []interface{}{pod, pod})
202+
err := ctrl.processSyncEvents(context.Background(), []any{pod, pod})
202203
require.NoError(t, err)
203204
assert.Equal(t, 1, poster.getPostClusterCalls(), "PostCluster should be called once")
204205
assert.Equal(t, 1, poster.clusterRecordCount, "PostCluster should receive only 1 record")
@@ -211,9 +212,9 @@ func TestProcessSyncEvents_PostCluster404(t *testing.T) {
211212
}
212213
ctrl := newTestController(poster)
213214
ctrl.workloadResolver = &mockResolver{name: "test-deploy"}
214-
pod := makePod("app", "test-deploy-abc123", "sha256:abc123", "ReplicaSet")
215+
pod := makeTestPod("app", "test-deploy-abc123", "sha256:abc123", "ReplicaSet")
215216

216-
err := ctrl.processSyncEvents(context.Background(), []interface{}{pod})
217+
err := ctrl.processSyncEvents(context.Background(), []any{pod})
217218
require.NoError(t, err, "ClusterNoRepositoriesError should not propagate")
218219
assert.Equal(t, 1, poster.getPostClusterCalls())
219220

@@ -226,19 +227,19 @@ func TestProcessSyncEvents_PostCluster404(t *testing.T) {
226227
func TestProcessSyncEvents_PostCluster500(t *testing.T) {
227228
t.Parallel()
228229
poster := &mockPoster{
229-
clusterErr: fmt.Errorf("server error"),
230+
clusterErr: errors.New("server error"),
230231
}
231232
ctrl := newTestController(poster)
232233
ctrl.workloadResolver = &mockResolver{name: "test-deploy"}
233-
pod := makePod("app", "test-deploy-abc123", "sha256:abc123", "ReplicaSet")
234+
pod := makeTestPod("app", "test-deploy-abc123", "sha256:abc123", "ReplicaSet")
234235

235-
err := ctrl.processSyncEvents(context.Background(), []interface{}{pod})
236+
err := ctrl.processSyncEvents(context.Background(), []any{pod})
236237
require.Error(t, err)
237238
assert.Contains(t, err.Error(), "failed to post sync cluster records")
238239
assert.Equal(t, 1, poster.getPostClusterCalls())
239240
}
240241

241-
func makePod(containerName string, parentName string, digest string, parentKind string) *corev1.Pod {
242+
func makeTestPod(containerName string, parentName string, digest string, parentKind string) *corev1.Pod {
242243
return &corev1.Pod{
243244
ObjectMeta: metav1.ObjectMeta{
244245
Name: "test-pod",

pkg/deploymentrecord/client.go

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ func (c *ClientError) Unwrap() error {
165165
return c.err
166166
}
167167

168+
// ClusterNoRepositoriesError represents a 404 response from the cluster endpoint
169+
// indicating no repositories were found for the given cluster.
168170
type ClusterNoRepositoriesError struct {
169171
err error
170172
}
@@ -195,27 +197,27 @@ func (n *NoArtifactError) Unwrap() error {
195197

196198
// PostOne posts a single deployment record to the GitHub deployment
197199
// records API.
198-
func (c *Client) PostOne(ctx context.Context, record *DeploymentRecord) error {
200+
func (c *Client) PostOne(ctx context.Context, record *Record) error {
199201
if record == nil {
200202
return errors.New("record cannot be nil")
201203
}
202204

203-
singleUrl := fmt.Sprintf("%s/orgs/%s/artifacts/metadata/deployment-record", c.baseURL, c.org)
205+
singleURL := fmt.Sprintf("%s/orgs/%s/artifacts/metadata/deployment-record", c.baseURL, c.org)
204206

205207
body, err := buildRequestBody(record)
206208
if err != nil {
207209
return fmt.Errorf("failed to marshal record: %w", err)
208210
}
209211

210-
respBody, statusCode, lastErr := c.postWithRetry(ctx, singleUrl, body)
212+
respBody, statusCode, lastErr := c.postWithRetry(ctx, singleURL, body)
211213

212214
var clientErr *ClientError
213215
switch {
214216
case errors.As(lastErr, &clientErr):
215217
dtmetrics.PostDeploymentRecordClientError.Inc()
216218
slog.Warn("client error, aborting",
217219
"status_code", statusCode,
218-
"url", singleUrl,
220+
"url", singleURL,
219221
"resp_msg", string(respBody),
220222
)
221223
return fmt.Errorf("client error: %w", lastErr)
@@ -244,28 +246,28 @@ func (c *Client) PostOne(ctx context.Context, record *DeploymentRecord) error {
244246

245247
// PostCluster sends a full cluster state of records to GitHub deployment
246248
// records cluster API.
247-
func (c *Client) PostCluster(ctx context.Context, records []*DeploymentRecord, cluster string) ([]byte, error) {
248-
if len(records) <= 0 {
249+
func (c *Client) PostCluster(ctx context.Context, records []*Record, cluster string) ([]byte, error) {
250+
if len(records) == 0 {
249251
slog.Debug("Records is empty, skipping")
250252
return nil, nil
251253
}
252254

253-
clusterUrl := fmt.Sprintf("%s/orgs/%s/artifacts/metadata/deployment-record/cluster/%s", c.baseURL, c.org, url.PathEscape(cluster))
255+
clusterURL := fmt.Sprintf("%s/orgs/%s/artifacts/metadata/deployment-record/cluster/%s", c.baseURL, c.org, url.PathEscape(cluster))
254256

255257
body, err := buildClusterRequestBody(records)
256258
if err != nil {
257259
return nil, fmt.Errorf("failed to marshal records: %w", err)
258260
}
259261

260-
respBody, statusCode, lastErr := c.postWithRetry(ctx, clusterUrl, body)
262+
respBody, statusCode, lastErr := c.postWithRetry(ctx, clusterURL, body)
261263

262264
var clientErr *ClientError
263265
switch {
264266
case errors.As(lastErr, &clientErr):
265267
dtmetrics.PostDeploymentRecordClientError.Inc()
266268
slog.Warn("client error, aborting",
267269
"status_code", statusCode,
268-
"url", clusterUrl,
270+
"url", clusterURL,
269271
"resp_msg", string(respBody),
270272
)
271273
return nil, fmt.Errorf("client error: %w", lastErr)
@@ -274,7 +276,7 @@ func (c *Client) PostCluster(ctx context.Context, records []*DeploymentRecord, c
274276
return respBody, nil
275277
case statusCode == 404:
276278
dtmetrics.PostDeploymentRecordUnknownArtifact.Inc()
277-
return nil, &ClusterNoRepositoriesError{err: fmt.Errorf("no repositories found")}
279+
return nil, &ClusterNoRepositoriesError{err: errors.New("no repositories found")}
278280
default:
279281
dtmetrics.PostDeploymentRecordHardFail.Inc()
280282
slog.Error("all retries exhausted",
@@ -286,7 +288,7 @@ func (c *Client) PostCluster(ctx context.Context, records []*DeploymentRecord, c
286288
}
287289
}
288290

289-
func (c *Client) postWithRetry(ctx context.Context, url string, body []byte) ([]byte, int, error) {
291+
func (c *Client) postWithRetry(ctx context.Context, targetURL string, body []byte) ([]byte, int, error) {
290292
bodyReader := bytes.NewReader(body)
291293
var lastErr error
292294
// The first attempt is not a retry!
@@ -306,7 +308,7 @@ func (c *Client) postWithRetry(ctx context.Context, url string, body []byte) ([]
306308
// Reset reader position for retries
307309
bodyReader.Reset(body)
308310

309-
req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bodyReader)
311+
req, err := http.NewRequestWithContext(ctx, http.MethodPost, targetURL, bodyReader)
310312
if err != nil {
311313
return nil, 0, fmt.Errorf("failed to create request: %w", err)
312314
}
@@ -363,7 +365,7 @@ func (c *Client) postWithRetry(ctx context.Context, url string, body []byte) ([]
363365
"retry-after", resp.Header.Get("Retry-After"),
364366
"x-ratelimit-remaining", resp.Header.Get("X-Ratelimit-Remaining"),
365367
"retry_delay", retryDelay.Seconds(),
366-
"url", url,
368+
"url", targetURL,
367369
"resp_msg", string(respBody),
368370
)
369371
lastErr = fmt.Errorf("rate limited, attempt %d", attempt)
@@ -377,7 +379,7 @@ func (c *Client) postWithRetry(ctx context.Context, url string, body []byte) ([]
377379
slog.Debug("retriable error",
378380
"attempt", attempt,
379381
"status_code", resp.StatusCode,
380-
"url", url,
382+
"url", targetURL,
381383
"resp_msg", string(respBody),
382384
)
383385
lastErr = fmt.Errorf("server error, attempt %d", attempt)
@@ -463,29 +465,29 @@ func parseRateLimitDelay(resp *http.Response) time.Duration {
463465

464466
// buildRequestBody adds return_records=false to a deployment record request body
465467
// which results in a minimal response payload.
466-
func buildRequestBody(record *DeploymentRecord) ([]byte, error) {
468+
func buildRequestBody(record *Record) ([]byte, error) {
467469
return json.Marshal(struct {
468-
DeploymentRecord
470+
Record
469471
ReturnRecords bool `json:"return_records"`
470472
}{
471-
DeploymentRecord: *record,
472-
ReturnRecords: false,
473+
Record: *record,
474+
ReturnRecords: false,
473475
})
474476
}
475477

476-
// buildClusterRequestBody count the total records, builds DeploymentRecords,
478+
// buildClusterRequestBody count the total records, builds ClusterRecordsBody,
477479
// and returns []byte.
478-
func buildClusterRequestBody(records []*DeploymentRecord) ([]byte, error) {
479-
if len(records) <= 0 {
480+
func buildClusterRequestBody(records []*Record) ([]byte, error) {
481+
if len(records) == 0 {
480482
return nil, nil
481483
}
482-
deploymentRecords := []DeploymentRecordBase{}
484+
deploymentRecords := []BaseRecord{}
483485

484486
for _, r := range records {
485-
deploymentRecords = append(deploymentRecords, r.DeploymentRecordBase)
487+
deploymentRecords = append(deploymentRecords, r.BaseRecord)
486488
}
487489

488-
return json.Marshal(DeploymentRecords{
490+
return json.Marshal(ClusterRecordsBody{
489491
LogicalEnvironment: records[0].LogicalEnvironment,
490492
PhysicalEnvironment: records[0].PhysicalEnvironment,
491493
PartialSuccess: true,

0 commit comments

Comments
 (0)