Allow registering search attributes without Advance Visibility enabled (

cadence-workflow#5185)

* remove validation & test for add search attribute with no advanced config

- Remove validation for Advance Visibility Store
- Add Advance Visibility Config check before update ElasticSearch/OpenSearch mapping
- Remove co-related test for 'no advanced config'

* Update CHANGELOG.md

Update CHANGELOG.md

* Add warn level message if skip updating OpenSearch/ElasticSearch mapping

* Add warn level message and add validSearchAttributes in development.yaml

---------

Co-authored-by: Quanzheng Long <[email protected]>

CHANGELOG.md

@@ -11,6 +11,7 @@ You can find a list of previous releases on the [github releases](https://github
 - Added TLS support for gRPC (#4606). Use `tls` config section under service `rpc` block to enable it.
 ### Changed
 - Default outbound between internal server components are now switched to gRPC. There is still an option to switch back to TChannel by setting dynamic config `system.enableGRPCOutbound` to `false`. However this is now considered deprecated and will be removed in the future release.
+- Allow registering search attributes when Advanced Visibility is not enabled
 
 ## [0.23.0] - TBD
 ### Added

config/dynamicconfig/development.yaml

@@ -13,3 +13,39 @@ history.enableCrossClusterOperations:
 history.useNewInitialFailoverVersion:
 - value: true
   constraints: {}
+frontend.validSearchAttributes:
+- value:
+    BinaryChecksums: 1
+    CadenceChangeVersion: 1
+    CloseStatus: 2
+    CloseTime: 2
+    CustomBoolField: 4
+    CustomDatetimeField: 5
+    CustomDomain: 1
+    CustomDoubleField: 3
+    CustomIntField: 2
+    CustomKeywordField: 1
+    CustomStringField: 0
+    DomainID: 1
+    ExecutionTime: 2
+    HistoryLength: 2
+    IsCron: 1
+    NewKey: 1
+    NumClusters: 2
+    Operator: 1
+    Passed: 4
+    RolloutID: 1
+    RunID: 1
+    ShardID: 2
+    StartTime: 2
+    TaskList: 1
+    UpdateTime: 2
+    WorkflowID: 1
+    WorkflowType: 1
+    addon: 1
+    addon-type: 1
+    environment: 1
+    project: 1
+    service: 1
+    user: 1
+  constraints: {}

service/frontend/adminHandler.go

@@ -216,9 +216,6 @@ func (adh *adminHandlerImpl) AddSearchAttribute(
 	if len(request.GetSearchAttribute()) == 0 {
 		return adh.error(&types.BadRequestError{Message: "SearchAttributes are not provided"}, scope)
 	}
-	if err := adh.validateConfigForAdvanceVisibility(); err != nil {
-		return adh.error(&types.BadRequestError{Message: "AdvancedVisibilityStore is not configured for this Cadence Cluster"}, scope)
-	}
 
 	searchAttr := request.GetSearchAttribute()
 	currentValidAttr, err := adh.params.DynamicConfig.GetMapValue(dc.ValidSearchAttributes, nil)
@@ -245,23 +242,27 @@ func (adh *adminHandlerImpl) AddSearchAttribute(
 		adh.GetLogger().Warn("Failed to update dynamicconfig. This is only useful in local dev environment for filebased config. Please ignore this warn if this is in a real Cluster, because your filebased dynamicconfig MUST be updated separately. Configstore dynamic config will also require separate updating via the CLI.")
 	}
 
-	// update elasticsearch mapping, new added field will not be able to remove or update
-	index := adh.params.ESConfig.GetVisibilityIndex()
-	for k, v := range searchAttr {
-		valueType := convertIndexedValueTypeToESDataType(v)
-		if len(valueType) == 0 {
-			return adh.error(&types.BadRequestError{Message: fmt.Sprintf("Unknown value type, %v", v)}, scope)
-		}
-		err := adh.params.ESClient.PutMapping(ctx, index, definition.Attr, k, valueType)
-		if adh.esClient.IsNotFoundError(err) {
-			err = adh.params.ESClient.CreateIndex(ctx, index)
+	// when have valid advance visibility config, update elasticsearch mapping, new added field will not be able to remove or update
+	if err := adh.validateConfigForAdvanceVisibility(); err != nil {
+		adh.GetLogger().Warn("Skip updating OpenSearch/ElasticSearch mapping since Advance Visibility hasn't been enabled.")
+	} else {
+		index := adh.params.ESConfig.GetVisibilityIndex()
+		for k, v := range searchAttr {
+			valueType := convertIndexedValueTypeToESDataType(v)
+			if len(valueType) == 0 {
+				return adh.error(&types.BadRequestError{Message: fmt.Sprintf("Unknown value type, %v", v)}, scope)
+			}
+			err := adh.params.ESClient.PutMapping(ctx, index, definition.Attr, k, valueType)
+			if adh.esClient.IsNotFoundError(err) {
+				err = adh.params.ESClient.CreateIndex(ctx, index)
+				if err != nil {
+					return adh.error(&types.InternalServiceError{Message: fmt.Sprintf("Failed to create ES index, err: %v", err)}, scope)
+				}
+				err = adh.params.ESClient.PutMapping(ctx, index, definition.Attr, k, valueType)
+			}
 			if err != nil {
-				return adh.error(&types.InternalServiceError{Message: fmt.Sprintf("Failed to create ES index, err: %v", err)}, scope)
+				return adh.error(&types.InternalServiceError{Message: fmt.Sprintf("Failed to update ES mapping, err: %v", err)}, scope)
 			}
-			err = adh.params.ESClient.PutMapping(ctx, index, definition.Attr, k, valueType)
-		}
-		if err != nil {
-			return adh.error(&types.InternalServiceError{Message: fmt.Sprintf("Failed to update ES mapping, err: %v", err)}, scope)
 		}
 	}
 

service/frontend/adminHandler_test.go

@@ -541,15 +541,6 @@ func (s *adminHandlerSuite) Test_AddSearchAttribute_Validate() {
 			Request:  &types.AddSearchAttributeRequest{},
 			Expected: &types.BadRequestError{Message: "SearchAttributes are not provided"},
 		},
-		{
-			Name: "no advanced config",
-			Request: &types.AddSearchAttributeRequest{
-				SearchAttribute: map[string]types.IndexedValueType{
-					"CustomKeywordField": 1,
-				},
-			},
-			Expected: &types.BadRequestError{Message: "AdvancedVisibilityStore is not configured for this Cadence Cluster"},
-		},
 	}
 	for _, testCase := range testCases1 {
 		s.Equal(testCase.Expected, handler.AddSearchAttribute(ctx, testCase.Request))