Skip to content

Commit 0b91c01

Browse files
c4rloc4rlo
authored
Code modernizations via go fix + manual follow-up (#6746)
* Run "go fix ./..." Excluding only changes made to - pkg/server/endpoints/bundle/internal/acmetest/ca.go - pkg/server/endpoints/bundle/internal/autocert/autocert.go As this is effectively "vendored" code. Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net> * Small simplification post "go fix" Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net> * Use "new(value)" construct in more places Finish the job that "go fix" started with this. Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net> * Fix (invalid) staticcheck complaint Our current staticcheck version is too old to know about the "new(value)" construct and gets confused by it. As it happens, we can slightly simplify the code here anyway. Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net> * Use "wg.Go" in more places Finish the job that "go fix" started with this. This actually includes some race conditions fixes, where we were previously calling "wg.Add" inside the goroutine, when we should have been calling it before spawning the goroutine. Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net> --------- Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net>
1 parent 5fe0953 commit 0b91c01

File tree

37 files changed

+146
-230
lines changed

37 files changed

+146
-230
lines changed

‎pkg/agent/agent.go‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -286,21 +286,17 @@ func (a *Agent) setupProfiling(ctx context.Context) (stop func()) {
286286

287287
// kick off a goroutine to serve the pprof endpoints and one to
288288
// gracefully shut down the server when profiling is being torn down
289-
wg.Add(1)
290-
go func() {
291-
defer wg.Done()
289+
wg.Go(func() {
292290
if err := server.ListenAndServe(); err != nil {
293291
a.c.Log.WithError(err).Warn("Unable to serve profiling server")
294292
}
295-
}()
296-
wg.Add(1)
297-
go func() {
298-
defer wg.Done()
293+
})
294+
wg.Go(func() {
299295
<-ctx.Done()
300296
if err := server.Shutdown(ctx); err != nil {
301297
a.c.Log.WithError(err).Warn("Unable to shut down cleanly")
302298
}
303-
}()
299+
})
304300
}
305301
if a.c.ProfilingFreq > 0 {
306302
c := &profiling.Config{
@@ -310,13 +306,11 @@ func (a *Agent) setupProfiling(ctx context.Context) (stop func()) {
310306
RunGCBeforeHeapProfile: true,
311307
Profiles: a.c.ProfilingNames,
312308
}
313-
wg.Add(1)
314-
go func() {
315-
defer wg.Done()
309+
wg.Go(func() {
316310
if err := profiling.Run(ctx, c); err != nil {
317311
a.c.Log.WithError(err).Warn("Failed to run profiling")
318312
}
319-
}()
313+
})
320314
}
321315

322316
return func() {

‎pkg/agent/client/client_test.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,9 @@ func TestFetchUpdates(t *testing.T) {
171171
var wg sync.WaitGroup
172172
var update *Update
173173
err := errors.New("a not nil error")
174-
wg.Add(1)
175-
go func() {
176-
defer wg.Done()
174+
wg.Go(func() {
177175
update, err = client.FetchUpdates(ctx)
178-
}()
176+
})
179177

180178
// The request should wait until the SVID rotation finishes
181179
require.Contains(t, "a not nil error", err.Error())
@@ -543,11 +541,9 @@ func TestNewX509SVIDs(t *testing.T) {
543541
var wg sync.WaitGroup
544542
var svids map[string]*X509SVID
545543
err := errors.New("a not nil error")
546-
wg.Add(1)
547-
go func() {
548-
defer wg.Done()
544+
wg.Go(func() {
549545
svids, err = sClient.NewX509SVIDs(ctx, newTestCSRs())
550-
}()
546+
})
551547

552548
// The request should wait until the SVID rotation finishes
553549
require.Contains(t, "a not nil error", err.Error())

‎pkg/agent/common/sigstore/config_test.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ func TestNewConfigFromHCL(t *testing.T) {
2121
"test-issuer-2": {"domain/ci.yaml@refs/tags/*"},
2222
},
2323
SkippedImages: []string{"registry/image@sha256:examplehash"},
24-
RekorURL: strPtr("https://test.dev"),
25-
IgnoreSCT: boolPtr(true),
26-
IgnoreTlog: boolPtr(true),
27-
IgnoreAttestations: boolPtr(true),
24+
RekorURL: new("https://test.dev"),
25+
IgnoreSCT: new(true),
26+
IgnoreTlog: new(true),
27+
IgnoreAttestations: new(true),
2828
RegistryCredentials: map[string]*RegistryCredential{
2929
"registry": {
3030
Username: "user",
@@ -75,11 +75,3 @@ func TestNewConfigFromHCL(t *testing.T) {
7575
})
7676
}
7777
}
78-
79-
func strPtr(s string) *string {
80-
return &s
81-
}
82-
83-
func boolPtr(b bool) *bool {
84-
return &b
85-
}

‎pkg/agent/manager/manager_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,13 +2125,11 @@ func initializeAndRunManager(t *testing.T, m *manager) (closer func()) {
21252125
func runManager(t *testing.T, m *manager) (closer func()) {
21262126
var wg sync.WaitGroup
21272127
ctx, cancel := context.WithCancel(context.Background())
2128-
wg.Add(1)
2129-
go func() {
2130-
defer wg.Done()
2128+
wg.Go(func() {
21312129
if err := m.Run(ctx); err != nil {
21322130
t.Error(err)
21332131
}
2134-
}()
2132+
})
21352133
return func() {
21362134
cancel()
21372135
wg.Wait()

‎pkg/agent/plugin/workloadattestor/docker/docker_test.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,10 @@ func TestNewConfigFromHCL(t *testing.T) {
258258
"test-issuer-2": {"domain/ci.yaml@refs/tags/*"},
259259
},
260260
SkippedImages: []string{"registry/image@sha256:examplehash"},
261-
RekorURL: strPtr("https://test.dev"),
262-
IgnoreSCT: boolPtr(true),
263-
IgnoreTlog: boolPtr(true),
264-
IgnoreAttestations: boolPtr(true),
261+
RekorURL: new("https://test.dev"),
262+
IgnoreSCT: new(true),
263+
IgnoreTlog: new(true),
264+
IgnoreAttestations: new(true),
265265
RegistryCredentials: map[string]*sigstore.RegistryCredential{
266266
"registry": {
267267
Username: "user",
@@ -544,11 +544,3 @@ func (f *fakeSigstoreVerifier) Verify(_ context.Context, imageID string) ([]stri
544544
}
545545
return f.selectors, f.err
546546
}
547-
548-
func strPtr(s string) *string {
549-
return &s
550-
}
551-
552-
func boolPtr(b bool) *bool {
553-
return &b
554-
}

‎pkg/common/api/middleware/names.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,9 @@ func makeNames(fullMethod string) (names api.Names) {
8585

8686
// Parse the slash separated service and method name. The separating slash
8787
// should always be present in practice.
88-
if slashIndex := strings.Index(fullMethod, "/"); slashIndex != -1 {
89-
names.RawService = fullMethod[0:slashIndex]
90-
names.Method = fullMethod[slashIndex+1:]
88+
if before, after, ok := strings.Cut(fullMethod, "/"); ok {
89+
names.RawService = before
90+
names.Method = after
9191
}
9292

9393
names.Service = serviceReplacer.Replace(names.RawService)

‎pkg/common/catalog/builtin.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,11 @@ func startPipeServer(server *grpc.Server, log logrus.FieldLogger) (_ *pipeConn,
135135
}
136136
}))
137137

138-
wg.Add(1)
139-
go func() {
140-
defer wg.Done()
138+
wg.Go(func() {
141139
if err := server.Serve(pipeNet); err != nil && !errors.Is(err, grpc.ErrServerStopped) {
142140
log.WithError(err).Error("Pipe server unexpectedly failed to serve")
143141
}
144-
}()
142+
})
145143

146144
// Dial the server
147145
conn, err := grpc.NewClient(

‎pkg/common/catalog/external.go‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -159,24 +159,20 @@ func (p *hcClientPlugin) GRPCClient(ctx context.Context, b *goplugin.GRPCBroker,
159159
server := newHostServer(p.config.Log, p.config.Name, p.config.HostServices)
160160

161161
var wg sync.WaitGroup
162-
wg.Add(1)
163-
go func() {
164-
defer wg.Done()
162+
wg.Go(func() {
165163
if err := server.Serve(listener); err != nil && !errors.Is(err, grpc.ErrServerStopped) {
166164
p.config.Log.WithError(err).Error("Host services server failed")
167165
c.Close()
168166
}
169-
}()
167+
})
170168

171169
ctx, cancel := context.WithCancel(ctx)
172-
wg.Add(1)
173-
go func() {
174-
defer wg.Done()
170+
wg.Go(func() {
175171
<-ctx.Done()
176172
if !gracefulStopWithTimeout(server) {
177173
p.config.Log.Warn("Forced timed-out host service server to stop")
178174
}
179-
}()
175+
})
180176

181177
return &hcPlugin{
182178
conn: c,

‎pkg/common/cliprinter/internal/structpretty/structpretty.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func printStruct(msg any, stdout, stderr io.Writer) error {
3737
msgValue := reflect.ValueOf(msg)
3838

3939
// We also want to accept pointers to structs
40-
if msgType.Kind() == reflect.Ptr {
40+
if msgType.Kind() == reflect.Pointer {
4141
if msgType.Elem().Kind() != reflect.Struct {
4242
err := fmt.Errorf("cannot print unsupported type %q", msgType.Elem().Kind().String())
4343
_ = errorpretty.Print(err, stdout, stderr)
@@ -120,7 +120,7 @@ func isCompositeTypePrintable(t reflect.Type) bool {
120120
func isUnprintableType(t reflect.Type) bool {
121121
switch t.Kind() {
122122
case reflect.Invalid, reflect.Chan, reflect.Func, reflect.Interface,
123-
reflect.Ptr, reflect.Struct, reflect.UnsafePointer:
123+
reflect.Pointer, reflect.Struct, reflect.UnsafePointer:
124124

125125
return true
126126
default:

‎pkg/common/entrypoint/entrypoint_windows_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,12 @@ func (s *fakeSystemCall) Run(string, svc.Handler) error {
5252
exitCode uint32
5353
)
5454

55-
wg.Add(1)
56-
go func() {
57-
defer wg.Done()
55+
wg.Go(func() {
5856
s.mtx.RLock()
5957
defer s.mtx.RUnlock()
6058

6159
svcSpecificEC, exitCode = s.s.Execute(s.args, s.changeRequestCh, s.statusCh)
62-
}()
60+
})
6361

6462
c := make(chan struct{})
6563
go func() {

0 commit comments

Comments
 (0)