refactor: refactor the domain / cli commands for active-active's cluster-attr (#7299)

<!-- Describe what has changed in this PR -->
**What changed?**

This is changing the cli to update the cluster-attributes for
active/active clusters. This allows domains to update the domain by
passing in the format `<cluster-attr>.<scope>:<name>` ie:
`region.manilla:cluster0,region.newyork:cluster1'`

<!-- 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]>

Author: davidporter-id-au

common/types/shared.go

@@ -1710,6 +1710,13 @@ type DescribeDomainResponse struct {
 	FailoverInfo             *FailoverInfo                   `json:"failoverInfo,omitempty"`
 }
 
+func (v *DomainReplicationConfiguration) GetActiveClusters() (o *ActiveClusters) {
+	if v != nil && v.ActiveClusters != nil {
+		return v.ActiveClusters
+	}
+	return
+}
+
 // GetDomainInfo is an internal getter (TBD...)
 func (v *DescribeDomainResponse) GetDomainInfo() (o *DomainInfo) {
 	if v != nil && v.DomainInfo != nil {
@@ -2253,6 +2260,20 @@ type DomainReplicationConfiguration struct {
 	ActiveClusters    *ActiveClusters                    `json:"activeClusters,omitempty"`
 }
 
+func (v *DomainReplicationConfiguration) IsActiveActiveDomain() bool {
+	if v == nil || v.ActiveClusters == nil {
+		return false
+	}
+	if v.ActiveClusters.AttributeScopes != nil && len(v.ActiveClusters.AttributeScopes) > 0 {
+		return true
+	}
+	// todo (david.porter) remove this once we have fully migrated to AttributeScopes
+	if v.ActiveClusters.ActiveClustersByRegion != nil && len(v.ActiveClusters.ActiveClustersByRegion) > 0 {
+		return true
+	}
+	return false
+}
+
 // GetActiveClusterName is an internal getter (TBD...)
 func (v *DomainReplicationConfiguration) GetActiveClusterName() (o string) {
 	if v != nil {
@@ -2269,13 +2290,6 @@ func (v *DomainReplicationConfiguration) GetClusters() (o []*ClusterReplicationC
 	return
 }
 
-func (v *DomainReplicationConfiguration) GetActiveClusters() (o *ActiveClusters) {
-	if v != nil && v.ActiveClusters != nil {
-		return v.ActiveClusters
-	}
-	return
-}
-
 // ByteSize returns the approximate memory used in bytes
 func (v *DomainReplicationConfiguration) ByteSize() uint64 {
 	if v == nil {

common/types/shared_test.go

@@ -120,6 +120,77 @@ func TestActiveClustersConfigDeepCopy(t *testing.T) {
 	}
 }
 
+func TestIsActiveActiveDomain(t *testing.T) {
+	tests := []struct {
+		name           string
+		activeClusters *DomainReplicationConfiguration
+		want           bool
+	}{
+		{
+			name:           "empty DomainReplicationConfiguration should return false",
+			activeClusters: &DomainReplicationConfiguration{},
+			want:           false,
+		},
+		{
+			name:           "nil receiver should return false",
+			activeClusters: nil,
+			want:           false,
+		},
+		{
+			name:           "empty ActiveClusters should return false",
+			activeClusters: &DomainReplicationConfiguration{ActiveClusters: &ActiveClusters{}},
+			want:           false,
+		},
+		{
+			name: "ActiveClusters with only old format populated should return true",
+			activeClusters: &DomainReplicationConfiguration{
+				ActiveClusters: &ActiveClusters{
+					ActiveClustersByRegion: map[string]ActiveClusterInfo{
+						"us-east-1": {ActiveClusterName: "cluster1"},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "ActiveClusters with only new format populated should return true",
+			activeClusters: &DomainReplicationConfiguration{
+				ActiveClusters: &ActiveClusters{
+					AttributeScopes: map[string]ClusterAttributeScope{
+						"region": {ClusterAttributes: map[string]ActiveClusterInfo{
+							"us-east-1": {ActiveClusterName: "cluster1"},
+						}},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "ActiveClusters with both formats populated should return true",
+			activeClusters: &DomainReplicationConfiguration{
+				ActiveClusters: &ActiveClusters{
+					ActiveClustersByRegion: map[string]ActiveClusterInfo{
+						"us-east-1": {ActiveClusterName: "cluster1"},
+					},
+					AttributeScopes: map[string]ClusterAttributeScope{
+						"region": {ClusterAttributes: map[string]ActiveClusterInfo{
+							"us-east-1": {ActiveClusterName: "cluster1"},
+						}},
+					},
+				},
+			},
+			want: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.activeClusters.IsActiveActiveDomain()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 // identicalByteArray returns true if a and b are the same slice, false otherwise.
 func identicalByteArray(a, b []byte) bool {
 	return len(a) == len(b) && unsafe.SliceData(a) == unsafe.SliceData(b)

tools/cli/domain_commands.go

@@ -27,6 +27,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -112,16 +113,6 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error {
 			return commoncli.Problem(fmt.Sprintf("Option %s format is invalid.", FlagIsGlobalDomain), err)
 		}
 	}
-	isActiveActiveDomain := false
-	if c.IsSet(FlagIsActiveActiveDomain) {
-		isActiveActiveDomain, err = strconv.ParseBool(c.String(FlagIsActiveActiveDomain))
-		if err != nil {
-			return commoncli.Problem(fmt.Sprintf("Option %s format is invalid.", FlagIsActiveActiveDomain), err)
-		}
-
-		// Also set isGlobalDomain to true if it is active-active domain
-		isGlobalDomain = isActiveActiveDomain
-	}
 
 	var domainData *flag.StringMap
 	if c.IsSet(FlagDomainData) {
@@ -136,17 +127,9 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error {
 
 	activeClusterName := ""
 	if c.IsSet(FlagActiveClusterName) {
-		if isActiveActiveDomain {
-			return commoncli.Problem("Option --active_cluster is not supported for active-active domain. Use --active_clusters_by_region instead.", nil)
-		}
 		activeClusterName = c.String(FlagActiveClusterName)
 	}
 
-	activeClustersByRegion, err := parseActiveClustersByRegion(c, isActiveActiveDomain)
-	if err != nil {
-		return err
-	}
-
 	var clusters []*types.ClusterReplicationConfiguration
 	if c.IsSet(FlagClusters) {
 		for _, clusterStr := range c.StringSlice(FlagClusters) {
@@ -165,6 +148,15 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error {
 		return fmt.Errorf("failed to parse %s flag: %w", FlagVisibilityArchivalStatus, err)
 	}
 
+	var activeClusters *types.ActiveClusters
+	if c.IsSet(FlagActiveClusters) {
+		ac, err := parseActiveClustersByClusterAttribute(c.String(FlagActiveClusters))
+		if err != nil {
+			return err
+		}
+		activeClusters = &ac
+	}
+
 	request := &types.RegisterDomainRequest{
 		Name:                                   domainName,
 		Description:                            description,
@@ -173,7 +165,7 @@ func (d *domainCLIImpl) RegisterDomain(c *cli.Context) error {
 		WorkflowExecutionRetentionPeriodInDays: int32(retentionDays),
 		Clusters:                               clusters,
 		ActiveClusterName:                      activeClusterName,
-		ActiveClustersByRegion:                 activeClustersByRegion,
+		ActiveClusters:                         activeClusters,
 		SecurityToken:                          securityToken,
 		HistoryArchivalStatus:                  has,
 		HistoryArchivalURI:                     c.String(FlagHistoryArchivalURI),
@@ -225,23 +217,16 @@ func (d *domainCLIImpl) UpdateDomain(c *cli.Context) error {
 			ActiveClusterName:        common.StringPtr(activeCluster),
 			FailoverTimeoutInSeconds: failoverTimeout,
 		}
-	} else if c.IsSet(FlagActiveClustersByRegion) { // active-active domain failover
-		activeClustersByRegion, err := parseActiveClustersByRegion(c, true)
+	} else if c.IsSet(FlagActiveClusters) { // active-active domain failover
+		activeClusters, err := parseActiveClustersByClusterAttribute(c.String(FlagActiveClusters))
 		if err != nil {
 			return err
 		}
 
-		acbr := make(map[string]types.ActiveClusterInfo)
-		for region, cluster := range activeClustersByRegion {
-			acbr[region] = types.ActiveClusterInfo{
-				ActiveClusterName: cluster,
-			}
-		}
-
 		updateRequest = &types.UpdateDomainRequest{
 			Name: domainName,
 			ActiveClusters: &types.ActiveClusters{
-				ActiveClustersByRegion: acbr,
+				AttributeScopes: activeClusters.AttributeScopes,
 			},
 		}
 	} else {
@@ -669,26 +654,27 @@ type ActiveClusterInfoRow struct {
 }
 
 type DomainRow struct {
-	Name                     string `header:"Name"`
-	UUID                     string `header:"UUID"`
-	Description              string
-	OwnerEmail               string
-	DomainData               map[string]string  `header:"Domain Data"`
-	Status                   types.DomainStatus `header:"Status"`
-	IsGlobal                 bool               `header:"Is Global Domain"`
-	ActiveCluster            string             `header:"Active Cluster"`
-	Clusters                 []string           `header:"Clusters"`
-	RetentionDays            int32              `header:"Retention Days"`
-	EmitMetrics              bool
-	HistoryArchivalStatus    types.ArchivalStatus `header:"History Archival Status"`
-	HistoryArchivalURI       string               `header:"History Archival URI"`
-	VisibilityArchivalStatus types.ArchivalStatus `header:"Visibility Archival Status"`
-	VisibilityArchivalURI    string               `header:"Visibility Archival URI"`
-	BadBinaries              []BadBinaryRow
-	FailoverInfo             *FailoverInfoRow
-	LongRunningWorkFlowNum   *int
-	IsActiveActiveDomain     bool
-	ActiveClustersByRegion   []ActiveClusterInfoRow
+	Name                             string `header:"Name"`
+	UUID                             string `header:"UUID"`
+	Description                      string
+	OwnerEmail                       string
+	DomainData                       map[string]string  `header:"Domain Data"`
+	Status                           types.DomainStatus `header:"Status"`
+	IsGlobal                         bool               `header:"Is Global Domain"`
+	ActiveCluster                    string             `header:"Active Cluster"`
+	Clusters                         []string           `header:"Clusters"`
+	RetentionDays                    int32              `header:"Retention Days"`
+	EmitMetrics                      bool
+	HistoryArchivalStatus            types.ArchivalStatus `header:"History Archival Status"`
+	HistoryArchivalURI               string               `header:"History Archival URI"`
+	VisibilityArchivalStatus         types.ArchivalStatus `header:"Visibility Archival Status"`
+	VisibilityArchivalURI            string               `header:"Visibility Archival URI"`
+	BadBinaries                      []BadBinaryRow
+	FailoverInfo                     *FailoverInfoRow
+	LongRunningWorkFlowNum           *int
+	IsActiveActiveDomain             bool
+	ActiveClustersByRegion           []ActiveClusterInfoRow // todo (david.porter) remove this as it's not in use
+	ActiveClustersByClusterAttribute []ActiveClusterInfoRow
 }
 
 type DomainMigrationRow struct {
@@ -732,7 +718,7 @@ func newDomainRow(domain *types.DescribeDomainResponse) DomainRow {
 		VisibilityArchivalURI:    domain.Configuration.GetVisibilityArchivalURI(),
 		BadBinaries:              newBadBinaryRows(domain.Configuration.BadBinaries),
 		FailoverInfo:             newFailoverInfoRow(domain.FailoverInfo),
-		IsActiveActiveDomain:     domain.ReplicationConfiguration.GetActiveClusters() != nil,
+		IsActiveActiveDomain:     domain.ReplicationConfiguration.IsActiveActiveDomain(),
 		ActiveClustersByRegion:   newActiveClustersByRegion(domain.ReplicationConfiguration.GetActiveClusters()),
 	}
 }
@@ -972,27 +958,42 @@ func clustersToStrings(clusters []*types.ClusterReplicationConfiguration) []stri
 	return res
 }
 
-func parseActiveClustersByRegion(c *cli.Context, isActiveActiveDomain bool) (map[string]string, error) {
-	var activeClustersByRegion map[string]string
-	if c.IsSet(FlagActiveClustersByRegion) {
-		if !isActiveActiveDomain {
-			return nil, commoncli.Problem("Option --active_clusters_by_region is only supported for active-active domain. Use --active_cluster instead.", nil)
+func parseActiveClustersByClusterAttribute(clusters string) (types.ActiveClusters, error) {
+	split := regexp.MustCompile(`(?P<attribute>[a-zA-Z0-9_]+).(?P<scope>[a-zA-Z0-9_]+):(?P<name>[a-zA-Z0-9_]+)`)
+	matches := split.FindAllStringSubmatch(clusters, -1)
+	if len(matches) == 0 {
+		return types.ActiveClusters{}, fmt.Errorf("option %s format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'", FlagActiveClusters)
+	}
+
+	out := types.ActiveClusters{
+		AttributeScopes: map[string]types.ClusterAttributeScope{},
+	}
+
+	for _, match := range matches {
+		if len(match) != 4 {
+			return types.ActiveClusters{}, fmt.Errorf("option %s format is invalid. Expected format is 'region.dca:dev2_dca,region.phx:dev2_phx'", FlagActiveClusters)
 		}
+		attribute := match[1]
+		scope := match[2]
+		name := match[3]
 
-		activeClustersByRegion = make(map[string]string)
-		for _, regionCluster := range c.StringSlice(FlagActiveClustersByRegion) {
-			splitted := strings.Split(regionCluster, ":")
-			if len(splitted) != 2 {
-				return nil, commoncli.Problem(fmt.Sprintf("Option --%s format is invalid. Expected format is 'region1:cluster1,region2:cluster2'", FlagActiveClustersByRegion), nil)
+		existing, ok := out.AttributeScopes[attribute]
+		if !ok {
+			out.AttributeScopes[attribute] = types.ClusterAttributeScope{
+				ClusterAttributes: map[string]types.ActiveClusterInfo{
+					scope: {ActiveClusterName: name},
+				},
 			}
-			region, cluster := strings.TrimSpace(splitted[0]), strings.TrimSpace(splitted[1])
-			activeClustersByRegion[region] = cluster
+		} else {
+			if _, ok := existing.ClusterAttributes[scope]; ok {
+				return types.ActiveClusters{}, fmt.Errorf("option active_clusters format is invalid. the key %q was duplicated. This can only map to a single active cluster", scope)
+			}
+			existing.ClusterAttributes[scope] = types.ActiveClusterInfo{ActiveClusterName: name}
+			out.AttributeScopes[attribute] = existing
 		}
-	}
 
-	if isActiveActiveDomain && len(activeClustersByRegion) == 0 {
-		return nil, commoncli.Problem("Option --active_clusters_by_region is required for active-active domain.", nil)
+		out.AttributeScopes[attribute].ClusterAttributes[scope] = types.ActiveClusterInfo{ActiveClusterName: name}
 	}
 
-	return activeClustersByRegion, nil
+	return out, nil
 }