Skip to content

Commit

Permalink
Fix slice reuse in cassandra/domain.go
Browse files Browse the repository at this point in the history
While reading domains from Cassandra the isolationGroups and asncWFConfigData are not properly reset for each row. This can result in the same backing array being reused across multiple rows, resulting in an incorrect or corrupted value being read. Notably this is used for the domain cache, so any operation relying on it may be unable to get the correct values of these fields.
  • Loading branch information
natemort committed Apr 24, 2024
1 parent 1a0c232 commit 9671c7a
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
4 changes: 4 additions & 0 deletions common/persistence/nosql/nosqlplugin/cassandra/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ func (db *cdb) SelectAllDomains(
replicationClusters = []map[string]interface{}{}
badBinariesData = []byte("")
badBinariesDataEncoding = ""
isolationGroups = []byte("")
isolationGroupsEncoding = ""
asyncWFConfigData = []byte("")
asyncWFConfigEncoding = ""
failoverEndTime = 0
lastUpdateTime = 0
retentionDays = 0
Expand Down
19 changes: 15 additions & 4 deletions common/persistence/persistence-tests/metadataPersistenceV2Test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1182,8 +1182,14 @@ func (m *MetadataPersistenceSuiteV2) TestListDomains() {
VisibilityArchivalStatus: types.ArchivalStatusEnabled,
VisibilityArchivalURI: "test://visibility/uri",
BadBinaries: testBinaries1,
IsolationGroups: types.IsolationGroupConfiguration{},
AsyncWorkflowConfig: types.AsyncWorkflowConfiguration{},
IsolationGroups: types.IsolationGroupConfiguration{
"name1": types.IsolationGroupPartition{Name: "name1"},
"name2": types.IsolationGroupPartition{Name: "name2"},
},
AsyncWorkflowConfig: types.AsyncWorkflowConfiguration{
Enabled: true,
PredefinedQueueName: "queue1",
},
},
ReplicationConfig: &p.DomainReplicationConfig{
ActiveClusterName: clusterActive1,
Expand Down Expand Up @@ -1211,8 +1217,13 @@ func (m *MetadataPersistenceSuiteV2) TestListDomains() {
VisibilityArchivalStatus: types.ArchivalStatusDisabled,
VisibilityArchivalURI: "",
BadBinaries: testBinaries2,
IsolationGroups: types.IsolationGroupConfiguration{},
AsyncWorkflowConfig: types.AsyncWorkflowConfiguration{},
IsolationGroups: types.IsolationGroupConfiguration{
"name3": types.IsolationGroupPartition{Name: "name3"},
},
AsyncWorkflowConfig: types.AsyncWorkflowConfiguration{
Enabled: false,
PredefinedQueueName: "queue2",
},
},
ReplicationConfig: &p.DomainReplicationConfig{
ActiveClusterName: clusterActive2,
Expand Down
27 changes: 27 additions & 0 deletions host/async_wf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ func (s *AsyncWFIntegrationSuite) SetupSuite() {

s.domainName = s.randomizeStr("integration-test-domain")
s.Require().NoError(s.registerDomain(s.domainName, 1, types.ArchivalStatusDisabled, "", types.ArchivalStatusDisabled, ""))
s.secondaryDomainName = s.randomizeStr("unused-test-domain")
s.Require().NoError(s.registerDomain(s.secondaryDomainName, 1, types.ArchivalStatusDisabled, "", types.ArchivalStatusDisabled, ""))

s.domainCacheRefresh()
}
Expand All @@ -128,24 +130,37 @@ func (s *AsyncWFIntegrationSuite) TestStartWorkflowExecutionAsync() {
name string
wantStartFailure bool
asyncWFCfg *types.AsyncWorkflowConfiguration
secondaryCfg *types.AsyncWorkflowConfiguration
}{
{
name: "start workflow execution async fails because domain missing async queue",
wantStartFailure: true,
secondaryCfg: &types.AsyncWorkflowConfiguration{
Enabled: true,
PredefinedQueueName: "test-async-wf-queue",
},
},
{
name: "start workflow execution async fails because async queue is disabled",
asyncWFCfg: &types.AsyncWorkflowConfiguration{
Enabled: false,
},
wantStartFailure: true,
secondaryCfg: &types.AsyncWorkflowConfiguration{
Enabled: true,
PredefinedQueueName: "test-async-wf-queue",
},
},
{
name: "start workflow execution async succeeds and workflow starts",
asyncWFCfg: &types.AsyncWorkflowConfiguration{
Enabled: true,
PredefinedQueueName: "test-async-wf-queue",
},
secondaryCfg: &types.AsyncWorkflowConfiguration{
Enabled: false,
PredefinedQueueName: "test-async-wf-queue",
},
},
}

Expand All @@ -170,6 +185,18 @@ func (s *AsyncWFIntegrationSuite) TestStartWorkflowExecutionAsync() {
s.domainCacheRefresh()
}

if tc.secondaryCfg != nil {
_, err := s.adminClient.UpdateDomainAsyncWorkflowConfiguraton(ctx, &types.UpdateDomainAsyncWorkflowConfiguratonRequest{
Domain: s.secondaryDomainName,
Configuration: tc.secondaryCfg,
})
if err != nil {
t.Fatalf("UpdateDomainAsyncWorkflowConfiguraton() failed: %v", err)
}

s.domainCacheRefresh()
}

startTime := s.testClusterConfig.TimeSource.Now().UnixNano()
wfID := fmt.Sprintf("async-wf-integration-start-workflow-test-%d", startTime)
wfType := "async-wf-integration-start-workflow-test-type"
Expand Down
1 change: 1 addition & 0 deletions host/integrationbase.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type (
adminClient AdminClient
Logger log.Logger
domainName string
secondaryDomainName string
testRawHistoryDomainName string
foreignDomainName string
archivalDomainName string
Expand Down

0 comments on commit 9671c7a

Please sign in to comment.