From 797bdaff86ce0006663110b84524dd91394f88d7 Mon Sep 17 00:00:00 2001 From: Bob Callaway Date: Mon, 9 Oct 2023 07:49:05 -0400 Subject: [PATCH] fix nits, add unit tests Signed-off-by: Bob Callaway --- pkg/api/error.go | 2 +- pkg/api/index.go | 6 +- pkg/indexstorage/redis/redis.go | 10 ++- pkg/indexstorage/redis/redis_test.go | 100 +++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 pkg/indexstorage/redis/redis_test.go diff --git a/pkg/api/error.go b/pkg/api/error.go index d05e9170e..e0a04fc93 100644 --- a/pkg/api/error.go +++ b/pkg/api/error.go @@ -42,7 +42,7 @@ const ( malformedUUID = "UUID must be a 64-character hexadecimal string" malformedPublicKey = "public key provided could not be parsed" failedToGenerateCanonicalKey = "error generating canonicalized public key" - redisUnexpectedResult = "unexpected result from searching index" + indexStorageUnexpectedResult = "unexpected result from searching index" lastSizeGreaterThanKnown = "the tree size requested(%d) was greater than what is currently observable(%d)" signingError = "error signing" sthGenerateError = "error generating signed tree head" diff --git a/pkg/api/index.go b/pkg/api/index.go index a271fe822..0fb5a7232 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -47,7 +47,7 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder { sha := util.PrefixSHA(params.Query.Hash) resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(sha)) if err != nil { - return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult) + return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult) } result.Add(resultUUIDs) } @@ -74,14 +74,14 @@ func SearchIndexHandler(params index.SearchIndexParams) middleware.Responder { keyHash := sha256.Sum256(canonicalKey) resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(hex.EncodeToString(keyHash[:]))) if err != nil { - return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult) + return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult) } result.Add(resultUUIDs) } if params.Query.Email != "" { resultUUIDs, err := indexStorageClient.LookupIndices(httpReqCtx, strings.ToLower(params.Query.Email.String())) if err != nil { - return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("redis client: %w", err), redisUnexpectedResult) + return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("index storage error: %w", err), indexStorageUnexpectedResult) } result.Add(resultUUIDs) } diff --git a/pkg/indexstorage/redis/redis.go b/pkg/indexstorage/redis/redis.go index 6758d68b3..bdf42e396 100644 --- a/pkg/indexstorage/redis/redis.go +++ b/pkg/indexstorage/redis/redis.go @@ -25,6 +25,7 @@ import ( const ProviderType = "redis" +// IndexStorageProvider implements indexstorage.IndexStorage type IndexStorageProvider struct { client *redis.Client } @@ -39,6 +40,8 @@ func NewProvider(address, port string) (*IndexStorageProvider, error) { return provider, nil } +// LookupIndices looks up and returns all indices for the specified key. The key value will be canonicalized +// by converting all characters into a lowercase value before looking up in Redis func (isp *IndexStorageProvider) LookupIndices(ctx context.Context, key string) ([]string, error) { if isp.client == nil { return []string{}, errors.New("redis client has not been initialized") @@ -46,12 +49,13 @@ func (isp *IndexStorageProvider) LookupIndices(ctx context.Context, key string) return isp.client.LRange(ctx, strings.ToLower(key), 0, -1).Result() } -func (isp *IndexStorageProvider) WriteIndex(ctx context.Context, key, value string) error { +// WriteIndex adds the index for the specified key. The key value will be canonicalized +// by converting all characters into a lowercase value before appending the index in Redis +func (isp *IndexStorageProvider) WriteIndex(ctx context.Context, key, index string) error { if isp.client == nil { return errors.New("redis client has not been initialized") } - _, err := isp.client.LPush(ctx, strings.ToLower(key), value).Result() - if err != nil { + if _, err := isp.client.LPush(ctx, strings.ToLower(key), index).Result(); err != nil { return fmt.Errorf("redis client: %w", err) } return nil diff --git a/pkg/indexstorage/redis/redis_test.go b/pkg/indexstorage/redis/redis_test.go new file mode 100644 index 000000000..509679604 --- /dev/null +++ b/pkg/indexstorage/redis/redis_test.go @@ -0,0 +1,100 @@ +// Copyright 2023 The Sigstore Authors. +// +// Licensed 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 redis + +import ( + "context" + "errors" + "testing" + + "github.com/go-redis/redismock/v9" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "go.uber.org/goleak" +) + +func TestLookupIndices(t *testing.T) { + key := "87c1b129fbadd7b6e9abc0a9ef7695436d767aece042bec198a97e949fcbe14c" + value := []string{"1e1f2c881ae0608ec77ebf88a75c66d3099113a7343238f2f7a0ebb91a4ed335"} + redisClient, mock := redismock.NewClientMock() + mock.Regexp().ExpectLRange(key, 0, -1).SetVal(value) + + isp := IndexStorageProvider{redisClient} + + indices, err := isp.LookupIndices(context.Background(), key) + if err != nil { + t.Error(err) + } + + less := func(a, b string) bool { return a < b } + if cmp.Diff(value, indices, cmpopts.SortSlices(less)) != "" { + t.Errorf("expected %s, got %s", value, indices) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } + + mock.ClearExpect() + errRedis := errors.New("redis error") + mock.Regexp().ExpectLRange(key, 0, -1).SetErr(errRedis) + if _, err := isp.LookupIndices(context.Background(), key); err == nil { + t.Error("unexpected success") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +func TestWriteIndex(t *testing.T) { + key := "87c1b129fbadd7b6e9abc0a9ef7695436d767aece042bec198a97e949fcbe14c" + value := []string{"1e1f2c881ae0608ec77ebf88a75c66d3099113a7343238f2f7a0ebb91a4ed335"} + redisClient, mock := redismock.NewClientMock() + mock.Regexp().ExpectLPush(key, value).SetVal(1) + + isp := IndexStorageProvider{redisClient} + if err := isp.WriteIndex(context.Background(), key, value[0]); err != nil { + t.Error(err) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } + + mock.ClearExpect() + errRedis := errors.New("redis error") + mock.Regexp().ExpectLPush(key, value).SetErr(errRedis) + if err := isp.WriteIndex(context.Background(), key, value[0]); err == nil { + t.Error("unexpected success") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Error(err) + } +} + +func TestUninitializedClient(t *testing.T) { + // this is not initialized with a real Redis client + isp := IndexStorageProvider{} + if _, err := isp.LookupIndices(context.Background(), "key"); err == nil { + t.Error("unexpected success") + } + if err := isp.WriteIndex(context.Background(), "key", "value"); err == nil { + t.Error("unexpected success") + } +} + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +}