From 650dc81657199035e30b15222872ee8b273867f6 Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jul 2024 15:08:00 +0200 Subject: [PATCH 1/5] Unify metrics for blob size/history size/history count --- .../history/decision/attribute_validator.go | 753 +++++++++++++++ .../decision/attribute_validator_test.go | 758 +++++++++++++++ service/history/decision/checker.go | 748 +------------- service/history/decision/checker_test.go | 912 ++++-------------- 4 files changed, 1716 insertions(+), 1455 deletions(-) create mode 100644 service/history/decision/attribute_validator.go create mode 100644 service/history/decision/attribute_validator_test.go diff --git a/service/history/decision/attribute_validator.go b/service/history/decision/attribute_validator.go new file mode 100644 index 00000000000..966737c1311 --- /dev/null +++ b/service/history/decision/attribute_validator.go @@ -0,0 +1,753 @@ +package decision + +import ( + "fmt" + "github.com/pborman/uuid" + "github.com/uber/cadence/common" + "github.com/uber/cadence/common/backoff" + "github.com/uber/cadence/common/cache" + "github.com/uber/cadence/common/elasticsearch/validator" + "github.com/uber/cadence/common/log" + "github.com/uber/cadence/common/log/tag" + "github.com/uber/cadence/common/metrics" + "github.com/uber/cadence/common/persistence" + "github.com/uber/cadence/common/types" + "github.com/uber/cadence/service/history/config" + "github.com/uber/cadence/service/history/execution" + "strings" +) + +type attrValidator struct { + config *config.Config + domainCache cache.DomainCache + metricsClient metrics.Client + logger log.Logger + searchAttributesValidator *validator.SearchAttributesValidator +} + +func newAttrValidator( + domainCache cache.DomainCache, + metricsClient metrics.Client, + config *config.Config, + logger log.Logger, +) *attrValidator { + return &attrValidator{ + config: config, + domainCache: domainCache, + metricsClient: metricsClient, + logger: logger, + searchAttributesValidator: validator.NewSearchAttributesValidator( + logger, + config.EnableQueryAttributeValidation, + config.ValidSearchAttributes, + config.SearchAttributesNumberOfKeysLimit, + config.SearchAttributesSizeOfValueLimit, + config.SearchAttributesTotalSizeLimit, + ), + } +} + +func (v *attrValidator) validateActivityScheduleAttributes( + domainID string, + targetDomainID string, + attributes *types.ScheduleActivityTaskDecisionAttributes, + wfTimeout int32, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "ScheduleActivityTaskDecisionAttributes is not set on decision."} + } + + defaultTaskListName := "" + if _, err := v.validatedTaskList(attributes.TaskList, defaultTaskListName, metricsScope, attributes.GetDomain()); err != nil { + return err + } + + if attributes.GetActivityID() == "" { + return &types.BadRequestError{Message: "ActivityId is not set on decision."} + } + + if attributes.ActivityType == nil || attributes.ActivityType.GetName() == "" { + return &types.BadRequestError{Message: "ActivityType is not set on decision."} + } + + if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { + return err + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetActivityID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.ActivityIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrActivityIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeActivityID) { + return &types.BadRequestError{Message: "ActivityID exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetActivityType().GetName(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.ActivityTypeMaxLength(attributes.GetDomain()), + metrics.CadenceErrActivityTypeExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeActivityType) { + return &types.BadRequestError{Message: "ActivityType exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + // Only attempt to deduce and fill in unspecified timeouts only when all timeouts are non-negative. + if attributes.GetScheduleToCloseTimeoutSeconds() < 0 || attributes.GetScheduleToStartTimeoutSeconds() < 0 || + attributes.GetStartToCloseTimeoutSeconds() < 0 || attributes.GetHeartbeatTimeoutSeconds() < 0 { + return &types.BadRequestError{Message: "A valid timeout may not be negative."} + } + + // ensure activity timeout never larger than workflow timeout + if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetScheduleToStartTimeoutSeconds() > wfTimeout { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetStartToCloseTimeoutSeconds() > wfTimeout { + attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetHeartbeatTimeoutSeconds() > wfTimeout { + attributes.HeartbeatTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + + validScheduleToClose := attributes.GetScheduleToCloseTimeoutSeconds() > 0 + validScheduleToStart := attributes.GetScheduleToStartTimeoutSeconds() > 0 + validStartToClose := attributes.GetStartToCloseTimeoutSeconds() > 0 + + if validScheduleToClose { + if !validScheduleToStart { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) + } + if !validStartToClose { + attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) + } + } else if validScheduleToStart && validStartToClose { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToStartTimeoutSeconds() + attributes.GetStartToCloseTimeoutSeconds()) + if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + } else { + // Deduction failed as there's not enough information to fill in missing timeouts. + return &types.BadRequestError{Message: "A valid ScheduleToCloseTimeout is not set on decision."} + } + + // ensure activity's SCHEDULE_TO_START and SCHEDULE_TO_CLOSE is as long as expiration on retry policy + // if SCHEDULE_TO_START timeout is retryable + p := attributes.RetryPolicy + if p != nil { + isScheduleToStartRetryable := true + scheduleToStartErrorReason := execution.TimerTypeToReason(execution.TimerTypeScheduleToStart) + for _, reason := range p.GetNonRetriableErrorReasons() { + if reason == scheduleToStartErrorReason { + isScheduleToStartRetryable = false + break + } + } + + expiration := p.GetExpirationIntervalInSeconds() + if expiration == 0 || expiration > wfTimeout { + expiration = wfTimeout + } + + if isScheduleToStartRetryable { + // If schedule to start timeout is retryable, we don't need to fail the activity and schedule + // it again on the same tasklist, as it's a no-op). Extending schedule to start timeout to achieve + // the same thing. + // + // Theoretically, we can extend schedule to start to be as long as the expiration time, + // but if user specifies a very long expiration time and activity task got lost after the activity is + // scheduled, workflow will be stuck for a long time. So here, we cap the schedule to start timeout + // to a maximum value, so that when the activity task got lost, timeout can happen sooner and schedule + // the activity again. + + domainName, _ := v.domainCache.GetDomainName(domainID) // if this call returns an error, we will just used the default value for max timeout + maximumScheduleToStartTimeoutForRetryInSeconds := int32(v.config.ActivityMaxScheduleToStartTimeoutForRetry(domainName).Seconds()) + scheduleToStartExpiration := common.MinInt32(expiration, maximumScheduleToStartTimeoutForRetryInSeconds) + if attributes.GetScheduleToStartTimeoutSeconds() < scheduleToStartExpiration { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(scheduleToStartExpiration) + } + + // TODO: uncomment the following code when the client side bug for calculating scheduleToClose deadline is fixed and + // fully rolled out. Before that, we still need to extend scheduleToClose timeout to be as long as the expiration interval + // + // scheduleToCloseExpiration := common.MinInt32(expiration, scheduleToStartExpiration+attributes.GetStartToCloseTimeoutSeconds()) + // if attributes.GetScheduleToCloseTimeoutSeconds() < scheduleToCloseExpiration { + // attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(scheduleToCloseExpiration) + // } + } + + if attributes.GetScheduleToCloseTimeoutSeconds() < expiration { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(expiration) + } + } + return nil +} + +func (v *attrValidator) validateTimerScheduleAttributes( + attributes *types.StartTimerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "StartTimerDecisionAttributes is not set on decision."} + } + if attributes.GetTimerID() == "" { + return &types.BadRequestError{Message: "TimerId is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetTimerID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TimerIDMaxLength(domain), + metrics.CadenceErrTimerIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTimerID) { + return &types.BadRequestError{Message: "TimerId exceeds length limit."} + } + if attributes.GetStartToFireTimeoutSeconds() <= 0 { + return &types.BadRequestError{ + Message: fmt.Sprintf("Invalid StartToFireTimeoutSeconds: %v", attributes.GetStartToFireTimeoutSeconds()), + } + } + return nil +} + +func (v *attrValidator) validateActivityCancelAttributes( + attributes *types.RequestCancelActivityTaskDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "RequestCancelActivityTaskDecisionAttributes is not set on decision."} + } + if attributes.GetActivityID() == "" { + return &types.BadRequestError{Message: "ActivityId is not set on decision."} + } + + if !common.IsValidIDLength( + attributes.GetActivityID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.ActivityIDMaxLength(domain), + metrics.CadenceErrActivityIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeActivityID) { + return &types.BadRequestError{Message: "ActivityId exceeds length limit."} + } + return nil +} + +func (v *attrValidator) validateTimerCancelAttributes( + attributes *types.CancelTimerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CancelTimerDecisionAttributes is not set on decision."} + } + if attributes.GetTimerID() == "" { + return &types.BadRequestError{Message: "TimerId is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetTimerID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TimerIDMaxLength(domain), + metrics.CadenceErrTimerIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTimerID) { + return &types.BadRequestError{Message: "TimerId exceeds length limit."} + } + return nil +} + +func (v *attrValidator) validateRecordMarkerAttributes( + attributes *types.RecordMarkerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "RecordMarkerDecisionAttributes is not set on decision."} + } + if attributes.GetMarkerName() == "" { + return &types.BadRequestError{Message: "MarkerName is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetMarkerName(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.MarkerNameMaxLength(domain), + metrics.CadenceErrMarkerNameExceededWarnLimit, + domain, + v.logger, + tag.IDTypeMarkerName) { + return &types.BadRequestError{Message: "MarkerName exceeds length limit."} + } + + return nil +} + +func (v *attrValidator) validateCompleteWorkflowExecutionAttributes( + attributes *types.CompleteWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CompleteWorkflowExecutionDecisionAttributes is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateFailWorkflowExecutionAttributes( + attributes *types.FailWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "FailWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.Reason == nil { + return &types.BadRequestError{Message: "Reason is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateCancelWorkflowExecutionAttributes( + attributes *types.CancelWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CancelWorkflowExecutionDecisionAttributes is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateCancelExternalWorkflowExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.RequestCancelExternalWorkflowExecutionDecisionAttributes, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "RequestCancelExternalWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.WorkflowID == "" { + return &types.BadRequestError{Message: "WorkflowId is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + runID := attributes.GetRunID() + if runID != "" && uuid.Parse(runID) == nil { + return &types.BadRequestError{Message: "Invalid RunId set on decision."} + } + + return nil +} + +func (v *attrValidator) validateSignalExternalWorkflowExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.SignalExternalWorkflowExecutionDecisionAttributes, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.Execution == nil { + return &types.BadRequestError{Message: "Execution is nil on decision."} + } + if attributes.Execution.WorkflowID == "" { + return &types.BadRequestError{Message: "WorkflowId is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.Execution.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + + targetRunID := attributes.Execution.GetRunID() + if targetRunID != "" && uuid.Parse(targetRunID) == nil { + return &types.BadRequestError{Message: "Invalid RunId set on decision."} + } + if attributes.SignalName == "" { + return &types.BadRequestError{Message: "SignalName is not set on decision."} + } + + return nil +} + +func (v *attrValidator) validateUpsertWorkflowSearchAttributes( + domainName string, + attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision."} + } + + if attributes.SearchAttributes == nil { + return &types.BadRequestError{Message: "SearchAttributes is not set on decision."} + } + + if len(attributes.GetSearchAttributes().GetIndexedFields()) == 0 { + return &types.BadRequestError{Message: "IndexedFields is empty on decision."} + } + + return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) +} + +func (v *attrValidator) validateContinueAsNewWorkflowExecutionAttributes( + attributes *types.ContinueAsNewWorkflowExecutionDecisionAttributes, + executionInfo *persistence.WorkflowExecutionInfo, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "ContinueAsNewWorkflowExecutionDecisionAttributes is not set on decision."} + } + + // Inherit workflow type from previous execution if not provided on decision + if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { + attributes.WorkflowType = &types.WorkflowType{Name: executionInfo.WorkflowTypeName} + } + + if !common.IsValidIDLength( + attributes.WorkflowType.GetName(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.WorkflowTypeMaxLength(domain), + metrics.CadenceErrWorkflowTypeExceededWarnLimit, + domain, + v.logger, + tag.IDTypeWorkflowType) { + return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} + } + + // Inherit Tasklist from previous execution if not provided on decision + taskList, err := v.validatedTaskList(attributes.TaskList, executionInfo.TaskList, metricsScope, domain) + if err != nil { + return err + } + attributes.TaskList = taskList + + // Inherit workflow timeout from previous execution if not provided on decision + if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { + attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.WorkflowTimeout) + } + + // Inherit decision task timeout from previous execution if not provided on decision + if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { + attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.DecisionStartToCloseTimeout) + } + + // Check next run decision task delay + if attributes.GetBackoffStartIntervalInSeconds() < 0 { + return &types.BadRequestError{Message: "BackoffStartInterval is less than 0."} + } + + domainName, err := v.domainCache.GetDomainName(executionInfo.DomainID) + if err != nil { + return err + } + return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) +} + +func (v *attrValidator) validateStartChildExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.StartChildWorkflowExecutionDecisionAttributes, + parentInfo *persistence.WorkflowExecutionInfo, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "StartChildWorkflowExecutionDecisionAttributes is not set on decision."} + } + + if attributes.GetWorkflowID() == "" { + return &types.BadRequestError{Message: "Required field WorkflowID is not set on decision."} + } + + if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { + return &types.BadRequestError{Message: "Required field WorkflowType is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + if !common.IsValidIDLength( + attributes.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.WorkflowType.GetName(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowTypeMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowTypeExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowType) { + return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} + } + + if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { + return err + } + + if attributes.GetCronSchedule() != "" { + if _, err := backoff.ValidateSchedule(attributes.GetCronSchedule()); err != nil { + return err + } + } + + // Inherit tasklist from parent workflow execution if not provided on decision + taskList, err := v.validatedTaskList(attributes.TaskList, parentInfo.TaskList, metricsScope, attributes.GetDomain()) + if err != nil { + return err + } + attributes.TaskList = taskList + + // Inherit workflow timeout from parent workflow execution if not provided on decision + if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { + attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.WorkflowTimeout) + } + + // Inherit decision task timeout from parent workflow execution if not provided on decision + if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { + attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.DecisionStartToCloseTimeout) + } + + return nil +} + +func (v *attrValidator) validatedTaskList( + taskList *types.TaskList, + defaultVal string, + metricsScope int, + domain string, +) (*types.TaskList, error) { + + if taskList == nil { + taskList = &types.TaskList{} + } + + if taskList.GetName() == "" { + if defaultVal == "" { + return taskList, &types.BadRequestError{Message: "missing task list name"} + } + taskList.Name = defaultVal + return taskList, nil + } + + name := taskList.GetName() + if !common.IsValidIDLength( + name, + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TaskListNameMaxLength(domain), + metrics.CadenceErrTaskListNameExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTaskListName) { + return taskList, &types.BadRequestError{ + Message: fmt.Sprintf("task list name exceeds length limit of %v", v.config.TaskListNameMaxLength(domain)), + } + } + + if strings.HasPrefix(name, common.ReservedTaskListPrefix) { + return taskList, &types.BadRequestError{ + Message: fmt.Sprintf("task list name cannot start with reserved prefix %v", common.ReservedTaskListPrefix), + } + } + + return taskList, nil +} + +func (v *attrValidator) validateCrossDomainCall( + sourceDomainID string, + targetDomainID string, +) error { + + // same name, no check needed + if sourceDomainID == targetDomainID { + return nil + } + + sourceDomainEntry, err := v.domainCache.GetDomainByID(sourceDomainID) + if err != nil { + return err + } + + targetDomainEntry, err := v.domainCache.GetDomainByID(targetDomainID) + if err != nil { + return err + } + + sourceClusters := sourceDomainEntry.GetReplicationConfig().Clusters + targetClusters := targetDomainEntry.GetReplicationConfig().Clusters + + // both "local domain" + // here a domain is "local domain" when: + // - IsGlobalDomain() returns false + // - domainCluster contains only one cluster + // case 1 can be actually be combined with this case + if len(sourceClusters) == 1 && len(targetClusters) == 1 { + if sourceClusters[0].ClusterName == targetClusters[0].ClusterName { + return nil + } + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) + } + + // both global domain with > 1 replication cluster + // when code reaches here, at least one domain has more than one cluster + if len(sourceClusters) == len(targetClusters) && + v.config.EnableCrossClusterOperationsForDomain(sourceDomainEntry.GetInfo().Name) { + // check if the source domain cluster matches those for the target domain + for _, sourceCluster := range sourceClusters { + found := false + for _, targetCluster := range targetClusters { + if sourceCluster.ClusterName == targetCluster.ClusterName { + found = true + break + } + } + if !found { + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) + } + } + return nil + } + + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) +} + +func (v *attrValidator) createCrossDomainCallError( + domainEntry *cache.DomainCacheEntry, + targetDomainEntry *cache.DomainCacheEntry, +) error { + return &types.BadRequestError{Message: fmt.Sprintf( + "cannot make cross domain call between %v and %v", + domainEntry.GetInfo().Name, + targetDomainEntry.GetInfo().Name, + )} +} diff --git a/service/history/decision/attribute_validator_test.go b/service/history/decision/attribute_validator_test.go new file mode 100644 index 00000000000..b7ed90c4388 --- /dev/null +++ b/service/history/decision/attribute_validator_test.go @@ -0,0 +1,758 @@ +// Copyright (c) 2017 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package decision + +import ( + "testing" + "time" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/uber/cadence/common" + "github.com/uber/cadence/common/cache" + "github.com/uber/cadence/common/cluster" + "github.com/uber/cadence/common/definition" + "github.com/uber/cadence/common/dynamicconfig" + "github.com/uber/cadence/common/log" + "github.com/uber/cadence/common/metrics" + "github.com/uber/cadence/common/persistence" + "github.com/uber/cadence/common/types" + "github.com/uber/cadence/service/history/config" + "github.com/uber/cadence/service/history/constants" +) + +type ( + attrValidatorSuite struct { + suite.Suite + *require.Assertions + + controller *gomock.Controller + mockDomainCache *cache.MockDomainCache + + validator *attrValidator + + testDomainID string + testTargetDomainID string + + testActivityMaxScheduleToStartTimeoutForRetryInSeconds int32 + } +) + +func TestAttrValidatorSuite(t *testing.T) { + s := new(attrValidatorSuite) + suite.Run(t, s) +} + +func (s *attrValidatorSuite) SetupSuite() { + s.testDomainID = "test domain ID" + s.testTargetDomainID = "test target domain ID" + s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds = 1800 +} + +func (s *attrValidatorSuite) TearDownSuite() { +} + +func (s *attrValidatorSuite) SetupTest() { + s.Assertions = require.New(s.T()) + + s.controller = gomock.NewController(s.T()) + s.mockDomainCache = cache.NewMockDomainCache(s.controller) + config := &config.Config{ + MaxIDLengthWarnLimit: dynamicconfig.GetIntPropertyFn(128), + DomainNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + IdentityMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + WorkflowIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + SignalNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + WorkflowTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + RequestIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + TaskListNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + ActivityIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + ActivityTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + 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), + ActivityMaxScheduleToStartTimeoutForRetry: dynamicconfig.GetDurationPropertyFnFilteredByDomain( + time.Duration(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds) * time.Second, + ), + EnableCrossClusterOperationsForDomain: dynamicconfig.GetBoolPropertyFnFilteredByDomain(false), + } + s.validator = newAttrValidator( + s.mockDomainCache, + metrics.NewNoopMetricsClient(), + config, + log.NewNoop(), + ) +} + +func (s *attrValidatorSuite) TearDownTest() { + s.controller.Finish() +} + +func (s *attrValidatorSuite) TestValidateSignalExternalWorkflowExecutionAttributes() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).AnyTimes() + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).AnyTimes() + + var attributes *types.SignalExternalWorkflowExecutionDecisionAttributes + + err := s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision.") + + attributes = &types.SignalExternalWorkflowExecutionDecisionAttributes{} + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "Execution is nil on decision.") + + attributes.Execution = &types.WorkflowExecution{} + attributes.Execution.WorkflowID = "workflow-id" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "SignalName is not set on decision.") + + attributes.Execution.RunID = "run-id" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "Invalid RunId set on decision.") + attributes.Execution.RunID = constants.TestRunID + + attributes.SignalName = "my signal name" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.NoError(err) + + attributes.Input = []byte("test input") + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.NoError(err) +} + +func (s *attrValidatorSuite) TestValidateUpsertWorkflowSearchAttributes() { + domainName := "testDomain" + var attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes + + err := s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision.") + + attributes = &types.UpsertWorkflowSearchAttributesDecisionAttributes{} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "SearchAttributes is not set on decision.") + + attributes.SearchAttributes = &types.SearchAttributes{} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "IndexedFields is empty on decision.") + + attributes.SearchAttributes.IndexedFields = map[string][]byte{"CustomKeywordField": []byte(`"bytes"`)} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToLocal() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_SameCluster() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_DiffCluster() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToGlobal() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 5678, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 5678, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToGlobal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 5678, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToLocal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToEffectiveLocal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 5678, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_SameDomain() { + targetDomainID := s.testDomainID + + err := s.validator.validateCrossDomainCall(s.testDomainID, targetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(2) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(2) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) + + s.validator.config.EnableCrossClusterOperationsForDomain = dynamicconfig.GetBoolPropertyFnFilteredByDomain(true) + err = s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: "cluster name for s.testDomainID"}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: "cluster name for s.testTargetDomainID"}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateTaskListName() { + taskList := func(name string) *types.TaskList { + kind := types.TaskListKindNormal + return &types.TaskList{Name: name, Kind: &kind} + } + + testCases := []struct { + defaultVal string + input *types.TaskList + output *types.TaskList + isOutputErr bool + }{ + {"tl-1", nil, &types.TaskList{Name: "tl-1"}, false}, + {"", taskList("tl-1"), taskList("tl-1"), false}, + {"tl-1", taskList("tl-1"), taskList("tl-1"), false}, + {"", taskList("/tl-1"), taskList("/tl-1"), false}, + {"", taskList("/__cadence_sys"), taskList("/__cadence_sys"), false}, + {"", nil, &types.TaskList{}, true}, + {"", taskList(""), taskList(""), true}, + {"", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, + {"tl-1", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, + {"", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, + {"tl-1", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, + } + + for _, tc := range testCases { + key := tc.defaultVal + "#" + if tc.input != nil { + key += tc.input.GetName() + } else { + key += "nil" + } + s.Run(key, func() { + output, err := s.validator.validatedTaskList(tc.input, tc.defaultVal, metrics.HistoryRespondDecisionTaskCompletedScope, "domain_name") + if tc.isOutputErr { + s.Error(err) + } else { + s.NoError(err) + } + s.EqualValues(tc.output, output) + }) + } +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_NoRetryPolicy() { + wfTimeout := int32(5) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(3), // ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(10), // larger then wfTimeout + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), + ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: common.Int32Ptr(wfTimeout), + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartRetryable() { + s.mockDomainCache.EXPECT().GetDomainName(s.testDomainID).Return("some random domain name", nil).Times(1) + + wfTimeout := int32(3000) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(1), + RetryPolicy: &types.RetryPolicy{ + InitialIntervalInSeconds: 1, + BackoffCoefficient: 1.1, + ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than maximumScheduleToStartTimeoutForRetryInSeconds + NonRetriableErrorReasons: []string{"non-retryable error"}, + }, + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(attributes.RetryPolicy.ExpirationIntervalInSeconds), + ScheduleToStartTimeoutSeconds: common.Int32Ptr(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds), + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, + RetryPolicy: attributes.RetryPolicy, + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartNonRetryable() { + wfTimeout := int32(1000) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(1), + RetryPolicy: &types.RetryPolicy{ + InitialIntervalInSeconds: 1, + BackoffCoefficient: 1.1, + ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than wfTimeout and maximumScheduleToStartTimeoutForRetryInSeconds + NonRetriableErrorReasons: []string{"cadenceInternal:Timeout SCHEDULE_TO_START"}, + }, + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), + ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, + RetryPolicy: attributes.RetryPolicy, + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} diff --git a/service/history/decision/checker.go b/service/history/decision/checker.go index 65e6cb2d2ec..0d03f93624c 100644 --- a/service/history/decision/checker.go +++ b/service/history/decision/checker.go @@ -21,33 +21,18 @@ package decision import ( - "fmt" - "strings" - - "github.com/pborman/uuid" + "time" "github.com/uber/cadence/common" - "github.com/uber/cadence/common/backoff" - "github.com/uber/cadence/common/cache" - "github.com/uber/cadence/common/elasticsearch/validator" "github.com/uber/cadence/common/log" "github.com/uber/cadence/common/log/tag" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/types" - "github.com/uber/cadence/service/history/config" "github.com/uber/cadence/service/history/execution" ) type ( - attrValidator struct { - config *config.Config - domainCache cache.DomainCache - metricsClient metrics.Client - logger log.Logger - searchAttributesValidator *validator.SearchAttributesValidator - } - workflowSizeChecker struct { blobSizeLimitWarn int blobSizeLimitError int @@ -66,28 +51,6 @@ type ( } ) -func newAttrValidator( - domainCache cache.DomainCache, - metricsClient metrics.Client, - config *config.Config, - logger log.Logger, -) *attrValidator { - return &attrValidator{ - config: config, - domainCache: domainCache, - metricsClient: metricsClient, - logger: logger, - searchAttributesValidator: validator.NewSearchAttributesValidator( - logger, - config.EnableQueryAttributeValidation, - config.ValidSearchAttributes, - config.SearchAttributesNumberOfKeysLimit, - config.SearchAttributesSizeOfValueLimit, - config.SearchAttributesTotalSizeLimit, - ), - } -} - func newWorkflowSizeChecker( blobSizeLimitWarn int, blobSizeLimitError int, @@ -154,6 +117,10 @@ func (c *workflowSizeChecker) failWorkflowSizeExceedsLimit() (bool, error) { historyCount := int(c.mutableState.GetNextEventID()) - 1 historySize := int(c.executionStats.HistorySize) + // metricsScope already has domainName and operation: "RespondDecisionTaskCompleted" + c.metricsScope.RecordTimer(metrics.HistorySize, time.Duration(historySize)) + c.metricsScope.RecordTimer(metrics.HistoryCount, time.Duration(historyCount)) + if historySize > c.historySizeLimitError || historyCount > c.historyCountLimitError { executionInfo := c.mutableState.GetExecutionInfo() c.logger.Error("history size exceeds error limit.", @@ -187,708 +154,3 @@ func (c *workflowSizeChecker) failWorkflowSizeExceedsLimit() (bool, error) { return false, nil } - -func (v *attrValidator) validateActivityScheduleAttributes( - domainID string, - targetDomainID string, - attributes *types.ScheduleActivityTaskDecisionAttributes, - wfTimeout int32, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "ScheduleActivityTaskDecisionAttributes is not set on decision."} - } - - defaultTaskListName := "" - if _, err := v.validatedTaskList(attributes.TaskList, defaultTaskListName, metricsScope, attributes.GetDomain()); err != nil { - return err - } - - if attributes.GetActivityID() == "" { - return &types.BadRequestError{Message: "ActivityId is not set on decision."} - } - - if attributes.ActivityType == nil || attributes.ActivityType.GetName() == "" { - return &types.BadRequestError{Message: "ActivityType is not set on decision."} - } - - if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { - return err - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetActivityID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.ActivityIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrActivityIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeActivityID) { - return &types.BadRequestError{Message: "ActivityID exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetActivityType().GetName(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.ActivityTypeMaxLength(attributes.GetDomain()), - metrics.CadenceErrActivityTypeExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeActivityType) { - return &types.BadRequestError{Message: "ActivityType exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - // Only attempt to deduce and fill in unspecified timeouts only when all timeouts are non-negative. - if attributes.GetScheduleToCloseTimeoutSeconds() < 0 || attributes.GetScheduleToStartTimeoutSeconds() < 0 || - attributes.GetStartToCloseTimeoutSeconds() < 0 || attributes.GetHeartbeatTimeoutSeconds() < 0 { - return &types.BadRequestError{Message: "A valid timeout may not be negative."} - } - - // ensure activity timeout never larger than workflow timeout - if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetScheduleToStartTimeoutSeconds() > wfTimeout { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetStartToCloseTimeoutSeconds() > wfTimeout { - attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetHeartbeatTimeoutSeconds() > wfTimeout { - attributes.HeartbeatTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - - validScheduleToClose := attributes.GetScheduleToCloseTimeoutSeconds() > 0 - validScheduleToStart := attributes.GetScheduleToStartTimeoutSeconds() > 0 - validStartToClose := attributes.GetStartToCloseTimeoutSeconds() > 0 - - if validScheduleToClose { - if !validScheduleToStart { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) - } - if !validStartToClose { - attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) - } - } else if validScheduleToStart && validStartToClose { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToStartTimeoutSeconds() + attributes.GetStartToCloseTimeoutSeconds()) - if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - } else { - // Deduction failed as there's not enough information to fill in missing timeouts. - return &types.BadRequestError{Message: "A valid ScheduleToCloseTimeout is not set on decision."} - } - - // ensure activity's SCHEDULE_TO_START and SCHEDULE_TO_CLOSE is as long as expiration on retry policy - // if SCHEDULE_TO_START timeout is retryable - p := attributes.RetryPolicy - if p != nil { - isScheduleToStartRetryable := true - scheduleToStartErrorReason := execution.TimerTypeToReason(execution.TimerTypeScheduleToStart) - for _, reason := range p.GetNonRetriableErrorReasons() { - if reason == scheduleToStartErrorReason { - isScheduleToStartRetryable = false - break - } - } - - expiration := p.GetExpirationIntervalInSeconds() - if expiration == 0 || expiration > wfTimeout { - expiration = wfTimeout - } - - if isScheduleToStartRetryable { - // If schedule to start timeout is retryable, we don't need to fail the activity and schedule - // it again on the same tasklist, as it's a no-op). Extending schedule to start timeout to achieve - // the same thing. - // - // Theoretically, we can extend schedule to start to be as long as the expiration time, - // but if user specifies a very long expiration time and activity task got lost after the activity is - // scheduled, workflow will be stuck for a long time. So here, we cap the schedule to start timeout - // to a maximum value, so that when the activity task got lost, timeout can happen sooner and schedule - // the activity again. - - domainName, _ := v.domainCache.GetDomainName(domainID) // if this call returns an error, we will just used the default value for max timeout - maximumScheduleToStartTimeoutForRetryInSeconds := int32(v.config.ActivityMaxScheduleToStartTimeoutForRetry(domainName).Seconds()) - scheduleToStartExpiration := common.MinInt32(expiration, maximumScheduleToStartTimeoutForRetryInSeconds) - if attributes.GetScheduleToStartTimeoutSeconds() < scheduleToStartExpiration { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(scheduleToStartExpiration) - } - - // TODO: uncomment the following code when the client side bug for calculating scheduleToClose deadline is fixed and - // fully rolled out. Before that, we still need to extend scheduleToClose timeout to be as long as the expiration interval - // - // scheduleToCloseExpiration := common.MinInt32(expiration, scheduleToStartExpiration+attributes.GetStartToCloseTimeoutSeconds()) - // if attributes.GetScheduleToCloseTimeoutSeconds() < scheduleToCloseExpiration { - // attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(scheduleToCloseExpiration) - // } - } - - if attributes.GetScheduleToCloseTimeoutSeconds() < expiration { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(expiration) - } - } - return nil -} - -func (v *attrValidator) validateTimerScheduleAttributes( - attributes *types.StartTimerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "StartTimerDecisionAttributes is not set on decision."} - } - if attributes.GetTimerID() == "" { - return &types.BadRequestError{Message: "TimerId is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetTimerID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TimerIDMaxLength(domain), - metrics.CadenceErrTimerIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTimerID) { - return &types.BadRequestError{Message: "TimerId exceeds length limit."} - } - if attributes.GetStartToFireTimeoutSeconds() <= 0 { - return &types.BadRequestError{ - Message: fmt.Sprintf("Invalid StartToFireTimeoutSeconds: %v", attributes.GetStartToFireTimeoutSeconds()), - } - } - return nil -} - -func (v *attrValidator) validateActivityCancelAttributes( - attributes *types.RequestCancelActivityTaskDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "RequestCancelActivityTaskDecisionAttributes is not set on decision."} - } - if attributes.GetActivityID() == "" { - return &types.BadRequestError{Message: "ActivityId is not set on decision."} - } - - if !common.IsValidIDLength( - attributes.GetActivityID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.ActivityIDMaxLength(domain), - metrics.CadenceErrActivityIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeActivityID) { - return &types.BadRequestError{Message: "ActivityId exceeds length limit."} - } - return nil -} - -func (v *attrValidator) validateTimerCancelAttributes( - attributes *types.CancelTimerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CancelTimerDecisionAttributes is not set on decision."} - } - if attributes.GetTimerID() == "" { - return &types.BadRequestError{Message: "TimerId is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetTimerID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TimerIDMaxLength(domain), - metrics.CadenceErrTimerIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTimerID) { - return &types.BadRequestError{Message: "TimerId exceeds length limit."} - } - return nil -} - -func (v *attrValidator) validateRecordMarkerAttributes( - attributes *types.RecordMarkerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "RecordMarkerDecisionAttributes is not set on decision."} - } - if attributes.GetMarkerName() == "" { - return &types.BadRequestError{Message: "MarkerName is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetMarkerName(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.MarkerNameMaxLength(domain), - metrics.CadenceErrMarkerNameExceededWarnLimit, - domain, - v.logger, - tag.IDTypeMarkerName) { - return &types.BadRequestError{Message: "MarkerName exceeds length limit."} - } - - return nil -} - -func (v *attrValidator) validateCompleteWorkflowExecutionAttributes( - attributes *types.CompleteWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CompleteWorkflowExecutionDecisionAttributes is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateFailWorkflowExecutionAttributes( - attributes *types.FailWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "FailWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.Reason == nil { - return &types.BadRequestError{Message: "Reason is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateCancelWorkflowExecutionAttributes( - attributes *types.CancelWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CancelWorkflowExecutionDecisionAttributes is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateCancelExternalWorkflowExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.RequestCancelExternalWorkflowExecutionDecisionAttributes, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "RequestCancelExternalWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.WorkflowID == "" { - return &types.BadRequestError{Message: "WorkflowId is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - runID := attributes.GetRunID() - if runID != "" && uuid.Parse(runID) == nil { - return &types.BadRequestError{Message: "Invalid RunId set on decision."} - } - - return nil -} - -func (v *attrValidator) validateSignalExternalWorkflowExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.SignalExternalWorkflowExecutionDecisionAttributes, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.Execution == nil { - return &types.BadRequestError{Message: "Execution is nil on decision."} - } - if attributes.Execution.WorkflowID == "" { - return &types.BadRequestError{Message: "WorkflowId is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.Execution.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - - targetRunID := attributes.Execution.GetRunID() - if targetRunID != "" && uuid.Parse(targetRunID) == nil { - return &types.BadRequestError{Message: "Invalid RunId set on decision."} - } - if attributes.SignalName == "" { - return &types.BadRequestError{Message: "SignalName is not set on decision."} - } - - return nil -} - -func (v *attrValidator) validateUpsertWorkflowSearchAttributes( - domainName string, - attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision."} - } - - if attributes.SearchAttributes == nil { - return &types.BadRequestError{Message: "SearchAttributes is not set on decision."} - } - - if len(attributes.GetSearchAttributes().GetIndexedFields()) == 0 { - return &types.BadRequestError{Message: "IndexedFields is empty on decision."} - } - - return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) -} - -func (v *attrValidator) validateContinueAsNewWorkflowExecutionAttributes( - attributes *types.ContinueAsNewWorkflowExecutionDecisionAttributes, - executionInfo *persistence.WorkflowExecutionInfo, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "ContinueAsNewWorkflowExecutionDecisionAttributes is not set on decision."} - } - - // Inherit workflow type from previous execution if not provided on decision - if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { - attributes.WorkflowType = &types.WorkflowType{Name: executionInfo.WorkflowTypeName} - } - - if !common.IsValidIDLength( - attributes.WorkflowType.GetName(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.WorkflowTypeMaxLength(domain), - metrics.CadenceErrWorkflowTypeExceededWarnLimit, - domain, - v.logger, - tag.IDTypeWorkflowType) { - return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} - } - - // Inherit Tasklist from previous execution if not provided on decision - taskList, err := v.validatedTaskList(attributes.TaskList, executionInfo.TaskList, metricsScope, domain) - if err != nil { - return err - } - attributes.TaskList = taskList - - // Inherit workflow timeout from previous execution if not provided on decision - if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { - attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.WorkflowTimeout) - } - - // Inherit decision task timeout from previous execution if not provided on decision - if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { - attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.DecisionStartToCloseTimeout) - } - - // Check next run decision task delay - if attributes.GetBackoffStartIntervalInSeconds() < 0 { - return &types.BadRequestError{Message: "BackoffStartInterval is less than 0."} - } - - domainName, err := v.domainCache.GetDomainName(executionInfo.DomainID) - if err != nil { - return err - } - return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) -} - -func (v *attrValidator) validateStartChildExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.StartChildWorkflowExecutionDecisionAttributes, - parentInfo *persistence.WorkflowExecutionInfo, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "StartChildWorkflowExecutionDecisionAttributes is not set on decision."} - } - - if attributes.GetWorkflowID() == "" { - return &types.BadRequestError{Message: "Required field WorkflowID is not set on decision."} - } - - if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { - return &types.BadRequestError{Message: "Required field WorkflowType is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - if !common.IsValidIDLength( - attributes.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.WorkflowType.GetName(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowTypeMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowTypeExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowType) { - return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} - } - - if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { - return err - } - - if attributes.GetCronSchedule() != "" { - if _, err := backoff.ValidateSchedule(attributes.GetCronSchedule()); err != nil { - return err - } - } - - // Inherit tasklist from parent workflow execution if not provided on decision - taskList, err := v.validatedTaskList(attributes.TaskList, parentInfo.TaskList, metricsScope, attributes.GetDomain()) - if err != nil { - return err - } - attributes.TaskList = taskList - - // Inherit workflow timeout from parent workflow execution if not provided on decision - if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { - attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.WorkflowTimeout) - } - - // Inherit decision task timeout from parent workflow execution if not provided on decision - if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { - attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.DecisionStartToCloseTimeout) - } - - return nil -} - -func (v *attrValidator) validatedTaskList( - taskList *types.TaskList, - defaultVal string, - metricsScope int, - domain string, -) (*types.TaskList, error) { - - if taskList == nil { - taskList = &types.TaskList{} - } - - if taskList.GetName() == "" { - if defaultVal == "" { - return taskList, &types.BadRequestError{Message: "missing task list name"} - } - taskList.Name = defaultVal - return taskList, nil - } - - name := taskList.GetName() - if !common.IsValidIDLength( - name, - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TaskListNameMaxLength(domain), - metrics.CadenceErrTaskListNameExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTaskListName) { - return taskList, &types.BadRequestError{ - Message: fmt.Sprintf("task list name exceeds length limit of %v", v.config.TaskListNameMaxLength(domain)), - } - } - - if strings.HasPrefix(name, common.ReservedTaskListPrefix) { - return taskList, &types.BadRequestError{ - Message: fmt.Sprintf("task list name cannot start with reserved prefix %v", common.ReservedTaskListPrefix), - } - } - - return taskList, nil -} - -func (v *attrValidator) validateCrossDomainCall( - sourceDomainID string, - targetDomainID string, -) error { - - // same name, no check needed - if sourceDomainID == targetDomainID { - return nil - } - - sourceDomainEntry, err := v.domainCache.GetDomainByID(sourceDomainID) - if err != nil { - return err - } - - targetDomainEntry, err := v.domainCache.GetDomainByID(targetDomainID) - if err != nil { - return err - } - - sourceClusters := sourceDomainEntry.GetReplicationConfig().Clusters - targetClusters := targetDomainEntry.GetReplicationConfig().Clusters - - // both "local domain" - // here a domain is "local domain" when: - // - IsGlobalDomain() returns false - // - domainCluster contains only one cluster - // case 1 can be actually be combined with this case - if len(sourceClusters) == 1 && len(targetClusters) == 1 { - if sourceClusters[0].ClusterName == targetClusters[0].ClusterName { - return nil - } - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) - } - - // both global domain with > 1 replication cluster - // when code reaches here, at least one domain has more than one cluster - if len(sourceClusters) == len(targetClusters) && - v.config.EnableCrossClusterOperationsForDomain(sourceDomainEntry.GetInfo().Name) { - // check if the source domain cluster matches those for the target domain - for _, sourceCluster := range sourceClusters { - found := false - for _, targetCluster := range targetClusters { - if sourceCluster.ClusterName == targetCluster.ClusterName { - found = true - break - } - } - if !found { - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) - } - } - return nil - } - - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) -} - -func (v *attrValidator) createCrossDomainCallError( - domainEntry *cache.DomainCacheEntry, - targetDomainEntry *cache.DomainCacheEntry, -) error { - return &types.BadRequestError{Message: fmt.Sprintf( - "cannot make cross domain call between %v and %v", - domainEntry.GetInfo().Name, - targetDomainEntry.GetInfo().Name, - )} -} diff --git a/service/history/decision/checker_test.go b/service/history/decision/checker_test.go index b7ed90c4388..088adb61781 100644 --- a/service/history/decision/checker_test.go +++ b/service/history/decision/checker_test.go @@ -1,758 +1,246 @@ -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - package decision import ( - "testing" - "time" - "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - + "github.com/uber-go/tally" "github.com/uber/cadence/common" - "github.com/uber/cadence/common/cache" - "github.com/uber/cadence/common/cluster" - "github.com/uber/cadence/common/definition" - "github.com/uber/cadence/common/dynamicconfig" - "github.com/uber/cadence/common/log" + "github.com/uber/cadence/common/log/testlogger" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/types" - "github.com/uber/cadence/service/history/config" - "github.com/uber/cadence/service/history/constants" + "github.com/uber/cadence/service/history/execution" + "go.uber.org/zap/zaptest/observer" + "golang.org/x/exp/maps" + "sort" + "testing" ) -type ( - attrValidatorSuite struct { - suite.Suite - *require.Assertions - - controller *gomock.Controller - mockDomainCache *cache.MockDomainCache - - validator *attrValidator - - testDomainID string - testTargetDomainID string - - testActivityMaxScheduleToStartTimeoutForRetryInSeconds int32 - } +const ( + testDomainID = "test-domain-id" + testDomainName = "test-domain" + testWorkflowID = "test-workflow-id" + testRunID = "test-run-id" ) -func TestAttrValidatorSuite(t *testing.T) { - s := new(attrValidatorSuite) - suite.Run(t, s) -} - -func (s *attrValidatorSuite) SetupSuite() { - s.testDomainID = "test domain ID" - s.testTargetDomainID = "test target domain ID" - s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds = 1800 -} - -func (s *attrValidatorSuite) TearDownSuite() { -} - -func (s *attrValidatorSuite) SetupTest() { - s.Assertions = require.New(s.T()) - - s.controller = gomock.NewController(s.T()) - s.mockDomainCache = cache.NewMockDomainCache(s.controller) - config := &config.Config{ - MaxIDLengthWarnLimit: dynamicconfig.GetIntPropertyFn(128), - DomainNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - IdentityMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - WorkflowIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - SignalNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - WorkflowTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - RequestIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - TaskListNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - ActivityIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - ActivityTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - 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), - ActivityMaxScheduleToStartTimeoutForRetry: dynamicconfig.GetDurationPropertyFnFilteredByDomain( - time.Duration(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds) * time.Second, - ), - EnableCrossClusterOperationsForDomain: dynamicconfig.GetBoolPropertyFnFilteredByDomain(false), - } - s.validator = newAttrValidator( - s.mockDomainCache, - metrics.NewNoopMetricsClient(), - config, - log.NewNoop(), - ) -} - -func (s *attrValidatorSuite) TearDownTest() { - s.controller.Finish() -} - -func (s *attrValidatorSuite) TestValidateSignalExternalWorkflowExecutionAttributes() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, +func TestWorkflowSizeChecker_failWorkflowIfBlobSizeExceedsLimit(t *testing.T) { + var ( + testDecisionTag = metrics.DecisionTypeTag(types.DecisionTypeCompleteWorkflowExecution.String()) + testEventID = int64(1) + testMessage = "test" ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).AnyTimes() - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).AnyTimes() - - var attributes *types.SignalExternalWorkflowExecutionDecisionAttributes - - err := s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision.") - - attributes = &types.SignalExternalWorkflowExecutionDecisionAttributes{} - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "Execution is nil on decision.") - - attributes.Execution = &types.WorkflowExecution{} - attributes.Execution.WorkflowID = "workflow-id" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "SignalName is not set on decision.") - - attributes.Execution.RunID = "run-id" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "Invalid RunId set on decision.") - attributes.Execution.RunID = constants.TestRunID - - attributes.SignalName = "my signal name" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.NoError(err) - - attributes.Input = []byte("test input") - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.NoError(err) -} - -func (s *attrValidatorSuite) TestValidateUpsertWorkflowSearchAttributes() { - domainName := "testDomain" - var attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes - - err := s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision.") - - attributes = &types.UpsertWorkflowSearchAttributesDecisionAttributes{} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "SearchAttributes is not set on decision.") - - attributes.SearchAttributes = &types.SearchAttributes{} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "IndexedFields is empty on decision.") - - attributes.SearchAttributes.IndexedFields = map[string][]byte{"CustomKeywordField": []byte(`"bytes"`)} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToLocal() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_SameCluster() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_DiffCluster() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToGlobal() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, + for name, tc := range map[string]struct { + blobSizeLimitWarn int + blobSizeLimitError int + blob []byte + assertLogsAndMetrics func(*testing.T, *observer.ObservedLogs, tally.TestScope) + expectFail bool + }{ + "no errors": { + blobSizeLimitWarn: 10, + blobSizeLimitError: 20, + blob: []byte("test"), + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + assert.Empty(t, logs.All()) + // ensure metrics with the size is emitted. + timerData := maps.Values(scope.Snapshot().Timers()) + assert.Len(t, timerData, 1) + assert.Equal(t, "test.event_blob_size", timerData[0].Name()) }, }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + "warn": { + blobSizeLimitWarn: 10, + blobSizeLimitError: 20, + blob: []byte("should-warn"), + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "Blob size exceeds limit.", logEntries[0].Message) + }, }, - 5678, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + "fail": { + blobSizeLimitWarn: 5, + blobSizeLimitError: 10, + blob: []byte("should-fail"), + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "Blob size exceeds limit.", logEntries[0].Message) + }, + expectFail: true, + }, + } { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + mutableState := execution.NewMockMutableState(ctrl) + logger, logs := testlogger.NewObserved(t) + metricsScope := tally.NewTestScope("test", nil) + checker := &workflowSizeChecker{ + blobSizeLimitWarn: tc.blobSizeLimitWarn, + blobSizeLimitError: tc.blobSizeLimitError, + completedID: testEventID, + mutableState: mutableState, + logger: logger, + metricsScope: metrics.NewClient(metricsScope, metrics.History).Scope(metrics.HistoryRespondDecisionTaskCompletedScope, metrics.DomainTag(testDomainName)), + } + mutableState.EXPECT().GetExecutionInfo().Return(&persistence.WorkflowExecutionInfo{ + DomainID: testDomainID, + WorkflowID: testWorkflowID, + RunID: testRunID, + }).Times(1) + if tc.expectFail { + mutableState.EXPECT().AddFailWorkflowEvent(testEventID, &types.FailWorkflowExecutionDecisionAttributes{ + Reason: common.StringPtr(common.FailureReasonDecisionBlobSizeExceedsLimit), + Details: []byte(testMessage), + }).Return(nil, nil).Times(1) + } + failed, err := checker.failWorkflowIfBlobSizeExceedsLimit(testDecisionTag, tc.blob, testMessage) + require.NoError(t, err) + if tc.assertLogsAndMetrics != nil { + tc.assertLogsAndMetrics(t, logs, metricsScope) + } + assert.Equal(t, tc.expectFail, failed) + }) + } - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) } -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 5678, - ) +func TestWorkflowSizeChecker_failWorkflowSizeExceedsLimit(t *testing.T) { + for name, tc := range map[string]struct { + historyCount int + historyCountLimitWarn int + historyCountLimitError int - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + historySize int + historySizeLimitWarn int + historySizeLimitError int - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} + noExecutionCall bool -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToGlobal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - }, - }, - 5678, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, + assertLogsAndMetrics func(*testing.T, *observer.ObservedLogs, tally.TestScope) + expectFail bool + }{ + "no errors": { + historyCount: 1, + historyCountLimitWarn: 10, + historyCountLimitError: 20, + historySize: 1, + historySizeLimitWarn: 10, + historySizeLimitError: 20, + noExecutionCall: true, + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + assert.Empty(t, logs.All()) + // ensure metrics with the size is emitted. + timerData := maps.Values(scope.Snapshot().Timers()) + assert.Len(t, timerData, 4) + timerNames := make([]string, 0, 4) + for _, timer := range timerData { + timerNames = append(timerNames, timer.Name()) + } + sort.Strings(timerNames) + + // timers are duplicated for specific domain and domain: all + assert.Equal(t, []string{"test.history_count", "test.history_count", "test.history_size", "test.history_size"}, timerNames) }, }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + "count warn": { + historyCount: 15, + historyCountLimitWarn: 10, + historyCountLimitError: 20, - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} + historySize: 1, + historySizeLimitWarn: 10, + historySizeLimitError: 20, -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToLocal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "history size exceeds warn limit.", logEntries[0].Message) }, }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) + "count error": { + historyCount: 25, + historyCountLimitWarn: 10, + historyCountLimitError: 20, - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + historySize: 1, + historySizeLimitWarn: 10, + historySizeLimitError: 20, - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToEffectiveLocal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "history size exceeds error limit.", logEntries[0].Message) }, + expectFail: true, }, - 5678, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - }, - }, - 1234, - ) + "size warn": { + historyCount: 1, + historyCountLimitWarn: 10, + historyCountLimitError: 20, - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + historySize: 15, + historySizeLimitWarn: 10, + historySizeLimitError: 20, - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_SameDomain() { - targetDomainID := s.testDomainID - - err := s.validator.validateCrossDomainCall(s.testDomainID, targetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: cluster.TestCurrentClusterName}, + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "history size exceeds warn limit.", logEntries[0].Message) }, }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(2) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(2) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) + "size error": { + historyCount: 1, + historyCountLimitWarn: 10, + historyCountLimitError: 20, - s.validator.config.EnableCrossClusterOperationsForDomain = dynamicconfig.GetBoolPropertyFnFilteredByDomain(true) - err = s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} + historySize: 25, + historySizeLimitWarn: 10, + historySizeLimitError: 20, -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: "cluster name for s.testDomainID"}, - }, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: "cluster name for s.testTargetDomainID"}, + assertLogsAndMetrics: func(t *testing.T, logs *observer.ObservedLogs, scope tally.TestScope) { + logEntries := logs.All() + require.Len(t, logEntries, 1) + assert.Equal(t, "history size exceeds error limit.", logEntries[0].Message) }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateTaskListName() { - taskList := func(name string) *types.TaskList { - kind := types.TaskListKindNormal - return &types.TaskList{Name: name, Kind: &kind} - } - - testCases := []struct { - defaultVal string - input *types.TaskList - output *types.TaskList - isOutputErr bool - }{ - {"tl-1", nil, &types.TaskList{Name: "tl-1"}, false}, - {"", taskList("tl-1"), taskList("tl-1"), false}, - {"tl-1", taskList("tl-1"), taskList("tl-1"), false}, - {"", taskList("/tl-1"), taskList("/tl-1"), false}, - {"", taskList("/__cadence_sys"), taskList("/__cadence_sys"), false}, - {"", nil, &types.TaskList{}, true}, - {"", taskList(""), taskList(""), true}, - {"", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, - {"tl-1", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, - {"", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, - {"tl-1", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, - } + expectFail: true, + }, + } { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + mutableState := execution.NewMockMutableState(ctrl) + logger, logs := testlogger.NewObserved(t) + metricsScope := tally.NewTestScope("test", nil) + + mutableState.EXPECT().GetNextEventID().Return(int64(tc.historyCount + 1)).Times(1) + if !tc.noExecutionCall { + mutableState.EXPECT().GetExecutionInfo().Return(&persistence.WorkflowExecutionInfo{ + DomainID: testDomainID, + WorkflowID: testWorkflowID, + RunID: testRunID, + }).Times(1) + } - for _, tc := range testCases { - key := tc.defaultVal + "#" - if tc.input != nil { - key += tc.input.GetName() - } else { - key += "nil" - } - s.Run(key, func() { - output, err := s.validator.validatedTaskList(tc.input, tc.defaultVal, metrics.HistoryRespondDecisionTaskCompletedScope, "domain_name") - if tc.isOutputErr { - s.Error(err) - } else { - s.NoError(err) + checker := &workflowSizeChecker{ + historyCountLimitWarn: tc.historyCountLimitWarn, + historyCountLimitError: tc.historyCountLimitError, + historySizeLimitWarn: tc.historySizeLimitWarn, + historySizeLimitError: tc.historySizeLimitError, + mutableState: mutableState, + executionStats: &persistence.ExecutionStats{ + HistorySize: int64(tc.historySize), + }, + logger: logger, + metricsScope: metrics.NewClient(metricsScope, metrics.History).Scope(metrics.HistoryRespondDecisionTaskCompletedScope, metrics.DomainTag(testDomainName)), + } + failed, err := checker.failWorkflowSizeExceedsLimit() + require.NoError(t, err) + if tc.assertLogsAndMetrics != nil { + tc.assertLogsAndMetrics(t, logs, metricsScope) } - s.EqualValues(tc.output, output) + assert.Equal(t, tc.expectFail, failed) }) } } - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_NoRetryPolicy() { - wfTimeout := int32(5) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(3), // ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(10), // larger then wfTimeout - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), - ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: common.Int32Ptr(wfTimeout), - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartRetryable() { - s.mockDomainCache.EXPECT().GetDomainName(s.testDomainID).Return("some random domain name", nil).Times(1) - - wfTimeout := int32(3000) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(1), - RetryPolicy: &types.RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1.1, - ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than maximumScheduleToStartTimeoutForRetryInSeconds - NonRetriableErrorReasons: []string{"non-retryable error"}, - }, - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(attributes.RetryPolicy.ExpirationIntervalInSeconds), - ScheduleToStartTimeoutSeconds: common.Int32Ptr(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds), - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, - RetryPolicy: attributes.RetryPolicy, - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartNonRetryable() { - wfTimeout := int32(1000) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(1), - RetryPolicy: &types.RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1.1, - ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than wfTimeout and maximumScheduleToStartTimeoutForRetryInSeconds - NonRetriableErrorReasons: []string{"cadenceInternal:Timeout SCHEDULE_TO_START"}, - }, - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), - ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, - RetryPolicy: attributes.RetryPolicy, - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} From 98019f2d06bfa873ab9e058bb0029ec36cc7984a Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jul 2024 15:15:29 +0200 Subject: [PATCH 2/5] make pre --- .../history/decision/attribute_validator.go | 26 ++++++++++++++- service/history/decision/checker_test.go | 32 ++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/service/history/decision/attribute_validator.go b/service/history/decision/attribute_validator.go index 966737c1311..22d36579a2b 100644 --- a/service/history/decision/attribute_validator.go +++ b/service/history/decision/attribute_validator.go @@ -1,8 +1,33 @@ +// The MIT License (MIT) + +// Copyright (c) 2017-2020 Uber Technologies Inc. + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + package decision import ( "fmt" + "strings" + "github.com/pborman/uuid" + "github.com/uber/cadence/common" "github.com/uber/cadence/common/backoff" "github.com/uber/cadence/common/cache" @@ -14,7 +39,6 @@ import ( "github.com/uber/cadence/common/types" "github.com/uber/cadence/service/history/config" "github.com/uber/cadence/service/history/execution" - "strings" ) type attrValidator struct { diff --git a/service/history/decision/checker_test.go b/service/history/decision/checker_test.go index 088adb61781..d0a836c9996 100644 --- a/service/history/decision/checker_test.go +++ b/service/history/decision/checker_test.go @@ -1,20 +1,44 @@ +// The MIT License (MIT) + +// Copyright (c) 2017-2020 Uber Technologies Inc. + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + package decision import ( + "sort" + "testing" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/uber-go/tally" + "go.uber.org/zap/zaptest/observer" + "golang.org/x/exp/maps" + "github.com/uber/cadence/common" "github.com/uber/cadence/common/log/testlogger" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/types" "github.com/uber/cadence/service/history/execution" - "go.uber.org/zap/zaptest/observer" - "golang.org/x/exp/maps" - "sort" - "testing" ) const ( From b4f2fccec71125ab9eb567df7f2d28765248ce29 Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jul 2024 15:21:39 +0200 Subject: [PATCH 3/5] revert refactoring --- .../history/decision/attribute_validator.go | 777 ------------------ .../decision/attribute_validator_test.go | 758 ----------------- service/history/decision/checker.go | 743 +++++++++++++++++ service/history/decision/checker_test.go | 725 ++++++++++++++++ 4 files changed, 1468 insertions(+), 1535 deletions(-) delete mode 100644 service/history/decision/attribute_validator.go delete mode 100644 service/history/decision/attribute_validator_test.go diff --git a/service/history/decision/attribute_validator.go b/service/history/decision/attribute_validator.go deleted file mode 100644 index 22d36579a2b..00000000000 --- a/service/history/decision/attribute_validator.go +++ /dev/null @@ -1,777 +0,0 @@ -// The MIT License (MIT) - -// Copyright (c) 2017-2020 Uber Technologies Inc. - -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -package decision - -import ( - "fmt" - "strings" - - "github.com/pborman/uuid" - - "github.com/uber/cadence/common" - "github.com/uber/cadence/common/backoff" - "github.com/uber/cadence/common/cache" - "github.com/uber/cadence/common/elasticsearch/validator" - "github.com/uber/cadence/common/log" - "github.com/uber/cadence/common/log/tag" - "github.com/uber/cadence/common/metrics" - "github.com/uber/cadence/common/persistence" - "github.com/uber/cadence/common/types" - "github.com/uber/cadence/service/history/config" - "github.com/uber/cadence/service/history/execution" -) - -type attrValidator struct { - config *config.Config - domainCache cache.DomainCache - metricsClient metrics.Client - logger log.Logger - searchAttributesValidator *validator.SearchAttributesValidator -} - -func newAttrValidator( - domainCache cache.DomainCache, - metricsClient metrics.Client, - config *config.Config, - logger log.Logger, -) *attrValidator { - return &attrValidator{ - config: config, - domainCache: domainCache, - metricsClient: metricsClient, - logger: logger, - searchAttributesValidator: validator.NewSearchAttributesValidator( - logger, - config.EnableQueryAttributeValidation, - config.ValidSearchAttributes, - config.SearchAttributesNumberOfKeysLimit, - config.SearchAttributesSizeOfValueLimit, - config.SearchAttributesTotalSizeLimit, - ), - } -} - -func (v *attrValidator) validateActivityScheduleAttributes( - domainID string, - targetDomainID string, - attributes *types.ScheduleActivityTaskDecisionAttributes, - wfTimeout int32, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "ScheduleActivityTaskDecisionAttributes is not set on decision."} - } - - defaultTaskListName := "" - if _, err := v.validatedTaskList(attributes.TaskList, defaultTaskListName, metricsScope, attributes.GetDomain()); err != nil { - return err - } - - if attributes.GetActivityID() == "" { - return &types.BadRequestError{Message: "ActivityId is not set on decision."} - } - - if attributes.ActivityType == nil || attributes.ActivityType.GetName() == "" { - return &types.BadRequestError{Message: "ActivityType is not set on decision."} - } - - if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { - return err - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetActivityID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.ActivityIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrActivityIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeActivityID) { - return &types.BadRequestError{Message: "ActivityID exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetActivityType().GetName(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.ActivityTypeMaxLength(attributes.GetDomain()), - metrics.CadenceErrActivityTypeExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeActivityType) { - return &types.BadRequestError{Message: "ActivityType exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - // Only attempt to deduce and fill in unspecified timeouts only when all timeouts are non-negative. - if attributes.GetScheduleToCloseTimeoutSeconds() < 0 || attributes.GetScheduleToStartTimeoutSeconds() < 0 || - attributes.GetStartToCloseTimeoutSeconds() < 0 || attributes.GetHeartbeatTimeoutSeconds() < 0 { - return &types.BadRequestError{Message: "A valid timeout may not be negative."} - } - - // ensure activity timeout never larger than workflow timeout - if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetScheduleToStartTimeoutSeconds() > wfTimeout { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetStartToCloseTimeoutSeconds() > wfTimeout { - attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - if attributes.GetHeartbeatTimeoutSeconds() > wfTimeout { - attributes.HeartbeatTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - - validScheduleToClose := attributes.GetScheduleToCloseTimeoutSeconds() > 0 - validScheduleToStart := attributes.GetScheduleToStartTimeoutSeconds() > 0 - validStartToClose := attributes.GetStartToCloseTimeoutSeconds() > 0 - - if validScheduleToClose { - if !validScheduleToStart { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) - } - if !validStartToClose { - attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) - } - } else if validScheduleToStart && validStartToClose { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToStartTimeoutSeconds() + attributes.GetStartToCloseTimeoutSeconds()) - if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) - } - } else { - // Deduction failed as there's not enough information to fill in missing timeouts. - return &types.BadRequestError{Message: "A valid ScheduleToCloseTimeout is not set on decision."} - } - - // ensure activity's SCHEDULE_TO_START and SCHEDULE_TO_CLOSE is as long as expiration on retry policy - // if SCHEDULE_TO_START timeout is retryable - p := attributes.RetryPolicy - if p != nil { - isScheduleToStartRetryable := true - scheduleToStartErrorReason := execution.TimerTypeToReason(execution.TimerTypeScheduleToStart) - for _, reason := range p.GetNonRetriableErrorReasons() { - if reason == scheduleToStartErrorReason { - isScheduleToStartRetryable = false - break - } - } - - expiration := p.GetExpirationIntervalInSeconds() - if expiration == 0 || expiration > wfTimeout { - expiration = wfTimeout - } - - if isScheduleToStartRetryable { - // If schedule to start timeout is retryable, we don't need to fail the activity and schedule - // it again on the same tasklist, as it's a no-op). Extending schedule to start timeout to achieve - // the same thing. - // - // Theoretically, we can extend schedule to start to be as long as the expiration time, - // but if user specifies a very long expiration time and activity task got lost after the activity is - // scheduled, workflow will be stuck for a long time. So here, we cap the schedule to start timeout - // to a maximum value, so that when the activity task got lost, timeout can happen sooner and schedule - // the activity again. - - domainName, _ := v.domainCache.GetDomainName(domainID) // if this call returns an error, we will just used the default value for max timeout - maximumScheduleToStartTimeoutForRetryInSeconds := int32(v.config.ActivityMaxScheduleToStartTimeoutForRetry(domainName).Seconds()) - scheduleToStartExpiration := common.MinInt32(expiration, maximumScheduleToStartTimeoutForRetryInSeconds) - if attributes.GetScheduleToStartTimeoutSeconds() < scheduleToStartExpiration { - attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(scheduleToStartExpiration) - } - - // TODO: uncomment the following code when the client side bug for calculating scheduleToClose deadline is fixed and - // fully rolled out. Before that, we still need to extend scheduleToClose timeout to be as long as the expiration interval - // - // scheduleToCloseExpiration := common.MinInt32(expiration, scheduleToStartExpiration+attributes.GetStartToCloseTimeoutSeconds()) - // if attributes.GetScheduleToCloseTimeoutSeconds() < scheduleToCloseExpiration { - // attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(scheduleToCloseExpiration) - // } - } - - if attributes.GetScheduleToCloseTimeoutSeconds() < expiration { - attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(expiration) - } - } - return nil -} - -func (v *attrValidator) validateTimerScheduleAttributes( - attributes *types.StartTimerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "StartTimerDecisionAttributes is not set on decision."} - } - if attributes.GetTimerID() == "" { - return &types.BadRequestError{Message: "TimerId is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetTimerID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TimerIDMaxLength(domain), - metrics.CadenceErrTimerIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTimerID) { - return &types.BadRequestError{Message: "TimerId exceeds length limit."} - } - if attributes.GetStartToFireTimeoutSeconds() <= 0 { - return &types.BadRequestError{ - Message: fmt.Sprintf("Invalid StartToFireTimeoutSeconds: %v", attributes.GetStartToFireTimeoutSeconds()), - } - } - return nil -} - -func (v *attrValidator) validateActivityCancelAttributes( - attributes *types.RequestCancelActivityTaskDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "RequestCancelActivityTaskDecisionAttributes is not set on decision."} - } - if attributes.GetActivityID() == "" { - return &types.BadRequestError{Message: "ActivityId is not set on decision."} - } - - if !common.IsValidIDLength( - attributes.GetActivityID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.ActivityIDMaxLength(domain), - metrics.CadenceErrActivityIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeActivityID) { - return &types.BadRequestError{Message: "ActivityId exceeds length limit."} - } - return nil -} - -func (v *attrValidator) validateTimerCancelAttributes( - attributes *types.CancelTimerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CancelTimerDecisionAttributes is not set on decision."} - } - if attributes.GetTimerID() == "" { - return &types.BadRequestError{Message: "TimerId is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetTimerID(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TimerIDMaxLength(domain), - metrics.CadenceErrTimerIDExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTimerID) { - return &types.BadRequestError{Message: "TimerId exceeds length limit."} - } - return nil -} - -func (v *attrValidator) validateRecordMarkerAttributes( - attributes *types.RecordMarkerDecisionAttributes, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "RecordMarkerDecisionAttributes is not set on decision."} - } - if attributes.GetMarkerName() == "" { - return &types.BadRequestError{Message: "MarkerName is not set on decision."} - } - if !common.IsValidIDLength( - attributes.GetMarkerName(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.MarkerNameMaxLength(domain), - metrics.CadenceErrMarkerNameExceededWarnLimit, - domain, - v.logger, - tag.IDTypeMarkerName) { - return &types.BadRequestError{Message: "MarkerName exceeds length limit."} - } - - return nil -} - -func (v *attrValidator) validateCompleteWorkflowExecutionAttributes( - attributes *types.CompleteWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CompleteWorkflowExecutionDecisionAttributes is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateFailWorkflowExecutionAttributes( - attributes *types.FailWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "FailWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.Reason == nil { - return &types.BadRequestError{Message: "Reason is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateCancelWorkflowExecutionAttributes( - attributes *types.CancelWorkflowExecutionDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "CancelWorkflowExecutionDecisionAttributes is not set on decision."} - } - return nil -} - -func (v *attrValidator) validateCancelExternalWorkflowExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.RequestCancelExternalWorkflowExecutionDecisionAttributes, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "RequestCancelExternalWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.WorkflowID == "" { - return &types.BadRequestError{Message: "WorkflowId is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - runID := attributes.GetRunID() - if runID != "" && uuid.Parse(runID) == nil { - return &types.BadRequestError{Message: "Invalid RunId set on decision."} - } - - return nil -} - -func (v *attrValidator) validateSignalExternalWorkflowExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.SignalExternalWorkflowExecutionDecisionAttributes, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision."} - } - if attributes.Execution == nil { - return &types.BadRequestError{Message: "Execution is nil on decision."} - } - if attributes.Execution.WorkflowID == "" { - return &types.BadRequestError{Message: "WorkflowId is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.Execution.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - - targetRunID := attributes.Execution.GetRunID() - if targetRunID != "" && uuid.Parse(targetRunID) == nil { - return &types.BadRequestError{Message: "Invalid RunId set on decision."} - } - if attributes.SignalName == "" { - return &types.BadRequestError{Message: "SignalName is not set on decision."} - } - - return nil -} - -func (v *attrValidator) validateUpsertWorkflowSearchAttributes( - domainName string, - attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision."} - } - - if attributes.SearchAttributes == nil { - return &types.BadRequestError{Message: "SearchAttributes is not set on decision."} - } - - if len(attributes.GetSearchAttributes().GetIndexedFields()) == 0 { - return &types.BadRequestError{Message: "IndexedFields is empty on decision."} - } - - return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) -} - -func (v *attrValidator) validateContinueAsNewWorkflowExecutionAttributes( - attributes *types.ContinueAsNewWorkflowExecutionDecisionAttributes, - executionInfo *persistence.WorkflowExecutionInfo, - metricsScope int, - domain string, -) error { - - if attributes == nil { - return &types.BadRequestError{Message: "ContinueAsNewWorkflowExecutionDecisionAttributes is not set on decision."} - } - - // Inherit workflow type from previous execution if not provided on decision - if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { - attributes.WorkflowType = &types.WorkflowType{Name: executionInfo.WorkflowTypeName} - } - - if !common.IsValidIDLength( - attributes.WorkflowType.GetName(), - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.WorkflowTypeMaxLength(domain), - metrics.CadenceErrWorkflowTypeExceededWarnLimit, - domain, - v.logger, - tag.IDTypeWorkflowType) { - return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} - } - - // Inherit Tasklist from previous execution if not provided on decision - taskList, err := v.validatedTaskList(attributes.TaskList, executionInfo.TaskList, metricsScope, domain) - if err != nil { - return err - } - attributes.TaskList = taskList - - // Inherit workflow timeout from previous execution if not provided on decision - if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { - attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.WorkflowTimeout) - } - - // Inherit decision task timeout from previous execution if not provided on decision - if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { - attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.DecisionStartToCloseTimeout) - } - - // Check next run decision task delay - if attributes.GetBackoffStartIntervalInSeconds() < 0 { - return &types.BadRequestError{Message: "BackoffStartInterval is less than 0."} - } - - domainName, err := v.domainCache.GetDomainName(executionInfo.DomainID) - if err != nil { - return err - } - return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) -} - -func (v *attrValidator) validateStartChildExecutionAttributes( - domainID string, - targetDomainID string, - attributes *types.StartChildWorkflowExecutionDecisionAttributes, - parentInfo *persistence.WorkflowExecutionInfo, - metricsScope int, -) error { - - if err := v.validateCrossDomainCall( - domainID, - targetDomainID, - ); err != nil { - return err - } - - if attributes == nil { - return &types.BadRequestError{Message: "StartChildWorkflowExecutionDecisionAttributes is not set on decision."} - } - - if attributes.GetWorkflowID() == "" { - return &types.BadRequestError{Message: "Required field WorkflowID is not set on decision."} - } - - if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { - return &types.BadRequestError{Message: "Required field WorkflowType is not set on decision."} - } - - idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() - if !common.IsValidIDLength( - attributes.GetDomain(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.DomainNameMaxLength(attributes.GetDomain()), - metrics.CadenceErrDomainNameExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeDomainName) { - return &types.BadRequestError{Message: "Domain exceeds length limit."} - } - if !common.IsValidIDLength( - attributes.GetWorkflowID(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowIDMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowIDExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowID) { - return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} - } - - if !common.IsValidIDLength( - attributes.WorkflowType.GetName(), - v.metricsClient.Scope(metricsScope), - idLengthWarnLimit, - v.config.WorkflowTypeMaxLength(attributes.GetDomain()), - metrics.CadenceErrWorkflowTypeExceededWarnLimit, - attributes.GetDomain(), - v.logger, - tag.IDTypeWorkflowType) { - return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} - } - - if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { - return err - } - - if attributes.GetCronSchedule() != "" { - if _, err := backoff.ValidateSchedule(attributes.GetCronSchedule()); err != nil { - return err - } - } - - // Inherit tasklist from parent workflow execution if not provided on decision - taskList, err := v.validatedTaskList(attributes.TaskList, parentInfo.TaskList, metricsScope, attributes.GetDomain()) - if err != nil { - return err - } - attributes.TaskList = taskList - - // Inherit workflow timeout from parent workflow execution if not provided on decision - if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { - attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.WorkflowTimeout) - } - - // Inherit decision task timeout from parent workflow execution if not provided on decision - if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { - attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.DecisionStartToCloseTimeout) - } - - return nil -} - -func (v *attrValidator) validatedTaskList( - taskList *types.TaskList, - defaultVal string, - metricsScope int, - domain string, -) (*types.TaskList, error) { - - if taskList == nil { - taskList = &types.TaskList{} - } - - if taskList.GetName() == "" { - if defaultVal == "" { - return taskList, &types.BadRequestError{Message: "missing task list name"} - } - taskList.Name = defaultVal - return taskList, nil - } - - name := taskList.GetName() - if !common.IsValidIDLength( - name, - v.metricsClient.Scope(metricsScope), - v.config.MaxIDLengthWarnLimit(), - v.config.TaskListNameMaxLength(domain), - metrics.CadenceErrTaskListNameExceededWarnLimit, - domain, - v.logger, - tag.IDTypeTaskListName) { - return taskList, &types.BadRequestError{ - Message: fmt.Sprintf("task list name exceeds length limit of %v", v.config.TaskListNameMaxLength(domain)), - } - } - - if strings.HasPrefix(name, common.ReservedTaskListPrefix) { - return taskList, &types.BadRequestError{ - Message: fmt.Sprintf("task list name cannot start with reserved prefix %v", common.ReservedTaskListPrefix), - } - } - - return taskList, nil -} - -func (v *attrValidator) validateCrossDomainCall( - sourceDomainID string, - targetDomainID string, -) error { - - // same name, no check needed - if sourceDomainID == targetDomainID { - return nil - } - - sourceDomainEntry, err := v.domainCache.GetDomainByID(sourceDomainID) - if err != nil { - return err - } - - targetDomainEntry, err := v.domainCache.GetDomainByID(targetDomainID) - if err != nil { - return err - } - - sourceClusters := sourceDomainEntry.GetReplicationConfig().Clusters - targetClusters := targetDomainEntry.GetReplicationConfig().Clusters - - // both "local domain" - // here a domain is "local domain" when: - // - IsGlobalDomain() returns false - // - domainCluster contains only one cluster - // case 1 can be actually be combined with this case - if len(sourceClusters) == 1 && len(targetClusters) == 1 { - if sourceClusters[0].ClusterName == targetClusters[0].ClusterName { - return nil - } - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) - } - - // both global domain with > 1 replication cluster - // when code reaches here, at least one domain has more than one cluster - if len(sourceClusters) == len(targetClusters) && - v.config.EnableCrossClusterOperationsForDomain(sourceDomainEntry.GetInfo().Name) { - // check if the source domain cluster matches those for the target domain - for _, sourceCluster := range sourceClusters { - found := false - for _, targetCluster := range targetClusters { - if sourceCluster.ClusterName == targetCluster.ClusterName { - found = true - break - } - } - if !found { - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) - } - } - return nil - } - - return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) -} - -func (v *attrValidator) createCrossDomainCallError( - domainEntry *cache.DomainCacheEntry, - targetDomainEntry *cache.DomainCacheEntry, -) error { - return &types.BadRequestError{Message: fmt.Sprintf( - "cannot make cross domain call between %v and %v", - domainEntry.GetInfo().Name, - targetDomainEntry.GetInfo().Name, - )} -} diff --git a/service/history/decision/attribute_validator_test.go b/service/history/decision/attribute_validator_test.go deleted file mode 100644 index b7ed90c4388..00000000000 --- a/service/history/decision/attribute_validator_test.go +++ /dev/null @@ -1,758 +0,0 @@ -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -package decision - -import ( - "testing" - "time" - - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - - "github.com/uber/cadence/common" - "github.com/uber/cadence/common/cache" - "github.com/uber/cadence/common/cluster" - "github.com/uber/cadence/common/definition" - "github.com/uber/cadence/common/dynamicconfig" - "github.com/uber/cadence/common/log" - "github.com/uber/cadence/common/metrics" - "github.com/uber/cadence/common/persistence" - "github.com/uber/cadence/common/types" - "github.com/uber/cadence/service/history/config" - "github.com/uber/cadence/service/history/constants" -) - -type ( - attrValidatorSuite struct { - suite.Suite - *require.Assertions - - controller *gomock.Controller - mockDomainCache *cache.MockDomainCache - - validator *attrValidator - - testDomainID string - testTargetDomainID string - - testActivityMaxScheduleToStartTimeoutForRetryInSeconds int32 - } -) - -func TestAttrValidatorSuite(t *testing.T) { - s := new(attrValidatorSuite) - suite.Run(t, s) -} - -func (s *attrValidatorSuite) SetupSuite() { - s.testDomainID = "test domain ID" - s.testTargetDomainID = "test target domain ID" - s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds = 1800 -} - -func (s *attrValidatorSuite) TearDownSuite() { -} - -func (s *attrValidatorSuite) SetupTest() { - s.Assertions = require.New(s.T()) - - s.controller = gomock.NewController(s.T()) - s.mockDomainCache = cache.NewMockDomainCache(s.controller) - config := &config.Config{ - MaxIDLengthWarnLimit: dynamicconfig.GetIntPropertyFn(128), - DomainNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - IdentityMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - WorkflowIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - SignalNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - WorkflowTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - RequestIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - TaskListNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - ActivityIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - ActivityTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), - 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), - ActivityMaxScheduleToStartTimeoutForRetry: dynamicconfig.GetDurationPropertyFnFilteredByDomain( - time.Duration(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds) * time.Second, - ), - EnableCrossClusterOperationsForDomain: dynamicconfig.GetBoolPropertyFnFilteredByDomain(false), - } - s.validator = newAttrValidator( - s.mockDomainCache, - metrics.NewNoopMetricsClient(), - config, - log.NewNoop(), - ) -} - -func (s *attrValidatorSuite) TearDownTest() { - s.controller.Finish() -} - -func (s *attrValidatorSuite) TestValidateSignalExternalWorkflowExecutionAttributes() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).AnyTimes() - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).AnyTimes() - - var attributes *types.SignalExternalWorkflowExecutionDecisionAttributes - - err := s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision.") - - attributes = &types.SignalExternalWorkflowExecutionDecisionAttributes{} - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "Execution is nil on decision.") - - attributes.Execution = &types.WorkflowExecution{} - attributes.Execution.WorkflowID = "workflow-id" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "SignalName is not set on decision.") - - attributes.Execution.RunID = "run-id" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.EqualError(err, "Invalid RunId set on decision.") - attributes.Execution.RunID = constants.TestRunID - - attributes.SignalName = "my signal name" - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.NoError(err) - - attributes.Input = []byte("test input") - err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) - s.NoError(err) -} - -func (s *attrValidatorSuite) TestValidateUpsertWorkflowSearchAttributes() { - domainName := "testDomain" - var attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes - - err := s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision.") - - attributes = &types.UpsertWorkflowSearchAttributesDecisionAttributes{} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "SearchAttributes is not set on decision.") - - attributes.SearchAttributes = &types.SearchAttributes{} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.EqualError(err, "IndexedFields is empty on decision.") - - attributes.SearchAttributes.IndexedFields = map[string][]byte{"CustomKeywordField": []byte(`"bytes"`)} - err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToLocal() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_SameCluster() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_DiffCluster() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToGlobal() { - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 5678, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestAlternativeClusterName, - Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, - }, - 5678, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToGlobal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - }, - }, - 5678, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToLocal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 1234, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToEffectiveLocal() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 5678, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_SameDomain() { - targetDomainID := s.testDomainID - - err := s.validator.validateCrossDomainCall(s.testDomainID, targetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_SameCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: cluster.TestCurrentClusterName}, - }, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(2) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(2) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) - - s.validator.config.EnableCrossClusterOperationsForDomain = dynamicconfig.GetBoolPropertyFnFilteredByDomain(true) - err = s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.Nil(err) -} - -func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_DiffCluster() { - domainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: "cluster name for s.testDomainID"}, - }, - }, - 1234, - ) - targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - &persistence.DomainReplicationConfig{ - ActiveClusterName: cluster.TestCurrentClusterName, - Clusters: []*persistence.ClusterReplicationConfig{ - {ClusterName: cluster.TestCurrentClusterName}, - {ClusterName: cluster.TestAlternativeClusterName}, - {ClusterName: "cluster name for s.testTargetDomainID"}, - }, - }, - 1234, - ) - - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) - s.IsType(&types.BadRequestError{}, err) -} - -func (s *attrValidatorSuite) TestValidateTaskListName() { - taskList := func(name string) *types.TaskList { - kind := types.TaskListKindNormal - return &types.TaskList{Name: name, Kind: &kind} - } - - testCases := []struct { - defaultVal string - input *types.TaskList - output *types.TaskList - isOutputErr bool - }{ - {"tl-1", nil, &types.TaskList{Name: "tl-1"}, false}, - {"", taskList("tl-1"), taskList("tl-1"), false}, - {"tl-1", taskList("tl-1"), taskList("tl-1"), false}, - {"", taskList("/tl-1"), taskList("/tl-1"), false}, - {"", taskList("/__cadence_sys"), taskList("/__cadence_sys"), false}, - {"", nil, &types.TaskList{}, true}, - {"", taskList(""), taskList(""), true}, - {"", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, - {"tl-1", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, - {"", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, - {"tl-1", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, - } - - for _, tc := range testCases { - key := tc.defaultVal + "#" - if tc.input != nil { - key += tc.input.GetName() - } else { - key += "nil" - } - s.Run(key, func() { - output, err := s.validator.validatedTaskList(tc.input, tc.defaultVal, metrics.HistoryRespondDecisionTaskCompletedScope, "domain_name") - if tc.isOutputErr { - s.Error(err) - } else { - s.NoError(err) - } - s.EqualValues(tc.output, output) - }) - } -} - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_NoRetryPolicy() { - wfTimeout := int32(5) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(3), // ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(10), // larger then wfTimeout - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), - ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: common.Int32Ptr(wfTimeout), - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartRetryable() { - s.mockDomainCache.EXPECT().GetDomainName(s.testDomainID).Return("some random domain name", nil).Times(1) - - wfTimeout := int32(3000) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(1), - RetryPolicy: &types.RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1.1, - ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than maximumScheduleToStartTimeoutForRetryInSeconds - NonRetriableErrorReasons: []string{"non-retryable error"}, - }, - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(attributes.RetryPolicy.ExpirationIntervalInSeconds), - ScheduleToStartTimeoutSeconds: common.Int32Ptr(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds), - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, - RetryPolicy: attributes.RetryPolicy, - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} - -func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartNonRetryable() { - wfTimeout := int32(1000) - attributes := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: "some random activityID", - ActivityType: &types.ActivityType{ - Name: "some random activity type", - }, - Domain: s.testDomainID, - TaskList: &types.TaskList{ - Name: "some random task list", - }, - Input: []byte{1, 2, 3}, - ScheduleToCloseTimeoutSeconds: nil, // not set - ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), - StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout - HeartbeatTimeoutSeconds: common.Int32Ptr(1), - RetryPolicy: &types.RetryPolicy{ - InitialIntervalInSeconds: 1, - BackoffCoefficient: 1.1, - ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than wfTimeout and maximumScheduleToStartTimeoutForRetryInSeconds - NonRetriableErrorReasons: []string{"cadenceInternal:Timeout SCHEDULE_TO_START"}, - }, - } - - expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ - ActivityID: attributes.ActivityID, - ActivityType: attributes.ActivityType, - Domain: attributes.Domain, - TaskList: attributes.TaskList, - Input: attributes.Input, - ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), - ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, - StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, - HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, - RetryPolicy: attributes.RetryPolicy, - } - - domainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( - &persistence.DomainInfo{Name: s.testTargetDomainID}, - nil, - cluster.TestCurrentClusterName, - ) - s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) - s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) - - err := s.validator.validateActivityScheduleAttributes( - s.testDomainID, - s.testTargetDomainID, - attributes, - wfTimeout, - metrics.HistoryRespondDecisionTaskCompletedScope, - ) - s.Nil(err) - s.Equal(expectedAttributesAfterValidation, attributes) -} diff --git a/service/history/decision/checker.go b/service/history/decision/checker.go index 0d03f93624c..82ffe5e73bc 100644 --- a/service/history/decision/checker.go +++ b/service/history/decision/checker.go @@ -21,18 +21,34 @@ package decision import ( + "fmt" + "strings" "time" + "github.com/pborman/uuid" + "github.com/uber/cadence/common" + "github.com/uber/cadence/common/backoff" + "github.com/uber/cadence/common/cache" + "github.com/uber/cadence/common/elasticsearch/validator" "github.com/uber/cadence/common/log" "github.com/uber/cadence/common/log/tag" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/types" + "github.com/uber/cadence/service/history/config" "github.com/uber/cadence/service/history/execution" ) type ( + attrValidator struct { + config *config.Config + domainCache cache.DomainCache + metricsClient metrics.Client + logger log.Logger + searchAttributesValidator *validator.SearchAttributesValidator + } + workflowSizeChecker struct { blobSizeLimitWarn int blobSizeLimitError int @@ -79,6 +95,28 @@ func newWorkflowSizeChecker( } } +func newAttrValidator( + domainCache cache.DomainCache, + metricsClient metrics.Client, + config *config.Config, + logger log.Logger, +) *attrValidator { + return &attrValidator{ + config: config, + domainCache: domainCache, + metricsClient: metricsClient, + logger: logger, + searchAttributesValidator: validator.NewSearchAttributesValidator( + logger, + config.EnableQueryAttributeValidation, + config.ValidSearchAttributes, + config.SearchAttributesNumberOfKeysLimit, + config.SearchAttributesSizeOfValueLimit, + config.SearchAttributesTotalSizeLimit, + ), + } +} + func (c *workflowSizeChecker) failWorkflowIfBlobSizeExceedsLimit( decisionTypeTag metrics.Tag, blob []byte, @@ -154,3 +192,708 @@ func (c *workflowSizeChecker) failWorkflowSizeExceedsLimit() (bool, error) { return false, nil } + +func (v *attrValidator) validateActivityScheduleAttributes( + domainID string, + targetDomainID string, + attributes *types.ScheduleActivityTaskDecisionAttributes, + wfTimeout int32, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "ScheduleActivityTaskDecisionAttributes is not set on decision."} + } + + defaultTaskListName := "" + if _, err := v.validatedTaskList(attributes.TaskList, defaultTaskListName, metricsScope, attributes.GetDomain()); err != nil { + return err + } + + if attributes.GetActivityID() == "" { + return &types.BadRequestError{Message: "ActivityId is not set on decision."} + } + + if attributes.ActivityType == nil || attributes.ActivityType.GetName() == "" { + return &types.BadRequestError{Message: "ActivityType is not set on decision."} + } + + if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { + return err + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetActivityID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.ActivityIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrActivityIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeActivityID) { + return &types.BadRequestError{Message: "ActivityID exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetActivityType().GetName(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.ActivityTypeMaxLength(attributes.GetDomain()), + metrics.CadenceErrActivityTypeExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeActivityType) { + return &types.BadRequestError{Message: "ActivityType exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + // Only attempt to deduce and fill in unspecified timeouts only when all timeouts are non-negative. + if attributes.GetScheduleToCloseTimeoutSeconds() < 0 || attributes.GetScheduleToStartTimeoutSeconds() < 0 || + attributes.GetStartToCloseTimeoutSeconds() < 0 || attributes.GetHeartbeatTimeoutSeconds() < 0 { + return &types.BadRequestError{Message: "A valid timeout may not be negative."} + } + + // ensure activity timeout never larger than workflow timeout + if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetScheduleToStartTimeoutSeconds() > wfTimeout { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetStartToCloseTimeoutSeconds() > wfTimeout { + attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + if attributes.GetHeartbeatTimeoutSeconds() > wfTimeout { + attributes.HeartbeatTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + + validScheduleToClose := attributes.GetScheduleToCloseTimeoutSeconds() > 0 + validScheduleToStart := attributes.GetScheduleToStartTimeoutSeconds() > 0 + validStartToClose := attributes.GetStartToCloseTimeoutSeconds() > 0 + + if validScheduleToClose { + if !validScheduleToStart { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) + } + if !validStartToClose { + attributes.StartToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToCloseTimeoutSeconds()) + } + } else if validScheduleToStart && validStartToClose { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(attributes.GetScheduleToStartTimeoutSeconds() + attributes.GetStartToCloseTimeoutSeconds()) + if attributes.GetScheduleToCloseTimeoutSeconds() > wfTimeout { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(wfTimeout) + } + } else { + // Deduction failed as there's not enough information to fill in missing timeouts. + return &types.BadRequestError{Message: "A valid ScheduleToCloseTimeout is not set on decision."} + } + + // ensure activity's SCHEDULE_TO_START and SCHEDULE_TO_CLOSE is as long as expiration on retry policy + // if SCHEDULE_TO_START timeout is retryable + p := attributes.RetryPolicy + if p != nil { + isScheduleToStartRetryable := true + scheduleToStartErrorReason := execution.TimerTypeToReason(execution.TimerTypeScheduleToStart) + for _, reason := range p.GetNonRetriableErrorReasons() { + if reason == scheduleToStartErrorReason { + isScheduleToStartRetryable = false + break + } + } + + expiration := p.GetExpirationIntervalInSeconds() + if expiration == 0 || expiration > wfTimeout { + expiration = wfTimeout + } + + if isScheduleToStartRetryable { + // If schedule to start timeout is retryable, we don't need to fail the activity and schedule + // it again on the same tasklist, as it's a no-op). Extending schedule to start timeout to achieve + // the same thing. + // + // Theoretically, we can extend schedule to start to be as long as the expiration time, + // but if user specifies a very long expiration time and activity task got lost after the activity is + // scheduled, workflow will be stuck for a long time. So here, we cap the schedule to start timeout + // to a maximum value, so that when the activity task got lost, timeout can happen sooner and schedule + // the activity again. + + domainName, _ := v.domainCache.GetDomainName(domainID) // if this call returns an error, we will just used the default value for max timeout + maximumScheduleToStartTimeoutForRetryInSeconds := int32(v.config.ActivityMaxScheduleToStartTimeoutForRetry(domainName).Seconds()) + scheduleToStartExpiration := common.MinInt32(expiration, maximumScheduleToStartTimeoutForRetryInSeconds) + if attributes.GetScheduleToStartTimeoutSeconds() < scheduleToStartExpiration { + attributes.ScheduleToStartTimeoutSeconds = common.Int32Ptr(scheduleToStartExpiration) + } + + // TODO: uncomment the following code when the client side bug for calculating scheduleToClose deadline is fixed and + // fully rolled out. Before that, we still need to extend scheduleToClose timeout to be as long as the expiration interval + // + // scheduleToCloseExpiration := common.MinInt32(expiration, scheduleToStartExpiration+attributes.GetStartToCloseTimeoutSeconds()) + // if attributes.GetScheduleToCloseTimeoutSeconds() < scheduleToCloseExpiration { + // attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(scheduleToCloseExpiration) + // } + } + + if attributes.GetScheduleToCloseTimeoutSeconds() < expiration { + attributes.ScheduleToCloseTimeoutSeconds = common.Int32Ptr(expiration) + } + } + return nil +} + +func (v *attrValidator) validateTimerScheduleAttributes( + attributes *types.StartTimerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "StartTimerDecisionAttributes is not set on decision."} + } + if attributes.GetTimerID() == "" { + return &types.BadRequestError{Message: "TimerId is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetTimerID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TimerIDMaxLength(domain), + metrics.CadenceErrTimerIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTimerID) { + return &types.BadRequestError{Message: "TimerId exceeds length limit."} + } + if attributes.GetStartToFireTimeoutSeconds() <= 0 { + return &types.BadRequestError{ + Message: fmt.Sprintf("Invalid StartToFireTimeoutSeconds: %v", attributes.GetStartToFireTimeoutSeconds()), + } + } + return nil +} + +func (v *attrValidator) validateActivityCancelAttributes( + attributes *types.RequestCancelActivityTaskDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "RequestCancelActivityTaskDecisionAttributes is not set on decision."} + } + if attributes.GetActivityID() == "" { + return &types.BadRequestError{Message: "ActivityId is not set on decision."} + } + + if !common.IsValidIDLength( + attributes.GetActivityID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.ActivityIDMaxLength(domain), + metrics.CadenceErrActivityIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeActivityID) { + return &types.BadRequestError{Message: "ActivityId exceeds length limit."} + } + return nil +} + +func (v *attrValidator) validateTimerCancelAttributes( + attributes *types.CancelTimerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CancelTimerDecisionAttributes is not set on decision."} + } + if attributes.GetTimerID() == "" { + return &types.BadRequestError{Message: "TimerId is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetTimerID(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TimerIDMaxLength(domain), + metrics.CadenceErrTimerIDExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTimerID) { + return &types.BadRequestError{Message: "TimerId exceeds length limit."} + } + return nil +} + +func (v *attrValidator) validateRecordMarkerAttributes( + attributes *types.RecordMarkerDecisionAttributes, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "RecordMarkerDecisionAttributes is not set on decision."} + } + if attributes.GetMarkerName() == "" { + return &types.BadRequestError{Message: "MarkerName is not set on decision."} + } + if !common.IsValidIDLength( + attributes.GetMarkerName(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.MarkerNameMaxLength(domain), + metrics.CadenceErrMarkerNameExceededWarnLimit, + domain, + v.logger, + tag.IDTypeMarkerName) { + return &types.BadRequestError{Message: "MarkerName exceeds length limit."} + } + + return nil +} + +func (v *attrValidator) validateCompleteWorkflowExecutionAttributes( + attributes *types.CompleteWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CompleteWorkflowExecutionDecisionAttributes is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateFailWorkflowExecutionAttributes( + attributes *types.FailWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "FailWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.Reason == nil { + return &types.BadRequestError{Message: "Reason is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateCancelWorkflowExecutionAttributes( + attributes *types.CancelWorkflowExecutionDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "CancelWorkflowExecutionDecisionAttributes is not set on decision."} + } + return nil +} + +func (v *attrValidator) validateCancelExternalWorkflowExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.RequestCancelExternalWorkflowExecutionDecisionAttributes, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "RequestCancelExternalWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.WorkflowID == "" { + return &types.BadRequestError{Message: "WorkflowId is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + runID := attributes.GetRunID() + if runID != "" && uuid.Parse(runID) == nil { + return &types.BadRequestError{Message: "Invalid RunId set on decision."} + } + + return nil +} + +func (v *attrValidator) validateSignalExternalWorkflowExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.SignalExternalWorkflowExecutionDecisionAttributes, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision."} + } + if attributes.Execution == nil { + return &types.BadRequestError{Message: "Execution is nil on decision."} + } + if attributes.Execution.WorkflowID == "" { + return &types.BadRequestError{Message: "WorkflowId is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.Execution.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + + targetRunID := attributes.Execution.GetRunID() + if targetRunID != "" && uuid.Parse(targetRunID) == nil { + return &types.BadRequestError{Message: "Invalid RunId set on decision."} + } + if attributes.SignalName == "" { + return &types.BadRequestError{Message: "SignalName is not set on decision."} + } + + return nil +} + +func (v *attrValidator) validateUpsertWorkflowSearchAttributes( + domainName string, + attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision."} + } + + if attributes.SearchAttributes == nil { + return &types.BadRequestError{Message: "SearchAttributes is not set on decision."} + } + + if len(attributes.GetSearchAttributes().GetIndexedFields()) == 0 { + return &types.BadRequestError{Message: "IndexedFields is empty on decision."} + } + + return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) +} + +func (v *attrValidator) validateContinueAsNewWorkflowExecutionAttributes( + attributes *types.ContinueAsNewWorkflowExecutionDecisionAttributes, + executionInfo *persistence.WorkflowExecutionInfo, + metricsScope int, + domain string, +) error { + + if attributes == nil { + return &types.BadRequestError{Message: "ContinueAsNewWorkflowExecutionDecisionAttributes is not set on decision."} + } + + // Inherit workflow type from previous execution if not provided on decision + if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { + attributes.WorkflowType = &types.WorkflowType{Name: executionInfo.WorkflowTypeName} + } + + if !common.IsValidIDLength( + attributes.WorkflowType.GetName(), + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.WorkflowTypeMaxLength(domain), + metrics.CadenceErrWorkflowTypeExceededWarnLimit, + domain, + v.logger, + tag.IDTypeWorkflowType) { + return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} + } + + // Inherit Tasklist from previous execution if not provided on decision + taskList, err := v.validatedTaskList(attributes.TaskList, executionInfo.TaskList, metricsScope, domain) + if err != nil { + return err + } + attributes.TaskList = taskList + + // Inherit workflow timeout from previous execution if not provided on decision + if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { + attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.WorkflowTimeout) + } + + // Inherit decision task timeout from previous execution if not provided on decision + if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { + attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(executionInfo.DecisionStartToCloseTimeout) + } + + // Check next run decision task delay + if attributes.GetBackoffStartIntervalInSeconds() < 0 { + return &types.BadRequestError{Message: "BackoffStartInterval is less than 0."} + } + + domainName, err := v.domainCache.GetDomainName(executionInfo.DomainID) + if err != nil { + return err + } + return v.searchAttributesValidator.ValidateSearchAttributes(attributes.GetSearchAttributes(), domainName) +} + +func (v *attrValidator) validateStartChildExecutionAttributes( + domainID string, + targetDomainID string, + attributes *types.StartChildWorkflowExecutionDecisionAttributes, + parentInfo *persistence.WorkflowExecutionInfo, + metricsScope int, +) error { + + if err := v.validateCrossDomainCall( + domainID, + targetDomainID, + ); err != nil { + return err + } + + if attributes == nil { + return &types.BadRequestError{Message: "StartChildWorkflowExecutionDecisionAttributes is not set on decision."} + } + + if attributes.GetWorkflowID() == "" { + return &types.BadRequestError{Message: "Required field WorkflowID is not set on decision."} + } + + if attributes.WorkflowType == nil || attributes.WorkflowType.GetName() == "" { + return &types.BadRequestError{Message: "Required field WorkflowType is not set on decision."} + } + + idLengthWarnLimit := v.config.MaxIDLengthWarnLimit() + if !common.IsValidIDLength( + attributes.GetDomain(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.DomainNameMaxLength(attributes.GetDomain()), + metrics.CadenceErrDomainNameExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeDomainName) { + return &types.BadRequestError{Message: "Domain exceeds length limit."} + } + if !common.IsValidIDLength( + attributes.GetWorkflowID(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowIDMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowIDExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowID) { + return &types.BadRequestError{Message: "WorkflowId exceeds length limit."} + } + + if !common.IsValidIDLength( + attributes.WorkflowType.GetName(), + v.metricsClient.Scope(metricsScope), + idLengthWarnLimit, + v.config.WorkflowTypeMaxLength(attributes.GetDomain()), + metrics.CadenceErrWorkflowTypeExceededWarnLimit, + attributes.GetDomain(), + v.logger, + tag.IDTypeWorkflowType) { + return &types.BadRequestError{Message: "WorkflowType exceeds length limit."} + } + + if err := common.ValidateRetryPolicy(attributes.RetryPolicy); err != nil { + return err + } + + if attributes.GetCronSchedule() != "" { + if _, err := backoff.ValidateSchedule(attributes.GetCronSchedule()); err != nil { + return err + } + } + + // Inherit tasklist from parent workflow execution if not provided on decision + taskList, err := v.validatedTaskList(attributes.TaskList, parentInfo.TaskList, metricsScope, attributes.GetDomain()) + if err != nil { + return err + } + attributes.TaskList = taskList + + // Inherit workflow timeout from parent workflow execution if not provided on decision + if attributes.GetExecutionStartToCloseTimeoutSeconds() <= 0 { + attributes.ExecutionStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.WorkflowTimeout) + } + + // Inherit decision task timeout from parent workflow execution if not provided on decision + if attributes.GetTaskStartToCloseTimeoutSeconds() <= 0 { + attributes.TaskStartToCloseTimeoutSeconds = common.Int32Ptr(parentInfo.DecisionStartToCloseTimeout) + } + + return nil +} + +func (v *attrValidator) validatedTaskList( + taskList *types.TaskList, + defaultVal string, + metricsScope int, + domain string, +) (*types.TaskList, error) { + + if taskList == nil { + taskList = &types.TaskList{} + } + + if taskList.GetName() == "" { + if defaultVal == "" { + return taskList, &types.BadRequestError{Message: "missing task list name"} + } + taskList.Name = defaultVal + return taskList, nil + } + + name := taskList.GetName() + if !common.IsValidIDLength( + name, + v.metricsClient.Scope(metricsScope), + v.config.MaxIDLengthWarnLimit(), + v.config.TaskListNameMaxLength(domain), + metrics.CadenceErrTaskListNameExceededWarnLimit, + domain, + v.logger, + tag.IDTypeTaskListName) { + return taskList, &types.BadRequestError{ + Message: fmt.Sprintf("task list name exceeds length limit of %v", v.config.TaskListNameMaxLength(domain)), + } + } + + if strings.HasPrefix(name, common.ReservedTaskListPrefix) { + return taskList, &types.BadRequestError{ + Message: fmt.Sprintf("task list name cannot start with reserved prefix %v", common.ReservedTaskListPrefix), + } + } + + return taskList, nil +} + +func (v *attrValidator) validateCrossDomainCall( + sourceDomainID string, + targetDomainID string, +) error { + + // same name, no check needed + if sourceDomainID == targetDomainID { + return nil + } + + sourceDomainEntry, err := v.domainCache.GetDomainByID(sourceDomainID) + if err != nil { + return err + } + + targetDomainEntry, err := v.domainCache.GetDomainByID(targetDomainID) + if err != nil { + return err + } + + sourceClusters := sourceDomainEntry.GetReplicationConfig().Clusters + targetClusters := targetDomainEntry.GetReplicationConfig().Clusters + + // both "local domain" + // here a domain is "local domain" when: + // - IsGlobalDomain() returns false + // - domainCluster contains only one cluster + // case 1 can be actually be combined with this case + if len(sourceClusters) == 1 && len(targetClusters) == 1 { + if sourceClusters[0].ClusterName == targetClusters[0].ClusterName { + return nil + } + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) + } + + // both global domain with > 1 replication cluster + // when code reaches here, at least one domain has more than one cluster + if len(sourceClusters) == len(targetClusters) && + v.config.EnableCrossClusterOperationsForDomain(sourceDomainEntry.GetInfo().Name) { + // check if the source domain cluster matches those for the target domain + for _, sourceCluster := range sourceClusters { + found := false + for _, targetCluster := range targetClusters { + if sourceCluster.ClusterName == targetCluster.ClusterName { + found = true + break + } + } + if !found { + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) + } + } + return nil + } + + return v.createCrossDomainCallError(sourceDomainEntry, targetDomainEntry) +} + +func (v *attrValidator) createCrossDomainCallError( + domainEntry *cache.DomainCacheEntry, + targetDomainEntry *cache.DomainCacheEntry, +) error { + return &types.BadRequestError{Message: fmt.Sprintf( + "cannot make cross domain call between %v and %v", + domainEntry.GetInfo().Name, + targetDomainEntry.GetInfo().Name, + )} +} diff --git a/service/history/decision/checker_test.go b/service/history/decision/checker_test.go index d0a836c9996..ce9b534a5ef 100644 --- a/service/history/decision/checker_test.go +++ b/service/history/decision/checker_test.go @@ -25,22 +25,747 @@ package decision import ( "sort" "testing" + "time" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" "github.com/uber-go/tally" "go.uber.org/zap/zaptest/observer" "golang.org/x/exp/maps" "github.com/uber/cadence/common" + "github.com/uber/cadence/common/cache" + "github.com/uber/cadence/common/cluster" + "github.com/uber/cadence/common/definition" + "github.com/uber/cadence/common/dynamicconfig" + "github.com/uber/cadence/common/log" "github.com/uber/cadence/common/log/testlogger" "github.com/uber/cadence/common/metrics" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/types" + "github.com/uber/cadence/service/history/config" + "github.com/uber/cadence/service/history/constants" "github.com/uber/cadence/service/history/execution" ) +type ( + attrValidatorSuite struct { + suite.Suite + *require.Assertions + + controller *gomock.Controller + mockDomainCache *cache.MockDomainCache + + validator *attrValidator + + testDomainID string + testTargetDomainID string + + testActivityMaxScheduleToStartTimeoutForRetryInSeconds int32 + } +) + +func TestAttrValidatorSuite(t *testing.T) { + s := new(attrValidatorSuite) + suite.Run(t, s) +} + +func (s *attrValidatorSuite) SetupSuite() { + s.testDomainID = "test domain ID" + s.testTargetDomainID = "test target domain ID" + s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds = 1800 +} + +func (s *attrValidatorSuite) TearDownSuite() { +} + +func (s *attrValidatorSuite) SetupTest() { + s.Assertions = require.New(s.T()) + + s.controller = gomock.NewController(s.T()) + s.mockDomainCache = cache.NewMockDomainCache(s.controller) + config := &config.Config{ + MaxIDLengthWarnLimit: dynamicconfig.GetIntPropertyFn(128), + DomainNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + IdentityMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + WorkflowIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + SignalNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + WorkflowTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + RequestIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + TaskListNameMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + ActivityIDMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + ActivityTypeMaxLength: dynamicconfig.GetIntPropertyFilteredByDomain(1000), + 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), + ActivityMaxScheduleToStartTimeoutForRetry: dynamicconfig.GetDurationPropertyFnFilteredByDomain( + time.Duration(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds) * time.Second, + ), + EnableCrossClusterOperationsForDomain: dynamicconfig.GetBoolPropertyFnFilteredByDomain(false), + } + s.validator = newAttrValidator( + s.mockDomainCache, + metrics.NewNoopMetricsClient(), + config, + log.NewNoop(), + ) +} + +func (s *attrValidatorSuite) TearDownTest() { + s.controller.Finish() +} + +func (s *attrValidatorSuite) TestValidateSignalExternalWorkflowExecutionAttributes() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).AnyTimes() + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).AnyTimes() + + var attributes *types.SignalExternalWorkflowExecutionDecisionAttributes + + err := s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "SignalExternalWorkflowExecutionDecisionAttributes is not set on decision.") + + attributes = &types.SignalExternalWorkflowExecutionDecisionAttributes{} + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "Execution is nil on decision.") + + attributes.Execution = &types.WorkflowExecution{} + attributes.Execution.WorkflowID = "workflow-id" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "SignalName is not set on decision.") + + attributes.Execution.RunID = "run-id" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.EqualError(err, "Invalid RunId set on decision.") + attributes.Execution.RunID = constants.TestRunID + + attributes.SignalName = "my signal name" + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.NoError(err) + + attributes.Input = []byte("test input") + err = s.validator.validateSignalExternalWorkflowExecutionAttributes(s.testDomainID, s.testTargetDomainID, attributes, metrics.HistoryRespondDecisionTaskCompletedScope) + s.NoError(err) +} + +func (s *attrValidatorSuite) TestValidateUpsertWorkflowSearchAttributes() { + domainName := "testDomain" + var attributes *types.UpsertWorkflowSearchAttributesDecisionAttributes + + err := s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "UpsertWorkflowSearchAttributesDecisionAttributes is not set on decision.") + + attributes = &types.UpsertWorkflowSearchAttributesDecisionAttributes{} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "SearchAttributes is not set on decision.") + + attributes.SearchAttributes = &types.SearchAttributes{} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.EqualError(err, "IndexedFields is empty on decision.") + + attributes.SearchAttributes.IndexedFields = map[string][]byte{"CustomKeywordField": []byte(`"bytes"`)} + err = s.validator.validateUpsertWorkflowSearchAttributes(domainName, attributes) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToLocal() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_SameCluster() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToEffectiveLocal_DiffCluster() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_LocalToGlobal() { + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToLocal_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 5678, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToEffectiveLocal_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestCurrentClusterName}}, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestAlternativeClusterName, + Clusters: []*persistence.ClusterReplicationConfig{{ClusterName: cluster.TestAlternativeClusterName}}, + }, + 5678, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_EffectiveLocalToGlobal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 5678, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToLocal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToEffectiveLocal() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 5678, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_SameDomain() { + targetDomainID := s.testDomainID + + err := s.validator.validateCrossDomainCall(s.testDomainID, targetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_SameCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: cluster.TestCurrentClusterName}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(2) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(2) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) + + s.validator.config.EnableCrossClusterOperationsForDomain = dynamicconfig.GetBoolPropertyFnFilteredByDomain(true) + err = s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.Nil(err) +} + +func (s *attrValidatorSuite) TestValidateCrossDomainCall_GlobalToGlobal_DiffDomain_DiffCluster() { + domainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: "cluster name for s.testDomainID"}, + }, + }, + 1234, + ) + targetDomainEntry := cache.NewGlobalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + &persistence.DomainReplicationConfig{ + ActiveClusterName: cluster.TestCurrentClusterName, + Clusters: []*persistence.ClusterReplicationConfig{ + {ClusterName: cluster.TestCurrentClusterName}, + {ClusterName: cluster.TestAlternativeClusterName}, + {ClusterName: "cluster name for s.testTargetDomainID"}, + }, + }, + 1234, + ) + + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateCrossDomainCall(s.testDomainID, s.testTargetDomainID) + s.IsType(&types.BadRequestError{}, err) +} + +func (s *attrValidatorSuite) TestValidateTaskListName() { + taskList := func(name string) *types.TaskList { + kind := types.TaskListKindNormal + return &types.TaskList{Name: name, Kind: &kind} + } + + testCases := []struct { + defaultVal string + input *types.TaskList + output *types.TaskList + isOutputErr bool + }{ + {"tl-1", nil, &types.TaskList{Name: "tl-1"}, false}, + {"", taskList("tl-1"), taskList("tl-1"), false}, + {"tl-1", taskList("tl-1"), taskList("tl-1"), false}, + {"", taskList("/tl-1"), taskList("/tl-1"), false}, + {"", taskList("/__cadence_sys"), taskList("/__cadence_sys"), false}, + {"", nil, &types.TaskList{}, true}, + {"", taskList(""), taskList(""), true}, + {"", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, + {"tl-1", taskList(common.ReservedTaskListPrefix), taskList(common.ReservedTaskListPrefix), true}, + {"", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, + {"tl-1", taskList(common.ReservedTaskListPrefix + "tl-1"), taskList(common.ReservedTaskListPrefix + "tl-1"), true}, + } + + for _, tc := range testCases { + key := tc.defaultVal + "#" + if tc.input != nil { + key += tc.input.GetName() + } else { + key += "nil" + } + s.Run(key, func() { + output, err := s.validator.validatedTaskList(tc.input, tc.defaultVal, metrics.HistoryRespondDecisionTaskCompletedScope, "domain_name") + if tc.isOutputErr { + s.Error(err) + } else { + s.NoError(err) + } + s.EqualValues(tc.output, output) + }) + } +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_NoRetryPolicy() { + wfTimeout := int32(5) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(3), // ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(10), // larger then wfTimeout + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), + ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: common.Int32Ptr(wfTimeout), + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartRetryable() { + s.mockDomainCache.EXPECT().GetDomainName(s.testDomainID).Return("some random domain name", nil).Times(1) + + wfTimeout := int32(3000) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(1), + RetryPolicy: &types.RetryPolicy{ + InitialIntervalInSeconds: 1, + BackoffCoefficient: 1.1, + ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than maximumScheduleToStartTimeoutForRetryInSeconds + NonRetriableErrorReasons: []string{"non-retryable error"}, + }, + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(attributes.RetryPolicy.ExpirationIntervalInSeconds), + ScheduleToStartTimeoutSeconds: common.Int32Ptr(s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds), + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, + RetryPolicy: attributes.RetryPolicy, + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} + +func (s *attrValidatorSuite) TestValidateActivityScheduleAttributes_WithRetryPolicy_ScheduleToStartNonRetryable() { + wfTimeout := int32(1000) + attributes := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: "some random activityID", + ActivityType: &types.ActivityType{ + Name: "some random activity type", + }, + Domain: s.testDomainID, + TaskList: &types.TaskList{ + Name: "some random task list", + }, + Input: []byte{1, 2, 3}, + ScheduleToCloseTimeoutSeconds: nil, // not set + ScheduleToStartTimeoutSeconds: common.Int32Ptr(3), + StartToCloseTimeoutSeconds: common.Int32Ptr(500), // extended ScheduleToStart + StartToClose > wfTimeout + HeartbeatTimeoutSeconds: common.Int32Ptr(1), + RetryPolicy: &types.RetryPolicy{ + InitialIntervalInSeconds: 1, + BackoffCoefficient: 1.1, + ExpirationIntervalInSeconds: s.testActivityMaxScheduleToStartTimeoutForRetryInSeconds + 1000, // larger than wfTimeout and maximumScheduleToStartTimeoutForRetryInSeconds + NonRetriableErrorReasons: []string{"cadenceInternal:Timeout SCHEDULE_TO_START"}, + }, + } + + expectedAttributesAfterValidation := &types.ScheduleActivityTaskDecisionAttributes{ + ActivityID: attributes.ActivityID, + ActivityType: attributes.ActivityType, + Domain: attributes.Domain, + TaskList: attributes.TaskList, + Input: attributes.Input, + ScheduleToCloseTimeoutSeconds: common.Int32Ptr(wfTimeout), + ScheduleToStartTimeoutSeconds: attributes.ScheduleToStartTimeoutSeconds, + StartToCloseTimeoutSeconds: attributes.StartToCloseTimeoutSeconds, + HeartbeatTimeoutSeconds: attributes.HeartbeatTimeoutSeconds, + RetryPolicy: attributes.RetryPolicy, + } + + domainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + targetDomainEntry := cache.NewLocalDomainCacheEntryForTest( + &persistence.DomainInfo{Name: s.testTargetDomainID}, + nil, + cluster.TestCurrentClusterName, + ) + s.mockDomainCache.EXPECT().GetDomainByID(s.testDomainID).Return(domainEntry, nil).Times(1) + s.mockDomainCache.EXPECT().GetDomainByID(s.testTargetDomainID).Return(targetDomainEntry, nil).Times(1) + + err := s.validator.validateActivityScheduleAttributes( + s.testDomainID, + s.testTargetDomainID, + attributes, + wfTimeout, + metrics.HistoryRespondDecisionTaskCompletedScope, + ) + s.Nil(err) + s.Equal(expectedAttributesAfterValidation, attributes) +} + const ( testDomainID = "test-domain-id" testDomainName = "test-domain" From 411fb4ef6e960618ee0ec7be3c5a48760701058d Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jul 2024 15:25:22 +0200 Subject: [PATCH 4/5] revert changes --- service/history/decision/checker.go | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/service/history/decision/checker.go b/service/history/decision/checker.go index 82ffe5e73bc..13dd803fffb 100644 --- a/service/history/decision/checker.go +++ b/service/history/decision/checker.go @@ -67,6 +67,28 @@ type ( } ) +func newAttrValidator( + domainCache cache.DomainCache, + metricsClient metrics.Client, + config *config.Config, + logger log.Logger, +) *attrValidator { + return &attrValidator{ + config: config, + domainCache: domainCache, + metricsClient: metricsClient, + logger: logger, + searchAttributesValidator: validator.NewSearchAttributesValidator( + logger, + config.EnableQueryAttributeValidation, + config.ValidSearchAttributes, + config.SearchAttributesNumberOfKeysLimit, + config.SearchAttributesSizeOfValueLimit, + config.SearchAttributesTotalSizeLimit, + ), + } +} + func newWorkflowSizeChecker( blobSizeLimitWarn int, blobSizeLimitError int, @@ -95,28 +117,6 @@ func newWorkflowSizeChecker( } } -func newAttrValidator( - domainCache cache.DomainCache, - metricsClient metrics.Client, - config *config.Config, - logger log.Logger, -) *attrValidator { - return &attrValidator{ - config: config, - domainCache: domainCache, - metricsClient: metricsClient, - logger: logger, - searchAttributesValidator: validator.NewSearchAttributesValidator( - logger, - config.EnableQueryAttributeValidation, - config.ValidSearchAttributes, - config.SearchAttributesNumberOfKeysLimit, - config.SearchAttributesSizeOfValueLimit, - config.SearchAttributesTotalSizeLimit, - ), - } -} - func (c *workflowSizeChecker) failWorkflowIfBlobSizeExceedsLimit( decisionTypeTag metrics.Tag, blob []byte, From 05f26b313dd6e74cf377d7ea6f09f3775fa29b1c Mon Sep 17 00:00:00 2001 From: Ilya Ozherelyev Date: Fri, 5 Jul 2024 16:03:54 +0200 Subject: [PATCH 5/5] fix tests --- service/history/decision/checker_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/service/history/decision/checker_test.go b/service/history/decision/checker_test.go index ce9b534a5ef..5ef1b1f4393 100644 --- a/service/history/decision/checker_test.go +++ b/service/history/decision/checker_test.go @@ -857,6 +857,10 @@ func TestWorkflowSizeChecker_failWorkflowIfBlobSizeExceedsLimit(t *testing.T) { } func TestWorkflowSizeChecker_failWorkflowSizeExceedsLimit(t *testing.T) { + var ( + testEventID = int64(1) + ) + for name, tc := range map[string]struct { historyCount int historyCountLimitWarn int @@ -971,8 +975,15 @@ func TestWorkflowSizeChecker_failWorkflowSizeExceedsLimit(t *testing.T) { RunID: testRunID, }).Times(1) } + if tc.expectFail { + mutableState.EXPECT().AddFailWorkflowEvent(testEventID, &types.FailWorkflowExecutionDecisionAttributes{ + Reason: common.StringPtr(common.FailureReasonSizeExceedsLimit), + Details: []byte("Workflow history size / count exceeds limit."), + }).Return(nil, nil).Times(1) + } checker := &workflowSizeChecker{ + completedID: testEventID, historyCountLimitWarn: tc.historyCountLimitWarn, historyCountLimitError: tc.historyCountLimitError, historySizeLimitWarn: tc.historySizeLimitWarn,