Move check back to validator

temporalio · Sep 8, 2022 · daba691 · daba691
1 parent ec85d2e
commit daba691
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 22 deletions.
diff --git a/common/rpc/interceptor/namespace_validator.go b/common/rpc/interceptor/namespace_validator.go
@@ -142,12 +142,17 @@ func (ni *NamespaceValidatorInterceptor) extractNamespaceFromRequest(req interfa
 	}
 	namespaceName := namespace.Name(reqWithNamespace.GetNamespace())
 
-	switch req.(type) {
+	switch request := req.(type) {
 	case *workflowservice.DescribeNamespaceRequest:
-		// Special case for DescribeNamespace API which can get namespace by Id and should bypass namespace registry.
+		// Special case for DescribeNamespace API which should read namespace directly from database.
+		// Therefore, it must bypass namespace registry and validator.
+		if request.GetId() == "" && namespaceName.IsEmpty() {
+			return nil, ErrNamespaceNotSet
+		}
 		return nil, nil
 	case *workflowservice.RegisterNamespaceRequest:
-		// Special case for RegisterNamespace API. `namespace` is name of namespace that about to be registered. There is no namespace entry for it.
+		// Special case for RegisterNamespace API. `namespaceName` is name of namespace that about to be registered.
+		// There is no namespace entry for it, therefore, it must bypass namespace registry and validator.
 		if namespaceName.IsEmpty() {
 			return nil, ErrNamespaceNotSet
 		}

diff --git a/common/rpc/interceptor/namespace_validator_test.go b/common/rpc/interceptor/namespace_validator_test.go
@@ -201,10 +201,22 @@ func (s *namespaceValidatorSuite) Test_Intercept_StatusFromNamespace() {
 		// DescribeNamespace
 		{
 			state:       enumspb.NAMESPACE_STATE_UNSPECIFIED,
-			expectedErr: nil,
+			expectedErr: ErrNamespaceNotSet,
 			method:      "/temporal/DescribeNamespace",
 			req:         &workflowservice.DescribeNamespaceRequest{},
 		},
+		{
+			state:       enumspb.NAMESPACE_STATE_UNSPECIFIED,
+			expectedErr: nil,
+			method:      "/temporal/DescribeNamespace",
+			req:         &workflowservice.DescribeNamespaceRequest{Id: "test-namespace-id"},
+		},
+		{
+			state:       enumspb.NAMESPACE_STATE_UNSPECIFIED,
+			expectedErr: nil,
+			method:      "/temporal/DescribeNamespace",
+			req:         &workflowservice.DescribeNamespaceRequest{Namespace: "test-namespace"},
+		},
 		// RegisterNamespace
 		{
 			state:       enumspb.NAMESPACE_STATE_UNSPECIFIED,
@@ -400,8 +412,8 @@ func (s *namespaceValidatorSuite) Test_Intercept_DescribeNamespace_Id() {
 		return &workflowservice.DescribeNamespaceResponse{}, nil
 	})
 
-	s.NoError(err)
-	s.True(handlerCalled)
+	s.IsType(&serviceerror.InvalidArgument{}, err)
+	s.False(handlerCalled)
 }
 
 func (s *namespaceValidatorSuite) Test_Intercept_GetClusterInfo() {

diff --git a/service/frontend/workflow_handler.go b/service/frontend/workflow_handler.go
@@ -256,10 +256,6 @@ func (wh *WorkflowHandler) DescribeNamespace(ctx context.Context, request *workf
 		return nil, errRequestNotSet
 	}
 
-	if request.GetId() == "" && request.GetNamespace() == "" {
-		return nil, interceptor.ErrNamespaceNotSet
-	}
-
 	resp, err := wh.namespaceHandler.DescribeNamespace(ctx, request)
 	if err != nil {
 		return resp, err

diff --git a/service/frontend/workflow_handler_test.go b/service/frontend/workflow_handler_test.go
@@ -74,7 +74,6 @@ import (
 	"go.temporal.io/server/common/primitives"
 	"go.temporal.io/server/common/primitives/timestamp"
 	"go.temporal.io/server/common/resource"
-	"go.temporal.io/server/common/rpc/interceptor"
 	"go.temporal.io/server/common/searchattribute"
 	"go.temporal.io/server/service/worker/batcher"
 )
@@ -994,7 +993,7 @@ func (s *workflowHandlerSuite) TestDescribeNamespace_Success_ArchivalEnabled() {
 	wh := s.getWorkflowHandler(s.newConfig())
 
 	req := &workflowservice.DescribeNamespaceRequest{
-		Id: "test-namespace-id",
+		Namespace: "test-namespace",
 	}
 	result, err := wh.DescribeNamespace(context.Background(), req)
 
@@ -1007,16 +1006,6 @@ func (s *workflowHandlerSuite) TestDescribeNamespace_Success_ArchivalEnabled() {
 	s.Equal(testVisibilityArchivalURI, result.Config.GetVisibilityArchivalUri())
 }
 
-func (s *workflowHandlerSuite) TestDescribeNamespace_NamespaceNotSet() {
-	wh := s.getWorkflowHandler(s.newConfig())
-
-	req := &workflowservice.DescribeNamespaceRequest{}
-	result, err := wh.DescribeNamespace(context.Background(), req)
-
-	s.ErrorIs(err, interceptor.ErrNamespaceNotSet)
-	s.Nil(result)
-}
-
 func (s *workflowHandlerSuite) TestUpdateNamespace_Failure_UpdateExistingArchivalURI() {
 	s.mockMetadataMgr.EXPECT().GetMetadata(gomock.Any()).Return(&persistence.GetMetadataResponse{
 		NotificationVersion: int64(0),