refactor: Require ActiveClusterName for active-active domains

common/domain/attrValidator.go

@@ -101,6 +101,10 @@ func (d *AttrValidatorImpl) validateDomainReplicationConfigForGlobalDomain(
 	clusters := replicationConfig.Clusters
 	activeClusters := replicationConfig.ActiveClusters
 
+	if activeCluster == "" {
+		return errActiveClusterNameRequired
+	}
+
 	for _, clusterConfig := range clusters {
 		if err := d.validateClusterName(clusterConfig.ClusterName); err != nil {
 			return err
@@ -116,7 +120,16 @@ func (d *AttrValidatorImpl) validateDomainReplicationConfigForGlobalDomain(
 		return false
 	}
 
+	if err := d.validateClusterName(activeCluster); err != nil {
+		return err
+	}
+
+	if !isInClusters(activeCluster) {
+		return errActiveClusterNotInClusters
+	}
+
 	if replicationConfig.IsActiveActive() {
+		// For active-active domains, also validate that all clusters in ActiveClustersByRegion are valid
 		for _, cluster := range activeClusters.ActiveClustersByRegion {
 			if err := d.validateClusterName(cluster.ActiveClusterName); err != nil {
 				return err
@@ -126,14 +139,6 @@ func (d *AttrValidatorImpl) validateDomainReplicationConfigForGlobalDomain(
 				return errActiveClusterNotInClusters
 			}
 		}
-	} else {
-		if err := d.validateClusterName(activeCluster); err != nil {
-			return err
-		}
-
-		if !isInClusters(activeCluster) {
-			return errActiveClusterNotInClusters
-		}
 	}
 
 	return nil

common/domain/attrValidator_test.go

@@ -237,6 +237,55 @@ func (s *attrValidatorSuite) TestValidateDomainReplicationConfigForGlobalDomain(
 		},
 	)
 	s.NoError(err)
+
+	// When ActiveClusterName is not provided, and ActiveClusters are not provided, it should return an error
+	err = s.validator.validateDomainReplicationConfigForGlobalDomain(
+		&persistence.DomainReplicationConfig{
+			ActiveClusterName: "",
+			Clusters: []*persistence.ClusterReplicationConfig{
+				{ClusterName: cluster.TestCurrentClusterName},
+			},
+		},
+	)
+	s.Error(err)
+	s.IsType(&types.BadRequestError{}, err)
+
+	// When ActiveClusterName and ActiveClusters are provided, it should not return an error
+	err = s.validator.validateDomainReplicationConfigForGlobalDomain(
+		&persistence.DomainReplicationConfig{
+			ActiveClusterName: cluster.TestCurrentClusterName,
+			Clusters: []*persistence.ClusterReplicationConfig{
+				{ClusterName: cluster.TestCurrentClusterName},
+				{ClusterName: cluster.TestAlternativeClusterName},
+			},
+			ActiveClusters: &types.ActiveClusters{
+				ActiveClustersByRegion: map[string]types.ActiveClusterInfo{
+					cluster.TestRegion1: {ActiveClusterName: cluster.TestCurrentClusterName},
+					cluster.TestRegion2: {ActiveClusterName: cluster.TestAlternativeClusterName},
+				},
+			},
+		},
+	)
+	s.NoError(err)
+
+	// When ActiveClusterName is not provided, and ActiveClusters are provided, it should return an error
+	err = s.validator.validateDomainReplicationConfigForGlobalDomain(
+		&persistence.DomainReplicationConfig{
+			ActiveClusterName: "",
+			Clusters: []*persistence.ClusterReplicationConfig{
+				{ClusterName: cluster.TestCurrentClusterName},
+				{ClusterName: cluster.TestAlternativeClusterName},
+			},
+			ActiveClusters: &types.ActiveClusters{
+				ActiveClustersByRegion: map[string]types.ActiveClusterInfo{
+					cluster.TestRegion1: {ActiveClusterName: cluster.TestCurrentClusterName},
+					cluster.TestRegion2: {ActiveClusterName: cluster.TestAlternativeClusterName},
+				},
+			},
+		},
+	)
+	s.Error(err)
+	s.IsType(&types.BadRequestError{}, err)
 }
 
 func (s *attrValidatorSuite) TestValidateDomainReplicationConfigClustersDoesNotRemove() {

common/domain/errors.go

@@ -32,6 +32,7 @@ var (
 	errGracefulFailoverInActiveCluster     = &types.BadRequestError{Message: "Cannot start the graceful failover from an active cluster to an active cluster."}
 	errOngoingGracefulFailover             = &types.BadRequestError{Message: "Cannot start concurrent graceful failover."}
 	errInvalidGracefulFailover             = &types.BadRequestError{Message: "Cannot start graceful failover without updating active cluster or in local domain."}
+	errActiveClusterNameRequired           = &types.BadRequestError{Message: "ActiveClusterName is required for all global domains."}
 
 	errInvalidRetentionPeriod = &types.BadRequestError{Message: "A valid retention period is not set on request."}
 	errInvalidArchivalConfig  = &types.BadRequestError{Message: "Invalid to enable archival without specifying a uri."}

common/domain/handler.go

@@ -268,12 +268,6 @@ func (d *handlerImpl) RegisterDomain(
 		return err
 	}
 
-	if activeClusters != nil {
-		// TODO: Leave a default activeClusterName for active-active domains
-		// active-active domain, activeClusterName is not used
-		activeClusterName = ""
-	}
-
 	replicationConfig := &persistence.DomainReplicationConfig{
 		ActiveClusterName: activeClusterName,
 		Clusters:          clusters,

common/domain/handler_test.go

@@ -366,7 +366,31 @@ func TestRegisterDomain(t *testing.T) {
 			expectedErr: &types.BadRequestError{},
 		},
 		{
-			name: "active-active domain successfully registered",
+			name: "active-active domain successfully registered with explicit ActiveClusterName",
+			request: &types.RegisterDomainRequest{
+				Name:              "active-active-domain",
+				IsGlobalDomain:    true,
+				ActiveClusterName: cluster.TestCurrentClusterName,
+				ActiveClustersByRegion: map[string]string{
+					cluster.TestRegion1: cluster.TestCurrentClusterName,
+					cluster.TestRegion2: cluster.TestAlternativeClusterName,
+				},
+				Clusters: []*types.ClusterReplicationConfiguration{
+					{ClusterName: cluster.TestCurrentClusterName},
+					{ClusterName: cluster.TestAlternativeClusterName},
+				},
+				WorkflowExecutionRetentionPeriodInDays: 3,
+			},
+			isPrimaryCluster: true,
+			mockSetup: func(mockDomainMgr *persistence.MockDomainManager, mockReplicator *MockReplicator, request *types.RegisterDomainRequest) {
+				mockDomainMgr.EXPECT().GetDomain(gomock.Any(), &persistence.GetDomainRequest{Name: request.Name}).Return(nil, &types.EntityNotExistsError{})
+				mockDomainMgr.EXPECT().CreateDomain(gomock.Any(), gomock.Any()).Return(&persistence.CreateDomainResponse{ID: "test-domain-id"}, nil)
+				mockReplicator.EXPECT().HandleTransmissionTask(gomock.Any(), types.DomainOperationCreate, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), commonconstants.InitialPreviousFailoverVersion, true).Return(nil)
+			},
+			wantErr: false,
+		},
+		{
+			name: "active-active domain successfully registered without explicit ActiveClusterName (uses current cluster)",
 			request: &types.RegisterDomainRequest{
 				Name:           "active-active-domain",
 				IsGlobalDomain: true,

common/persistence/data_manager_interfaces.go

@@ -1148,12 +1148,14 @@ type (
 		Clusters []*ClusterReplicationConfig
 
 		// ActiveClusterName is the name of the cluster that the domain is active in.
-		// Applicable for active-passive domains.
+		// Required for all global domains (both active-passive and active-active).
+		// For active-passive domains, this is the single active cluster.
+		// For active-active domains this is the default cluster whenever a ClusterAttribute is not provided
 		ActiveClusterName string
 
-		// TODO(c-warren): Update documentation once ActiveClusterName is the default for active-active domains.
 		// ActiveClusters is only applicable for active-active domains.
-		// If this is set, ActiveClusterName is ignored.
+		// When this is set, the domain is considered active-active and workflows are routed
+		// based on their ClusterAttributes.
 		ActiveClusters *types.ActiveClusters
 	}