From 6d0bc0055dac05a3a6dd1572268cfae87510542f Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 16:43:56 +0200 Subject: [PATCH 1/6] feat: Add custom search attributes support to TemporalNamespace Adds the ability to configure custom search attributes on TemporalNamespace resources, enabling workflow search by custom attributes. Attributes are reconciled via the Temporal OperatorService API during namespace reconciliation. Closes #567 --- api/v1beta1/temporalnamespace_types.go | 8 + api/v1beta1/zz_generated.deepcopy.go | 7 + .../bases/temporal.io_temporalnamespaces.yaml | 12 + controllers/temporalnamespace_controller.go | 15 + pkg/temporal/search_attributes.go | 150 +++++++++ pkg/temporal/search_attributes_test.go | 312 ++++++++++++++++++ 6 files changed, 504 insertions(+) create mode 100644 pkg/temporal/search_attributes.go create mode 100644 pkg/temporal/search_attributes_test.go diff --git a/api/v1beta1/temporalnamespace_types.go b/api/v1beta1/temporalnamespace_types.go index 1ab2d45a..7a7e4c87 100644 --- a/api/v1beta1/temporalnamespace_types.go +++ b/api/v1beta1/temporalnamespace_types.go @@ -67,6 +67,14 @@ type TemporalNamespaceSpec struct { // If not set, the default cluster configuration is used. // +optional Archival *TemporalNamespaceArchivalSpec `json:"archival,omitempty"` + // CustomSearchAttributes is an optional mapping of custom search attribute names to types. + // Supported types: Text, Keyword, Int, Double, Bool, DateTime, KeywordList. + // +optional + CustomSearchAttributes map[string]string `json:"customSearchAttributes,omitempty"` + // AllowSearchAttributeDeletion makes the controller remove custom search attributes + // from the Temporal server if they are not present in the spec. + // +optional + AllowSearchAttributeDeletion bool `json:"allowSearchAttributeDeletion,omitempty"` } // TemporalNamespaceStatus defines the observed state of Namespace. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index ce8d20cb..6f33f6af 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1845,6 +1845,13 @@ func (in *TemporalNamespaceSpec) DeepCopyInto(out *TemporalNamespaceSpec) { *out = new(TemporalNamespaceArchivalSpec) (*in).DeepCopyInto(*out) } + if in.CustomSearchAttributes != nil { + in, out := &in.CustomSearchAttributes, &out.CustomSearchAttributes + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TemporalNamespaceSpec. diff --git a/config/crd/bases/temporal.io_temporalnamespaces.yaml b/config/crd/bases/temporal.io_temporalnamespaces.yaml index 37ec3aaa..66bee4b3 100644 --- a/config/crd/bases/temporal.io_temporalnamespaces.yaml +++ b/config/crd/bases/temporal.io_temporalnamespaces.yaml @@ -50,6 +50,11 @@ spec: AllowDeletion makes the controller delete the Temporal namespace if the CRD is deleted. type: boolean + allowSearchAttributeDeletion: + description: |- + AllowSearchAttributeDeletion makes the controller remove custom search attributes + from the Temporal server if they are not present in the spec. + type: boolean archival: description: |- Archival is a per-namespace archival configuration. @@ -130,6 +135,13 @@ spec: items: type: string type: array + customSearchAttributes: + additionalProperties: + type: string + description: |- + CustomSearchAttributes is an optional mapping of custom search attribute names to types. + Supported types: Text, Keyword, Int, Double, Bool, DateTime, KeywordList. + type: object data: additionalProperties: type: string diff --git a/controllers/temporalnamespace_controller.go b/controllers/temporalnamespace_controller.go index cfae9fd2..83d3ab15 100644 --- a/controllers/temporalnamespace_controller.go +++ b/controllers/temporalnamespace_controller.go @@ -136,6 +136,21 @@ func (r *TemporalNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Re } } + // Reconcile custom search attributes if any are configured or deletion is allowed. + if len(namespace.Spec.CustomSearchAttributes) > 0 || namespace.Spec.AllowSearchAttributeDeletion { + clusterClient, err := temporal.GetClusterClient(ctx, r.Client, cluster) + if err != nil { + err = fmt.Errorf("can't create cluster client for search attributes: %w", err) + return r.handleError(namespace, v1beta1.ReconcileErrorReason, err) + } + defer clusterClient.Close() + + if err := temporal.ReconcileSearchAttributes(ctx, clusterClient.OperatorService(), namespace); err != nil { + err = fmt.Errorf("can't reconcile search attributes for \"%s\" namespace: %w", namespace.GetName(), err) + return r.handleError(namespace, v1beta1.ReconcileErrorReason, err) + } + } + logger.Info("Successfully reconciled namespace", "namespace", namespace.GetName()) v1beta1.SetTemporalNamespaceReady(namespace, metav1.ConditionTrue, v1beta1.TemporalNamespaceCreatedReason, "Namespace successfully created") diff --git a/pkg/temporal/search_attributes.go b/pkg/temporal/search_attributes.go new file mode 100644 index 00000000..a9ba8afe --- /dev/null +++ b/pkg/temporal/search_attributes.go @@ -0,0 +1,150 @@ +// Licensed to Alexandre VILAIN under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Alexandre VILAIN licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package temporal + +import ( + "context" + "fmt" + + "github.com/alexandrevilain/temporal-operator/api/v1beta1" + enumsv1 "go.temporal.io/api/enums/v1" + "go.temporal.io/api/operatorservice/v1" + "google.golang.org/grpc" +) + +// searchAttributeTypes maps user-facing type names to Temporal IndexedValueType. +var searchAttributeTypes = map[string]enumsv1.IndexedValueType{ + "Text": enumsv1.INDEXED_VALUE_TYPE_TEXT, + "Keyword": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, + "Int": enumsv1.INDEXED_VALUE_TYPE_INT, + "Double": enumsv1.INDEXED_VALUE_TYPE_DOUBLE, + "Bool": enumsv1.INDEXED_VALUE_TYPE_BOOL, + "DateTime": enumsv1.INDEXED_VALUE_TYPE_DATETIME, + "KeywordList": enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST, +} + +// searchAttributeTypeNames is the reverse mapping from IndexedValueType to string. +var searchAttributeTypeNames map[enumsv1.IndexedValueType]string + +func init() { + searchAttributeTypeNames = make(map[enumsv1.IndexedValueType]string, len(searchAttributeTypes)) + for name, val := range searchAttributeTypes { + searchAttributeTypeNames[val] = name + } +} + +// SearchAttributeTypeFromString converts a user-facing type name to its IndexedValueType. +func SearchAttributeTypeFromString(s string) (enumsv1.IndexedValueType, error) { + t, ok := searchAttributeTypes[s] + if !ok { + return enumsv1.INDEXED_VALUE_TYPE_UNSPECIFIED, fmt.Errorf("invalid search attribute type %q: valid types are Text, Keyword, Int, Double, Bool, DateTime, KeywordList", s) + } + return t, nil +} + +// SearchAttributeTypeToString converts an IndexedValueType to its user-facing name. +func SearchAttributeTypeToString(t enumsv1.IndexedValueType) (string, error) { + name, ok := searchAttributeTypeNames[t] + if !ok { + return "", fmt.Errorf("unknown IndexedValueType: %v", t) + } + return name, nil +} + +// OperatorServiceClient is an interface for the Temporal OperatorService gRPC methods +// needed by search attribute reconciliation. It is satisfied by the client returned +// from temporalclient.Client.OperatorService(). +type OperatorServiceClient interface { + ListSearchAttributes(ctx context.Context, in *operatorservice.ListSearchAttributesRequest, opts ...grpc.CallOption) (*operatorservice.ListSearchAttributesResponse, error) + AddSearchAttributes(ctx context.Context, in *operatorservice.AddSearchAttributesRequest, opts ...grpc.CallOption) (*operatorservice.AddSearchAttributesResponse, error) + RemoveSearchAttributes(ctx context.Context, in *operatorservice.RemoveSearchAttributesRequest, opts ...grpc.CallOption) (*operatorservice.RemoveSearchAttributesResponse, error) +} + +// ReconcileSearchAttributes ensures the custom search attributes on the Temporal server +// match the desired state declared in the TemporalNamespace spec. +func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceClient, namespace *v1beta1.TemporalNamespace) error { + // 1. List current custom attributes from server. + listResp, err := operatorSvc.ListSearchAttributes(ctx, &operatorservice.ListSearchAttributesRequest{ + Namespace: namespace.GetName(), + }) + if err != nil { + return fmt.Errorf("listing search attributes: %w", err) + } + + existing := listResp.GetCustomAttributes() + + // 2. Parse desired spec into map[string]IndexedValueType. + desired := make(map[string]enumsv1.IndexedValueType, len(namespace.Spec.CustomSearchAttributes)) + for name, typeStr := range namespace.Spec.CustomSearchAttributes { + t, err := SearchAttributeTypeFromString(typeStr) + if err != nil { + return fmt.Errorf("search attribute %q: %w", name, err) + } + desired[name] = t + } + + // 3. Detect type mismatches and compute adds. + toAdd := make(map[string]enumsv1.IndexedValueType) + for name, desiredType := range desired { + existingType, exists := existing[name] + if exists { + if existingType != desiredType { + existingTypeName, _ := SearchAttributeTypeToString(existingType) + desiredTypeName, _ := SearchAttributeTypeToString(desiredType) + return fmt.Errorf("search attribute %q has type %s on server but %s in spec; Temporal does not allow type changes", name, existingTypeName, desiredTypeName) + } + // Already exists with correct type, nothing to do. + continue + } + toAdd[name] = desiredType + } + + // 4. Compute removals (only if AllowSearchAttributeDeletion is true). + var toRemove []string + if namespace.Spec.AllowSearchAttributeDeletion { + for name := range existing { + if _, inSpec := desired[name]; !inSpec { + toRemove = append(toRemove, name) + } + } + } + + // 5. Add new attributes. + if len(toAdd) > 0 { + _, err := operatorSvc.AddSearchAttributes(ctx, &operatorservice.AddSearchAttributesRequest{ + SearchAttributes: toAdd, + Namespace: namespace.GetName(), + }) + if err != nil { + return fmt.Errorf("adding search attributes: %w", err) + } + } + + // 6. Remove stale attributes. + if len(toRemove) > 0 { + _, err := operatorSvc.RemoveSearchAttributes(ctx, &operatorservice.RemoveSearchAttributesRequest{ + SearchAttributes: toRemove, + Namespace: namespace.GetName(), + }) + if err != nil { + return fmt.Errorf("removing search attributes: %w", err) + } + } + + return nil +} diff --git a/pkg/temporal/search_attributes_test.go b/pkg/temporal/search_attributes_test.go new file mode 100644 index 00000000..0e67921b --- /dev/null +++ b/pkg/temporal/search_attributes_test.go @@ -0,0 +1,312 @@ +// Licensed to Alexandre VILAIN under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Alexandre VILAIN licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package temporal + +import ( + "context" + "fmt" + "testing" + + "github.com/alexandrevilain/temporal-operator/api/v1beta1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + enumsv1 "go.temporal.io/api/enums/v1" + "go.temporal.io/api/operatorservice/v1" + "google.golang.org/grpc" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// mockOperatorServiceClient is a mock for testing. +type mockOperatorServiceClient struct { + listResponse *operatorservice.ListSearchAttributesResponse + listError error + addError error + removeError error + + addCalled bool + removeCalled bool + addRequest *operatorservice.AddSearchAttributesRequest + removeRequest *operatorservice.RemoveSearchAttributesRequest +} + +func (m *mockOperatorServiceClient) ListSearchAttributes(_ context.Context, _ *operatorservice.ListSearchAttributesRequest, _ ...grpc.CallOption) (*operatorservice.ListSearchAttributesResponse, error) { + return m.listResponse, m.listError +} + +func (m *mockOperatorServiceClient) AddSearchAttributes(_ context.Context, req *operatorservice.AddSearchAttributesRequest, _ ...grpc.CallOption) (*operatorservice.AddSearchAttributesResponse, error) { + m.addCalled = true + m.addRequest = req + return &operatorservice.AddSearchAttributesResponse{}, m.addError +} + +func (m *mockOperatorServiceClient) RemoveSearchAttributes(_ context.Context, req *operatorservice.RemoveSearchAttributesRequest, _ ...grpc.CallOption) (*operatorservice.RemoveSearchAttributesResponse, error) { + m.removeCalled = true + m.removeRequest = req + return &operatorservice.RemoveSearchAttributesResponse{}, m.removeError +} + +func TestSearchAttributeTypeFromString(t *testing.T) { + tests := map[string]struct { + input string + expected enumsv1.IndexedValueType + wantErr bool + }{ + "Text": {input: "Text", expected: enumsv1.INDEXED_VALUE_TYPE_TEXT}, + "Keyword": {input: "Keyword", expected: enumsv1.INDEXED_VALUE_TYPE_KEYWORD}, + "Int": {input: "Int", expected: enumsv1.INDEXED_VALUE_TYPE_INT}, + "Double": {input: "Double", expected: enumsv1.INDEXED_VALUE_TYPE_DOUBLE}, + "Bool": {input: "Bool", expected: enumsv1.INDEXED_VALUE_TYPE_BOOL}, + "DateTime": {input: "DateTime", expected: enumsv1.INDEXED_VALUE_TYPE_DATETIME}, + "KeywordList": {input: "KeywordList", expected: enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST}, + "invalid": {input: "invalid", wantErr: true}, + "empty": {input: "", wantErr: true}, + "lowercase": {input: "text", wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := SearchAttributeTypeFromString(tc.input) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestSearchAttributeTypeToString(t *testing.T) { + tests := map[string]struct { + input enumsv1.IndexedValueType + expected string + wantErr bool + }{ + "Text": {input: enumsv1.INDEXED_VALUE_TYPE_TEXT, expected: "Text"}, + "Keyword": {input: enumsv1.INDEXED_VALUE_TYPE_KEYWORD, expected: "Keyword"}, + "Int": {input: enumsv1.INDEXED_VALUE_TYPE_INT, expected: "Int"}, + "Double": {input: enumsv1.INDEXED_VALUE_TYPE_DOUBLE, expected: "Double"}, + "Bool": {input: enumsv1.INDEXED_VALUE_TYPE_BOOL, expected: "Bool"}, + "DateTime": {input: enumsv1.INDEXED_VALUE_TYPE_DATETIME, expected: "DateTime"}, + "KeywordList": {input: enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST, expected: "KeywordList"}, + "unspecified": {input: enumsv1.INDEXED_VALUE_TYPE_UNSPECIFIED, wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result, err := SearchAttributeTypeToString(tc.input) + if tc.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.expected, result) + }) + } +} + +func newNamespace(name string, attrs map[string]string, allowDeletion bool) *v1beta1.TemporalNamespace { + return &v1beta1.TemporalNamespace{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1beta1.TemporalNamespaceSpec{ + CustomSearchAttributes: attrs, + AllowSearchAttributeDeletion: allowDeletion, + }, + } +} + +func TestReconcileSearchAttributes(t *testing.T) { + ctx := context.Background() + + t.Run("add new attributes", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{}, + }, + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "Keyword", + "OrderTotal": "Double", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + require.NoError(t, err) + assert.True(t, mock.addCalled) + assert.False(t, mock.removeCalled) + assert.Equal(t, map[string]enumsv1.IndexedValueType{ + "CustomerId": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, + "OrderTotal": enumsv1.INDEXED_VALUE_TYPE_DOUBLE, + }, mock.addRequest.SearchAttributes) + assert.Equal(t, "test-ns", mock.addRequest.Namespace) + }) + + t.Run("remove stale with flag on", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + }, + }, + } + ns := newNamespace("test-ns", map[string]string{}, true) + + err := ReconcileSearchAttributes(ctx, mock, ns) + require.NoError(t, err) + assert.False(t, mock.addCalled) + assert.True(t, mock.removeCalled) + assert.Equal(t, []string{"OldAttr"}, mock.removeRequest.SearchAttributes) + }) + + t.Run("no removal with flag off", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + }, + }, + } + ns := newNamespace("test-ns", map[string]string{}, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + require.NoError(t, err) + assert.False(t, mock.addCalled) + assert.False(t, mock.removeCalled) + }) + + t.Run("no changes when spec matches server", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "CustomerId": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, + }, + }, + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "Keyword", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + require.NoError(t, err) + assert.False(t, mock.addCalled) + assert.False(t, mock.removeCalled) + }) + + t.Run("mixed add and remove", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "Existing": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, + "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + }, + }, + } + ns := newNamespace("test-ns", map[string]string{ + "Existing": "Keyword", + "NewAttr": "Bool", + }, true) + + err := ReconcileSearchAttributes(ctx, mock, ns) + require.NoError(t, err) + assert.True(t, mock.addCalled) + assert.True(t, mock.removeCalled) + assert.Equal(t, map[string]enumsv1.IndexedValueType{ + "NewAttr": enumsv1.INDEXED_VALUE_TYPE_BOOL, + }, mock.addRequest.SearchAttributes) + assert.Equal(t, []string{"OldAttr"}, mock.removeRequest.SearchAttributes) + }) + + t.Run("type mismatch returns error", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "CustomerId": enumsv1.INDEXED_VALUE_TYPE_TEXT, + }, + }, + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "Keyword", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + assert.Error(t, err) + assert.Contains(t, err.Error(), "does not allow type changes") + assert.False(t, mock.addCalled) + assert.False(t, mock.removeCalled) + }) + + t.Run("invalid type string returns error", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{}, + }, + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "InvalidType", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid search attribute type") + assert.False(t, mock.addCalled) + }) + + t.Run("list error propagated", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listError: fmt.Errorf("connection refused"), + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "Keyword", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + assert.Error(t, err) + assert.Contains(t, err.Error(), "connection refused") + }) + + t.Run("add error propagated", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{}, + }, + addError: fmt.Errorf("server error"), + } + ns := newNamespace("test-ns", map[string]string{ + "CustomerId": "Keyword", + }, false) + + err := ReconcileSearchAttributes(ctx, mock, ns) + assert.Error(t, err) + assert.Contains(t, err.Error(), "server error") + }) + + t.Run("remove error propagated", func(t *testing.T) { + mock := &mockOperatorServiceClient{ + listResponse: &operatorservice.ListSearchAttributesResponse{ + CustomAttributes: map[string]enumsv1.IndexedValueType{ + "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + }, + }, + removeError: fmt.Errorf("removal failed"), + } + ns := newNamespace("test-ns", map[string]string{}, true) + + err := ReconcileSearchAttributes(ctx, mock, ns) + assert.Error(t, err) + assert.Contains(t, err.Error(), "removal failed") + }) +} From 71c6253e9897003353138ed763f87856142f8656 Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 19:22:47 +0200 Subject: [PATCH 2/6] fix: address SonarCloud code analysis findings Reduce cognitive complexity of ReconcileSearchAttributes by extracting helper functions. Replace duplicated test namespace literal with constant and fix gofmt/unparam lint issues. --- pkg/temporal/search_attributes.go | 81 ++++++++++++++++---------- pkg/temporal/search_attributes_test.go | 34 ++++++----- 2 files changed, 68 insertions(+), 47 deletions(-) diff --git a/pkg/temporal/search_attributes.go b/pkg/temporal/search_attributes.go index a9ba8afe..9faec63c 100644 --- a/pkg/temporal/search_attributes.go +++ b/pkg/temporal/search_attributes.go @@ -75,10 +75,52 @@ type OperatorServiceClient interface { RemoveSearchAttributes(ctx context.Context, in *operatorservice.RemoveSearchAttributesRequest, opts ...grpc.CallOption) (*operatorservice.RemoveSearchAttributesResponse, error) } +// parseDesiredAttributes converts the spec's string type map into typed IndexedValueType map. +func parseDesiredAttributes(spec map[string]string) (map[string]enumsv1.IndexedValueType, error) { + desired := make(map[string]enumsv1.IndexedValueType, len(spec)) + for name, typeStr := range spec { + t, err := SearchAttributeTypeFromString(typeStr) + if err != nil { + return nil, fmt.Errorf("search attribute %q: %w", name, err) + } + desired[name] = t + } + return desired, nil +} + +// computeAttributesToAdd returns attributes present in desired but not in existing, +// and returns an error if any existing attribute has a type mismatch. +func computeAttributesToAdd(desired, existing map[string]enumsv1.IndexedValueType) (map[string]enumsv1.IndexedValueType, error) { + toAdd := make(map[string]enumsv1.IndexedValueType) + for name, desiredType := range desired { + existingType, exists := existing[name] + if !exists { + toAdd[name] = desiredType + continue + } + if existingType != desiredType { + existingTypeName, _ := SearchAttributeTypeToString(existingType) + desiredTypeName, _ := SearchAttributeTypeToString(desiredType) + return nil, fmt.Errorf("search attribute %q has type %s on server but %s in spec; Temporal does not allow type changes", name, existingTypeName, desiredTypeName) + } + } + return toAdd, nil +} + +// computeAttributesToRemove returns attribute names present in existing but not in desired. +func computeAttributesToRemove(desired, existing map[string]enumsv1.IndexedValueType) []string { + var toRemove []string + for name := range existing { + if _, inSpec := desired[name]; !inSpec { + toRemove = append(toRemove, name) + } + } + return toRemove +} + // ReconcileSearchAttributes ensures the custom search attributes on the Temporal server // match the desired state declared in the TemporalNamespace spec. func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceClient, namespace *v1beta1.TemporalNamespace) error { - // 1. List current custom attributes from server. listResp, err := operatorSvc.ListSearchAttributes(ctx, &operatorservice.ListSearchAttributesRequest{ Namespace: namespace.GetName(), }) @@ -88,43 +130,21 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC existing := listResp.GetCustomAttributes() - // 2. Parse desired spec into map[string]IndexedValueType. - desired := make(map[string]enumsv1.IndexedValueType, len(namespace.Spec.CustomSearchAttributes)) - for name, typeStr := range namespace.Spec.CustomSearchAttributes { - t, err := SearchAttributeTypeFromString(typeStr) - if err != nil { - return fmt.Errorf("search attribute %q: %w", name, err) - } - desired[name] = t + desired, err := parseDesiredAttributes(namespace.Spec.CustomSearchAttributes) + if err != nil { + return err } - // 3. Detect type mismatches and compute adds. - toAdd := make(map[string]enumsv1.IndexedValueType) - for name, desiredType := range desired { - existingType, exists := existing[name] - if exists { - if existingType != desiredType { - existingTypeName, _ := SearchAttributeTypeToString(existingType) - desiredTypeName, _ := SearchAttributeTypeToString(desiredType) - return fmt.Errorf("search attribute %q has type %s on server but %s in spec; Temporal does not allow type changes", name, existingTypeName, desiredTypeName) - } - // Already exists with correct type, nothing to do. - continue - } - toAdd[name] = desiredType + toAdd, err := computeAttributesToAdd(desired, existing) + if err != nil { + return err } - // 4. Compute removals (only if AllowSearchAttributeDeletion is true). var toRemove []string if namespace.Spec.AllowSearchAttributeDeletion { - for name := range existing { - if _, inSpec := desired[name]; !inSpec { - toRemove = append(toRemove, name) - } - } + toRemove = computeAttributesToRemove(desired, existing) } - // 5. Add new attributes. if len(toAdd) > 0 { _, err := operatorSvc.AddSearchAttributes(ctx, &operatorservice.AddSearchAttributesRequest{ SearchAttributes: toAdd, @@ -135,7 +155,6 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC } } - // 6. Remove stale attributes. if len(toRemove) > 0 { _, err := operatorSvc.RemoveSearchAttributes(ctx, &operatorservice.RemoveSearchAttributesRequest{ SearchAttributes: toRemove, diff --git a/pkg/temporal/search_attributes_test.go b/pkg/temporal/search_attributes_test.go index 0e67921b..64d0b22f 100644 --- a/pkg/temporal/search_attributes_test.go +++ b/pkg/temporal/search_attributes_test.go @@ -31,6 +31,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const testNamespace = "test-ns" + // mockOperatorServiceClient is a mock for testing. type mockOperatorServiceClient struct { listResponse *operatorservice.ListSearchAttributesResponse @@ -38,9 +40,9 @@ type mockOperatorServiceClient struct { addError error removeError error - addCalled bool - removeCalled bool - addRequest *operatorservice.AddSearchAttributesRequest + addCalled bool + removeCalled bool + addRequest *operatorservice.AddSearchAttributesRequest removeRequest *operatorservice.RemoveSearchAttributesRequest } @@ -120,9 +122,9 @@ func TestSearchAttributeTypeToString(t *testing.T) { } } -func newNamespace(name string, attrs map[string]string, allowDeletion bool) *v1beta1.TemporalNamespace { +func newNamespace(attrs map[string]string, allowDeletion bool) *v1beta1.TemporalNamespace { return &v1beta1.TemporalNamespace{ - ObjectMeta: metav1.ObjectMeta{Name: name}, + ObjectMeta: metav1.ObjectMeta{Name: testNamespace}, Spec: v1beta1.TemporalNamespaceSpec{ CustomSearchAttributes: attrs, AllowSearchAttributeDeletion: allowDeletion, @@ -139,7 +141,7 @@ func TestReconcileSearchAttributes(t *testing.T) { CustomAttributes: map[string]enumsv1.IndexedValueType{}, }, } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "Keyword", "OrderTotal": "Double", }, false) @@ -152,7 +154,7 @@ func TestReconcileSearchAttributes(t *testing.T) { "CustomerId": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, "OrderTotal": enumsv1.INDEXED_VALUE_TYPE_DOUBLE, }, mock.addRequest.SearchAttributes) - assert.Equal(t, "test-ns", mock.addRequest.Namespace) + assert.Equal(t, testNamespace, mock.addRequest.Namespace) }) t.Run("remove stale with flag on", func(t *testing.T) { @@ -163,7 +165,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, }, } - ns := newNamespace("test-ns", map[string]string{}, true) + ns := newNamespace(map[string]string{}, true) err := ReconcileSearchAttributes(ctx, mock, ns) require.NoError(t, err) @@ -180,7 +182,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, }, } - ns := newNamespace("test-ns", map[string]string{}, false) + ns := newNamespace(map[string]string{}, false) err := ReconcileSearchAttributes(ctx, mock, ns) require.NoError(t, err) @@ -196,7 +198,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, }, } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "Keyword", }, false) @@ -215,7 +217,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, }, } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "Existing": "Keyword", "NewAttr": "Bool", }, true) @@ -238,7 +240,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, }, } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "Keyword", }, false) @@ -255,7 +257,7 @@ func TestReconcileSearchAttributes(t *testing.T) { CustomAttributes: map[string]enumsv1.IndexedValueType{}, }, } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "InvalidType", }, false) @@ -269,7 +271,7 @@ func TestReconcileSearchAttributes(t *testing.T) { mock := &mockOperatorServiceClient{ listError: fmt.Errorf("connection refused"), } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "Keyword", }, false) @@ -285,7 +287,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, addError: fmt.Errorf("server error"), } - ns := newNamespace("test-ns", map[string]string{ + ns := newNamespace(map[string]string{ "CustomerId": "Keyword", }, false) @@ -303,7 +305,7 @@ func TestReconcileSearchAttributes(t *testing.T) { }, removeError: fmt.Errorf("removal failed"), } - ns := newNamespace("test-ns", map[string]string{}, true) + ns := newNamespace(map[string]string{}, true) err := ReconcileSearchAttributes(ctx, mock, ns) assert.Error(t, err) From edc7d66d3f681102de882f831f148d76cac937ba Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 20:10:57 +0200 Subject: [PATCH 3/6] feat: add structured logging to search attribute reconciliation Log key lifecycle events: existing attribute count, when attributes are up to date, additions with count, and removals with count and names. --- pkg/temporal/search_attributes.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/temporal/search_attributes.go b/pkg/temporal/search_attributes.go index 9faec63c..0412122a 100644 --- a/pkg/temporal/search_attributes.go +++ b/pkg/temporal/search_attributes.go @@ -25,6 +25,7 @@ import ( enumsv1 "go.temporal.io/api/enums/v1" "go.temporal.io/api/operatorservice/v1" "google.golang.org/grpc" + "sigs.k8s.io/controller-runtime/pkg/log" ) // searchAttributeTypes maps user-facing type names to Temporal IndexedValueType. @@ -121,14 +122,18 @@ func computeAttributesToRemove(desired, existing map[string]enumsv1.IndexedValue // ReconcileSearchAttributes ensures the custom search attributes on the Temporal server // match the desired state declared in the TemporalNamespace spec. func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceClient, namespace *v1beta1.TemporalNamespace) error { + logger := log.FromContext(ctx) + nsName := namespace.GetName() + listResp, err := operatorSvc.ListSearchAttributes(ctx, &operatorservice.ListSearchAttributesRequest{ - Namespace: namespace.GetName(), + Namespace: nsName, }) if err != nil { return fmt.Errorf("listing search attributes: %w", err) } existing := listResp.GetCustomAttributes() + logger.Info("Listed existing custom search attributes", "namespace", nsName, "count", len(existing)) desired, err := parseDesiredAttributes(namespace.Spec.CustomSearchAttributes) if err != nil { @@ -145,10 +150,16 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC toRemove = computeAttributesToRemove(desired, existing) } + if len(toAdd) == 0 && len(toRemove) == 0 { + logger.Info("Search attributes are up to date", "namespace", nsName) + return nil + } + if len(toAdd) > 0 { + logger.Info("Adding search attributes", "namespace", nsName, "count", len(toAdd)) _, err := operatorSvc.AddSearchAttributes(ctx, &operatorservice.AddSearchAttributesRequest{ SearchAttributes: toAdd, - Namespace: namespace.GetName(), + Namespace: nsName, }) if err != nil { return fmt.Errorf("adding search attributes: %w", err) @@ -156,9 +167,10 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC } if len(toRemove) > 0 { + logger.Info("Removing search attributes", "namespace", nsName, "count", len(toRemove), "attributes", toRemove) _, err := operatorSvc.RemoveSearchAttributes(ctx, &operatorservice.RemoveSearchAttributesRequest{ SearchAttributes: toRemove, - Namespace: namespace.GetName(), + Namespace: nsName, }) if err != nil { return fmt.Errorf("removing search attributes: %w", err) From b390952b37925587a617183fcf97f3f504d50b51 Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 20:16:04 +0200 Subject: [PATCH 4/6] fix: demote routine search attribute logs to debug level Only log at INFO when actually adding or removing attributes. Listing and up-to-date checks are debug (V(1)) to reduce noise. --- pkg/temporal/search_attributes.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/temporal/search_attributes.go b/pkg/temporal/search_attributes.go index 0412122a..e493482c 100644 --- a/pkg/temporal/search_attributes.go +++ b/pkg/temporal/search_attributes.go @@ -133,7 +133,7 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC } existing := listResp.GetCustomAttributes() - logger.Info("Listed existing custom search attributes", "namespace", nsName, "count", len(existing)) + logger.V(1).Info("Listed existing custom search attributes", "namespace", nsName, "count", len(existing)) desired, err := parseDesiredAttributes(namespace.Spec.CustomSearchAttributes) if err != nil { @@ -151,7 +151,7 @@ func ReconcileSearchAttributes(ctx context.Context, operatorSvc OperatorServiceC } if len(toAdd) == 0 && len(toRemove) == 0 { - logger.Info("Search attributes are up to date", "namespace", nsName) + logger.V(1).Info("Search attributes are up to date", "namespace", nsName) return nil } From 38481e12cdc7ebc92a2f7cc47de367fda918e8db Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 20:28:39 +0200 Subject: [PATCH 5/6] refactor: use bare enums import and remove init() Match namespace.go's import style (unaliased enums instead of enumsv1). Replace init() with a static map literal for the reverse type mapping. --- pkg/temporal/search_attributes.go | 49 ++++++++-------- pkg/temporal/search_attributes_test.go | 78 +++++++++++++------------- 2 files changed, 64 insertions(+), 63 deletions(-) diff --git a/pkg/temporal/search_attributes.go b/pkg/temporal/search_attributes.go index e493482c..86b749e5 100644 --- a/pkg/temporal/search_attributes.go +++ b/pkg/temporal/search_attributes.go @@ -22,44 +22,45 @@ import ( "fmt" "github.com/alexandrevilain/temporal-operator/api/v1beta1" - enumsv1 "go.temporal.io/api/enums/v1" + "go.temporal.io/api/enums/v1" "go.temporal.io/api/operatorservice/v1" "google.golang.org/grpc" "sigs.k8s.io/controller-runtime/pkg/log" ) // searchAttributeTypes maps user-facing type names to Temporal IndexedValueType. -var searchAttributeTypes = map[string]enumsv1.IndexedValueType{ - "Text": enumsv1.INDEXED_VALUE_TYPE_TEXT, - "Keyword": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, - "Int": enumsv1.INDEXED_VALUE_TYPE_INT, - "Double": enumsv1.INDEXED_VALUE_TYPE_DOUBLE, - "Bool": enumsv1.INDEXED_VALUE_TYPE_BOOL, - "DateTime": enumsv1.INDEXED_VALUE_TYPE_DATETIME, - "KeywordList": enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST, +var searchAttributeTypes = map[string]enums.IndexedValueType{ + "Text": enums.INDEXED_VALUE_TYPE_TEXT, + "Keyword": enums.INDEXED_VALUE_TYPE_KEYWORD, + "Int": enums.INDEXED_VALUE_TYPE_INT, + "Double": enums.INDEXED_VALUE_TYPE_DOUBLE, + "Bool": enums.INDEXED_VALUE_TYPE_BOOL, + "DateTime": enums.INDEXED_VALUE_TYPE_DATETIME, + "KeywordList": enums.INDEXED_VALUE_TYPE_KEYWORD_LIST, } // searchAttributeTypeNames is the reverse mapping from IndexedValueType to string. -var searchAttributeTypeNames map[enumsv1.IndexedValueType]string - -func init() { - searchAttributeTypeNames = make(map[enumsv1.IndexedValueType]string, len(searchAttributeTypes)) - for name, val := range searchAttributeTypes { - searchAttributeTypeNames[val] = name - } +var searchAttributeTypeNames = map[enums.IndexedValueType]string{ + enums.INDEXED_VALUE_TYPE_TEXT: "Text", + enums.INDEXED_VALUE_TYPE_KEYWORD: "Keyword", + enums.INDEXED_VALUE_TYPE_INT: "Int", + enums.INDEXED_VALUE_TYPE_DOUBLE: "Double", + enums.INDEXED_VALUE_TYPE_BOOL: "Bool", + enums.INDEXED_VALUE_TYPE_DATETIME: "DateTime", + enums.INDEXED_VALUE_TYPE_KEYWORD_LIST: "KeywordList", } // SearchAttributeTypeFromString converts a user-facing type name to its IndexedValueType. -func SearchAttributeTypeFromString(s string) (enumsv1.IndexedValueType, error) { +func SearchAttributeTypeFromString(s string) (enums.IndexedValueType, error) { t, ok := searchAttributeTypes[s] if !ok { - return enumsv1.INDEXED_VALUE_TYPE_UNSPECIFIED, fmt.Errorf("invalid search attribute type %q: valid types are Text, Keyword, Int, Double, Bool, DateTime, KeywordList", s) + return enums.INDEXED_VALUE_TYPE_UNSPECIFIED, fmt.Errorf("invalid search attribute type %q: valid types are Text, Keyword, Int, Double, Bool, DateTime, KeywordList", s) } return t, nil } // SearchAttributeTypeToString converts an IndexedValueType to its user-facing name. -func SearchAttributeTypeToString(t enumsv1.IndexedValueType) (string, error) { +func SearchAttributeTypeToString(t enums.IndexedValueType) (string, error) { name, ok := searchAttributeTypeNames[t] if !ok { return "", fmt.Errorf("unknown IndexedValueType: %v", t) @@ -77,8 +78,8 @@ type OperatorServiceClient interface { } // parseDesiredAttributes converts the spec's string type map into typed IndexedValueType map. -func parseDesiredAttributes(spec map[string]string) (map[string]enumsv1.IndexedValueType, error) { - desired := make(map[string]enumsv1.IndexedValueType, len(spec)) +func parseDesiredAttributes(spec map[string]string) (map[string]enums.IndexedValueType, error) { + desired := make(map[string]enums.IndexedValueType, len(spec)) for name, typeStr := range spec { t, err := SearchAttributeTypeFromString(typeStr) if err != nil { @@ -91,8 +92,8 @@ func parseDesiredAttributes(spec map[string]string) (map[string]enumsv1.IndexedV // computeAttributesToAdd returns attributes present in desired but not in existing, // and returns an error if any existing attribute has a type mismatch. -func computeAttributesToAdd(desired, existing map[string]enumsv1.IndexedValueType) (map[string]enumsv1.IndexedValueType, error) { - toAdd := make(map[string]enumsv1.IndexedValueType) +func computeAttributesToAdd(desired, existing map[string]enums.IndexedValueType) (map[string]enums.IndexedValueType, error) { + toAdd := make(map[string]enums.IndexedValueType) for name, desiredType := range desired { existingType, exists := existing[name] if !exists { @@ -109,7 +110,7 @@ func computeAttributesToAdd(desired, existing map[string]enumsv1.IndexedValueTyp } // computeAttributesToRemove returns attribute names present in existing but not in desired. -func computeAttributesToRemove(desired, existing map[string]enumsv1.IndexedValueType) []string { +func computeAttributesToRemove(desired, existing map[string]enums.IndexedValueType) []string { var toRemove []string for name := range existing { if _, inSpec := desired[name]; !inSpec { diff --git a/pkg/temporal/search_attributes_test.go b/pkg/temporal/search_attributes_test.go index 64d0b22f..ff310c3d 100644 --- a/pkg/temporal/search_attributes_test.go +++ b/pkg/temporal/search_attributes_test.go @@ -25,7 +25,7 @@ import ( "github.com/alexandrevilain/temporal-operator/api/v1beta1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - enumsv1 "go.temporal.io/api/enums/v1" + "go.temporal.io/api/enums/v1" "go.temporal.io/api/operatorservice/v1" "google.golang.org/grpc" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -65,16 +65,16 @@ func (m *mockOperatorServiceClient) RemoveSearchAttributes(_ context.Context, re func TestSearchAttributeTypeFromString(t *testing.T) { tests := map[string]struct { input string - expected enumsv1.IndexedValueType + expected enums.IndexedValueType wantErr bool }{ - "Text": {input: "Text", expected: enumsv1.INDEXED_VALUE_TYPE_TEXT}, - "Keyword": {input: "Keyword", expected: enumsv1.INDEXED_VALUE_TYPE_KEYWORD}, - "Int": {input: "Int", expected: enumsv1.INDEXED_VALUE_TYPE_INT}, - "Double": {input: "Double", expected: enumsv1.INDEXED_VALUE_TYPE_DOUBLE}, - "Bool": {input: "Bool", expected: enumsv1.INDEXED_VALUE_TYPE_BOOL}, - "DateTime": {input: "DateTime", expected: enumsv1.INDEXED_VALUE_TYPE_DATETIME}, - "KeywordList": {input: "KeywordList", expected: enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST}, + "Text": {input: "Text", expected: enums.INDEXED_VALUE_TYPE_TEXT}, + "Keyword": {input: "Keyword", expected: enums.INDEXED_VALUE_TYPE_KEYWORD}, + "Int": {input: "Int", expected: enums.INDEXED_VALUE_TYPE_INT}, + "Double": {input: "Double", expected: enums.INDEXED_VALUE_TYPE_DOUBLE}, + "Bool": {input: "Bool", expected: enums.INDEXED_VALUE_TYPE_BOOL}, + "DateTime": {input: "DateTime", expected: enums.INDEXED_VALUE_TYPE_DATETIME}, + "KeywordList": {input: "KeywordList", expected: enums.INDEXED_VALUE_TYPE_KEYWORD_LIST}, "invalid": {input: "invalid", wantErr: true}, "empty": {input: "", wantErr: true}, "lowercase": {input: "text", wantErr: true}, @@ -95,18 +95,18 @@ func TestSearchAttributeTypeFromString(t *testing.T) { func TestSearchAttributeTypeToString(t *testing.T) { tests := map[string]struct { - input enumsv1.IndexedValueType + input enums.IndexedValueType expected string wantErr bool }{ - "Text": {input: enumsv1.INDEXED_VALUE_TYPE_TEXT, expected: "Text"}, - "Keyword": {input: enumsv1.INDEXED_VALUE_TYPE_KEYWORD, expected: "Keyword"}, - "Int": {input: enumsv1.INDEXED_VALUE_TYPE_INT, expected: "Int"}, - "Double": {input: enumsv1.INDEXED_VALUE_TYPE_DOUBLE, expected: "Double"}, - "Bool": {input: enumsv1.INDEXED_VALUE_TYPE_BOOL, expected: "Bool"}, - "DateTime": {input: enumsv1.INDEXED_VALUE_TYPE_DATETIME, expected: "DateTime"}, - "KeywordList": {input: enumsv1.INDEXED_VALUE_TYPE_KEYWORD_LIST, expected: "KeywordList"}, - "unspecified": {input: enumsv1.INDEXED_VALUE_TYPE_UNSPECIFIED, wantErr: true}, + "Text": {input: enums.INDEXED_VALUE_TYPE_TEXT, expected: "Text"}, + "Keyword": {input: enums.INDEXED_VALUE_TYPE_KEYWORD, expected: "Keyword"}, + "Int": {input: enums.INDEXED_VALUE_TYPE_INT, expected: "Int"}, + "Double": {input: enums.INDEXED_VALUE_TYPE_DOUBLE, expected: "Double"}, + "Bool": {input: enums.INDEXED_VALUE_TYPE_BOOL, expected: "Bool"}, + "DateTime": {input: enums.INDEXED_VALUE_TYPE_DATETIME, expected: "DateTime"}, + "KeywordList": {input: enums.INDEXED_VALUE_TYPE_KEYWORD_LIST, expected: "KeywordList"}, + "unspecified": {input: enums.INDEXED_VALUE_TYPE_UNSPECIFIED, wantErr: true}, } for name, tc := range tests { @@ -138,7 +138,7 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("add new attributes", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{}, + CustomAttributes: map[string]enums.IndexedValueType{}, }, } ns := newNamespace(map[string]string{ @@ -150,9 +150,9 @@ func TestReconcileSearchAttributes(t *testing.T) { require.NoError(t, err) assert.True(t, mock.addCalled) assert.False(t, mock.removeCalled) - assert.Equal(t, map[string]enumsv1.IndexedValueType{ - "CustomerId": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, - "OrderTotal": enumsv1.INDEXED_VALUE_TYPE_DOUBLE, + assert.Equal(t, map[string]enums.IndexedValueType{ + "CustomerId": enums.INDEXED_VALUE_TYPE_KEYWORD, + "OrderTotal": enums.INDEXED_VALUE_TYPE_DOUBLE, }, mock.addRequest.SearchAttributes) assert.Equal(t, testNamespace, mock.addRequest.Namespace) }) @@ -160,8 +160,8 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("remove stale with flag on", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + CustomAttributes: map[string]enums.IndexedValueType{ + "OldAttr": enums.INDEXED_VALUE_TYPE_TEXT, }, }, } @@ -177,8 +177,8 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("no removal with flag off", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + CustomAttributes: map[string]enums.IndexedValueType{ + "OldAttr": enums.INDEXED_VALUE_TYPE_TEXT, }, }, } @@ -193,8 +193,8 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("no changes when spec matches server", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "CustomerId": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, + CustomAttributes: map[string]enums.IndexedValueType{ + "CustomerId": enums.INDEXED_VALUE_TYPE_KEYWORD, }, }, } @@ -211,9 +211,9 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("mixed add and remove", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "Existing": enumsv1.INDEXED_VALUE_TYPE_KEYWORD, - "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + CustomAttributes: map[string]enums.IndexedValueType{ + "Existing": enums.INDEXED_VALUE_TYPE_KEYWORD, + "OldAttr": enums.INDEXED_VALUE_TYPE_TEXT, }, }, } @@ -226,8 +226,8 @@ func TestReconcileSearchAttributes(t *testing.T) { require.NoError(t, err) assert.True(t, mock.addCalled) assert.True(t, mock.removeCalled) - assert.Equal(t, map[string]enumsv1.IndexedValueType{ - "NewAttr": enumsv1.INDEXED_VALUE_TYPE_BOOL, + assert.Equal(t, map[string]enums.IndexedValueType{ + "NewAttr": enums.INDEXED_VALUE_TYPE_BOOL, }, mock.addRequest.SearchAttributes) assert.Equal(t, []string{"OldAttr"}, mock.removeRequest.SearchAttributes) }) @@ -235,8 +235,8 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("type mismatch returns error", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "CustomerId": enumsv1.INDEXED_VALUE_TYPE_TEXT, + CustomAttributes: map[string]enums.IndexedValueType{ + "CustomerId": enums.INDEXED_VALUE_TYPE_TEXT, }, }, } @@ -254,7 +254,7 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("invalid type string returns error", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{}, + CustomAttributes: map[string]enums.IndexedValueType{}, }, } ns := newNamespace(map[string]string{ @@ -283,7 +283,7 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("add error propagated", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{}, + CustomAttributes: map[string]enums.IndexedValueType{}, }, addError: fmt.Errorf("server error"), } @@ -299,8 +299,8 @@ func TestReconcileSearchAttributes(t *testing.T) { t.Run("remove error propagated", func(t *testing.T) { mock := &mockOperatorServiceClient{ listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enumsv1.IndexedValueType{ - "OldAttr": enumsv1.INDEXED_VALUE_TYPE_TEXT, + CustomAttributes: map[string]enums.IndexedValueType{ + "OldAttr": enums.INDEXED_VALUE_TYPE_TEXT, }, }, removeError: fmt.Errorf("removal failed"), From f900c131ba27d86d2a14efcc6c9bd7571be44d14 Mon Sep 17 00:00:00 2001 From: Ido Date: Mon, 16 Mar 2026 20:37:53 +0200 Subject: [PATCH 6/6] test: remove trivial tests that only verify map lookups Remove TestSearchAttributeTypeFromString, TestSearchAttributeTypeToString (just mirror the map data), and error propagation tests (just verify fmt.Errorf wrapping). Keep tests that exercise actual reconciliation logic. --- pkg/temporal/search_attributes_test.go | 106 ------------------------- 1 file changed, 106 deletions(-) diff --git a/pkg/temporal/search_attributes_test.go b/pkg/temporal/search_attributes_test.go index ff310c3d..72d212a8 100644 --- a/pkg/temporal/search_attributes_test.go +++ b/pkg/temporal/search_attributes_test.go @@ -19,7 +19,6 @@ package temporal import ( "context" - "fmt" "testing" "github.com/alexandrevilain/temporal-operator/api/v1beta1" @@ -62,66 +61,6 @@ func (m *mockOperatorServiceClient) RemoveSearchAttributes(_ context.Context, re return &operatorservice.RemoveSearchAttributesResponse{}, m.removeError } -func TestSearchAttributeTypeFromString(t *testing.T) { - tests := map[string]struct { - input string - expected enums.IndexedValueType - wantErr bool - }{ - "Text": {input: "Text", expected: enums.INDEXED_VALUE_TYPE_TEXT}, - "Keyword": {input: "Keyword", expected: enums.INDEXED_VALUE_TYPE_KEYWORD}, - "Int": {input: "Int", expected: enums.INDEXED_VALUE_TYPE_INT}, - "Double": {input: "Double", expected: enums.INDEXED_VALUE_TYPE_DOUBLE}, - "Bool": {input: "Bool", expected: enums.INDEXED_VALUE_TYPE_BOOL}, - "DateTime": {input: "DateTime", expected: enums.INDEXED_VALUE_TYPE_DATETIME}, - "KeywordList": {input: "KeywordList", expected: enums.INDEXED_VALUE_TYPE_KEYWORD_LIST}, - "invalid": {input: "invalid", wantErr: true}, - "empty": {input: "", wantErr: true}, - "lowercase": {input: "text", wantErr: true}, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - result, err := SearchAttributeTypeFromString(tc.input) - if tc.wantErr { - assert.Error(t, err) - return - } - require.NoError(t, err) - assert.Equal(t, tc.expected, result) - }) - } -} - -func TestSearchAttributeTypeToString(t *testing.T) { - tests := map[string]struct { - input enums.IndexedValueType - expected string - wantErr bool - }{ - "Text": {input: enums.INDEXED_VALUE_TYPE_TEXT, expected: "Text"}, - "Keyword": {input: enums.INDEXED_VALUE_TYPE_KEYWORD, expected: "Keyword"}, - "Int": {input: enums.INDEXED_VALUE_TYPE_INT, expected: "Int"}, - "Double": {input: enums.INDEXED_VALUE_TYPE_DOUBLE, expected: "Double"}, - "Bool": {input: enums.INDEXED_VALUE_TYPE_BOOL, expected: "Bool"}, - "DateTime": {input: enums.INDEXED_VALUE_TYPE_DATETIME, expected: "DateTime"}, - "KeywordList": {input: enums.INDEXED_VALUE_TYPE_KEYWORD_LIST, expected: "KeywordList"}, - "unspecified": {input: enums.INDEXED_VALUE_TYPE_UNSPECIFIED, wantErr: true}, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - result, err := SearchAttributeTypeToString(tc.input) - if tc.wantErr { - assert.Error(t, err) - return - } - require.NoError(t, err) - assert.Equal(t, tc.expected, result) - }) - } -} - func newNamespace(attrs map[string]string, allowDeletion bool) *v1beta1.TemporalNamespace { return &v1beta1.TemporalNamespace{ ObjectMeta: metav1.ObjectMeta{Name: testNamespace}, @@ -266,49 +205,4 @@ func TestReconcileSearchAttributes(t *testing.T) { assert.Contains(t, err.Error(), "invalid search attribute type") assert.False(t, mock.addCalled) }) - - t.Run("list error propagated", func(t *testing.T) { - mock := &mockOperatorServiceClient{ - listError: fmt.Errorf("connection refused"), - } - ns := newNamespace(map[string]string{ - "CustomerId": "Keyword", - }, false) - - err := ReconcileSearchAttributes(ctx, mock, ns) - assert.Error(t, err) - assert.Contains(t, err.Error(), "connection refused") - }) - - t.Run("add error propagated", func(t *testing.T) { - mock := &mockOperatorServiceClient{ - listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enums.IndexedValueType{}, - }, - addError: fmt.Errorf("server error"), - } - ns := newNamespace(map[string]string{ - "CustomerId": "Keyword", - }, false) - - err := ReconcileSearchAttributes(ctx, mock, ns) - assert.Error(t, err) - assert.Contains(t, err.Error(), "server error") - }) - - t.Run("remove error propagated", func(t *testing.T) { - mock := &mockOperatorServiceClient{ - listResponse: &operatorservice.ListSearchAttributesResponse{ - CustomAttributes: map[string]enums.IndexedValueType{ - "OldAttr": enums.INDEXED_VALUE_TYPE_TEXT, - }, - }, - removeError: fmt.Errorf("removal failed"), - } - ns := newNamespace(map[string]string{}, true) - - err := ReconcileSearchAttributes(ctx, mock, ns) - assert.Error(t, err) - assert.Contains(t, err.Error(), "removal failed") - }) }