Skip to content

Commit 986dab9

Browse files
davidporter-id-audavidporter-id-au
authored
fix: mapping in requests for domain update/registration (#7301)
<!-- Describe what has changed in this PR --> **What changed?** This fixes some fields being set for Active/active on registration and domain updating. It also adds some regression tests to catch silent misses in the future. <!-- Tell your future self why have you made these changes --> **Why?** <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> **How did you test it?** <!-- Assuming the worst case, what can be broken when deploying this change to production? --> **Potential risks** <!-- Is it notable for release? e.g. schema updates, configuration or data migration required? If so, please mention it, and also update CHANGELOG.md --> **Release notes** <!-- Is there any documentation updates should be made for config, https://cadenceworkflow.io/docs/operation-guide/setup/ ? If so, please open an PR in https://github.com/cadence-workflow/cadence-docs --> **Documentation Changes** --------- Signed-off-by: David Porter <[email protected]>
1 parent 6ad8d17 commit 986dab9

File tree

6 files changed

+83
-13
lines changed

6 files changed

+83
-13
lines changed

‎common/types/mapper/proto/api.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2594,6 +2594,7 @@ func FromRegisterDomainRequest(t *types.RegisterDomainRequest) *apiv1.RegisterDo
25942594
Clusters: FromClusterReplicationConfigurationArray(t.Clusters),
25952595
ActiveClusterName: t.ActiveClusterName,
25962596
ActiveClustersByRegion: t.ActiveClustersByRegion,
2597+
ActiveClusters: FromActiveClusters(t.ActiveClusters),
25972598
Data: t.Data,
25982599
SecurityToken: t.SecurityToken,
25992600
IsGlobalDomain: t.IsGlobalDomain,
@@ -2613,10 +2614,11 @@ func ToRegisterDomainRequest(t *apiv1.RegisterDomainRequest) *types.RegisterDoma
26132614
Description: t.Description,
26142615
OwnerEmail: t.OwnerEmail,
26152616
WorkflowExecutionRetentionPeriodInDays: *durationToDays(t.WorkflowExecutionRetentionPeriod),
2616-
EmitMetric: common.BoolPtr(true),
2617+
EmitMetric: common.BoolPtr(true), // this is a legacy field that doesn't exist in proto and probably can be removed
26172618
Clusters: ToClusterReplicationConfigurationArray(t.Clusters),
26182619
ActiveClusterName: t.ActiveClusterName,
26192620
ActiveClustersByRegion: t.ActiveClustersByRegion,
2621+
ActiveClusters: ToActiveClusters(t.ActiveClusters),
26202622
Data: t.Data,
26212623
SecurityToken: t.SecurityToken,
26222624
IsGlobalDomain: t.IsGlobalDomain,

‎common/types/mapper/proto/api_test.go‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ package proto
2323
import (
2424
"testing"
2525

26+
fuzz "github.com/google/gofuzz"
2627
"github.com/stretchr/testify/assert"
2728
apiv1 "github.com/uber/cadence-idl/go/proto/api/v1"
2829

2930
"github.com/uber/cadence/common"
3031
"github.com/uber/cadence/common/types"
32+
"github.com/uber/cadence/common/types/mapper/testutils"
3133
"github.com/uber/cadence/common/types/testdata"
3234
)
3335

@@ -440,10 +442,45 @@ func TestRecordMarkerDecisionAttributes(t *testing.T) {
440442
assert.Equal(t, item, ToRecordMarkerDecisionAttributes(FromRecordMarkerDecisionAttributes(item)))
441443
}
442444
}
443-
func TestRegisterDomainRequest(t *testing.T) {
444-
for _, item := range []*types.RegisterDomainRequest{nil, {EmitMetric: common.BoolPtr(true)}, &testdata.RegisterDomainRequest} {
445-
assert.Equal(t, item, ToRegisterDomainRequest(FromRegisterDomainRequest(item)))
446-
}
445+
446+
func TestRegisterDomainRequestFuzz(t *testing.T) {
447+
t.Run("round trip from internal", func(t *testing.T) {
448+
testutils.EnsureFuzzCoverage(t, []string{
449+
"nil", "empty", "filled",
450+
}, func(t *testing.T, f *fuzz.Fuzzer) string {
451+
// Configure fuzzer to generate valid enum values and reasonable day ranges
452+
fuzzer := f.Funcs(
453+
func(e *types.ArchivalStatus, c fuzz.Continue) {
454+
*e = types.ArchivalStatus(c.Intn(2)) // 0-1 are valid values (Disabled=0, Enabled=1)
455+
},
456+
func(days *int32, c fuzz.Continue) {
457+
// Generate reasonable retention period values to avoid precision loss in conversion
458+
*days = int32(c.Intn(10000)) // 0-9999 days is reasonable range
459+
},
460+
).NilChance(0.3)
461+
462+
var orig *types.RegisterDomainRequest
463+
fuzzer.Fuzz(&orig)
464+
out := ToRegisterDomainRequest(FromRegisterDomainRequest(orig))
465+
466+
// Proto RegisterDomainRequest doesn't support EmitMetric field, it's always fixed on
467+
if orig != nil {
468+
expected := *orig // Copy the struct
469+
expected.EmitMetric = common.BoolPtr(true) // this is a legacy field which is always true. It's probably safe to remove
470+
assert.Equal(t, &expected, out, "RegisterDomainRequest did not survive round-tripping")
471+
} else {
472+
assert.Equal(t, orig, out, "RegisterDomainRequest did not survive round-tripping")
473+
}
474+
475+
if orig == nil {
476+
return "nil"
477+
}
478+
if orig.Name == "" && orig.ActiveClusterName == "" && orig.ActiveClusters == nil {
479+
return "empty"
480+
}
481+
return "filled"
482+
})
483+
})
447484
}
448485
func TestRequestCancelActivityTaskDecisionAttributes(t *testing.T) {
449486
for _, item := range []*types.RequestCancelActivityTaskDecisionAttributes{nil, {}, &testdata.RequestCancelActivityTaskDecisionAttributes} {

‎common/types/mapper/thrift/shared.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4298,6 +4298,7 @@ func FromRegisterDomainRequest(t *types.RegisterDomainRequest) *shared.RegisterD
42984298
Clusters: FromClusterReplicationConfigurationArray(t.Clusters),
42994299
ActiveClusterName: &t.ActiveClusterName,
43004300
ActiveClustersByRegion: t.ActiveClustersByRegion,
4301+
ActiveClusters: FromActiveClusters(t.ActiveClusters),
43014302
Data: t.Data,
43024303
SecurityToken: &t.SecurityToken,
43034304
IsGlobalDomain: &t.IsGlobalDomain,
@@ -4322,6 +4323,7 @@ func ToRegisterDomainRequest(t *shared.RegisterDomainRequest) *types.RegisterDom
43224323
Clusters: ToClusterReplicationConfigurationArray(t.Clusters),
43234324
ActiveClusterName: t.GetActiveClusterName(),
43244325
ActiveClustersByRegion: t.ActiveClustersByRegion,
4326+
ActiveClusters: ToActiveClusters(t.ActiveClusters),
43254327
Data: t.Data,
43264328
SecurityToken: t.GetSecurityToken(),
43274329
IsGlobalDomain: t.GetIsGlobalDomain(),

‎common/types/mapper/thrift/shared_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2277,6 +2277,34 @@ func TestRegisterDomainRequestConversion(t *testing.T) {
22772277
}
22782278
}
22792279

2280+
func TestRegisterDomainRequestFuzz(t *testing.T) {
2281+
t.Run("round trip from internal", func(t *testing.T) {
2282+
testutils.EnsureFuzzCoverage(t, []string{
2283+
"nil", "empty", "filled",
2284+
}, func(t *testing.T, f *fuzz.Fuzzer) string {
2285+
// Configure fuzzer to generate valid enum values
2286+
fuzzer := f.Funcs(
2287+
func(e *types.ArchivalStatus, c fuzz.Continue) {
2288+
*e = types.ArchivalStatus(c.Intn(2)) // 0-1 are valid values (Disabled=0, Enabled=1)
2289+
},
2290+
).NilChance(0.3)
2291+
2292+
var orig *types.RegisterDomainRequest
2293+
fuzzer.Fuzz(&orig)
2294+
out := ToRegisterDomainRequest(FromRegisterDomainRequest(orig))
2295+
assert.Equal(t, orig, out, "RegisterDomainRequest did not survive round-tripping")
2296+
2297+
if orig == nil {
2298+
return "nil"
2299+
}
2300+
if orig.Name == "" && orig.ActiveClusterName == "" && orig.ActiveClusters == nil {
2301+
return "empty"
2302+
}
2303+
return "filled"
2304+
})
2305+
})
2306+
}
2307+
22802308
func TestRemoteSyncMatchedErrorConversion(t *testing.T) {
22812309
testCases := []*types.RemoteSyncMatchedError{
22822310
nil,

‎common/types/shared.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2313,11 +2313,11 @@ func (v *DomainReplicationConfiguration) ByteSize() uint64 {
23132313
// TODO(c-warren): Rename to ClusterAttributes
23142314
type ActiveClusters struct {
23152315
// TODO(c-warren): Remove once refactor to ClusterAttribute is complete
2316-
ActiveClustersByRegion map[string]ActiveClusterInfo `json:"activeClustersByRegion,omitempty"`
2317-
// AttributeScopes maps scope types to their cluster attribute configurations.
2318-
// Keyed by a scope type (e.g., region, datacenter, city, etc.).
2319-
// The value is a ClusterAttributeScope - a map of unique names (e.g., seattle, san_francisco, etc.) to an ActiveClusterInfo.
2320-
AttributeScopes map[string]ClusterAttributeScope `json:"attributeScopes,omitempty"`
2316+
ActiveClustersByRegion map[string]ActiveClusterInfo `json:"activeClustersByRegion,omitempty" yaml:"activeClustersByRegion,omitempty"`
2317+
// ClusterAttributes
2318+
// Keyed by a scope type (e.g region, datacenter, city, etc.).
2319+
// The value is a ClusterAttributeScope - a map of unique names (e.g seattle, san_francisco, etc.) to an ActiveClusterInfo.
2320+
AttributeScopes map[string]ClusterAttributeScope `json:"attributeScopes,omitempty" yaml:"attributeScopes,omitempty"`
23212321
}
23222322

23232323
// DefaultAttributeScopeType is the default scope type for backward compatibility with ActiveClustersByRegion
@@ -2495,7 +2495,7 @@ func (v *ActiveClusters) ByteSize() uint64 {
24952495
// ClusterAttributeScope is a map of unique attribute names to the active cluster for that attribute.
24962496
// It can be used to determine the current failover version for a workflow associated with that attribute.
24972497
type ClusterAttributeScope struct {
2498-
ClusterAttributes map[string]ActiveClusterInfo `json:"clusterAttributes,omitempty"`
2498+
ClusterAttributes map[string]ActiveClusterInfo `json:"clusterAttributes,omitempty" yaml:"clusterAttributes,omitempty"`
24992499
}
25002500

25012501
func (v *ClusterAttributeScope) GetActiveClusterByClusterAttribute(name string) (ActiveClusterInfo, error) {
@@ -2528,8 +2528,8 @@ func (v *ClusterAttributeScope) ByteSize() uint64 {
25282528

25292529
// ActiveClusterInfo defines failover information for a ClusterAttribute.
25302530
type ActiveClusterInfo struct {
2531-
ActiveClusterName string `json:"activeClusterName,omitempty"`
2532-
FailoverVersion int64 `json:"failoverVersion,omitempty"`
2531+
ActiveClusterName string `json:"activeClusterName,omitempty" yaml:"activeClusterName,omitempty"`
2532+
FailoverVersion int64 `json:"failoverVersion,omitempty" yaml:"failoverVersion,omitempty"`
25332533
}
25342534

25352535
// ByteSize returns the approximate memory used in bytes

‎common/types/testdata/service_frontend.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ var (
3535
Clusters: ClusterReplicationConfigurationArray,
3636
ActiveClusterName: ClusterName1,
3737
ActiveClustersByRegion: map[string]string{Region1: ClusterName1, Region2: ClusterName2},
38+
ActiveClusters: &ActiveClusters,
3839
Data: DomainData,
3940
SecurityToken: SecurityToken,
4041
IsGlobalDomain: true,

0 commit comments

Comments
 (0)