Search attribute validation toggling (uber#5033)

* Introduce dyn. config flag for toggling attr. validation, implement flag check before performing validation

* Toggle validation on attaching attr. in WF start, toggle indexProcessor level validation

* Update construction of validation tests

* Add new dynamic config property to unit test

* Handle nil case for new config property in validation

Co-authored-by: David Porter <david.porter@uber.com>
Co-authored-by: Zack Kirsch <zackk@uber.com>

common/dynamicconfig/constants.go

@@ -1417,6 +1417,12 @@ const (
 	// Default value: false
 	// Allowed filters: DomainName
 	FrontendEmitSignalNameMetricsTag
+	// EnableQueryAttributeValidation enables validation of queries' search attributes against the dynamic config whitelist
+	// Keyname: frontend.enableQueryAttributeValidation
+	// Value type: Bool
+	// Default value: true
+	// Allowed filters: N/A
+	EnableQueryAttributeValidation
 
 	// key for matching
 
@@ -3474,6 +3480,11 @@ var BoolKeys = map[BoolKey]DynamicBool{
 		Description:  "FrontendEmitSignalNameMetricsTag enables emitting signal name tag in metrics in frontend client",
 		DefaultValue: false,
 	},
+	EnableQueryAttributeValidation: DynamicBool{
+		KeyName:      "frontend.enableQueryAttributeValidation",
+		Description:  "EnableQueryAttributeValidation enables validation of queries' search attributes against the dynamic config whitelist",
+		DefaultValue: true,
+	},
 	MatchingEnableSyncMatch: DynamicBool{
 		KeyName:      "matching.enableSyncMatch",
 		Description:  "MatchingEnableSyncMatch is to enable sync match",

common/elasticsearch/validator/queryValidator.go

@@ -35,13 +35,17 @@ import (
 
 // VisibilityQueryValidator for sql query validation
 type VisibilityQueryValidator struct {
-	validSearchAttributes dynamicconfig.MapPropertyFn
+	validSearchAttributes          dynamicconfig.MapPropertyFn
+	enableQueryAttributeValidation dynamicconfig.BoolPropertyFn
 }
 
 // NewQueryValidator create VisibilityQueryValidator
-func NewQueryValidator(validSearchAttributes dynamicconfig.MapPropertyFn) *VisibilityQueryValidator {
+func NewQueryValidator(
+	validSearchAttributes dynamicconfig.MapPropertyFn,
+	enableQueryAttributeValidation dynamicconfig.BoolPropertyFn) *VisibilityQueryValidator {
 	return &VisibilityQueryValidator{
-		validSearchAttributes: validSearchAttributes,
+		validSearchAttributes:          validSearchAttributes,
+		enableQueryAttributeValidation: enableQueryAttributeValidation,
 	}
 }
 
@@ -198,7 +202,10 @@ func (qv *VisibilityQueryValidator) validateOrderByExpr(orderBy sqlparser.OrderB
 
 // isValidSearchAttributes return true if key is registered
 func (qv *VisibilityQueryValidator) isValidSearchAttributes(key string) bool {
-	validAttr := qv.validSearchAttributes()
-	_, isValidKey := validAttr[key]
-	return isValidKey
+	if qv.enableQueryAttributeValidation() {
+		validAttr := qv.validSearchAttributes()
+		_, isValidKey := validAttr[key]
+		return isValidKey
+	}
+	return true
 }

common/elasticsearch/validator/queryValidator_test.go

@@ -131,7 +131,8 @@ func TestValidateQuery(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.msg, func(t *testing.T) {
 			validSearchAttr := dynamicconfig.GetMapPropertyFn(definition.GetDefaultIndexedKeys())
-			qv := NewQueryValidator(validSearchAttr)
+			validateSearchAttr := dynamicconfig.GetBoolPropertyFn(true)
+			qv := NewQueryValidator(validSearchAttr, validateSearchAttr)
 			validated, err := qv.ValidateQuery(tt.query)
 			if err != nil {
 				assert.Equal(t, tt.err, err.Error())

common/elasticsearch/validator/searchAttrValidator.go

@@ -35,6 +35,7 @@ import (
 type SearchAttributesValidator struct {
 	logger log.Logger
 
+	enableQueryAttributeValidation    dynamicconfig.BoolPropertyFn
 	validSearchAttributes             dynamicconfig.MapPropertyFn
 	searchAttributesNumberOfKeysLimit dynamicconfig.IntPropertyFnWithDomainFilter
 	searchAttributesSizeOfValueLimit  dynamicconfig.IntPropertyFnWithDomainFilter
@@ -44,13 +45,15 @@ type SearchAttributesValidator struct {
 // NewSearchAttributesValidator create SearchAttributesValidator
 func NewSearchAttributesValidator(
 	logger log.Logger,
+	enableQueryAttributeValidation dynamicconfig.BoolPropertyFn,
 	validSearchAttributes dynamicconfig.MapPropertyFn,
 	searchAttributesNumberOfKeysLimit dynamicconfig.IntPropertyFnWithDomainFilter,
 	searchAttributesSizeOfValueLimit dynamicconfig.IntPropertyFnWithDomainFilter,
 	searchAttributesTotalSizeLimit dynamicconfig.IntPropertyFnWithDomainFilter,
 ) *SearchAttributesValidator {
 	return &SearchAttributesValidator{
 		logger:                            logger,
+		enableQueryAttributeValidation:    enableQueryAttributeValidation,
 		validSearchAttributes:             validSearchAttributes,
 		searchAttributesNumberOfKeysLimit: searchAttributesNumberOfKeysLimit,
 		searchAttributesSizeOfValueLimit:  searchAttributesSizeOfValueLimit,
@@ -74,19 +77,27 @@ func (sv *SearchAttributesValidator) ValidateSearchAttributes(input *types.Searc
 	}
 
 	totalSize := 0
+
+	validateAttrFn := sv.enableQueryAttributeValidation
+	validateAttr := true
+	if validateAttrFn != nil {
+		validateAttr = validateAttrFn()
+	}
 	validAttr := sv.validSearchAttributes()
 	for key, val := range fields {
-		// verify: key is whitelisted
-		if !sv.isValidSearchAttributesKey(validAttr, key) {
-			sv.logger.WithTags(tag.ESKey(key), tag.WorkflowDomainName(domain)).
-				Error("invalid search attribute key")
-			return &types.BadRequestError{Message: fmt.Sprintf("%s is not a valid search attribute key", key)}
-		}
-		// verify: value has the correct type
-		if !sv.isValidSearchAttributesValue(validAttr, key, val) {
-			sv.logger.WithTags(tag.ESKey(key), tag.ESValue(val), tag.WorkflowDomainName(domain)).
-				Error("invalid search attribute value")
-			return &types.BadRequestError{Message: fmt.Sprintf("%s is not a valid search attribute value for key %s", val, key)}
+		if validateAttr {
+			// verify: key is whitelisted
+			if !sv.isValidSearchAttributesKey(validAttr, key) {
+				sv.logger.WithTags(tag.ESKey(key), tag.WorkflowDomainName(domain)).
+					Error("invalid search attribute key")
+				return &types.BadRequestError{Message: fmt.Sprintf("%s is not a valid search attribute key", key)}
+			}
+			// verify: value has the correct type
+			if !sv.isValidSearchAttributesValue(validAttr, key, val) {
+				sv.logger.WithTags(tag.ESKey(key), tag.ESValue(val), tag.WorkflowDomainName(domain)).
+					Error("invalid search attribute value")
+				return &types.BadRequestError{Message: fmt.Sprintf("%s is not a valid search attribute value for key %s", val, key)}
+			}
 		}
 		// verify: key is not system reserved
 		if definition.IsSystemIndexedKey(key) {

common/elasticsearch/validator/searchAttrValidator_test.go

@@ -46,6 +46,7 @@ func (s *searchAttributesValidatorSuite) TestValidateSearchAttributes() {
 	sizeOfTotalLimit := 20
 
 	validator := NewSearchAttributesValidator(log.NewNoop(),
+		dynamicconfig.GetBoolPropertyFn(true),
 		dynamicconfig.GetMapPropertyFn(definition.GetDefaultIndexedKeys()),
 		dynamicconfig.GetIntPropertyFilteredByDomain(numOfKeysLimit),
 		dynamicconfig.GetIntPropertyFilteredByDomain(sizeOfValueLimit),

service/frontend/service.go

@@ -61,6 +61,7 @@ type Config struct {
 	GlobalDomainWorkerRPS             dynamicconfig.IntPropertyFnWithDomainFilter
 	GlobalDomainVisibilityRPS         dynamicconfig.IntPropertyFnWithDomainFilter
 	EnableClientVersionCheck          dynamicconfig.BoolPropertyFn
+	EnableQueryAttributeValidation    dynamicconfig.BoolPropertyFn
 	DisallowQuery                     dynamicconfig.BoolPropertyFnWithDomainFilter
 	ShutdownDrainDuration             dynamicconfig.DurationPropertyFn
 	Lockdown                          dynamicconfig.BoolPropertyFnWithDomainFilter
@@ -160,6 +161,7 @@ func NewConfig(dc *dynamicconfig.Collection, numHistoryShards int, isAdvancedVis
 		DomainFailoverRefreshInterval:               dc.GetDurationProperty(dynamicconfig.DomainFailoverRefreshInterval),
 		DomainFailoverRefreshTimerJitterCoefficient: dc.GetFloat64Property(dynamicconfig.DomainFailoverRefreshTimerJitterCoefficient),
 		EnableClientVersionCheck:                    dc.GetBoolProperty(dynamicconfig.EnableClientVersionCheck),
+		EnableQueryAttributeValidation:              dc.GetBoolProperty(dynamicconfig.EnableQueryAttributeValidation),
 		ValidSearchAttributes:                       dc.GetMapProperty(dynamicconfig.ValidSearchAttributes),
 		SearchAttributesNumberOfKeysLimit:           dc.GetIntPropertyFilteredByDomain(dynamicconfig.SearchAttributesNumberOfKeysLimit),
 		SearchAttributesSizeOfValueLimit:            dc.GetIntPropertyFilteredByDomain(dynamicconfig.SearchAttributesSizeOfValueLimit),

service/frontend/workflowHandler.go

@@ -217,9 +217,13 @@ func NewWorkflowHandler(
 			resource.GetArchiverProvider(),
 			resource.GetTimeSource(),
 		),
-		visibilityQueryValidator: validator.NewQueryValidator(config.ValidSearchAttributes),
+		visibilityQueryValidator: validator.NewQueryValidator(
+			config.ValidSearchAttributes,
+			config.EnableQueryAttributeValidation,
+		),
 		searchAttributesValidator: validator.NewSearchAttributesValidator(
 			resource.GetLogger(),
+			config.EnableQueryAttributeValidation,
 			config.ValidSearchAttributes,
 			config.SearchAttributesNumberOfKeysLimit,
 			config.SearchAttributesSizeOfValueLimit,

service/history/config/config.go

@@ -244,6 +244,7 @@ type Config struct {
 	PendingActivityValidationEnabled dynamicconfig.BoolPropertyFn
 
 	// ValidSearchAttributes is legal indexed keys that can be used in list APIs
+	EnableQueryAttributeValidation    dynamicconfig.BoolPropertyFn
 	ValidSearchAttributes             dynamicconfig.MapPropertyFn
 	SearchAttributesNumberOfKeysLimit dynamicconfig.IntPropertyFnWithDomainFilter
 	SearchAttributesSizeOfValueLimit  dynamicconfig.IntPropertyFnWithDomainFilter

service/history/decision/checker.go

@@ -79,6 +79,7 @@ func newAttrValidator(
 		logger:        logger,
 		searchAttributesValidator: validator.NewSearchAttributesValidator(
 			logger,
+			config.EnableQueryAttributeValidation,
 			config.ValidSearchAttributes,
 			config.SearchAttributesNumberOfKeysLimit,
 			config.SearchAttributesSizeOfValueLimit,

service/history/decision/checker_test.go

@@ -91,6 +91,7 @@ func (s *attrValidatorSuite) SetupTest() {
 		MarkerNameMaxLength:               dynamicconfig.GetIntPropertyFilteredByDomain(1000),
 		TimerIDMaxLength:                  dynamicconfig.GetIntPropertyFilteredByDomain(1000),
 		ValidSearchAttributes:             dynamicconfig.GetMapPropertyFn(definition.GetDefaultIndexedKeys()),
+		EnableQueryAttributeValidation:    dynamicconfig.GetBoolPropertyFn(true),
 		SearchAttributesNumberOfKeysLimit: dynamicconfig.GetIntPropertyFilteredByDomain(100),
 		SearchAttributesSizeOfValueLimit:  dynamicconfig.GetIntPropertyFilteredByDomain(2 * 1024),
 		SearchAttributesTotalSizeLimit:    dynamicconfig.GetIntPropertyFilteredByDomain(40 * 1024),

service/worker/indexer/indexer.go

@@ -47,12 +47,13 @@ type (
 
 	// Config contains all configs for indexer
 	Config struct {
-		IndexerConcurrency       dynamicconfig.IntPropertyFn
-		ESProcessorNumOfWorkers  dynamicconfig.IntPropertyFn
-		ESProcessorBulkActions   dynamicconfig.IntPropertyFn // max number of requests in bulk
-		ESProcessorBulkSize      dynamicconfig.IntPropertyFn // max total size of bytes in bulk
-		ESProcessorFlushInterval dynamicconfig.DurationPropertyFn
-		ValidSearchAttributes    dynamicconfig.MapPropertyFn
+		IndexerConcurrency             dynamicconfig.IntPropertyFn
+		ESProcessorNumOfWorkers        dynamicconfig.IntPropertyFn
+		ESProcessorBulkActions         dynamicconfig.IntPropertyFn // max number of requests in bulk
+		ESProcessorBulkSize            dynamicconfig.IntPropertyFn // max total size of bytes in bulk
+		ESProcessorFlushInterval       dynamicconfig.DurationPropertyFn
+		ValidSearchAttributes          dynamicconfig.MapPropertyFn
+		EnableQueryAttributeValidation dynamicconfig.BoolPropertyFn
 	}
 )
 

service/worker/indexer/processor.go

@@ -285,6 +285,9 @@ func (p *indexProcessor) dumpFieldsToMap(fields map[string]*indexer.Field) map[s
 }
 
 func (p *indexProcessor) isValidFieldToES(field string) bool {
+	if !p.config.EnableQueryAttributeValidation() {
+		return true
+	}
 	if _, ok := p.config.ValidSearchAttributes()[field]; ok {
 		return true
 	}

service/worker/service.go

@@ -194,12 +194,13 @@ func NewConfig(params *resource.Params) *Config {
 	)
 	if common.IsAdvancedVisibilityWritingEnabled(advancedVisWritingMode(), params.PersistenceConfig.IsAdvancedVisibilityConfigExist()) {
 		config.IndexerCfg = &indexer.Config{
-			IndexerConcurrency:       dc.GetIntProperty(dynamicconfig.WorkerIndexerConcurrency),
-			ESProcessorNumOfWorkers:  dc.GetIntProperty(dynamicconfig.WorkerESProcessorNumOfWorkers),
-			ESProcessorBulkActions:   dc.GetIntProperty(dynamicconfig.WorkerESProcessorBulkActions),
-			ESProcessorBulkSize:      dc.GetIntProperty(dynamicconfig.WorkerESProcessorBulkSize),
-			ESProcessorFlushInterval: dc.GetDurationProperty(dynamicconfig.WorkerESProcessorFlushInterval),
-			ValidSearchAttributes:    dc.GetMapProperty(dynamicconfig.ValidSearchAttributes),
+			IndexerConcurrency:             dc.GetIntProperty(dynamicconfig.WorkerIndexerConcurrency),
+			ESProcessorNumOfWorkers:        dc.GetIntProperty(dynamicconfig.WorkerESProcessorNumOfWorkers),
+			ESProcessorBulkActions:         dc.GetIntProperty(dynamicconfig.WorkerESProcessorBulkActions),
+			ESProcessorBulkSize:            dc.GetIntProperty(dynamicconfig.WorkerESProcessorBulkSize),
+			ESProcessorFlushInterval:       dc.GetDurationProperty(dynamicconfig.WorkerESProcessorFlushInterval),
+			ValidSearchAttributes:          dc.GetMapProperty(dynamicconfig.ValidSearchAttributes),
+			EnableQueryAttributeValidation: dc.GetBoolProperty(dynamicconfig.EnableQueryAttributeValidation),
 		}
 	}
 	return config