Skip to content

Commit 78caede

Browse files
bergundybergundy
authored
Forbid the use of pborman/uuid and remove uses (#8541)
## What Used the `depguard` linter to prevent importing the package and modified all uses of it. Also fixed a bug in the membership code where we were not updating the next page token: https://github.com/temporalio/temporal/pull/8541/files#diff-985dd64ffa4c6bfe0248b982dc47439b725862baa3e1f2c559b7ff522bf21969R350 ## Why? From the package README in GitHub: > This package now leverages the github.com/google/uuid package (which is based off an earlier version of this package). Also, there's no point in having two uuid implementations in the codebase. ## Test Added the invalid import and confirmed it works: ``` ../service/frontend/admin_handler_test.go:13:2: import 'github.com/pborman/uuid' is not allowed from list 'main': Importing github.com/pborman/uuid is disallowed; use github.com/google/uuid instead (depguard) _ "github.com/pborman/uuid" ```
1 parent ffae22b commit 78caede

File tree

175 files changed

+1945
-1906
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

175 files changed

+1945
-1906
lines changed

‎.github/.golangci.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ linters:
1010
enable:
1111
- errcheck
1212
- importas
13+
- depguard
1314
- revive
1415
- staticcheck
1516
- govet
@@ -35,6 +36,13 @@ linters:
3536
msg: "Please use require.Eventually or assert.Eventually instead unless you've no other option"
3637
- pattern: panic
3738
msg: "Please avoid using panic in application code"
39+
depguard:
40+
rules:
41+
main:
42+
list-mode: lax
43+
deny:
44+
- pkg: github.com/pborman/uuid
45+
desc: "Importing github.com/pborman/uuid is disallowed; use github.com/google/uuid instead"
3846
importas:
3947
# Enforce the aliases below.
4048
no-unaliased: true

‎chasm/lib/scheduler/backfiller.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package scheduler
33
import (
44
"time"
55

6-
"github.com/pborman/uuid"
6+
"github.com/google/uuid"
77
schedulespb "go.temporal.io/server/api/schedule/v1"
88
"go.temporal.io/server/chasm"
99
"go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1"
@@ -33,7 +33,7 @@ func newBackfiller(
3333
ctx chasm.MutableContext,
3434
scheduler *Scheduler,
3535
) *Backfiller {
36-
id := uuid.New()
36+
id := uuid.NewString()
3737
backfiller := &Backfiller{
3838
BackfillerState: &schedulerpb.BackfillerState{
3939
BackfillId: id,

‎common/checksum/crc_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"testing"
77
"time"
88

9-
"github.com/pborman/uuid"
9+
"github.com/google/uuid"
1010
"github.com/stretchr/testify/assert"
1111
commonpb "go.temporal.io/api/common/v1"
1212
workflowpb "go.temporal.io/api/workflow/v1"
@@ -21,8 +21,8 @@ func TestCRC32OverProto(t *testing.T) {
2121
// different set of serialized bytes
2222
obj := &workflowpb.WorkflowExecutionInfo{
2323
Execution: &commonpb.WorkflowExecution{
24-
WorkflowId: uuid.New(),
25-
RunId: uuid.New(),
24+
WorkflowId: uuid.NewString(),
25+
RunId: uuid.NewString(),
2626
},
2727
StartTime: timestamppb.New(time.Now().UTC()),
2828
HistoryLength: 550,

‎common/cluster/metadata_test.go‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"testing"
66
"time"
77

8-
"github.com/pborman/uuid"
8+
"github.com/google/uuid"
99
"github.com/stretchr/testify/require"
1010
"github.com/stretchr/testify/suite"
1111
persistencespb "go.temporal.io/server/api/persistence/v1"
@@ -51,29 +51,29 @@ func (s *metadataSuite) SetupTest() {
5151

5252
s.isGlobalNamespaceEnabled = true
5353
s.failoverVersionIncrement = 100
54-
s.clusterName = uuid.New()
55-
s.secondClusterName = uuid.New()
56-
s.thirdClusterName = uuid.New()
54+
s.clusterName = uuid.NewString()
55+
s.secondClusterName = uuid.NewString()
56+
s.thirdClusterName = uuid.NewString()
5757

5858
clusterInfo := map[string]ClusterInformation{
5959
s.clusterName: {
6060
Enabled: true,
6161
InitialFailoverVersion: int64(1),
62-
RPCAddress: uuid.New(),
62+
RPCAddress: uuid.NewString(),
6363
ShardCount: 1,
6464
version: 1,
6565
},
6666
s.secondClusterName: {
6767
Enabled: true,
6868
InitialFailoverVersion: int64(4),
69-
RPCAddress: uuid.New(),
69+
RPCAddress: uuid.NewString(),
7070
ShardCount: 2,
7171
version: 1,
7272
},
7373
s.thirdClusterName: {
7474
Enabled: true,
7575
InitialFailoverVersion: int64(5),
76-
RPCAddress: uuid.New(),
76+
RPCAddress: uuid.NewString(),
7777
ShardCount: 1,
7878
version: 1,
7979
},
@@ -135,7 +135,7 @@ func (s *metadataSuite) Test_RegisterMetadataChangeCallback() {
135135
}
136136

137137
func (s *metadataSuite) Test_RefreshClusterMetadata_Success() {
138-
id := uuid.New()
138+
id := uuid.NewString()
139139
s.metadata.clusterChangeCallback[id] = func(oldClusterMetadata map[string]*ClusterInformation, newClusterMetadata map[string]*ClusterInformation) {
140140
oldMetadata, ok := oldClusterMetadata[id]
141141
s.True(ok)
@@ -169,8 +169,8 @@ func (s *metadataSuite) Test_RefreshClusterMetadata_Success() {
169169
IsConnectionEnabled: true,
170170
InitialFailoverVersion: 1,
171171
HistoryShardCount: 1,
172-
ClusterAddress: uuid.New(),
173-
HttpAddress: uuid.New(),
172+
ClusterAddress: uuid.NewString(),
173+
HttpAddress: uuid.NewString(),
174174
},
175175
Version: 1,
176176
},
@@ -181,8 +181,8 @@ func (s *metadataSuite) Test_RefreshClusterMetadata_Success() {
181181
IsConnectionEnabled: true,
182182
InitialFailoverVersion: 1,
183183
HistoryShardCount: 1,
184-
ClusterAddress: uuid.New(),
185-
HttpAddress: uuid.New(),
184+
ClusterAddress: uuid.NewString(),
185+
HttpAddress: uuid.NewString(),
186186
Tags: map[string]string{"test": "test"},
187187
},
188188
Version: 2,
@@ -194,8 +194,8 @@ func (s *metadataSuite) Test_RefreshClusterMetadata_Success() {
194194
IsConnectionEnabled: true,
195195
InitialFailoverVersion: 2,
196196
HistoryShardCount: 2,
197-
ClusterAddress: uuid.New(),
198-
HttpAddress: uuid.New(),
197+
ClusterAddress: uuid.NewString(),
198+
HttpAddress: uuid.NewString(),
199199
Tags: map[string]string{"test": "test"},
200200
},
201201
Version: 2,
@@ -211,7 +211,7 @@ func (s *metadataSuite) Test_RefreshClusterMetadata_Success() {
211211

212212
func (s *metadataSuite) Test_ListAllClusterMetadataFromDB_Success() {
213213
nextPageSizeToken := []byte{1}
214-
newClusterName := uuid.New()
214+
newClusterName := uuid.NewString()
215215
s.mockClusterMetadataStore.EXPECT().ListClusterMetadata(gomock.Any(), &persistence.ListClusterMetadataRequest{
216216
PageSize: defaultClusterMetadataPageSize,
217217
NextPageToken: nil,
@@ -224,8 +224,8 @@ func (s *metadataSuite) Test_ListAllClusterMetadataFromDB_Success() {
224224
IsConnectionEnabled: true,
225225
InitialFailoverVersion: 1,
226226
HistoryShardCount: 1,
227-
ClusterAddress: uuid.New(),
228-
HttpAddress: uuid.New(),
227+
ClusterAddress: uuid.NewString(),
228+
HttpAddress: uuid.NewString(),
229229
},
230230
Version: 1,
231231
},
@@ -244,8 +244,8 @@ func (s *metadataSuite) Test_ListAllClusterMetadataFromDB_Success() {
244244
IsConnectionEnabled: true,
245245
InitialFailoverVersion: 2,
246246
HistoryShardCount: 2,
247-
ClusterAddress: uuid.New(),
248-
HttpAddress: uuid.New(),
247+
ClusterAddress: uuid.NewString(),
248+
HttpAddress: uuid.NewString(),
249249
},
250250
Version: 2,
251251
},

‎common/collection/concurrent_tx_map_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"sync/atomic"
88
"testing"
99

10-
"github.com/pborman/uuid"
10+
"github.com/google/uuid"
1111
"github.com/stretchr/testify/require"
1212
"github.com/stretchr/testify/suite"
1313
)
@@ -55,7 +55,7 @@ func (s *ConcurrentTxMapSuite) TestLen() {
5555

5656
func (s *ConcurrentTxMapSuite) TestGetAndDo() {
5757
testMap := NewShardedConcurrentTxMap(1, UUIDHashCode)
58-
key := uuid.New()
58+
key := uuid.NewString()
5959
var value intType
6060
fnApplied := false
6161

@@ -86,7 +86,7 @@ func (s *ConcurrentTxMapSuite) TestGetAndDo() {
8686

8787
func (s *ConcurrentTxMapSuite) TestPutOrDo() {
8888
testMap := NewShardedConcurrentTxMap(1, UUIDHashCode)
89-
key := uuid.New()
89+
key := uuid.NewString()
9090
var value intType
9191
fnApplied := false
9292

@@ -117,7 +117,7 @@ func (s *ConcurrentTxMapSuite) TestPutOrDo() {
117117

118118
func (s *ConcurrentTxMapSuite) TestRemoveIf() {
119119
testMap := NewShardedConcurrentTxMap(1, UUIDHashCode)
120-
key := uuid.New()
120+
key := uuid.NewString()
121121
value := intType(1)
122122
testMap.Put(key, &value)
123123

@@ -142,7 +142,7 @@ func (s *ConcurrentTxMapSuite) TestGetAfterPut() {
142142
testMap := NewShardedConcurrentTxMap(1, UUIDHashCode)
143143

144144
for i := 0; i < 1024; i++ {
145-
key := uuid.New()
145+
key := uuid.NewString()
146146
countMap[key] = 0
147147
testMap.Put(key, boolType(true))
148148
}
@@ -175,7 +175,7 @@ func (s *ConcurrentTxMapSuite) TestGetAfterPut() {
175175

176176
func (s *ConcurrentTxMapSuite) TestPutIfNotExist() {
177177
testMap := NewShardedConcurrentTxMap(1, UUIDHashCode)
178-
key := uuid.New()
178+
key := uuid.NewString()
179179
ok := testMap.PutIfNotExist(key, boolType(true))
180180
s.True(ok, "PutIfNotExist failed to insert item")
181181
ok = testMap.PutIfNotExist(key, boolType(true))
@@ -186,7 +186,7 @@ func (s *ConcurrentTxMapSuite) TestMapConcurrency() {
186186
nKeys := 1024
187187
keys := make([]string, nKeys)
188188
for i := 0; i < nKeys; i++ {
189-
keys[i] = uuid.New()
189+
keys[i] = uuid.NewString()
190190
}
191191

192192
var total int32

‎common/membership/ringpop/monitor.go‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"sync"
1212
"time"
1313

14-
"github.com/pborman/uuid"
14+
"github.com/google/uuid"
1515
"github.com/temporalio/ringpop-go"
1616
"github.com/temporalio/ringpop-go/discovery/statichosts"
1717
"github.com/temporalio/ringpop-go/swim"
@@ -64,7 +64,7 @@ type monitor struct {
6464
logger log.Logger
6565
metadataManager persistence.ClusterMetadataManager
6666
broadcastHostPortResolver func() (string, error)
67-
hostID uuid.UUID
67+
hostID []byte
6868
initialized *future.FutureImpl[struct{}]
6969
}
7070

@@ -87,6 +87,8 @@ func newMonitor(
8787
lifecycleCtx,
8888
headers.SystemBackgroundHighCallerInfo,
8989
)
90+
hostID, _ := uuid.New().MarshalBinary()
91+
// MarshalBinary should never error.
9092

9193
rpo := &monitor{
9294
status: common.DaemonStatusInitialized,
@@ -101,7 +103,7 @@ func newMonitor(
101103
logger: logger,
102104
metadataManager: metadataManager,
103105
broadcastHostPortResolver: broadcastHostPortResolver,
104-
hostID: uuid.NewUUID(),
106+
hostID: hostID,
105107
initialized: future.NewFuture[struct{}](),
106108
maxJoinDuration: maxJoinDuration,
107109
propagationTime: propagationTime,
@@ -251,10 +253,14 @@ func (rpo *monitor) upsertMyMembership(
251253
err := rpo.metadataManager.UpsertClusterMembership(ctx, request)
252254

253255
if err == nil {
256+
hostID, err := uuid.FromBytes(request.HostID)
257+
if err != nil {
258+
return err
259+
}
254260
rpo.logger.Debug("Membership heartbeat upserted successfully",
255261
tag.Address(request.RPCAddress.String()),
256262
tag.Port(int(request.RPCPort)),
257-
tag.HostID(request.HostID.String()))
263+
tag.HostID(hostID.String()))
258264
}
259265

260266
return err
@@ -313,10 +319,14 @@ func (rpo *monitor) startHeartbeat(broadcastHostport string) error {
313319
// read side by filtering on the last time a heartbeat was seen.
314320
err = rpo.upsertMyMembership(rpo.lifecycleCtx, req)
315321
if err == nil {
322+
hostID, err := uuid.FromBytes(rpo.hostID)
323+
if err != nil {
324+
return err
325+
}
316326
rpo.logger.Info("Membership heartbeat upserted successfully",
317327
tag.Address(broadcastAddress.String()),
318328
tag.Port(int(broadcastPort)),
319-
tag.HostID(rpo.hostID.String()))
329+
tag.HostID(hostID.String()))
320330

321331
rpo.startHeartbeatUpsertLoop(req)
322332
}
@@ -347,11 +357,10 @@ func (rpo *monitor) fetchCurrentBootstrapHostports() ([]string, error) {
347357
for _, host := range resp.ActiveMembers {
348358
set[net.JoinHostPort(host.RPCAddress.String(), convert.Uint16ToString(host.RPCPort))] = struct{}{}
349359
}
350-
351360
nextPageToken = resp.NextPageToken
352361

353362
// Stop iterating once we have either 500 unique ip:port combos or there is no more results.
354-
if nextPageToken == nil || len(set) >= 500 {
363+
if len(nextPageToken) == 0 || len(set) >= 500 {
355364
bootstrapHostPorts := make([]string, 0, len(set))
356365
for k := range set {
357366
bootstrapHostPorts = append(bootstrapHostPorts, k)

‎common/membership/ringpop/test_cluster.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"testing"
66
"time"
77

8-
"github.com/pborman/uuid"
8+
"github.com/google/uuid"
99
"github.com/temporalio/ringpop-go"
1010
"github.com/temporalio/tchannel-go"
1111
"go.temporal.io/server/common/config"
@@ -71,7 +71,7 @@ func newTestCluster(
7171
logger.Error("tchannel listen failed", tag.Error(err))
7272
return nil
7373
}
74-
cluster.hostUUIDs[i] = uuid.New()
74+
cluster.hostUUIDs[i] = uuid.NewString()
7575
cluster.hostAddrs[i], err = buildBroadcastHostPort(cluster.channels[i].PeerInfo(), broadcastAddress)
7676
if err != nil {
7777
logger.Error("Failed to build broadcast hostport", tag.Error(err))
@@ -91,8 +91,11 @@ func newTestCluster(
9191
logger.Error("unable to split host port", tag.Error(err))
9292
return nil
9393
}
94+
// MarshalBinary never fails for UUIDs
95+
hostID, _ := uuid.New().MarshalBinary()
96+
9497
seedMember := &persistence.ClusterMember{
95-
HostID: uuid.NewUUID(),
98+
HostID: hostID,
9699
RPCAddress: seedAddress,
97100
RPCPort: seedPort,
98101
SessionStart: time.Now().UTC(),
@@ -104,12 +107,13 @@ func newTestCluster(
104107
func(_ context.Context, _ *persistence.GetClusterMembersRequest) (*persistence.GetClusterMembersResponse, error) {
105108
res := &persistence.GetClusterMembersResponse{ActiveMembers: []*persistence.ClusterMember{seedMember}}
106109

110+
hostID, _ := uuid.New().MarshalBinary()
107111
if firstGetClusterMemberCall {
108112
// The first time GetClusterMembers is invoked, we simulate returning a stale/bad heartbeat.
109113
// All subsequent calls only return the single "good" seed member
110114
// This ensures that we exercise the retry path in bootstrap properly.
111115
badSeedMember := &persistence.ClusterMember{
112-
HostID: uuid.NewUUID(),
116+
HostID: hostID,
113117
RPCAddress: seedAddress,
114118
RPCPort: seedPort + 1,
115119
SessionStart: time.Now().UTC(),

‎common/metrics/tally_metrics_handler_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"testing"
66
"time"
77

8-
"github.com/pborman/uuid"
8+
"github.com/google/uuid"
99
"github.com/stretchr/testify/assert"
1010
"github.com/uber-go/tally/v4"
1111
)
@@ -62,7 +62,7 @@ func TestTallyScope(t *testing.T) {
6262
assert.EqualValues(t, map[time.Duration]int64(nil), histograms["test.transmission+"].Durations())
6363
assert.EqualValues(t, map[string]string{}, histograms["test.transmission+"].Tags())
6464

65-
newTaggedHandler := mp.WithTags(NamespaceTag(uuid.New()))
65+
newTaggedHandler := mp.WithTags(NamespaceTag(uuid.NewString()))
6666
recordTallyMetrics(newTaggedHandler)
6767
snap = scope.Snapshot()
6868
counters = snap.Counters()

0 commit comments

Comments
 (0)