From ffaee322910fdd9300f8c9169e4a81ce73d116fd Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Thu, 16 Mar 2023 15:45:02 -0400 Subject: [PATCH 1/7] Allow bin/test_unit to run single test case --- bin/test_unit | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bin/test_unit b/bin/test_unit index 704f01b4..809e9509 100755 --- a/bin/test_unit +++ b/bin/test_unit @@ -5,6 +5,8 @@ set -eox pipefail junit_output_file="./junit.output" . bin/build_utils +test_case="${1-}" + function main() { retrieve_cyberark_ca_cert build_docker_ut_image @@ -18,12 +20,19 @@ function build_docker_ut_image() { function run_unit_tests() { echo "Running unit tests..." + + test_case_cmd="" + if [[ "$test_case" != "" ]]; then + test_case_cmd="--run $test_case" + fi + docker run --rm -t \ --volume "$PWD"/:/secrets-provider-for-k8s/test/ \ secrets-provider-for-k8s-test-runner:latest \ -coverprofile="./test/c.out" \ ./cmd/... \ ./pkg/... \ + $test_case_cmd \ | tee -a "$junit_output_file" echo "Unit test exit status: $?" } From 331ba00441e7b2a235872b4d1794103b740de903 Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 29 Mar 2023 20:49:32 -0400 Subject: [PATCH 2/7] Move cmd dir contents into entrypoint package --- cmd/secrets-provider/main.go | 260 +---------------- pkg/entrypoint/entrypoint.go | 265 ++++++++++++++++++ .../entrypoint}/trace.go | 2 +- 3 files changed, 268 insertions(+), 259 deletions(-) create mode 100644 pkg/entrypoint/entrypoint.go rename {cmd/secrets-provider => pkg/entrypoint}/trace.go (99%) diff --git a/cmd/secrets-provider/main.go b/cmd/secrets-provider/main.go index 4a1c76d0..b29d138b 100644 --- a/cmd/secrets-provider/main.go +++ b/cmd/secrets-provider/main.go @@ -1,265 +1,9 @@ package main import ( - "context" - "errors" - "fmt" - "io/ioutil" - "os" - "time" - - authnConfigProvider "github.com/cyberark/conjur-authn-k8s-client/pkg/authenticator/config" - "github.com/cyberark/conjur-authn-k8s-client/pkg/log" - "github.com/cyberark/conjur-opentelemetry-tracer/pkg/trace" - "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" - "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets" - "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/annotations" - "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/clients/conjur" - secretsConfigProvider "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/config" - k8sSecretsStorage "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/k8s_secrets_storage" - "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/pushtofile" - "go.opentelemetry.io/otel/attribute" -) - -const ( - defaultContainerMode = "init" - annotationsFilePath = "/conjur/podinfo/annotations" - secretsBasePath = "/conjur/secrets" - templatesBasePath = "/conjur/templates" - tracerName = "secrets-provider" - tracerService = "secrets-provider" - tracerEnvironment = "production" - tracerID = 1 + "github.com/cyberark/secrets-provider-for-k8s/pkg/entrypoint" ) -var annotationsMap map[string]string - -var envAnnotationsConversion = map[string]string{ - "CONJUR_AUTHN_LOGIN": "conjur.org/authn-identity", - "CONTAINER_MODE": "conjur.org/container-mode", - "SECRETS_DESTINATION": "conjur.org/secrets-destination", - "K8S_SECRETS": "conjur.org/k8s-secrets", - "RETRY_COUNT_LIMIT": "conjur.org/retry-count-limit", - "RETRY_INTERVAL_SEC": "conjur.org/retry-interval-sec", - "DEBUG": "conjur.org/debug-logging", - "JAEGER_COLLECTOR_URL": "conjur.org/jaeger-collector-url", - "LOG_TRACES": "conjur.org/log-traces", - "JWT_TOKEN_PATH": "conjur.org/jwt-token-path", - "REMOVE_DELETED_SECRETS": "conjur.org/remove-deleted-secrets-enabled", -} - func main() { - // os.Exit() does not call deferred functions, so defer exit until after - // all other deferred functions have been called. - exitCode := 0 - defer func() { os.Exit(exitCode) }() - - logError := func(errStr string) { - log.Error(errStr) - exitCode = 1 - } - - log.Info(messages.CSPFK008I, secrets.FullVersionName) - - // Create a TracerProvider, Tracer, and top-level (parent) Span - tracerType, tracerURL := getTracerConfig() - ctx, tracer, deferFunc, err := createTracer(tracerType, tracerURL) - defer deferFunc(ctx) - if err != nil { - logError(err.Error()) - return - } - - // Process Pod Annotations - if err := processAnnotations(ctx, tracer); err != nil { - logError(err.Error()) - return - } - - // Gather K8s authenticator config and create a Conjur secret retriever - secretRetriever, err := secretRetriever(ctx, tracer) - if err != nil { - logError(err.Error()) - return - } - - // Gather secrets config and create a repeatable Secrets Provider - provideSecrets, _, err := repeatableSecretsProvider(ctx, tracer, secretRetriever) - if err != nil { - logError(err.Error()) - return - } - - // Provide secrets - if err = provideSecrets(); err != nil { - logError(err.Error()) - } -} - -func processAnnotations(ctx context.Context, tracer trace.Tracer) error { - // Only attempt to populate from annotations if the annotations file exists - // TODO: Figure out strategy for dealing with explicit annotation file path - // set by user. In that case we can't just ignore that the file is missing. - if _, err := os.Stat(annotationsFilePath); err == nil { - _, span := tracer.Start(ctx, "Process Annotations") - defer span.End() - annotationsMap, err = annotations.NewAnnotationsFromFile(annotationsFilePath) - if err != nil { - log.Error(err.Error()) - span.RecordErrorAndSetStatus(err) - return err - } - - errLogs, infoLogs := secretsConfigProvider.ValidateAnnotations(annotationsMap) - if err := logErrorsAndInfos(errLogs, infoLogs); err != nil { - log.Error(messages.CSPFK049E) - span.RecordErrorAndSetStatus(errors.New(messages.CSPFK049E)) - return err - } - } - return nil -} - -func secretRetriever(ctx context.Context, - tracer trace.Tracer) (*conjur.SecretRetriever, error) { - // Gather authenticator config - _, span := tracer.Start(ctx, "Gather authenticator config") - defer span.End() - - authnConfig, err := authnConfigProvider.NewConfigFromCustomEnv(ioutil.ReadFile, customEnv) - if err != nil { - span.RecordErrorAndSetStatus(err) - log.Error(messages.CSPFK008E) - return nil, err - } - - // Initialize a Conjur secret retriever - secretRetriever, err := conjur.NewSecretRetriever(authnConfig) - if err != nil { - log.Error(err.Error()) - return nil, err - } - return secretRetriever, nil -} - -func repeatableSecretsProvider( - ctx context.Context, - tracer trace.Tracer, - secretRetriever *conjur.SecretRetriever) (secrets.RepeatableProviderFunc, *secretsConfigProvider.Config, error) { - - _, span := tracer.Start(ctx, "Create repeatable secrets provider") - defer span.End() - - // Initialize Secrets Provider configuration - secretsConfig, err := setupSecretsConfig() - if err != nil { - log.Error(err.Error()) - span.RecordErrorAndSetStatus(err) - return nil, nil, err - } - providerConfig := &secrets.ProviderConfig{ - CommonProviderConfig: secrets.CommonProviderConfig{ - StoreType: secretsConfig.StoreType, - SanitizeEnabled: secretsConfig.SanitizeEnabled, - }, - K8sProviderConfig: k8sSecretsStorage.K8sProviderConfig{ - PodNamespace: secretsConfig.PodNamespace, - RequiredK8sSecrets: secretsConfig.RequiredK8sSecrets, - }, - P2FProviderConfig: pushtofile.P2FProviderConfig{ - SecretFileBasePath: secretsBasePath, - TemplateFileBasePath: templatesBasePath, - AnnotationsMap: annotationsMap, - }, - } - - // Tag the span with the secrets provider mode - span.SetAttributes(attribute.String("store_type", secretsConfig.StoreType)) - - // Create a secrets provider - provideSecrets, errs := secrets.NewProviderForType(ctx, - secretRetriever.Retrieve, *providerConfig) - if err := logErrorsAndInfos(errs, nil); err != nil { - log.Error(messages.CSPFK053E) - span.RecordErrorAndSetStatus(errors.New(messages.CSPFK053E)) - return nil, nil, err - } - - provideSecrets = secrets.RetryableSecretProvider( - time.Duration(secretsConfig.RetryIntervalSec)*time.Second, - secretsConfig.RetryCountLimit, - provideSecrets, - ) - - // Create a channel to send a quit signal to the periodic secret provider. - // TODO: Currently, this is just used for testing, but in the future we - // may want to create a SIGTERM or SIGHUP handler to catch a signal from - // a user / external entity, and then send an (empty struct) quit signal - // on this channel to trigger a graceful shut down of the Secrets Provider. - providerQuit := make(chan struct{}) - - refreshConfig := secrets.ProviderRefreshConfig{ - Mode: getContainerMode(), - SecretRefreshInterval: secretsConfig.SecretsRefreshInterval, - ProviderQuit: providerQuit, - } - - repeatableProvideSecrets := secrets.RepeatableSecretProvider( - refreshConfig, - provideSecrets, - ) - return repeatableProvideSecrets, secretsConfig, nil -} - -func customEnv(key string) string { - if annotation, ok := envAnnotationsConversion[key]; ok { - if value := annotationsMap[annotation]; value != "" { - log.Info(messages.CSPFK014I, key, fmt.Sprintf("annotation %s", annotation)) - return value - } - - if value := os.Getenv(key); value == "" && key == "CONTAINER_MODE" { - log.Info(messages.CSPFK014I, key, "default") - return defaultContainerMode - } - - log.Info(messages.CSPFK014I, key, "environment") - } - - return os.Getenv(key) -} - -func setupSecretsConfig() (*secretsConfigProvider.Config, error) { - secretsProviderSettings := secretsConfigProvider.GatherSecretsProviderSettings(annotationsMap) - - errLogs, infoLogs := secretsConfigProvider.ValidateSecretsProviderSettings(secretsProviderSettings) - if err := logErrorsAndInfos(errLogs, infoLogs); err != nil { - log.Error(messages.CSPFK015E) - return nil, err - } - - return secretsConfigProvider.NewConfig(secretsProviderSettings), nil -} - -func logErrorsAndInfos(errLogs []error, infoLogs []error) error { - for _, err := range infoLogs { - log.Info(err.Error()) - } - if len(errLogs) > 0 { - for _, err := range errLogs { - log.Error(err.Error()) - } - return errors.New("fatal errors occurred, check Secrets Provider logs") - } - return nil -} - -func getContainerMode() string { - containerMode := "init" - if mode, exists := annotationsMap[secretsConfigProvider.ContainerModeKey]; exists { - containerMode = mode - } else if mode = os.Getenv("CONTAINER_MODE"); mode == "sidecar" || mode == "application" { - containerMode = mode - } - return containerMode + entrypoint.StartSecretsProvider() } diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go new file mode 100644 index 00000000..bed9abcd --- /dev/null +++ b/pkg/entrypoint/entrypoint.go @@ -0,0 +1,265 @@ +package entrypoint + +import ( + "context" + "errors" + "fmt" + "io/ioutil" + "os" + "time" + + authnConfigProvider "github.com/cyberark/conjur-authn-k8s-client/pkg/authenticator/config" + "github.com/cyberark/conjur-authn-k8s-client/pkg/log" + "github.com/cyberark/conjur-opentelemetry-tracer/pkg/trace" + "github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/annotations" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/clients/conjur" + secretsConfigProvider "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/config" + k8sSecretsStorage "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/k8s_secrets_storage" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/pushtofile" + "go.opentelemetry.io/otel/attribute" +) + +const ( + defaultContainerMode = "init" + annotationsFilePath = "/conjur/podinfo/annotations" + secretsBasePath = "/conjur/secrets" + templatesBasePath = "/conjur/templates" + tracerName = "secrets-provider" + tracerService = "secrets-provider" + tracerEnvironment = "production" + tracerID = 1 +) + +var annotationsMap map[string]string + +var envAnnotationsConversion = map[string]string{ + "CONJUR_AUTHN_LOGIN": "conjur.org/authn-identity", + "CONTAINER_MODE": "conjur.org/container-mode", + "SECRETS_DESTINATION": "conjur.org/secrets-destination", + "K8S_SECRETS": "conjur.org/k8s-secrets", + "RETRY_COUNT_LIMIT": "conjur.org/retry-count-limit", + "RETRY_INTERVAL_SEC": "conjur.org/retry-interval-sec", + "DEBUG": "conjur.org/debug-logging", + "JAEGER_COLLECTOR_URL": "conjur.org/jaeger-collector-url", + "LOG_TRACES": "conjur.org/log-traces", + "JWT_TOKEN_PATH": "conjur.org/jwt-token-path", + "REMOVE_DELETED_SECRETS": "conjur.org/remove-deleted-secrets-enabled", +} + +func StartSecretsProvider() { + // os.Exit() does not call deferred functions, so defer exit until after + // all other deferred functions have been called. + exitCode := 0 + defer func() { os.Exit(exitCode) }() + + logError := func(errStr string) { + log.Error(errStr) + exitCode = 1 + } + + log.Info(messages.CSPFK008I, secrets.FullVersionName) + + // Create a TracerProvider, Tracer, and top-level (parent) Span + tracerType, tracerURL := getTracerConfig() + ctx, tracer, deferFunc, err := createTracer(tracerType, tracerURL) + defer deferFunc(ctx) + if err != nil { + logError(err.Error()) + return + } + + // Process Pod Annotations + if err := processAnnotations(ctx, tracer); err != nil { + logError(err.Error()) + return + } + + // Gather K8s authenticator config and create a Conjur secret retriever + secretRetriever, err := secretRetriever(ctx, tracer) + if err != nil { + logError(err.Error()) + return + } + + // Gather secrets config and create a repeatable Secrets Provider + provideSecrets, _, err := repeatableSecretsProvider(ctx, tracer, secretRetriever) + if err != nil { + logError(err.Error()) + return + } + + // Provide secrets + if err = provideSecrets(); err != nil { + logError(err.Error()) + } +} + +func processAnnotations(ctx context.Context, tracer trace.Tracer) error { + // Only attempt to populate from annotations if the annotations file exists + // TODO: Figure out strategy for dealing with explicit annotation file path + // set by user. In that case we can't just ignore that the file is missing. + if _, err := os.Stat(annotationsFilePath); err == nil { + _, span := tracer.Start(ctx, "Process Annotations") + defer span.End() + annotationsMap, err = annotations.NewAnnotationsFromFile(annotationsFilePath) + if err != nil { + log.Error(err.Error()) + span.RecordErrorAndSetStatus(err) + return err + } + + errLogs, infoLogs := secretsConfigProvider.ValidateAnnotations(annotationsMap) + if err := logErrorsAndInfos(errLogs, infoLogs); err != nil { + log.Error(messages.CSPFK049E) + span.RecordErrorAndSetStatus(errors.New(messages.CSPFK049E)) + return err + } + } + return nil +} + +func secretRetriever(ctx context.Context, + tracer trace.Tracer) (*conjur.SecretRetriever, error) { + // Gather authenticator config + _, span := tracer.Start(ctx, "Gather authenticator config") + defer span.End() + + authnConfig, err := authnConfigProvider.NewConfigFromCustomEnv(ioutil.ReadFile, customEnv) + if err != nil { + span.RecordErrorAndSetStatus(err) + log.Error(messages.CSPFK008E) + return nil, err + } + + // Initialize a Conjur secret retriever + secretRetriever, err := conjur.NewSecretRetriever(authnConfig) + if err != nil { + log.Error(err.Error()) + return nil, err + } + return secretRetriever, nil +} + +func repeatableSecretsProvider( + ctx context.Context, + tracer trace.Tracer, + secretRetriever *conjur.SecretRetriever) (secrets.RepeatableProviderFunc, *secretsConfigProvider.Config, error) { + + _, span := tracer.Start(ctx, "Create repeatable secrets provider") + defer span.End() + + // Initialize Secrets Provider configuration + secretsConfig, err := setupSecretsConfig() + if err != nil { + log.Error(err.Error()) + span.RecordErrorAndSetStatus(err) + return nil, nil, err + } + providerConfig := &secrets.ProviderConfig{ + CommonProviderConfig: secrets.CommonProviderConfig{ + StoreType: secretsConfig.StoreType, + SanitizeEnabled: secretsConfig.SanitizeEnabled, + }, + K8sProviderConfig: k8sSecretsStorage.K8sProviderConfig{ + PodNamespace: secretsConfig.PodNamespace, + RequiredK8sSecrets: secretsConfig.RequiredK8sSecrets, + }, + P2FProviderConfig: pushtofile.P2FProviderConfig{ + SecretFileBasePath: secretsBasePath, + TemplateFileBasePath: templatesBasePath, + AnnotationsMap: annotationsMap, + }, + } + + // Tag the span with the secrets provider mode + span.SetAttributes(attribute.String("store_type", secretsConfig.StoreType)) + + // Create a secrets provider + provideSecrets, errs := secrets.NewProviderForType(ctx, + secretRetriever.Retrieve, *providerConfig) + if err := logErrorsAndInfos(errs, nil); err != nil { + log.Error(messages.CSPFK053E) + span.RecordErrorAndSetStatus(errors.New(messages.CSPFK053E)) + return nil, nil, err + } + + provideSecrets = secrets.RetryableSecretProvider( + time.Duration(secretsConfig.RetryIntervalSec)*time.Second, + secretsConfig.RetryCountLimit, + provideSecrets, + ) + + // Create a channel to send a quit signal to the periodic secret provider. + // TODO: Currently, this is just used for testing, but in the future we + // may want to create a SIGTERM or SIGHUP handler to catch a signal from + // a user / external entity, and then send an (empty struct) quit signal + // on this channel to trigger a graceful shut down of the Secrets Provider. + providerQuit := make(chan struct{}) + + refreshConfig := secrets.ProviderRefreshConfig{ + Mode: getContainerMode(), + SecretRefreshInterval: secretsConfig.SecretsRefreshInterval, + ProviderQuit: providerQuit, + } + + repeatableProvideSecrets := secrets.RepeatableSecretProvider( + refreshConfig, + provideSecrets, + ) + return repeatableProvideSecrets, secretsConfig, nil +} + +func customEnv(key string) string { + if annotation, ok := envAnnotationsConversion[key]; ok { + if value := annotationsMap[annotation]; value != "" { + log.Info(messages.CSPFK014I, key, fmt.Sprintf("annotation %s", annotation)) + return value + } + + if value := os.Getenv(key); value == "" && key == "CONTAINER_MODE" { + log.Info(messages.CSPFK014I, key, "default") + return defaultContainerMode + } + + log.Info(messages.CSPFK014I, key, "environment") + } + + return os.Getenv(key) +} + +func setupSecretsConfig() (*secretsConfigProvider.Config, error) { + secretsProviderSettings := secretsConfigProvider.GatherSecretsProviderSettings(annotationsMap) + + errLogs, infoLogs := secretsConfigProvider.ValidateSecretsProviderSettings(secretsProviderSettings) + if err := logErrorsAndInfos(errLogs, infoLogs); err != nil { + log.Error(messages.CSPFK015E) + return nil, err + } + + return secretsConfigProvider.NewConfig(secretsProviderSettings), nil +} + +func logErrorsAndInfos(errLogs []error, infoLogs []error) error { + for _, err := range infoLogs { + log.Info(err.Error()) + } + if len(errLogs) > 0 { + for _, err := range errLogs { + log.Error(err.Error()) + } + return errors.New("fatal errors occurred, check Secrets Provider logs") + } + return nil +} + +func getContainerMode() string { + containerMode := "init" + if mode, exists := annotationsMap[secretsConfigProvider.ContainerModeKey]; exists { + containerMode = mode + } else if mode = os.Getenv("CONTAINER_MODE"); mode == "sidecar" || mode == "application" { + containerMode = mode + } + return containerMode +} diff --git a/cmd/secrets-provider/trace.go b/pkg/entrypoint/trace.go similarity index 99% rename from cmd/secrets-provider/trace.go rename to pkg/entrypoint/trace.go index 89acdcf4..7398e7c2 100644 --- a/cmd/secrets-provider/trace.go +++ b/pkg/entrypoint/trace.go @@ -1,4 +1,4 @@ -package main +package entrypoint import ( "context" From bf44153cfa08ff0c3c06f2632ead51b3b1cd7fe3 Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 29 Mar 2023 21:11:25 -0400 Subject: [PATCH 3/7] Inject dependencies into entrypoint with factories --- pkg/entrypoint/entrypoint.go | 35 +++++++--- .../conjur/conjur_secrets_retriever.go | 14 ++-- pkg/secrets/provide_conjur_secrets.go | 28 ++++---- pkg/secrets/provide_conjur_secrets_test.go | 2 +- pkg/secrets/provider_status.go | 65 +++++++++++-------- pkg/secrets/provider_status_test.go | 4 +- 6 files changed, 90 insertions(+), 58 deletions(-) diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index bed9abcd..9a6d8954 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -49,6 +49,18 @@ var envAnnotationsConversion = map[string]string{ } func StartSecretsProvider() { + startSecretsProviderWithDeps( + conjur.NewSecretRetriever, + secrets.NewProviderForType, + secrets.DefaultStatusUpdater, + ) +} + +func startSecretsProviderWithDeps( + retrieverFactory conjur.RetrieverFactory, + providerFactory secrets.ProviderFactory, + statusUpdaterFactory secrets.StatusUpdaterFactory, +) { // os.Exit() does not call deferred functions, so defer exit until after // all other deferred functions have been called. exitCode := 0 @@ -77,14 +89,14 @@ func StartSecretsProvider() { } // Gather K8s authenticator config and create a Conjur secret retriever - secretRetriever, err := secretRetriever(ctx, tracer) + secretRetriever, err := secretRetriever(ctx, tracer, retrieverFactory) if err != nil { logError(err.Error()) return } // Gather secrets config and create a repeatable Secrets Provider - provideSecrets, _, err := repeatableSecretsProvider(ctx, tracer, secretRetriever) + provideSecrets, _, err := repeatableSecretsProvider(ctx, tracer, secretRetriever, providerFactory, statusUpdaterFactory) if err != nil { logError(err.Error()) return @@ -120,8 +132,11 @@ func processAnnotations(ctx context.Context, tracer trace.Tracer) error { return nil } -func secretRetriever(ctx context.Context, - tracer trace.Tracer) (*conjur.SecretRetriever, error) { +func secretRetriever( + ctx context.Context, + tracer trace.Tracer, + retrieverFactory conjur.RetrieverFactory, +) (conjur.RetrieveSecretsFunc, error) { // Gather authenticator config _, span := tracer.Start(ctx, "Gather authenticator config") defer span.End() @@ -134,7 +149,7 @@ func secretRetriever(ctx context.Context, } // Initialize a Conjur secret retriever - secretRetriever, err := conjur.NewSecretRetriever(authnConfig) + secretRetriever, err := retrieverFactory(authnConfig) if err != nil { log.Error(err.Error()) return nil, err @@ -145,7 +160,10 @@ func secretRetriever(ctx context.Context, func repeatableSecretsProvider( ctx context.Context, tracer trace.Tracer, - secretRetriever *conjur.SecretRetriever) (secrets.RepeatableProviderFunc, *secretsConfigProvider.Config, error) { + secretRetriever conjur.RetrieveSecretsFunc, + providerFactory secrets.ProviderFactory, + statusUpdaterFactory secrets.StatusUpdaterFactory, +) (secrets.RepeatableProviderFunc, *secretsConfigProvider.Config, error) { _, span := tracer.Start(ctx, "Create repeatable secrets provider") defer span.End() @@ -177,8 +195,8 @@ func repeatableSecretsProvider( span.SetAttributes(attribute.String("store_type", secretsConfig.StoreType)) // Create a secrets provider - provideSecrets, errs := secrets.NewProviderForType(ctx, - secretRetriever.Retrieve, *providerConfig) + provideSecrets, errs := providerFactory(ctx, + secretRetriever, *providerConfig) if err := logErrorsAndInfos(errs, nil); err != nil { log.Error(messages.CSPFK053E) span.RecordErrorAndSetStatus(errors.New(messages.CSPFK053E)) @@ -207,6 +225,7 @@ func repeatableSecretsProvider( repeatableProvideSecrets := secrets.RepeatableSecretProvider( refreshConfig, provideSecrets, + statusUpdaterFactory(), ) return repeatableProvideSecrets, secretsConfig, nil } diff --git a/pkg/secrets/clients/conjur/conjur_secrets_retriever.go b/pkg/secrets/clients/conjur/conjur_secrets_retriever.go index 3ffeca3a..02c403f8 100644 --- a/pkg/secrets/clients/conjur/conjur_secrets_retriever.go +++ b/pkg/secrets/clients/conjur/conjur_secrets_retriever.go @@ -19,16 +19,20 @@ import ( // SecretRetriever implements a Retrieve function that is capable of // authenticating with Conjur and retrieving multiple Conjur variables // in bulk. -type SecretRetriever struct { +type secretRetriever struct { authn authenticator.Authenticator } // RetrieveSecretsFunc defines a function type for retrieving secrets. type RetrieveSecretsFunc func(variableIDs []string, traceContext context.Context) (map[string][]byte, error) +// RetrieverFactory defines a function type for creating a RetrieveSecretsFunc +// implementation given an authenticator config. +type RetrieverFactory func(authnConfig config.Configuration) (RetrieveSecretsFunc, error) + // NewSecretRetriever creates a new SecretRetriever and Authenticator // given an authenticator config. -func NewSecretRetriever(authnConfig config.Configuration) (*SecretRetriever, error) { +func NewSecretRetriever(authnConfig config.Configuration) (RetrieveSecretsFunc, error) { accessToken, err := memory.NewAccessToken() if err != nil { return nil, fmt.Errorf("%s", messages.CSPFK001E) @@ -39,14 +43,14 @@ func NewSecretRetriever(authnConfig config.Configuration) (*SecretRetriever, err return nil, fmt.Errorf("%s", messages.CSPFK009E) } - return &SecretRetriever{ + return secretRetriever{ authn: authn, - }, nil + }.Retrieve, nil } // Retrieve implements a RetrieveSecretsFunc for a given SecretRetriever. // Authenticates the client, and retrieves a given batch of variables from Conjur. -func (retriever SecretRetriever) Retrieve(variableIDs []string, traceContext context.Context) (map[string][]byte, error) { +func (retriever secretRetriever) Retrieve(variableIDs []string, traceContext context.Context) (map[string][]byte, error) { authn := retriever.authn diff --git a/pkg/secrets/provide_conjur_secrets.go b/pkg/secrets/provide_conjur_secrets.go index d2891ba7..d499a7bf 100644 --- a/pkg/secrets/provide_conjur_secrets.go +++ b/pkg/secrets/provide_conjur_secrets.go @@ -43,6 +43,10 @@ type ProviderFunc func() (updated bool, err error) // indefinitely while providing secrets to unspecified targets. type RepeatableProviderFunc func() error +// ProviderFactory defines a function type for creating a ProviderFunc given a +// RetrieveSecretsFunc and ProviderConfig. +type ProviderFactory func(traceContent context.Context, secretsRetrieverFunc conjur.RetrieveSecretsFunc, providerConfig ProviderConfig) (ProviderFunc, []error) + // NewProviderForType returns a ProviderFunc responsible for providing secrets in a given mode. func NewProviderForType( traceContext context.Context, @@ -118,21 +122,13 @@ type ProviderRefreshConfig struct { // RepeatableSecretProvider returns a new ProviderFunc, which wraps a retryable // ProviderFunc inside a function that operates in one of three modes: -// - Run once and return (for init or application container modes) -// - Run once and sleep forever (for sidecar mode without periodic refresh) -// - Run periodically (for sidecar mode with periodic refresh) +// - Run once and return (for init or application container modes) +// - Run once and sleep forever (for sidecar mode without periodic refresh) +// - Run periodically (for sidecar mode with periodic refresh) func RepeatableSecretProvider( - refreshConfig ProviderRefreshConfig, - provideSecrets ProviderFunc, -) RepeatableProviderFunc { - return repeatableSecretProvider(refreshConfig, provideSecrets, - defaultStatusUpdater) -} - -func repeatableSecretProvider( config ProviderRefreshConfig, provideSecrets ProviderFunc, - status statusUpdater, + status StatusUpdater, ) RepeatableProviderFunc { var periodicQuit = make(chan struct{}) @@ -141,14 +137,14 @@ func repeatableSecretProvider( var err error return func() error { - if err = status.copyScripts(); err != nil { + if err = status.CopyScripts(); err != nil { return err } if _, err = provideSecrets(); err != nil { // Return immediately upon error, regardless of operating mode return err } - err = status.setSecretsProvided() + err = status.SetSecretsProvided() if err != nil { return err } @@ -200,7 +196,7 @@ type periodicConfig struct { func periodicSecretProvider( provideSecrets ProviderFunc, config periodicConfig, - status statusUpdater, + status StatusUpdater, ) { for { select { @@ -209,7 +205,7 @@ func periodicSecretProvider( case <-config.ticker.C: updated, err := provideSecrets() if err == nil && updated { - err = status.setSecretsUpdated() + err = status.SetSecretsUpdated() } if err != nil { config.periodicError <- err diff --git a/pkg/secrets/provide_conjur_secrets_test.go b/pkg/secrets/provide_conjur_secrets_test.go index 4f22b9ad..3ff8bb67 100644 --- a/pkg/secrets/provide_conjur_secrets_test.go +++ b/pkg/secrets/provide_conjur_secrets_test.go @@ -298,7 +298,7 @@ func TestRepeatableSecretProvider(t *testing.T) { SecretRefreshInterval: tc.interval, ProviderQuit: providerQuit, } - provideSecrets := repeatableSecretProvider( + provideSecrets := RepeatableSecretProvider( refreshConfig, tc.provider.provide, fileUpdater) // Run the secrets provider diff --git a/pkg/secrets/provider_status.go b/pkg/secrets/provider_status.go index 5505e408..4cf4a1a0 100644 --- a/pkg/secrets/provider_status.go +++ b/pkg/secrets/provider_status.go @@ -12,36 +12,40 @@ const ( scriptFileMode = 0755 ) -// statusUpdater defines an interface for recording a secret provider's +// StatusUpdater defines an interface for recording a secret provider's // status, and for copying utility scripts for checking that recorded status. // -// setSecretsProvided: A function that records that the secrets provider -// has finished providing secrets (at least for its -// initial iteration). -// setSecretsUpdated: A function that records that the secrets provider -// has just updated the secret files or Kubernetes Secrets -// with recently updated secret values retrieved from -// Conjur. -// copyScripts: Copy utility scripts for checking provider status from -// a "baked-in" container directory into a volume that is -// potentially shared with application container(s). -type statusUpdater interface { - setSecretsProvided() error - setSecretsUpdated() error - copyScripts() error +// SetSecretsProvided: A function that records that the secrets provider +// +// has finished providing secrets (at least for its +// initial iteration). +// +// SetSecretsUpdated: A function that records that the secrets provider +// +// has just updated the secret files or Kubernetes Secrets +// with recently updated secret values retrieved from +// Conjur. +// +// CopyScripts: Copy utility scripts for checking provider status from +// +// a "baked-in" container directory into a volume that is +// potentially shared with application container(s). +type StatusUpdater interface { + SetSecretsProvided() error + SetSecretsUpdated() error + CopyScripts() error } -type chmodFunc func(string, os.FileMode) error -type createFunc func(string) (*os.File, error) -type openFunc func(string) (*os.File, error) +type chmodFunc func(string, os.FileMode) error +type createFunc func(string) (*os.File, error) +type openFunc func(string) (*os.File, error) type mkdirAllFunc func(string, os.FileMode) error - type osFuncs struct { - chmod chmodFunc - create createFunc - open openFunc - mkdirAll mkdirAllFunc + chmod chmodFunc + create createFunc + open openFunc + mkdirAll mkdirAllFunc } var stdOSFuncs = osFuncs{ @@ -51,6 +55,15 @@ var stdOSFuncs = osFuncs{ mkdirAll: os.MkdirAll, } +// StatusUpdaterFactory defines a function type for creating a StatusUpdater +// implementation. +type StatusUpdaterFactory func() StatusUpdater + +// DefaultStatusUpdater returns the default StatusUpdater. +func DefaultStatusUpdater() StatusUpdater { + return defaultStatusUpdater +} + // fileUpdater implements the statusUpdater interface. It records provider // status by creating empty sentinel files. type fileUpdater struct { @@ -80,15 +93,15 @@ func (f fileUpdater) setStatus(path string) error { return f.os.chmod(file.Name(), statusFileMode) } -func (f fileUpdater) setSecretsProvided() error { +func (f fileUpdater) SetSecretsProvided() error { return f.setStatus(f.providedFile) } -func (f fileUpdater) setSecretsUpdated() error { +func (f fileUpdater) SetSecretsUpdated() error { return f.setStatus(f.updatedFile) } -func (f fileUpdater) copyScripts() error { +func (f fileUpdater) CopyScripts() error { // Create the directory err := f.os.mkdirAll(f.scriptDestDir, os.ModePerm) diff --git a/pkg/secrets/provider_status_test.go b/pkg/secrets/provider_status_test.go index 84a424d7..dfa5d05c 100644 --- a/pkg/secrets/provider_status_test.go +++ b/pkg/secrets/provider_status_test.go @@ -239,10 +239,10 @@ func TestCopyScripts(t *testing.T) { // Run test fileUpdater := updater.fileUpdater - err = fileUpdater.copyScripts() + err = fileUpdater.CopyScripts() if tc.runTwice { assert.NoError(t, err) - err = fileUpdater.copyScripts() + err = fileUpdater.CopyScripts() } // Check results From b66c6eeb41ae0012c2ce8aebee1515ca81587305 Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 29 Mar 2023 21:43:42 -0400 Subject: [PATCH 4/7] Inject filepaths into entrypoint --- pkg/entrypoint/entrypoint.go | 40 +++++++++++++++++++++++++----------- pkg/entrypoint/trace.go | 6 +++--- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index 9a6d8954..5a14c108 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -22,14 +22,14 @@ import ( ) const ( - defaultContainerMode = "init" - annotationsFilePath = "/conjur/podinfo/annotations" - secretsBasePath = "/conjur/secrets" - templatesBasePath = "/conjur/templates" - tracerName = "secrets-provider" - tracerService = "secrets-provider" - tracerEnvironment = "production" - tracerID = 1 + defaultContainerMode = "init" + defaultAnnotationsFilePath = "/conjur/podinfo/annotations" + defaultSecretsBasePath = "/conjur/secrets" + defaultTemplatesBasePath = "/conjur/templates" + tracerName = "secrets-provider" + tracerService = "secrets-provider" + tracerEnvironment = "production" + tracerID = 1 ) var annotationsMap map[string]string @@ -50,6 +50,9 @@ var envAnnotationsConversion = map[string]string{ func StartSecretsProvider() { startSecretsProviderWithDeps( + defaultAnnotationsFilePath, + defaultSecretsBasePath, + defaultTemplatesBasePath, conjur.NewSecretRetriever, secrets.NewProviderForType, secrets.DefaultStatusUpdater, @@ -57,6 +60,9 @@ func StartSecretsProvider() { } func startSecretsProviderWithDeps( + annotationsFilePath string, + secretsBasePath string, + templatesBasePath string, retrieverFactory conjur.RetrieverFactory, providerFactory secrets.ProviderFactory, statusUpdaterFactory secrets.StatusUpdaterFactory, @@ -74,7 +80,7 @@ func startSecretsProviderWithDeps( log.Info(messages.CSPFK008I, secrets.FullVersionName) // Create a TracerProvider, Tracer, and top-level (parent) Span - tracerType, tracerURL := getTracerConfig() + tracerType, tracerURL := getTracerConfig(annotationsFilePath) ctx, tracer, deferFunc, err := createTracer(tracerType, tracerURL) defer deferFunc(ctx) if err != nil { @@ -83,7 +89,7 @@ func startSecretsProviderWithDeps( } // Process Pod Annotations - if err := processAnnotations(ctx, tracer); err != nil { + if err := processAnnotations(ctx, tracer, annotationsFilePath); err != nil { logError(err.Error()) return } @@ -96,7 +102,15 @@ func startSecretsProviderWithDeps( } // Gather secrets config and create a repeatable Secrets Provider - provideSecrets, _, err := repeatableSecretsProvider(ctx, tracer, secretRetriever, providerFactory, statusUpdaterFactory) + provideSecrets, _, err := repeatableSecretsProvider( + ctx, + tracer, + secretsBasePath, + templatesBasePath, + secretRetriever, + providerFactory, + statusUpdaterFactory, + ) if err != nil { logError(err.Error()) return @@ -108,7 +122,7 @@ func startSecretsProviderWithDeps( } } -func processAnnotations(ctx context.Context, tracer trace.Tracer) error { +func processAnnotations(ctx context.Context, tracer trace.Tracer, annotationsFilePath string) error { // Only attempt to populate from annotations if the annotations file exists // TODO: Figure out strategy for dealing with explicit annotation file path // set by user. In that case we can't just ignore that the file is missing. @@ -160,6 +174,8 @@ func secretRetriever( func repeatableSecretsProvider( ctx context.Context, tracer trace.Tracer, + secretsBasePath string, + templatesBasePath string, secretRetriever conjur.RetrieveSecretsFunc, providerFactory secrets.ProviderFactory, statusUpdaterFactory secrets.StatusUpdaterFactory, diff --git a/pkg/entrypoint/trace.go b/pkg/entrypoint/trace.go index 7398e7c2..f74396cb 100644 --- a/pkg/entrypoint/trace.go +++ b/pkg/entrypoint/trace.go @@ -9,10 +9,10 @@ import ( "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/annotations" ) -func getTracerConfig() (trace.TracerProviderType, string) { +func getTracerConfig(annotationsFilePath string) (trace.TracerProviderType, string) { // First try to get the tracer config from annotations log.Debug("Getting tracer config from annotations") - traceType, jaegerUrl, err := getTracerConfigFromAnnotations() + traceType, jaegerUrl, err := getTracerConfigFromAnnotations(annotationsFilePath) // If no tracer is specified in annotations, get it from environment variables if err != nil || traceType == trace.NoopProviderType { @@ -35,7 +35,7 @@ func getTracerConfigFromEnv() (trace.TracerProviderType, string) { return trace.NoopProviderType, "" } -func getTracerConfigFromAnnotations() (trace.TracerProviderType, string, error) { +func getTracerConfigFromAnnotations(annotationsFilePath string) (trace.TracerProviderType, string, error) { annotationsMap, err := annotations.NewAnnotationsFromFile(annotationsFilePath) if err != nil { return trace.NoopProviderType, "", err From 19df1405fd955e26ffac2ea5e3fa6a9097b28ddf Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 29 Mar 2023 22:00:42 -0400 Subject: [PATCH 5/7] Unit test entrypoint function --- pkg/entrypoint/entrypoint.go | 11 +- pkg/entrypoint/entrypoint_test.go | 290 ++++++++++++++++++++++++++++++ 2 files changed, 295 insertions(+), 6 deletions(-) create mode 100644 pkg/entrypoint/entrypoint_test.go diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index 5a14c108..db30d41d 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -49,7 +49,7 @@ var envAnnotationsConversion = map[string]string{ } func StartSecretsProvider() { - startSecretsProviderWithDeps( + exitCode := startSecretsProviderWithDeps( defaultAnnotationsFilePath, defaultSecretsBasePath, defaultTemplatesBasePath, @@ -57,6 +57,7 @@ func StartSecretsProvider() { secrets.NewProviderForType, secrets.DefaultStatusUpdater, ) + os.Exit(exitCode) } func startSecretsProviderWithDeps( @@ -66,11 +67,8 @@ func startSecretsProviderWithDeps( retrieverFactory conjur.RetrieverFactory, providerFactory secrets.ProviderFactory, statusUpdaterFactory secrets.StatusUpdaterFactory, -) { - // os.Exit() does not call deferred functions, so defer exit until after - // all other deferred functions have been called. - exitCode := 0 - defer func() { os.Exit(exitCode) }() +) (exitCode int) { + exitCode = 0 logError := func(errStr string) { log.Error(errStr) @@ -120,6 +118,7 @@ func startSecretsProviderWithDeps( if err = provideSecrets(); err != nil { logError(err.Error()) } + return } func processAnnotations(ctx context.Context, tracer trace.Tracer, annotationsFilePath string) error { diff --git a/pkg/entrypoint/entrypoint_test.go b/pkg/entrypoint/entrypoint_test.go new file mode 100644 index 00000000..a89dabf4 --- /dev/null +++ b/pkg/entrypoint/entrypoint_test.go @@ -0,0 +1,290 @@ +package entrypoint + +import ( + "bytes" + "context" + "errors" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/cyberark/conjur-authn-k8s-client/pkg/authenticator/config" + "github.com/cyberark/conjur-authn-k8s-client/pkg/log" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets" + "github.com/cyberark/secrets-provider-for-k8s/pkg/secrets/clients/conjur" + "github.com/stretchr/testify/assert" +) + +type mockRetrieverFactory struct { + retriever conjur.RetrieveSecretsFunc + err error +} + +func (r mockRetrieverFactory) GetRetriever(a config.Configuration) (conjur.RetrieveSecretsFunc, error) { + return r.retriever, r.err +} + +type mockRetriever struct { + data map[string][]byte + err error +} + +func (r mockRetriever) Retrieve(ids []string, c context.Context) (map[string][]byte, error) { + return r.data, r.err +} + +type mockProviderFactory struct { + providerFunc secrets.ProviderFunc + errs []error +} + +func (p mockProviderFactory) GetProvider(traceContext context.Context, secretsRetrieverFunc conjur.RetrieveSecretsFunc, providerConfig secrets.ProviderConfig) (secrets.ProviderFunc, []error) { + return p.providerFunc, p.errs +} + +func getMockStatusUpdater() secrets.StatusUpdater { + return mockStatusUpdater{} +} + +type mockStatusUpdater struct{} + +func (s mockStatusUpdater) SetSecretsProvided() error { + return nil +} + +func (s mockStatusUpdater) SetSecretsUpdated() error { + return nil +} + +func (s mockStatusUpdater) CopyScripts() error { + return nil +} + +type editableMap map[string]string + +func (m editableMap) Delete(key string) editableMap { + delete(m, key) + return m +} + +func (m editableMap) Edit(key string, value string) editableMap { + m[key] = value + return m +} + +func (m editableMap) Copy() editableMap { + mCopy := editableMap{} + for k, v := range m { + mCopy[k] = v + } + return mCopy +} + +func TestStartSecretsProvider(t *testing.T) { + tmpDir, _ := ioutil.TempDir("", "entrypoint_test") + defer os.RemoveAll(tmpDir) + + env := map[string]string{ + "MY_POD_NAME": "podname", + "MY_POD_NAMESPACE": "podnamespace", + "CONJUR_ACCOUNT": "default", + "CONJUR_APPLIANCE_URL": "https://conjur.myorg.com", + "CONJUR_AUTHN_URL": "https://conjur.myorg.com/authn-k8s/authn-service", + "CONJUR_SSL_CERTIFICATE": "cert-data", + "CONJUR_AUTHENTICATOR_ID": "authn-service", + } + annots := editableMap{ + "conjur.org/authn-identity": "host/alice", + "conjur.org/container-mode": "init", + "conjur.org/secrets-destination": "file", + "conjur.org/conjur-secrets.groupA": `- alias: path/to/secret\n`, + } + annotationsFilePath := filepath.Join(tmpDir, "annotations") + secretsBasePath := filepath.Join(tmpDir, "secrets") + templatesBasePath := filepath.Join(tmpDir, "templates") + + TestCases := []struct { + description string + environment map[string]string + annotations map[string]string + retrieverFactory mockRetrieverFactory + providerFactory mockProviderFactory + assertions func(*testing.T, int, string) + }{ + { + description: "happy path", + environment: env, + annotations: annots, + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: map[string][]byte{ + "path/to/secret": []byte("secret value"), + }, + err: nil, + }.Retrieve, + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 0, code) + }, + }, + { + description: "bad provider factory", + environment: env, + annotations: annots, + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: map[string][]byte{ + "path/to/secret": []byte("secret value"), + }, + err: nil, + }.Retrieve, + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{errors.New("provider factory failure")}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 1, code) + assert.Contains(t, logs, "CSPFK053E") + assert.Contains(t, logs, "provider factory failure") + }, + }, + { + description: "bad retriever factory", + environment: env, + annotations: annots, + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: nil, + err: nil, + }.Retrieve, + err: errors.New("retriever factory failure"), + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 1, code) + assert.Contains(t, logs, "retriever factory failure") + }, + }, + { + description: "annotation validation failure", + environment: env, + annotations: annots.Copy().Edit("conjur.org/retry-interval-sec", "not-an-integer"), + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: nil, + err: nil, + }.Retrieve, + err: nil, + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 1, code) + assert.Contains(t, logs, "CSPFK049E") + }, + }, + { + description: "authenticator config validation failure", + environment: env, + annotations: annots.Copy().Edit("conjur.org/authn-identity", "1"), + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: nil, + err: nil, + }.Retrieve, + err: nil, + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 1, code) + assert.Contains(t, logs, "CSPFK008E") + }, + }, + { + description: "secrets provider setting validation failure", + environment: env, + annotations: annots.Copy().Delete("conjur.org/secrets-destination"), + retrieverFactory: mockRetrieverFactory{ + retriever: mockRetriever{ + data: nil, + err: nil, + }.Retrieve, + err: nil, + }, + providerFactory: mockProviderFactory{ + providerFunc: func() (bool, error) { + return true, nil + }, + errs: []error{}, + }, + assertions: func(t *testing.T, code int, logs string) { + assert.Equal(t, 1, code) + assert.Contains(t, logs, "CSPFK015E") + }, + }, + } + + for _, tc := range TestCases { + t.Run(tc.description, func(t *testing.T) { + // Capture error logs + buf := &bytes.Buffer{} + log.ErrorLogger.SetOutput(buf) + // Burn info logs + log.InfoLogger.SetOutput(&bytes.Buffer{}) + // Setup envvars + for k, v := range tc.environment { + os.Setenv(k, v) + } + // Setup annotation file + annotationFileContent := "" + for k, v := range tc.annotations { + annotationFileContent = fmt.Sprintf("%s%s=\"%s\"\n", annotationFileContent, k, v) + } + err := os.WriteFile(annotationsFilePath, []byte(annotationFileContent), 0666) + assert.Nil(t, err) + + exitCode := startSecretsProviderWithDeps( + annotationsFilePath, + secretsBasePath, + templatesBasePath, + tc.retrieverFactory.GetRetriever, + tc.providerFactory.GetProvider, + getMockStatusUpdater, + ) + + tc.assertions(t, exitCode, buf.String()) + + // Restore logs + log.ErrorLogger.SetOutput(os.Stderr) + // Teardown envvars + for k := range tc.environment { + os.Setenv(k, "") + } + }) + } +} From 9778c3bc1489b7ea6e7bd6932f2b6610755ca75a Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Wed, 29 Mar 2023 22:35:06 -0400 Subject: [PATCH 6/7] Refactor repeatableSecretsProvider function --- pkg/entrypoint/entrypoint.go | 48 ++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index db30d41d..020f0ea5 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -99,21 +99,33 @@ func startSecretsProviderWithDeps( return } - // Gather secrets config and create a repeatable Secrets Provider - provideSecrets, _, err := repeatableSecretsProvider( + provideSecretsFunc, secretsConfig, err := secretsProvider( ctx, tracer, secretsBasePath, templatesBasePath, secretRetriever, providerFactory, - statusUpdaterFactory, ) if err != nil { logError(err.Error()) return } + provideSecretsFunc = secrets.RetryableSecretProvider( + time.Duration(secretsConfig.RetryIntervalSec)*time.Second, + secretsConfig.RetryCountLimit, + provideSecretsFunc, + ) + + provideSecrets := repeatableSecretsProvider( + ctx, + tracer, + secretsConfig.SecretsRefreshInterval, + provideSecretsFunc, + statusUpdaterFactory, + ) + // Provide secrets if err = provideSecrets(); err != nil { logError(err.Error()) @@ -170,17 +182,15 @@ func secretRetriever( return secretRetriever, nil } -func repeatableSecretsProvider( +func secretsProvider( ctx context.Context, tracer trace.Tracer, secretsBasePath string, templatesBasePath string, secretRetriever conjur.RetrieveSecretsFunc, providerFactory secrets.ProviderFactory, - statusUpdaterFactory secrets.StatusUpdaterFactory, -) (secrets.RepeatableProviderFunc, *secretsConfigProvider.Config, error) { - - _, span := tracer.Start(ctx, "Create repeatable secrets provider") +) (secrets.ProviderFunc, *secretsConfigProvider.Config, error) { + _, span := tracer.Start(ctx, "Create single-use secrets provider") defer span.End() // Initialize Secrets Provider configuration @@ -218,11 +228,18 @@ func repeatableSecretsProvider( return nil, nil, err } - provideSecrets = secrets.RetryableSecretProvider( - time.Duration(secretsConfig.RetryIntervalSec)*time.Second, - secretsConfig.RetryCountLimit, - provideSecrets, - ) + return provideSecrets, secretsConfig, nil +} + +func repeatableSecretsProvider( + ctx context.Context, + tracer trace.Tracer, + refreshInterval time.Duration, + provideSecrets secrets.ProviderFunc, + statusUpdaterFactory secrets.StatusUpdaterFactory, +) secrets.RepeatableProviderFunc { + _, span := tracer.Start(ctx, "Create repeatable secrets provider") + defer span.End() // Create a channel to send a quit signal to the periodic secret provider. // TODO: Currently, this is just used for testing, but in the future we @@ -233,16 +250,15 @@ func repeatableSecretsProvider( refreshConfig := secrets.ProviderRefreshConfig{ Mode: getContainerMode(), - SecretRefreshInterval: secretsConfig.SecretsRefreshInterval, + SecretRefreshInterval: refreshInterval, ProviderQuit: providerQuit, } - repeatableProvideSecrets := secrets.RepeatableSecretProvider( + return secrets.RepeatableSecretProvider( refreshConfig, provideSecrets, statusUpdaterFactory(), ) - return repeatableProvideSecrets, secretsConfig, nil } func customEnv(key string) string { From 2d7027392b9d15f1f577fec4434fe7df9a64049e Mon Sep 17 00:00:00 2001 From: John ODonnell Date: Thu, 30 Mar 2023 11:57:03 -0400 Subject: [PATCH 7/7] Review changes with @doodlesbykumbi - Remove defaultStatusUpdater singleton in favor of NewStatusUpdater factory. - Convert RepeatableSecretsProvider, which returns a RepeatableProviderFunc, to RunSecretsProvider, which runs execution logic. - ./bin/test_unit passes all args to go test command. --- bin/test_unit | 13 +-- pkg/entrypoint/entrypoint.go | 62 ++++---------- pkg/secrets/provide_conjur_secrets.go | 95 +++++++++++----------- pkg/secrets/provide_conjur_secrets_test.go | 6 +- pkg/secrets/provider_status.go | 22 +++-- 5 files changed, 79 insertions(+), 119 deletions(-) diff --git a/bin/test_unit b/bin/test_unit index 809e9509..0e4abba6 100755 --- a/bin/test_unit +++ b/bin/test_unit @@ -5,12 +5,10 @@ set -eox pipefail junit_output_file="./junit.output" . bin/build_utils -test_case="${1-}" - function main() { retrieve_cyberark_ca_cert build_docker_ut_image - run_unit_tests + run_unit_tests $@ } function build_docker_ut_image() { @@ -21,20 +19,15 @@ function build_docker_ut_image() { function run_unit_tests() { echo "Running unit tests..." - test_case_cmd="" - if [[ "$test_case" != "" ]]; then - test_case_cmd="--run $test_case" - fi - docker run --rm -t \ --volume "$PWD"/:/secrets-provider-for-k8s/test/ \ secrets-provider-for-k8s-test-runner:latest \ -coverprofile="./test/c.out" \ ./cmd/... \ ./pkg/... \ - $test_case_cmd \ + $@ \ | tee -a "$junit_output_file" echo "Unit test exit status: $?" } -main +main $@ diff --git a/pkg/entrypoint/entrypoint.go b/pkg/entrypoint/entrypoint.go index 020f0ea5..723b77c6 100644 --- a/pkg/entrypoint/entrypoint.go +++ b/pkg/entrypoint/entrypoint.go @@ -55,7 +55,7 @@ func StartSecretsProvider() { defaultTemplatesBasePath, conjur.NewSecretRetriever, secrets.NewProviderForType, - secrets.DefaultStatusUpdater, + secrets.NewStatusUpdater, ) os.Exit(exitCode) } @@ -99,7 +99,7 @@ func startSecretsProviderWithDeps( return } - provideSecretsFunc, secretsConfig, err := secretsProvider( + provideSecrets, secretsConfig, err := secretsProvider( ctx, tracer, secretsBasePath, @@ -112,22 +112,26 @@ func startSecretsProviderWithDeps( return } - provideSecretsFunc = secrets.RetryableSecretProvider( + provideSecrets = secrets.RetryableSecretProvider( time.Duration(secretsConfig.RetryIntervalSec)*time.Second, secretsConfig.RetryCountLimit, - provideSecretsFunc, - ) - - provideSecrets := repeatableSecretsProvider( - ctx, - tracer, - secretsConfig.SecretsRefreshInterval, - provideSecretsFunc, - statusUpdaterFactory, + provideSecrets, ) - // Provide secrets - if err = provideSecrets(); err != nil { + if err = secrets.RunSecretsProvider( + secrets.ProviderRefreshConfig{ + Mode: getContainerMode(), + SecretRefreshInterval: secretsConfig.SecretsRefreshInterval, + // Create a channel to send a quit signal to the periodic secret provider. + // TODO: Currently, this is just used for testing, but in the future we + // may want to create a SIGTERM or SIGHUP handler to catch a signal from + // a user / external entity, and then send an (empty struct) quit signal + // on this channel to trigger a graceful shut down of the Secrets Provider. + ProviderQuit: make(chan struct{}), + }, + provideSecrets, + statusUpdaterFactory(), + ); err != nil { logError(err.Error()) } return @@ -231,36 +235,6 @@ func secretsProvider( return provideSecrets, secretsConfig, nil } -func repeatableSecretsProvider( - ctx context.Context, - tracer trace.Tracer, - refreshInterval time.Duration, - provideSecrets secrets.ProviderFunc, - statusUpdaterFactory secrets.StatusUpdaterFactory, -) secrets.RepeatableProviderFunc { - _, span := tracer.Start(ctx, "Create repeatable secrets provider") - defer span.End() - - // Create a channel to send a quit signal to the periodic secret provider. - // TODO: Currently, this is just used for testing, but in the future we - // may want to create a SIGTERM or SIGHUP handler to catch a signal from - // a user / external entity, and then send an (empty struct) quit signal - // on this channel to trigger a graceful shut down of the Secrets Provider. - providerQuit := make(chan struct{}) - - refreshConfig := secrets.ProviderRefreshConfig{ - Mode: getContainerMode(), - SecretRefreshInterval: refreshInterval, - ProviderQuit: providerQuit, - } - - return secrets.RepeatableSecretProvider( - refreshConfig, - provideSecrets, - statusUpdaterFactory(), - ) -} - func customEnv(key string) string { if annotation, ok := envAnnotationsConversion[key]; ok { if value := annotationsMap[annotation]; value != "" { diff --git a/pkg/secrets/provide_conjur_secrets.go b/pkg/secrets/provide_conjur_secrets.go index d499a7bf..380101da 100644 --- a/pkg/secrets/provide_conjur_secrets.go +++ b/pkg/secrets/provide_conjur_secrets.go @@ -120,71 +120,68 @@ type ProviderRefreshConfig struct { ProviderQuit chan struct{} } -// RepeatableSecretProvider returns a new ProviderFunc, which wraps a retryable -// ProviderFunc inside a function that operates in one of three modes: +// RunSecretsProvider takes a retryable ProviderFunc, and runs it in one of three modes: // - Run once and return (for init or application container modes) // - Run once and sleep forever (for sidecar mode without periodic refresh) // - Run periodically (for sidecar mode with periodic refresh) -func RepeatableSecretProvider( +func RunSecretsProvider( config ProviderRefreshConfig, provideSecrets ProviderFunc, status StatusUpdater, -) RepeatableProviderFunc { +) error { var periodicQuit = make(chan struct{}) var periodicError = make(chan error) var ticker *time.Ticker var err error - return func() error { - if err = status.CopyScripts(); err != nil { - return err - } - if _, err = provideSecrets(); err != nil { - // Return immediately upon error, regardless of operating mode - return err - } - err = status.SetSecretsProvided() - if err != nil { - return err - } - switch { - case config.Mode != "sidecar": - // Run once and return if not in sidecar mode - return nil - case config.SecretRefreshInterval > 0: - // Run periodically if in sidecar mode with periodic refresh - ticker = time.NewTicker(config.SecretRefreshInterval) - config := periodicConfig{ - ticker: ticker, - periodicQuit: periodicQuit, - periodicError: periodicError, - } - go periodicSecretProvider(provideSecrets, config, status) - default: - // Run once and sleep forever if in sidecar mode without - // periodic refresh (fall through) + if err = status.CopyScripts(); err != nil { + return err + } + if _, err = provideSecrets(); err != nil { + // Return immediately upon error, regardless of operating mode + return err + } + err = status.SetSecretsProvided() + if err != nil { + return err + } + switch { + case config.Mode != "sidecar": + // Run once and return if not in sidecar mode + return nil + case config.SecretRefreshInterval > 0: + // Run periodically if in sidecar mode with periodic refresh + ticker = time.NewTicker(config.SecretRefreshInterval) + config := periodicConfig{ + ticker: ticker, + periodicQuit: periodicQuit, + periodicError: periodicError, } + go periodicSecretProvider(provideSecrets, config, status) + default: + // Run once and sleep forever if in sidecar mode without + // periodic refresh (fall through) + } - // Wait here for a signal to quit providing secrets or an error - // from the periodicSecretProvider() function - select { - case <-config.ProviderQuit: - break - case err = <-periodicError: - break - } + // Wait here for a signal to quit providing secrets or an error + // from the periodicSecretProvider() function + select { + case <-config.ProviderQuit: + break + case err = <-periodicError: + break + } - // Allow the periodicSecretProvider goroutine to gracefully shut down - if config.SecretRefreshInterval > 0 { - // Kill the ticker - ticker.Stop() - periodicQuit <- struct{}{} - // Let the go routine exit - time.Sleep(secretProviderGracePeriod) - } - return err + // Allow the periodicSecretProvider goroutine to gracefully shut down + if config.SecretRefreshInterval > 0 { + // Kill the ticker + ticker.Stop() + periodicQuit <- struct{}{} + // Let the go routine exit + time.Sleep(secretProviderGracePeriod) } + return err } type periodicConfig struct { diff --git a/pkg/secrets/provide_conjur_secrets_test.go b/pkg/secrets/provide_conjur_secrets_test.go index 3ff8bb67..d6b9e3e2 100644 --- a/pkg/secrets/provide_conjur_secrets_test.go +++ b/pkg/secrets/provide_conjur_secrets_test.go @@ -148,7 +148,7 @@ func TestRetryableSecretProvider(t *testing.T) { } } -func TestRepeatableSecretProvider(t *testing.T) { +func TestRunSecretsProvider(t *testing.T) { testCases := []struct { description string mode string @@ -298,13 +298,11 @@ func TestRepeatableSecretProvider(t *testing.T) { SecretRefreshInterval: tc.interval, ProviderQuit: providerQuit, } - provideSecrets := RepeatableSecretProvider( - refreshConfig, tc.provider.provide, fileUpdater) // Run the secrets provider testError := make(chan error) go func() { - err := provideSecrets() + err := RunSecretsProvider(refreshConfig, tc.provider.provide, fileUpdater) testError <- err }() select { diff --git a/pkg/secrets/provider_status.go b/pkg/secrets/provider_status.go index 4cf4a1a0..e1b398c1 100644 --- a/pkg/secrets/provider_status.go +++ b/pkg/secrets/provider_status.go @@ -59,9 +59,16 @@ var stdOSFuncs = osFuncs{ // implementation. type StatusUpdaterFactory func() StatusUpdater -// DefaultStatusUpdater returns the default StatusUpdater. -func DefaultStatusUpdater() StatusUpdater { - return defaultStatusUpdater +// NewStatusUpdater returns a new instance of the default StatusUpdater. +func NewStatusUpdater() StatusUpdater { + return fileUpdater{ + providedFile: "/conjur/status/CONJUR_SECRETS_PROVIDED", + updatedFile: "/conjur/status/CONJUR_SECRETS_UPDATED", + scripts: []string{"conjur-secrets-unchanged.sh"}, + scriptSrcDir: "/usr/local/bin", + scriptDestDir: "/conjur/status", + os: stdOSFuncs, + } } // fileUpdater implements the statusUpdater interface. It records provider @@ -75,15 +82,6 @@ type fileUpdater struct { os osFuncs } -var defaultStatusUpdater = fileUpdater{ - providedFile: "/conjur/status/CONJUR_SECRETS_PROVIDED", - updatedFile: "/conjur/status/CONJUR_SECRETS_UPDATED", - scripts: []string{"conjur-secrets-unchanged.sh"}, - scriptSrcDir: "/usr/local/bin", - scriptDestDir: "/conjur/status", - os: stdOSFuncs, -} - func (f fileUpdater) setStatus(path string) error { file, err := f.os.create(path) if err != nil {