diff --git a/cmd/api-server/main.go b/cmd/api-server/main.go index 73bbf58d5aa..97222a56e8f 100644 --- a/cmd/api-server/main.go +++ b/cmd/api-server/main.go @@ -13,6 +13,10 @@ import ( "syscall" "time" + corev1 "k8s.io/api/core/v1" + + "github.com/kubeshop/testkube/pkg/cache" + "github.com/nats-io/nats.go" "google.golang.org/grpc/metadata" "google.golang.org/protobuf/types/known/emptypb" @@ -487,7 +491,7 @@ func main() { inspector := imageinspector.NewInspector( cfg.TestkubeRegistry, imageinspector.NewSkopeoFetcher(), - imageinspector.NewSecretFetcher(secretClient), + imageinspector.NewSecretFetcher(secretClient, cache.NewInMemoryCache[*corev1.Secret](), imageinspector.WithSecretCacheTTL(cfg.TestkubeImageCredentialsCacheTTL)), inspectorStorages..., ) @@ -515,6 +519,7 @@ func main() { features, cfg.TestkubeDefaultStorageClassName, cfg.WhitelistedContainers, + cfg.TestkubeImageCredentialsCacheTTL, ) if err != nil { exitOnError("Creating container executor", err) diff --git a/cmd/tcl/testworkflow-toolkit/spawn/utils.go b/cmd/tcl/testworkflow-toolkit/spawn/utils.go index acfe2b137eb..6d6db65b967 100644 --- a/cmd/tcl/testworkflow-toolkit/spawn/utils.go +++ b/cmd/tcl/testworkflow-toolkit/spawn/utils.go @@ -299,6 +299,7 @@ func CreateBaseMachine() expressions.Machine { "images.toolkit": env.Config().Images.Toolkit, "images.persistence.enabled": strconv.FormatBool(env.Config().Images.InspectorPersistenceEnabled), "images.persistence.key": env.Config().Images.InspectorPersistenceCacheKey, + "images.cache.ttl": env.Config().Images.ImageCredentialsCacheTTL.String(), }), ) } diff --git a/cmd/testworkflow-toolkit/env/client.go b/cmd/testworkflow-toolkit/env/client.go index d59cb72c1a5..ab6ca5528d8 100644 --- a/cmd/testworkflow-toolkit/env/client.go +++ b/cmd/testworkflow-toolkit/env/client.go @@ -5,6 +5,10 @@ import ( "fmt" "math" + corev1 "k8s.io/api/core/v1" + + "github.com/kubeshop/testkube/pkg/cache" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -58,7 +62,7 @@ func ImageInspector() imageinspector.Inspector { return imageinspector.NewInspector( Config().System.DefaultRegistry, imageinspector.NewSkopeoFetcher(), - imageinspector.NewSecretFetcher(secretClient), + imageinspector.NewSecretFetcher(secretClient, cache.NewInMemoryCache[*corev1.Secret](), imageinspector.WithSecretCacheTTL(Config().Images.ImageCredentialsCacheTTL)), inspectorStorages..., ) } diff --git a/cmd/testworkflow-toolkit/env/config.go b/cmd/testworkflow-toolkit/env/config.go index 2d301a16a93..23862f9d799 100644 --- a/cmd/testworkflow-toolkit/env/config.go +++ b/cmd/testworkflow-toolkit/env/config.go @@ -60,10 +60,11 @@ type envSystemConfig struct { } type envImagesConfig struct { - Init string `envconfig:"TESTKUBE_TW_INIT_IMAGE"` - Toolkit string `envconfig:"TESTKUBE_TW_TOOLKIT_IMAGE"` - InspectorPersistenceEnabled bool `envconfig:"TK_IMG_P" default:"false"` - InspectorPersistenceCacheKey string `envconfig:"TK_IMG_PK"` + Init string `envconfig:"TESTKUBE_TW_INIT_IMAGE"` + Toolkit string `envconfig:"TESTKUBE_TW_TOOLKIT_IMAGE"` + InspectorPersistenceEnabled bool `envconfig:"TK_IMG_P" default:"false"` + InspectorPersistenceCacheKey string `envconfig:"TK_IMG_PK"` + ImageCredentialsCacheTTL time.Duration `envconfig:"TK_IMG_CRED_TTL" default:"0"` } type featuresConfig struct { diff --git a/internal/config/config.go b/internal/config/config.go index b07e70ace13..7f48a517350 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -82,36 +82,39 @@ type Config struct { TestkubeProTLSSecret string `envconfig:"TESTKUBE_PRO_TLS_SECRET" default:""` TestkubeProRunnerCustomCASecret string `envconfig:"TESTKUBE_PRO_RUNNER_CUSTOM_CA_SECRET" default:""` TestkubeWatcherNamespaces string `envconfig:"TESTKUBE_WATCHER_NAMESPACES" default:""` - GraphqlPort string `envconfig:"TESTKUBE_GRAPHQL_PORT" default:"8070"` TestkubeRegistry string `envconfig:"TESTKUBE_REGISTRY" default:""` TestkubePodStartTimeout time.Duration `envconfig:"TESTKUBE_POD_START_TIMEOUT" default:"30m"` - CDEventsTarget string `envconfig:"CDEVENTS_TARGET" default:""` - TestkubeDashboardURI string `envconfig:"TESTKUBE_DASHBOARD_URI" default:""` - DisableReconciler bool `envconfig:"DISABLE_RECONCILER" default:"false"` - TestkubeClusterName string `envconfig:"TESTKUBE_CLUSTER_NAME" default:""` - CompressArtifacts bool `envconfig:"COMPRESSARTIFACTS" default:"false"` - TestkubeHelmchartVersion string `envconfig:"TESTKUBE_HELMCHART_VERSION" default:""` - DebugListenAddr string `envconfig:"DEBUG_LISTEN_ADDR" default:"0.0.0.0:1337"` - EnableDebugServer bool `envconfig:"ENABLE_DEBUG_SERVER" default:"false"` - EnableSecretsEndpoint bool `envconfig:"ENABLE_SECRETS_ENDPOINT" default:"false"` - EnableListingAllSecrets bool `envconfig:"ENABLE_LISTING_ALL_SECRETS" default:"false"` - SecretCreationPrefix string `envconfig:"SECRET_CREATION_PREFIX" default:"testkube-"` - DisableMongoMigrations bool `envconfig:"DISABLE_MONGO_MIGRATIONS" default:"false"` - Debug bool `envconfig:"DEBUG" default:"false"` - Trace bool `envconfig:"TRACE" default:"false"` - EnableImageDataPersistentCache bool `envconfig:"TESTKUBE_ENABLE_IMAGE_DATA_PERSISTENT_CACHE" default:"false"` - ImageDataPersistentCacheKey string `envconfig:"TESTKUBE_IMAGE_DATA_PERSISTENT_CACHE_KEY" default:"testkube-image-cache"` - LogServerGrpcAddress string `envconfig:"LOG_SERVER_GRPC_ADDRESS" default:":9090"` - LogServerSecure bool `envconfig:"LOG_SERVER_SECURE" default:"false"` - LogServerSkipVerify bool `envconfig:"LOG_SERVER_SKIP_VERIFY" default:"false"` - LogServerCertFile string `envconfig:"LOG_SERVER_CERT_FILE" default:""` - LogServerKeyFile string `envconfig:"LOG_SERVER_KEY_FILE" default:""` - LogServerCAFile string `envconfig:"LOG_SERVER_CA_FILE" default:""` - DisableSecretCreation bool `envconfig:"DISABLE_SECRET_CREATION" default:"false"` - TestkubeExecutionNamespaces string `envconfig:"TESTKUBE_EXECUTION_NAMESPACES" default:""` - TestkubeDefaultStorageClassName string `envconfig:"TESTKUBE_DEFAULT_STORAGE_CLASS_NAME" default:""` - GlobalWorkflowTemplateName string `envconfig:"TESTKUBE_GLOBAL_WORKFLOW_TEMPLATE_NAME" default:""` - EnableK8sEvents bool `envconfig:"ENABLE_K8S_EVENTS" default:"true"` + // TestkubeImageCredentialsCacheTTL is the duration for which the image pull credentials should be cached provided as a Go duration string. + // If set to 0, the cache is disabled. + TestkubeImageCredentialsCacheTTL time.Duration `envconfig:"TESTKUBE_IMAGE_CREDENTIALS_CACHE_TTL" default:"30m"` + GraphqlPort string `envconfig:"TESTKUBE_GRAPHQL_PORT" default:"8070"` + CDEventsTarget string `envconfig:"CDEVENTS_TARGET" default:""` + TestkubeDashboardURI string `envconfig:"TESTKUBE_DASHBOARD_URI" default:""` + DisableReconciler bool `envconfig:"DISABLE_RECONCILER" default:"false"` + TestkubeClusterName string `envconfig:"TESTKUBE_CLUSTER_NAME" default:""` + CompressArtifacts bool `envconfig:"COMPRESSARTIFACTS" default:"false"` + TestkubeHelmchartVersion string `envconfig:"TESTKUBE_HELMCHART_VERSION" default:""` + DebugListenAddr string `envconfig:"DEBUG_LISTEN_ADDR" default:"0.0.0.0:1337"` + EnableDebugServer bool `envconfig:"ENABLE_DEBUG_SERVER" default:"false"` + EnableSecretsEndpoint bool `envconfig:"ENABLE_SECRETS_ENDPOINT" default:"false"` + EnableListingAllSecrets bool `envconfig:"ENABLE_LISTING_ALL_SECRETS" default:"false"` + SecretCreationPrefix string `envconfig:"SECRET_CREATION_PREFIX" default:"testkube-"` + DisableMongoMigrations bool `envconfig:"DISABLE_MONGO_MIGRATIONS" default:"false"` + Debug bool `envconfig:"DEBUG" default:"false"` + Trace bool `envconfig:"TRACE" default:"false"` + EnableImageDataPersistentCache bool `envconfig:"TESTKUBE_ENABLE_IMAGE_DATA_PERSISTENT_CACHE" default:"false"` + ImageDataPersistentCacheKey string `envconfig:"TESTKUBE_IMAGE_DATA_PERSISTENT_CACHE_KEY" default:"testkube-image-cache"` + LogServerGrpcAddress string `envconfig:"LOG_SERVER_GRPC_ADDRESS" default:":9090"` + LogServerSecure bool `envconfig:"LOG_SERVER_SECURE" default:"false"` + LogServerSkipVerify bool `envconfig:"LOG_SERVER_SKIP_VERIFY" default:"false"` + LogServerCertFile string `envconfig:"LOG_SERVER_CERT_FILE" default:""` + LogServerKeyFile string `envconfig:"LOG_SERVER_KEY_FILE" default:""` + LogServerCAFile string `envconfig:"LOG_SERVER_CA_FILE" default:""` + DisableSecretCreation bool `envconfig:"DISABLE_SECRET_CREATION" default:"false"` + TestkubeExecutionNamespaces string `envconfig:"TESTKUBE_EXECUTION_NAMESPACES" default:""` + TestkubeDefaultStorageClassName string `envconfig:"TESTKUBE_DEFAULT_STORAGE_CLASS_NAME" default:""` + GlobalWorkflowTemplateName string `envconfig:"TESTKUBE_GLOBAL_WORKFLOW_TEMPLATE_NAME" default:""` + EnableK8sEvents bool `envconfig:"ENABLE_K8S_EVENTS" default:"true"` // DEPRECATED: Use TestkubeProAPIKey instead TestkubeCloudAPIKey string `envconfig:"TESTKUBE_CLOUD_API_KEY" default:""` diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go new file mode 100644 index 00000000000..0d84f8b4603 --- /dev/null +++ b/pkg/cache/cache.go @@ -0,0 +1,33 @@ +package cache + +import ( + "context" + "math" + "time" + + "github.com/pkg/errors" +) + +var ( + ErrNotFound = errors.New("item not found") +) + +type Cache[T any] interface { + // Get retrieves the cached value for the given key. + // If the key is not found or expired, the method should return ErrNotFound. + Get(ctx context.Context, key string) (T, error) + // Set stores the value in the cache with the given key. + // If ttl is 0, the item should not be cached and this method should return no error. + Set(ctx context.Context, key string, value T, ttl time.Duration) error +} + +// IsCacheMiss returns true if the error is a cache miss error. +// This is a helper function to determine so users don't have to compare errors manually. +func IsCacheMiss(err error) bool { + return errors.Is(err, ErrNotFound) +} + +// InfiniteTTL returns a time.Duration that represents an infinite TTL. +func InfiniteTTL() time.Duration { + return math.MaxInt64 +} diff --git a/pkg/cache/inmem.go b/pkg/cache/inmem.go new file mode 100644 index 00000000000..22b9138cb59 --- /dev/null +++ b/pkg/cache/inmem.go @@ -0,0 +1,71 @@ +package cache + +import ( + "context" + "sync" + "time" + + "github.com/pkg/errors" +) + +type item[T any] struct { + value T + expiresAt *time.Time +} + +// timeGetter is a function that returns the current time. +type timeGetter func() time.Time + +type InMemoryCache[T any] struct { + cache sync.Map + timeGetter timeGetter +} + +// NewInMemoryCache creates a new in-memory cache. +// The underlying cache implementation uses a sync.Map so it is thread-safe. +func NewInMemoryCache[T any]() *InMemoryCache[T] { + return &InMemoryCache[T]{ + timeGetter: time.Now, + } +} + +func (c *InMemoryCache[T]) Get(ctx context.Context, key string) (T, error) { + var defaultVal T + rawItem, ok := c.cache.Load(key) + if !ok { + return defaultVal, ErrNotFound + } + i, ok := rawItem.(*item[T]) + if !ok { + return defaultVal, errors.New("unexpected item type found in cache") + } + + if i.expiresAt != nil && i.expiresAt.Before(time.Now()) { + c.cache.Delete(key) + return defaultVal, ErrNotFound + } + + return i.value, nil +} + +func (c *InMemoryCache[T]) Set(ctx context.Context, key string, value T, ttl time.Duration) error { + if ttl < 0 { + return errors.New("ttl must be greater than 0") + } + if ttl == 0 { + return nil + } + + i := &item[T]{ + value: value, + } + if ttl > 0 { + expiresAt := c.timeGetter().Add(ttl) + i.expiresAt = &expiresAt + } + c.cache.Store(key, i) + + return nil +} + +var _ Cache[any] = &InMemoryCache[any]{} diff --git a/pkg/cache/inmem_test.go b/pkg/cache/inmem_test.go new file mode 100644 index 00000000000..e98d19836af --- /dev/null +++ b/pkg/cache/inmem_test.go @@ -0,0 +1,223 @@ +package cache + +import ( + "context" + "testing" + "time" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" +) + +func TestInMemoryCache_Get(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + tests := []struct { + name string + setup func(cache *InMemoryCache[string]) + key string + want any + wantError error + }{ + { + name: "Get existing item without TTL", + setup: func(c *InMemoryCache[string]) { + i := &item[string]{ + value: "value", + } + c.cache.Store("existing", i) + }, + key: "existing", + want: "value", + wantError: nil, + }, + { + name: "Get existing item with expired TTL", + setup: func(cache *InMemoryCache[string]) { + expiresAt := time.Now().Add(-1 * time.Hour) + i := &item[string]{ + value: "value", + expiresAt: &expiresAt, + } + cache.cache.Store("stale", i) + }, + key: "stale", + want: nil, + wantError: ErrNotFound, + }, + { + name: "Get non-existing item", + setup: func(cache *InMemoryCache[string]) {}, + key: "non-existing", + want: nil, + wantError: ErrNotFound, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := NewInMemoryCache[string]() + tt.setup(cache) + got, err := cache.Get(ctx, tt.key) + if tt.wantError != nil { + assert.EqualError(t, err, tt.wantError.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} + +func TestInMemoryCache_Set(t *testing.T) { + t.Parallel() + + ctx := context.Background() + staticTimeGetter := func() time.Time { + return time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) + } + + tests := []struct { + name string + key string + value string + ttl time.Duration + wantErr error + }{ + { + name: "Set item without TTL", + key: "key", + value: "value", + wantErr: nil, + }, + { + name: "Set item with TTL", + key: "key", + value: "value", + ttl: 1 * time.Hour, + wantErr: nil, + }, + { + name: "Set item with infinite TTL", + key: "key", + value: "value", + ttl: InfiniteTTL(), + wantErr: nil, + }, + { + name: "Set item with invalid TTL", + key: "key", + value: "value", + ttl: -1 * time.Minute, + wantErr: errors.New("ttl must be greater than 0"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := InMemoryCache[string]{ + timeGetter: staticTimeGetter, + } + err := c.Set(ctx, tt.key, tt.value, tt.ttl) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + if tt.ttl == 0 { + // Assert that the item is not expired + _, err := c.Get(ctx, tt.key) + assert.ErrorIs(t, err, ErrNotFound) + return + } + rawItem, ok := c.cache.Load(tt.key) + if !ok { + t.Fatalf("expected item to be set in cache") + } + i, ok := rawItem.(*item[string]) + if !ok { + t.Fatalf("unexpected item type found in cache") + } + assert.Equal(t, tt.value, i.value) + if tt.ttl > 0 { + if i.expiresAt == nil { + t.Fatalf("expected item to have an expiry time") + } + assert.Equal(t, staticTimeGetter().Add(tt.ttl), *i.expiresAt) + } else { + assert.Nil(t, i.expiresAt) + } + } + }) + } +} + +func TestInMemoryCache(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + tests := []struct { + name string + key string + value string + ttl time.Duration + waitForExpiration bool + want any + }{ + { + name: "Set and Get existing item without TTL", + key: "existing", + value: "value", + ttl: 0, + }, + { + name: "Set and Get existing item with TTL", + key: "existingWithTTL", + value: "value", + ttl: 1 * time.Hour, + want: "value", + }, + { + name: "Set and Get item which expired", + key: "existingWithTTL", + value: "value", + ttl: 100 * time.Millisecond, + waitForExpiration: true, + want: "value", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := NewInMemoryCache[string]() + + err := cache.Set(ctx, tt.key, tt.value, tt.ttl) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.ttl == 0 { + // Set should not have cached the item if TTL is 0 + _, err := cache.Get(ctx, tt.key) + assert.ErrorIs(t, err, ErrNotFound) + return + } + if tt.waitForExpiration { + // Assert that a not found error eventually gets returned + assert.Eventually(t, func() bool { + _, err := cache.Get(ctx, tt.key) + return errors.Is(err, ErrNotFound) + }, 300*time.Millisecond, 30*time.Millisecond) + // Assert that any subsequent Get calls return a not found error + _, err := cache.Get(ctx, tt.key) + assert.Equal(t, ErrNotFound, err) + + return + } + got, err := cache.Get(ctx, tt.key) + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/executor/containerexecutor/containerexecutor.go b/pkg/executor/containerexecutor/containerexecutor.go index 0da5513b1e7..83a476f7916 100644 --- a/pkg/executor/containerexecutor/containerexecutor.go +++ b/pkg/executor/containerexecutor/containerexecutor.go @@ -6,6 +6,8 @@ import ( "path/filepath" "time" + "github.com/kubeshop/testkube/pkg/cache" + "github.com/pkg/errors" "github.com/kubeshop/testkube/pkg/featureflags" @@ -85,6 +87,7 @@ func NewContainerExecutor( features featureflags.FeatureFlags, defaultStorageClassName string, whitelistedContainers []string, + imageCredentialsCacheTTL time.Duration, ) (client *ContainerExecutor, err error) { clientSet, err := k8sclient.ConnectToK8s() if err != nil { @@ -96,31 +99,32 @@ func NewContainerExecutor( } return &ContainerExecutor{ - clientSet: clientSet, - repository: repo, - log: log.DefaultLogger, - images: images, - templates: templates, - imageInspector: imageInspector, - configMap: configMap, - serviceAccountNames: serviceAccountNames, - metrics: metrics, - emitter: emiter, - testsClient: testsClient, - executorsClient: executorsClient, - testExecutionsClient: testExecutionsClient, - templatesClient: templatesClient, - registry: registry, - podStartTimeout: podStartTimeout, - clusterID: clusterID, - dashboardURI: dashboardURI, - apiURI: apiURI, - natsURI: natsUri, - debug: debug, - logsStream: logsStream, - features: features, - defaultStorageClassName: defaultStorageClassName, - whitelistedContainers: whitelistedContainers, + clientSet: clientSet, + repository: repo, + log: log.DefaultLogger, + images: images, + templates: templates, + imageInspector: imageInspector, + configMap: configMap, + serviceAccountNames: serviceAccountNames, + metrics: metrics, + emitter: emiter, + testsClient: testsClient, + executorsClient: executorsClient, + testExecutionsClient: testExecutionsClient, + templatesClient: templatesClient, + registry: registry, + podStartTimeout: podStartTimeout, + clusterID: clusterID, + dashboardURI: dashboardURI, + apiURI: apiURI, + natsURI: natsUri, + debug: debug, + logsStream: logsStream, + features: features, + defaultStorageClassName: defaultStorageClassName, + whitelistedContainers: whitelistedContainers, + imageCredentialsCacheTTL: imageCredentialsCacheTTL, }, nil } @@ -156,6 +160,8 @@ type ContainerExecutor struct { defaultStorageClassName string // whitelistedContainers is a list of containers from which logs are allowed to be streamed. whitelistedContainers []string + // imageCredentialsCacheTTL defines the ttl for image credentials cache. + imageCredentialsCacheTTL time.Duration } type JobOptions struct { @@ -323,7 +329,11 @@ func (c *ContainerExecutor) createJob(ctx context.Context, execution testkube.Ex if err != nil { return nil, errors.Wrap(err, "failed to build secrets client") } - inspector = imageinspector.NewInspector(c.registry, imageinspector.NewSkopeoFetcher(), imageinspector.NewSecretFetcher(secretClient)) + inspector = imageinspector.NewInspector( + c.registry, + imageinspector.NewSkopeoFetcher(), + imageinspector.NewSecretFetcher(secretClient, cache.NewInMemoryCache[*corev1.Secret](), imageinspector.WithSecretCacheTTL(c.imageCredentialsCacheTTL)), + ) } jobOptions, err := NewJobOptions(c.log, c.templatesClient, c.images, c.templates, inspector, diff --git a/pkg/imageinspector/secretfetcher.go b/pkg/imageinspector/secretfetcher.go index 44b81f51159..e03b6d84567 100644 --- a/pkg/imageinspector/secretfetcher.go +++ b/pkg/imageinspector/secretfetcher.go @@ -2,35 +2,54 @@ package imageinspector import ( "context" - "sync" + "time" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + "github.com/kubeshop/testkube/pkg/cache" + "github.com/kubeshop/testkube/pkg/log" + "github.com/kubeshop/testkube/pkg/secret" ) +type SecretFetcherOption func(*secretFetcher) + +// WithSecretCacheTTL sets the time to live for the cached secrets. +func WithSecretCacheTTL(ttl time.Duration) SecretFetcherOption { + return func(s *secretFetcher) { + s.ttl = ttl + } +} + type secretFetcher struct { client secret.Interface - cache map[string]*corev1.Secret - mu sync.RWMutex + cache cache.Cache[*corev1.Secret] + ttl time.Duration } -func NewSecretFetcher(client secret.Interface) SecretFetcher { - return &secretFetcher{ +func NewSecretFetcher(client secret.Interface, cache cache.Cache[*corev1.Secret], opts ...SecretFetcherOption) SecretFetcher { + s := &secretFetcher{ client: client, - cache: make(map[string]*corev1.Secret), + cache: cache, + } + for _, opt := range opts { + opt(s) } + return s } func (s *secretFetcher) Get(ctx context.Context, name string) (*corev1.Secret, error) { - // Get cached secret - s.mu.RLock() - if v, ok := s.cache[name]; ok { - s.mu.RUnlock() - return v, nil + if s.ttl > 0 { + // Get cached secret + cached, err := s.getFromCache(ctx, name) + if err != nil { + return nil, err + } + if cached != nil { + return cached, nil + } } - s.mu.RUnlock() // Load secret from the Kubernetes obj, err := s.client.GetObject(name) @@ -38,13 +57,28 @@ func (s *secretFetcher) Get(ctx context.Context, name string) (*corev1.Secret, e return nil, errors.Wrap(err, "fetching image pull secret") } - // Save in cache - s.mu.Lock() - s.cache[name] = obj - s.mu.Unlock() + if s.ttl > 0 { + // Save in cache + if err := s.cache.Set(ctx, name, obj, s.ttl); err != nil { + log.DefaultLogger.Warnw("error while saving secret in cache", "name", name, "error", err) + } + } if ctx.Err() != nil { return nil, ctx.Err() } return obj, nil } + +func (s *secretFetcher) getFromCache(ctx context.Context, name string) (*corev1.Secret, error) { + cached, err := s.cache.Get(ctx, name) + if err != nil { + if cache.IsCacheMiss(err) { + return nil, nil + } + log.DefaultLogger.Warnw("error while getting secret from cache", "name", name, "error", err) + return nil, err + } + + return cached, nil +} diff --git a/pkg/imageinspector/secretfetcher_test.go b/pkg/imageinspector/secretfetcher_test.go index e60e7cd5ee6..b9828d45ac1 100644 --- a/pkg/imageinspector/secretfetcher_test.go +++ b/pkg/imageinspector/secretfetcher_test.go @@ -3,6 +3,9 @@ package imageinspector import ( "context" "testing" + "time" + + "github.com/kubeshop/testkube/pkg/cache" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -16,7 +19,7 @@ import ( func TestSecretFetcherGetExisting(t *testing.T) { ctrl := gomock.NewController(t) client := secret.NewMockInterface(ctrl) - fetcher := NewSecretFetcher(client) + fetcher := NewSecretFetcher(client, cache.NewInMemoryCache[*corev1.Secret]()) expected := corev1.Secret{ StringData: map[string]string{"key": "value"}, @@ -31,7 +34,7 @@ func TestSecretFetcherGetExisting(t *testing.T) { func TestSecretFetcherGetCache(t *testing.T) { ctrl := gomock.NewController(t) client := secret.NewMockInterface(ctrl) - fetcher := NewSecretFetcher(client) + fetcher := NewSecretFetcher(client, cache.NewInMemoryCache[*corev1.Secret](), WithSecretCacheTTL(1*time.Minute)) expected := corev1.Secret{ StringData: map[string]string{"key": "value"}, @@ -46,10 +49,25 @@ func TestSecretFetcherGetCache(t *testing.T) { assert.Equal(t, &expected, result2) } +func TestSecretFetcherGetDisabledCache(t *testing.T) { + ctrl := gomock.NewController(t) + client := secret.NewMockInterface(ctrl) + fetcher := NewSecretFetcher(client, newNoCache(t), WithSecretCacheTTL(0)) + + expected := corev1.Secret{ + StringData: map[string]string{"key": "value"}, + } + client.EXPECT().GetObject("dummy").Return(&expected, nil) + + result1, err1 := fetcher.Get(context.Background(), "dummy") + assert.NoError(t, err1) + assert.Equal(t, &expected, result1) +} + func TestSecretFetcherGetError(t *testing.T) { ctrl := gomock.NewController(t) client := secret.NewMockInterface(ctrl) - fetcher := NewSecretFetcher(client) + fetcher := NewSecretFetcher(client, cache.NewInMemoryCache[*corev1.Secret]()) client.EXPECT().GetObject("dummy").Return(nil, k8serrors.NewNotFound(schema.GroupResource{}, "dummy")) client.EXPECT().GetObject("dummy").Return(nil, k8serrors.NewNotFound(schema.GroupResource{}, "dummy")) @@ -64,3 +82,21 @@ func TestSecretFetcherGetError(t *testing.T) { assert.Equal(t, noSecret, result1) assert.Equal(t, noSecret, result2) } + +type noCache struct { + t *testing.T +} + +func newNoCache(t *testing.T) *noCache { + return &noCache{t: t} +} + +func (n *noCache) Set(ctx context.Context, key string, value *corev1.Secret, ttl time.Duration) error { + n.t.Fatalf("set method should not be invoked when cache is disabled") + return nil +} + +func (n *noCache) Get(ctx context.Context, key string) (*corev1.Secret, error) { + n.t.Fatalf("get method should not be invoked when cache is disabled") + return nil, nil +} diff --git a/pkg/testworkflows/testworkflowexecutor/executor.go b/pkg/testworkflows/testworkflowexecutor/executor.go index 46e89e5e536..b30f359c6d6 100644 --- a/pkg/testworkflows/testworkflowexecutor/executor.go +++ b/pkg/testworkflows/testworkflowexecutor/executor.go @@ -429,6 +429,7 @@ func (e *executor) Execute(ctx context.Context, workflow testworkflowsv1.TestWor "images.toolkit": constants.DefaultToolkitImage, "images.persistence.enabled": strconv.FormatBool(e.enableImageDataPersistentCache), "images.persistence.key": e.imageDataPersistentCacheKey, + "images.cache.ttl": common.GetOr(os.Getenv("TESTKUBE_IMAGE_CREDENTIALS_CACHE_TTL"), "30m"), }). Register("workflow", map[string]string{ "name": workflow.Name, diff --git a/pkg/testworkflows/testworkflowprocessor/container.go b/pkg/testworkflows/testworkflowprocessor/container.go index fba2a2d4950..0b2b7b95370 100644 --- a/pkg/testworkflows/testworkflowprocessor/container.go +++ b/pkg/testworkflows/testworkflowprocessor/container.go @@ -458,6 +458,7 @@ func (c *container) EnableToolkit(ref string) Container { "TESTKUBE_TW_INIT_IMAGE": "{{internal.images.init}}", "TK_IMG_P": "{{internal.images.persistence.enabled}}", "TK_IMG_PK": "{{internal.images.persistence.key}}", + "TK_IMG_CRED_TTL": "{{internal.images.cache.ttl}}", }) }