From d181ae8b2662b0a85a0babd082aa4ec0a826ae76 Mon Sep 17 00:00:00 2001
From: Rob Pothier <Robert.Pothier@cyberark.com>
Date: Tue, 18 Jan 2022 14:15:55 -0500
Subject: [PATCH] Add Secrets Provider refresh interval

---
 PUSH_TO_FILE.md                            |   4 +-
 cmd/secrets-provider/main.go               |  27 ++++-
 pkg/log/messages/error_messages.go         |   3 +
 pkg/secrets/config/config.go               |  78 ++++++++++++++-
 pkg/secrets/config/config_test.go          |  60 +++++++++++
 pkg/secrets/provide_conjur_secrets.go      |  67 ++++++++++++-
 pkg/secrets/provide_conjur_secrets_test.go | 111 +++++++++++++++++++++
 7 files changed, 341 insertions(+), 9 deletions(-)

diff --git a/PUSH_TO_FILE.md b/PUSH_TO_FILE.md
index 7f17b4f72..4eedf316c 100644
--- a/PUSH_TO_FILE.md
+++ b/PUSH_TO_FILE.md
@@ -323,12 +323,14 @@ for a description of each environment variable setting:
 | K8s Annotation  | Equivalent<br>Environment Variable | Description, Notes |
 |-----------------------------------------|---------------------|----------------------------------|
 | `conjur.org/authn-identity`         | `CONJUR_AUTHN_LOGIN`  | Required value. Example: `host/conjur/authn-k8s/cluster/apps/inventory-api` |
-| `conjur.org/container-mode`         | `CONTAINER_MODE`      | Allowed values: <ul><li>`init`</li><li>`application`</li></ul>Defaults to `init`.<br>Must be set (or default) to `init` for Push to File mode.|
+| `conjur.org/container-mode`         | `CONTAINER_MODE`      | Allowed values: <ul><li>`init`</li><li>`application`</li><li>`side-car`</li></ul>Defaults to `init`.<br>Must be set (or default) to `init` or `side-car`for Push to File mode.|
 | `conjur.org/secrets-destination`    | `SECRETS_DESTINATION` | Allowed values: <ul><li>`file`</li><li>`k8s_secrets`</li></ul> |
 | `conjur.org/k8s-secrets`            | `K8S_SECRETS`         | This list is ignored when `conjur.org/secrets-destination` annotation is set to **`file`** |
 | `conjur.org/retry-count-limit`      | `RETRY_COUNT_LIMIT`   | Defaults to 5
 | `conjur.org/retry-interval-sec`     | `RETRY_INTERVAL_SEC`  | Defaults to 1 (sec)              |
 | `conjur.org/debug-logging`          | `DEBUG`               | Defaults to `false`              |
+| `conjur.org/secrets-refresh-enabled`| Note\* | Can be set to `true` or `false`. Secrets Provider will exit with error if this is explicitly set to `false` and `conjur.org/secrets-rotation-interval` is explicitly set. |
+| `conjur.org/secrets-refresh-interval`           | Note\* | Set to a valid duration string as defined [here](https://pkg.go.dev/time#ParseDuration). Valid time units are `s`, `m`, and `h` (for seconds, minutes, and hours, respectively). Some examples of valid duration strings:<ul><li>`5m`</li><li>`2h30m`</li><li>`48h`</li></ul>The minimum refresh interval is 1 second. A refresh interval of 0 seconds is treated as a fatal configuration error. The maximum refresh interval is approximately 290 years. |
 | `conjur.org/conjur-secrets.{secret-group}`      | Note\* | List of secrets to be retrieved from Conjur. Each entry can be either:<ul><li>A Conjur variable path</li><li> A key/value pairs of the form `<alias>:<Conjur variable path>` where the `alias` represents the name of the secret to be written to the secrets file |
 | `conjur.org/conjur-secrets-policy-path.{secret-group}` | Note\* | Defines a common Conjur policy path, assumed to be relative to the root policy.<br><br>When this annotation is set, the policy paths defined by `conjur.org/conjur-secrets.{secret-group}` are relative to this common path.<br><br>When this annotation is not set, the policy paths defined by `conjur.org/conjur-secrets.{secret-group}` are themselves relative to the root policy.<br><br>(See [Example Common Policy Path](#example-common-policy-path) for an explicit example of this relationship.)|
 | `conjur.org/secret-file-path.{secret-group}`    | Note\* | Relative path for secret file or directory to be written. This path is assumed to be relative to the respective mount path for the shared secrets volume for each container.<br><br>If the `conjur.org/secret-file-template.{secret-group}` is set, then this secret file path defaults to `{secret-group}.out`. For example, if the secret group name is `my-app`, the the secret file path defaults to `my-app.out`.<br><br>If the `conjur.org/secret-file-template.{secret-group}` is not set, then this secret file path defaults to `{secret-group}.{secret-group-file-format}`. For example, if the secret group name is `my-app`, and the secret file format is set for YAML, the the secret file path defaults to `my-app.yaml`.
diff --git a/cmd/secrets-provider/main.go b/cmd/secrets-provider/main.go
index efceb96b9..23743d4b3 100644
--- a/cmd/secrets-provider/main.go
+++ b/cmd/secrets-provider/main.go
@@ -85,6 +85,7 @@ func main() {
 		logError(err.Error())
 		return
 	}
+	fmt.Printf("***TEMP*** Begin refresh loop\n")
 
 	// Provide secrets
 	err = provideSecrets()
@@ -92,6 +93,7 @@ func main() {
 		errStr := fmt.Sprintf(messages.CSPFK039E, secretsConfig.StoreType, err.Error())
 		logError(errStr)
 	}
+	fmt.Printf("***TEMP*** Secrets Provider complete!\n")
 }
 
 func processAnnotations(ctx context.Context, tracer trace.Tracer) error {
@@ -130,11 +132,12 @@ func secretRetriever(ctx context.Context,
 		log.Error(messages.CSPFK008E)
 		return nil, err
 	}
-	if err = validateContainerMode(authnConfig.GetContainerMode()); err != nil {
+	// Do we need to check this? Container mode is checked in ValidateAnnotations
+	/*if err = validateContainerMode(authnConfig.GetContainerMode()); err != nil {
 		span.RecordErrorAndSetStatus(err)
 		log.Error(err.Error())
 		return nil, err
-	}
+	}*/
 
 	// Initialize a Conjur secret retriever
 	secretRetriever, err := conjur.NewSecretRetriever(authnConfig)
@@ -186,6 +189,14 @@ func retryableSecretsProvider(
 		secretsConfig.RetryCountLimit,
 		provideSecrets,
 	)
+
+	// new retryable secrets
+	provideSecrets = secrets.PeriodicSecretProvider(
+		secretsConfig.SecretsRefreshInterval,
+		getContainerMode(),
+		provideSecrets,
+		time.Duration(100 * 365 * 24)*time.Hour,
+	)
 	return provideSecrets, secretsConfig, nil
 }
 
@@ -232,16 +243,24 @@ func logErrorsAndInfos(errLogs []error, infoLogs []error) error {
 	return nil
 }
 
-func validateContainerMode(containerMode string) error {
+/*func validateContainerMode(containerMode string) error {
 	validContainerModes := []string{
 		"init",
 		"application",
 	}
-
 	for _, validContainerModeType := range validContainerModes {
 		if containerMode == validContainerModeType {
 			return nil
 		}
 	}
 	return fmt.Errorf(messages.CSPFK007E, containerMode, validContainerModes)
+}*/
+
+func getContainerMode() string {
+	containerMode := "init"
+	if mode, exists := annotationsMap[secretsConfigProvider.ContainerModeKey]; exists {
+		fmt.Printf("***TEMP*** found container mode %s\n", mode)
+		containerMode = mode
+	}
+	return containerMode
 }
diff --git a/pkg/log/messages/error_messages.go b/pkg/log/messages/error_messages.go
index b28a41668..642014f22 100644
--- a/pkg/log/messages/error_messages.go
+++ b/pkg/log/messages/error_messages.go
@@ -68,6 +68,9 @@ const CSPFK046E string = "CSPFK046E Secret Store Type needs to be configured, ei
 const CSPFK047E string = "CSPFK047E Secrets Provider in Push-to-File mode can only be configured with Pod annotations"
 const CSPFK048E string = "CSPFK048E Secrets Provider in K8s Secrets mode requires either the 'K8S_SECRETS' environment variable or 'conjur.org/k8s-secrets' Pod annotation"
 const CSPFK049E string = "CSPFK049E Failed to validate Pod annotations"
+const CSPFK050E string = "CSPFK050E Invalid secrets refresh interval annotation: %s %s"
+//const CSPFK051E string = "CSPFK051E Invalid secrets refresh enable annotation: %s %s"
+
 
 // Push to File
 const CSPFK053E string = "CSPFK053E Unable to initialize Secrets Provider: unable to create secret group collection"
diff --git a/pkg/secrets/config/config.go b/pkg/secrets/config/config.go
index 4ab6d9600..38e0956aa 100644
--- a/pkg/secrets/config/config.go
+++ b/pkg/secrets/config/config.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/cyberark/secrets-provider-for-k8s/pkg/log/messages"
 )
@@ -19,6 +20,8 @@ const (
 	DefaultRetryCountLimit  = 5
 	DefaultRetryIntervalSec = 1
 	MinRetryValue           = 0
+	DefaultRefreshInterval  = "5m"
+	MinRefreshInterval      = time.Second
 )
 
 // Config defines the configuration parameters
@@ -29,6 +32,7 @@ type Config struct {
 	RetryCountLimit    int
 	RetryIntervalSec   int
 	StoreType          string
+	SecretsRefreshInterval time.Duration
 }
 
 type annotationType int
@@ -46,14 +50,26 @@ type annotationRestraints struct {
 	allowedValues []string
 }
 
+// Annotations used to support Conjur secrets rotation.
+// SecretsRefreshIntervalKey is the Annotation key for setting the interval
+// for retrieving Conjur secrets and updating Kubernetes Secrets or
+// application secret files if necessary.
+const (
+	ContainerModeKey = "conjur.org/container-mode"
+	SecretsRefreshIntervalKey = "conjur.org/secrets-refresh-interval"
+	SecretsRefreshEnabledKey = "conjur.org/secrets-refresh-enabled"
+)
+
 // Define supported annotation keys for Secrets Provider config, as well as value restraints for each
 var secretsProviderAnnotations = map[string]annotationRestraints{
 	"conjur.org/authn-identity":       {TYPESTRING, []string{}},
-	"conjur.org/container-mode":       {TYPESTRING, []string{"init", "application"}},
+	ContainerModeKey:                  {TYPESTRING, []string{"init", "application", "side-car"}},
 	"conjur.org/secrets-destination":  {TYPESTRING, []string{"file", "k8s_secrets"}},
 	"conjur.org/k8s-secrets":          {TYPESTRING, []string{}},
 	"conjur.org/retry-count-limit":    {TYPEINT, []string{}},
 	"conjur.org/retry-interval-sec":   {TYPEINT, []string{}},
+	SecretsRefreshIntervalKey:         {TYPESTRING, []string{}},
+	SecretsRefreshEnabledKey:          {TYPEBOOL, []string{}},
 	"conjur.org/debug-logging":        {TYPEBOOL, []string{}},
 	"conjur.org/log-traces":           {TYPEBOOL, []string{}},
 	"conjur.org/jaeger-collector-url": {TYPESTRING, []string{}},
@@ -68,8 +84,8 @@ var pushToFileAnnotationPrefixes = map[string]annotationRestraints{
 	"conjur.org/conjur-secrets-policy-path.": {TYPESTRING, []string{}},
 	"conjur.org/secret-file-path.":           {TYPESTRING, []string{}},
 	"conjur.org/secret-file-format.":         {TYPESTRING, []string{"yaml", "json", "dotenv", "bash", "template"}},
-	"conjur.org/secret-file-permissions.": 	  {TYPESTRING, []string{}},
-	"conjur.org/secret-file-template":        {TYPESTRING, []string{}},
+	"conjur.org/secret-file-permissions.":    {TYPESTRING, []string{}},
+	"conjur.org/secret-file-template.":       {TYPESTRING, []string{}},
 }
 
 // Define environment variables used in Secrets Provider config
@@ -183,6 +199,21 @@ func ValidateSecretsProviderSettings(envAndAnnots map[string]string) ([]error, [
 		infoList = append(infoList, fmt.Errorf(messages.CSPFK012I, "RetryIntervalSec", "RETRY_INTERVAL_SEC", "conjur.org/retry-interval-sec"))
 	}
 
+	annotSecretsRefreshEnable := envAndAnnots[SecretsRefreshEnabledKey]
+	/* this is not needed
+	reason :=  validRefreshEnable(annotSecretsRefreshEnable)
+	if reason != "" {
+		errorList = append(errorList, fmt.Errorf(messages.CSPFK051E, annotSecretsRefreshEnable, reason))
+	}*/
+
+	annotSecretsRefreshInterval := envAndAnnots[SecretsRefreshIntervalKey]
+	if !(annotSecretsRefreshInterval == "" && annotSecretsRefreshEnable == ""){
+		valid, reason := validRefreshInterval(annotSecretsRefreshInterval, annotSecretsRefreshEnable)
+		if !valid {
+			errorList = append(errorList, fmt.Errorf(messages.CSPFK050E, annotSecretsRefreshInterval, reason))
+		}
+	}
+
 	return errorList, infoList
 }
 
@@ -222,12 +253,22 @@ func NewConfig(settings map[string]string) *Config {
 	}
 	retryIntervalSec := parseIntFromStringOrDefault(retryIntervalSecStr, DefaultRetryIntervalSec, MinRetryValue)
 
+	refreshIntervalStr := settings[SecretsRefreshIntervalKey]
+	refreshEnableStr   := settings[SecretsRefreshEnabledKey]
+	fmt.Printf("***TEMP*** NewConfig: secrets-refresh-interval=%s en=%s\n", refreshIntervalStr, refreshEnableStr)
+	if refreshIntervalStr == "" && refreshEnableStr == "true" {
+		refreshIntervalStr = DefaultRefreshInterval
+	}
+	refreshInterval, _ := time.ParseDuration(refreshIntervalStr)
+	fmt.Printf("***TEMP*** NewConfig2: secrets-refresh-interval=%v\n", refreshInterval)
+
 	return &Config{
 		PodNamespace:       podNamespace,
 		RequiredK8sSecrets: k8sSecretsArr,
 		RetryCountLimit:    retryCountLimit,
 		RetryIntervalSec:   retryIntervalSec,
 		StoreType:          storeType,
+		SecretsRefreshInterval: refreshInterval,
 	}
 }
 
@@ -304,3 +345,34 @@ func validStoreType(storeType string) bool {
 	}
 	return false
 }
+
+/*func validRefreshEnable(enableStr string) string {
+
+	if enableStr != "" {
+		_, err := strconv.ParseBool(enableStr)
+		if err != nil {
+			return err.Error()
+		}
+	}
+	return  ""
+}*/
+
+func validRefreshInterval(interval string, enableStr string) (bool, string) {
+
+	fmt.Printf("***TEMP*** in validRefreshInterval\n")
+	duration , err := time.ParseDuration(interval)
+	if err != nil {
+		return false, err.Error()
+	}
+	// check if the user explicitly set enable to false
+	if enableStr != "" {
+		enable, _ := strconv.ParseBool(enableStr)
+		if duration > 0 && !enable {
+			return false, "Secrets refresh interval set to value while enable is false"
+		}
+	}
+    if duration < MinRefreshInterval {
+		return false, "Secrets refresh interval must be at least one second"
+	}
+	return true, ""
+}
diff --git a/pkg/secrets/config/config_test.go b/pkg/secrets/config/config_test.go
index 4bbbe99f5..49f9f6ced 100644
--- a/pkg/secrets/config/config_test.go
+++ b/pkg/secrets/config/config_test.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -59,6 +60,7 @@ var validateAnnotationsTestCases = []validateAnnotationsTestCase{
 			"conjur.org/k8s-secrets":                   "- secret-1\n- secret-2\n- secret-3\n",
 			"conjur.org/retry-count-limit":             "12",
 			"conjur.org/retry-interval-sec":            "2",
+			"conjur.org/secrets-refresh-interval":      "5s",
 			"conjur.org/conjur-secrets.this-group":     "- test/url\n- test-password: test/password\n- test-username: test/username\n",
 			"conjur.org/secret-file-path.this-group":   "this-relative-path",
 			"conjur.org/secret-file-format.this-group": "yaml",
@@ -194,6 +196,7 @@ var validateSecretsProviderSettingsTestCases = []validateSecretsProviderSettings
 			"conjur.org/retry-count-limit":   "10",
 			"conjur.org/retry-interval-sec":  "20",
 			"conjur.org/k8s-secrets":         "- secret-1\n- secret-2\n- secret-3\n",
+			"conjur.org/secrets-refresh-interval":      "5s",
 		},
 		assert: assertEmptyErrorList(),
 	},
@@ -311,6 +314,47 @@ var validateSecretsProviderSettingsTestCases = []validateSecretsProviderSettings
 		},
 		assert: assertErrorInList(fmt.Errorf(messages.CSPFK005E, "SECRETS_DESTINATION")),
 	},
+	{
+		description: "if refresh interval is malformed, an error is returned",
+		envAndAnnots: map[string]string{
+			"MY_POD_NAMESPACE":               "test-namespace",
+			SecretsRefreshIntervalKey: "5",
+		},
+		assert: assertErrorInList(fmt.Errorf(messages.CSPFK050E, "5", "time: missing unit in duration \"5\"")),
+	},
+	{
+		description: "if refresh interval is zero, an error is returned",
+		envAndAnnots: map[string]string{
+			"MY_POD_NAMESPACE":               "test-namespace",
+			SecretsRefreshIntervalKey: "0",
+		},
+		assert: assertErrorInList(fmt.Errorf(messages.CSPFK050E, "0", "Secrets refresh interval must be at least one second")),
+	},
+	{
+		description: "if refresh interval is too small, an error is returned",
+		envAndAnnots: map[string]string{
+			"MY_POD_NAMESPACE":               "test-namespace",
+			SecretsRefreshIntervalKey: "500ms",
+		},
+		assert: assertErrorInList(fmt.Errorf(messages.CSPFK050E, "500ms", "Secrets refresh interval must be at least one second")),
+	},
+	{
+		description: "if refresh interval is negative, an error is returned",
+		envAndAnnots: map[string]string{
+			"MY_POD_NAMESPACE":               "test-namespace",
+			SecretsRefreshIntervalKey: "-5s",
+		},
+		assert: assertErrorInList(fmt.Errorf(messages.CSPFK050E, "-5s", "Secrets refresh interval must be at least one second")),
+	},
+	{
+		description: "if refresh interval is set and enable is false",
+		envAndAnnots: map[string]string{
+			"MY_POD_NAMESPACE":                    "test-namespace",
+			SecretsRefreshIntervalKey: "5s",
+			SecretsRefreshEnabledKey:   "false",
+		},
+		assert: assertErrorInList(fmt.Errorf(messages.CSPFK050E, "5s", "Secrets refresh interval set to value while enable is false")),
+	},
 }
 
 type newConfigTestCase struct {
@@ -387,6 +431,22 @@ var newConfigTestCases = []newConfigTestCase{
 			RetryIntervalSec:   20,
 		}),
 	},
+	{
+		description: "a valid map of annotation-based Secrets Provider settings returns a valid Config",
+		settings: map[string]string{
+			"MY_POD_NAMESPACE":               "test-namespace",
+			"conjur.org/secrets-destination": "file",
+			SecretsRefreshEnabledKey:  "true",
+		},
+		assert: assertGoodConfig(&Config{
+			PodNamespace:       "test-namespace",
+			StoreType:          "file",
+			RequiredK8sSecrets: []string{},
+			RetryCountLimit:    5,
+			RetryIntervalSec:   1,
+			SecretsRefreshInterval: time.Minute * 5,
+		}),
+	},
 }
 
 func TestValidateAnnotations(t *testing.T) {
diff --git a/pkg/secrets/provide_conjur_secrets.go b/pkg/secrets/provide_conjur_secrets.go
index 0799538df..64e957860 100644
--- a/pkg/secrets/provide_conjur_secrets.go
+++ b/pkg/secrets/provide_conjur_secrets.go
@@ -32,6 +32,12 @@ type ProviderConfig struct {
 	AnnotationsMap       map[string]string
 }
 
+const (
+	Init = iota
+	SideCar
+	App
+)
+
 // ProviderFunc describes a function type responsible for providing secrets to an unspecified target.
 type ProviderFunc func() error
 
@@ -87,7 +93,6 @@ func RetryableSecretProvider(
 			if limitedBackOff.RetryCount() > 0 {
 				log.Info(fmt.Sprintf(messages.CSPFK010I, limitedBackOff.RetryCount(), limitedBackOff.RetryLimit))
 			}
-
 			return provideSecrets()
 		}, limitedBackOff)
 
@@ -97,3 +102,63 @@ func RetryableSecretProvider(
 		return err
 	}
 }
+
+// Make global for now
+var TickerQuit = make(chan bool)
+var ProviderDone = make(chan bool)
+func PeriodicSecretProvider(
+	secretRefreshInterval time.Duration,
+	mode string,
+	provideSecrets ProviderFunc,
+	timeout time.Duration,
+	) ProviderFunc {
+	fmt.Printf("***TEMP*** init PeriodicSecretProvider %v\n", secretRefreshInterval)
+	return func() error {
+		fmt.Printf("***TEMP*** running PeriodicSecretProvider\n")
+		err := provideSecrets()
+		switch {
+		case mode == "init":
+			fmt.Printf("***TEMP*** init - exiting periodic provider\n")
+			return err
+		case mode == "side-car" && secretRefreshInterval > 0:
+			ticker := time.NewTicker(secretRefreshInterval)
+			go periodicSecretProvider(provideSecrets, ticker)
+		default:
+			fmt.Printf("***TEMP*** application or 0 interval \n")
+			select {
+			case <-ProviderDone: // Todo change to SIGTERM or SIGINTERRUPT
+			case <-time.After(timeout):
+			}
+			return nil
+		}
+		fmt.Printf("***TEMP*** Waiting for Provider complete!\n")
+		select {
+		case <-ProviderDone: // Todo change to SIGTERM or SIGINTERRUPT
+		case <-time.After(timeout):
+		}
+		TickerQuit <- true
+		time.Sleep(1 * time.Second)// give a second for the go routine to shut down
+		fmt.Printf("***TEMP*** exiting periodic provider\n")
+		return nil
+	}
+}
+
+func periodicSecretProvider(provideSecrets ProviderFunc, ticker *time.Ticker) {
+	for {
+		fmt.Printf("***TEMP*** Waiting ...%s\n", time.Now())
+		select {
+		case <-ticker.C:
+			fmt.Printf("***TEMP*** Got a tick %s\n", time.Now())
+			err := provideSecrets()
+			if err != nil {
+				fmt.Printf("***TEMP*** Errror in go routine\n")
+				//errStr := fmt.Sprintf(messages.CSPFK039E, secretsConfig.StoreType, err.Error())
+				//logError(errStr)
+			}
+		case <-TickerQuit:
+			fmt.Printf("***TEMP*** Got the quit signal\n")
+			ticker.Stop()
+			return
+		}
+	}
+}
\ No newline at end of file
diff --git a/pkg/secrets/provide_conjur_secrets_test.go b/pkg/secrets/provide_conjur_secrets_test.go
index bb23e6ff0..498a3f5a2 100644
--- a/pkg/secrets/provide_conjur_secrets_test.go
+++ b/pkg/secrets/provide_conjur_secrets_test.go
@@ -18,7 +18,15 @@ func badProvider() error {
 	return errors.New("Failed to Provide")
 }
 
+var count = 0
 func goodProvider() error {
+	count++
+	return nil
+}
+
+func slowProvider() error {
+	count++
+	time.Sleep(6 * time.Second)
 	return nil
 }
 
@@ -41,6 +49,109 @@ func eventualProvider(successAfterNtries int) func() error {
 	}
 }
 
+func TestPeriodicSecretProvider(t *testing.T) {
+	TestCases := []struct {
+		description string
+		mode        string
+		interval    time.Duration
+		testTime    time.Duration // total test time for all tests must be less than 3m
+		expectedCount         int
+		provider    ProviderFunc
+		assertOn    string
+	}{
+		{
+			description: "init",
+			mode:        "init",
+			interval:    time.Duration(5) * time.Second,
+			testTime:    time.Duration(10) * time.Second,
+			expectedCount:  1,
+			provider:    goodProvider,
+			assertOn:    "success",
+		},
+		{
+			description: "side-car",
+			mode:        "side-car",
+			interval:    time.Duration(5) * time.Second,
+			testTime:    time.Duration(30) * time.Second,
+			expectedCount:  30/5+1,
+			provider:    goodProvider,
+			assertOn:    "success",
+		},
+		{
+			description: "side-car with zero duration",
+			mode:        "side-car",
+			interval:    time.Duration(0) * time.Second,
+			testTime:    time.Duration(30) * time.Second,
+			expectedCount:  1,
+			provider:    goodProvider,
+			assertOn:    "success",
+		},
+		{
+			description: "application",
+			mode:        "application",
+			interval:    time.Duration(5) * time.Second,
+			testTime:    time.Duration(30) * time.Second,
+			expectedCount:  1,
+			provider:    goodProvider,
+			assertOn:    "success",
+		},
+		{
+			description: "side-car",
+			mode:        "side-car",
+			interval:    time.Duration(10) * time.Second,
+			testTime:    time.Duration(30) * time.Second,
+			expectedCount:  30/10+1,
+			provider:    goodProvider,
+			assertOn:    "success",
+		},
+		{
+			description: "side-car with slow provider",
+			mode:        "side-car",
+			interval:    time.Duration(10) * time.Second,
+			testTime:    time.Duration(30) * time.Second,
+			expectedCount:  30/10+1,
+			provider:    slowProvider,
+			assertOn:    "success",
+		},
+		/* This test is inconsistent, 11-13 ticks
+		{
+			description: "side-car with duration less than fetch time",
+			mode:        "side-car",
+			interval:    time.Duration(3) * time.Second,
+			testTime:    time.Duration(60) * time.Second,
+			expectedCount:  60/6+1,
+			provider:    slowProvider,
+			assertOn:    "success",
+		},*/
+		//Todo check with badProvider
+	    /*{
+			description: "init",
+			mode:        "init",
+			interval:    time.Duration(5) * time.Second,
+			testTime:    time.Duration(60) * time.Second,
+			expectedCount:  1,
+			provider:    badProvider,
+			assertOn:    "success",
+		},*/
+	}
+
+	for _, tc := range TestCases {
+		periodicProvider := PeriodicSecretProvider(
+			tc.interval, tc.mode, tc.provider, tc.testTime,
+		)
+		count = 0
+		start := time.Now()
+		err := periodicProvider()
+		duration := time.Since(start)
+		fmt.Printf("***TEMP*** count = %d\n", count)
+		if err == nil && count != tc.expectedCount {
+			err = fmt.Errorf("incorrect number of timer ticks")
+		}
+		fmt.Printf("***TEMP***  time=%s count=%d\n", duration.String(), count)
+		assert.NoError(t, err)
+	}
+}
+
 func TestRetryableSecretProvider(t *testing.T) {
 	TestCases := []struct {
 		description string