Merge pull request #91 from Conjur-Enterprise/allow-empty-secrets

CNJR-12566: Allow empty RequiredK8sSecrets config

Author: gl-johnson

bootstrap.env

@@ -23,7 +23,6 @@ export APP_NAMESPACE_NAME=app-$UNIQUE_TEST_ID
 # export RUN_IN_DOCKER=false
 # export CONJUR_DEPLOYMENT=oss
 # export SUMMON_ENV=gke
-# export RELOAD_ENV=false
 # export STOP_RUNNING_ENV=true
 # export CONJUR_ACCOUNT=cucumber
 # export CONJUR_LOG_LEVEL=debug

pkg/log/messages/error_messages.go

@@ -95,4 +95,7 @@ const CSPFK066E string = "CSPFK066E Stopped fetching additional secrets after %d
 const CSPFK068E string = "CSPFK068E Retrieved secrets did not include secret '%s' requested by secret group '%s'"
 
 // URL Parsing
-const CSPFK069E string = "CSPFK069E Invalid URL format: %s"
+const CSPFK069E string = "CSPFK069E Invalid URL format: %s"
+
+// Label-based mode
+const CSPFK070E string = "CSPFK070E No labeled secrets were discovered"

pkg/log/messages/info_messages.go

@@ -31,3 +31,4 @@ const CSPFK020I string = "CSPFK020I No change in Kubernetes secret, no secrets u
 const CSPFK021I string = "CSPFK021I Error fetching Conjur secrets, clearing Kubernetes secrets"
 const CSPFK022I string = "CSPFK022I Storing secret with base64 content-type '%s' in destination '%s'"
 const CSPFK023I string = "CSPFK023I Retrieving all available secrets from Conjur"
+const CSPFK024I string = "CSPFK024I Secrets Provider set to retrieve Kubernetes secrets by label"

pkg/secrets/config/config.go

@@ -198,7 +198,8 @@ func ValidateSecretsProviderSettings(envAndAnnots map[string]string) ([]error, [
 	annotK8sSecretsStr := envAndAnnots[k8sSecretsKey]
 	if storeType == "k8s_secrets" {
 		if envK8sSecretsStr == "" && annotK8sSecretsStr == "" {
-			errorList = append(errorList, errors.New(messages.CSPFK048E))
+			// This is no longer considered an error, but rather a signal to use the label-based K8s Secrets mode
+			infoList = append(infoList, fmt.Errorf(messages.CSPFK024I))
 		} else if envK8sSecretsStr != "" && annotK8sSecretsStr != "" {
 			infoList = append(infoList, fmt.Errorf(messages.CSPFK012I, "RequiredK8sSecrets", "K8S_SECRETS", k8sSecretsKey))
 		}
@@ -245,7 +246,10 @@ func NewConfig(settings map[string]string) *Config {
 		} else {
 			k8sSecretsStr = settings["K8S_SECRETS"]
 			k8sSecretsStr = strings.ReplaceAll(k8sSecretsStr, " ", "")
-			k8sSecretsArr = strings.Split(k8sSecretsStr, ",")
+			// Only split if the string is not empty to avoid creating a slice with one empty element
+			if k8sSecretsStr != "" {
+				k8sSecretsArr = strings.Split(k8sSecretsStr, ",")
+			}
 		}
 	}
 

pkg/secrets/config/config_test.go

@@ -299,21 +299,21 @@ var validateSecretsProviderSettingsTestCases = []validateSecretsProviderSettings
 		assert: assertErrorInList(fmt.Errorf(messages.CSPFK043E, SecretsDestinationKey, "invalid", []string{File, K8s})),
 	},
 	{
-		description: "if RequiredK8sSecrets is not configured in K8s Secrets mode, an error is returned",
+		description: "if RequiredK8sSecrets is not configured in K8s Secrets mode, an info-level message is returned (indicating label-based mode)",
 		envAndAnnots: map[string]string{
 			"MY_POD_NAMESPACE":    "test-namespace",
 			SecretsDestinationKey: "k8s_secrets",
 		},
-		assert: assertErrorInList(errors.New(messages.CSPFK048E)),
+		assert: assertInfoInList(fmt.Errorf(messages.CSPFK024I)),
 	},
 	{
-		description: "if RequiredK8sSecrets is set to a null string in K8s Secrets mode, an error is returned",
+		description: "if RequiredK8sSecrets is set to a null string in K8s Secrets mode, an info-level message is returned (indicating label-based mode)",
 		envAndAnnots: map[string]string{
 			"MY_POD_NAMESPACE":    "test-namespace",
 			"SECRETS_DESTINATION": "k8s_secrets",
 			"K8S_SECRETS":         "",
 		},
-		assert: assertErrorInList(errors.New(messages.CSPFK048E)),
+		assert: assertInfoInList(fmt.Errorf(messages.CSPFK024I)),
 	},
 	{
 		description: "if envVar 'SECRETS_DESTINATION' is malformed in the absence of annotation 'conjur.org/secrets-destination', an error is returned",

pkg/secrets/k8s_secrets_storage/provide_conjur_secrets.go

@@ -159,6 +159,12 @@ func (p *K8sProvider) Provide() (bool, error) {
 		return false, p.log.recordedError(messages.CSPFK021E)
 	}
 
+	// In label-based mode with no secrets discovered, return gracefully
+	if len(p.requiredK8sSecrets) == 0 {
+		p.log.warn(messages.CSPFK070E)
+		return false, nil
+	}
+
 	// Retrieve Conjur secrets for all K8s Secrets.
 	var updated bool
 	retrievedConjurSecrets, err := p.retrieveConjurSecrets(tr)
@@ -219,14 +225,20 @@ func (p *K8sProvider) retrieveRequiredK8sSecrets(tracer trace.Tracer) error {
 	spanCtx, span := tracer.Start(p.traceContext, "Gather required K8s Secrets")
 	defer span.End()
 
-	for _, k8sSecretName := range p.requiredK8sSecrets {
-		_, childSpan := tracer.Start(spanCtx, "Retrieve K8s Secret")
-		defer childSpan.End()
-		if err := p.retrieveRequiredK8sSecret(k8sSecretName); err != nil {
-			childSpan.RecordErrorAndSetStatus(err)
-			span.RecordErrorAndSetStatus(err)
-			return err
+	if len(p.requiredK8sSecrets) > 0 {
+		// Default behavior - pre-configured secrets
+		for _, k8sSecretName := range p.requiredK8sSecrets {
+			_, childSpan := tracer.Start(spanCtx, "Retrieve K8s Secret")
+			defer childSpan.End()
+			if err := p.retrieveRequiredK8sSecret(k8sSecretName); err != nil {
+				childSpan.RecordErrorAndSetStatus(err)
+				span.RecordErrorAndSetStatus(err)
+				return err
+			}
 		}
+	} else {
+		// TODO: Check for labeled secrets
+		return nil
 	}
 	return nil
 }

pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go

@@ -142,6 +142,13 @@ func assertErrorLogged(msg string, args ...interface{}) assertFunc {
 	}
 }
 
+func assertNoErrorLogged() assertFunc {
+	return func(t *testing.T, mocks testMocks, updated bool, err error, desc string) {
+		// Check that no errors were logged by seeing if ErrorWasLogged returns false for any common error
+		assert.False(t, mocks.logger.ErrorWasLogged(""), desc+": should have no error logs")
+	}
+}
+
 func assertLogged(expected bool, logLevel string, msg string, args ...interface{}) assertFunc {
 	return func(t *testing.T, mocks testMocks, updated bool, err error, desc string) {
 		logStr := fmt.Sprintf(msg, args...)
@@ -814,6 +821,18 @@ func TestProvide(t *testing.T) {
 				assertErrorContains(fmt.Sprintf(messages.CSPFK034E, "no variables to retrieve"), false),
 			},
 		},
+		{
+			desc:            "Label-based mode with no pre-configured secrets returns gracefully",
+			k8sSecrets:      k8sStorageMocks.K8sSecrets{},
+			requiredSecrets: []string{},
+			asserts: []assertFunc{
+				assertNoErrorLogged(),
+				func(t *testing.T, mocks testMocks, updated bool, err error, desc string) {
+					assert.NoError(t, err, desc+": should not error")
+					assert.False(t, updated, desc+": should not have updates")
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1170,28 +1189,28 @@ func TestProvideSanitization(t *testing.T) {
 }
 
 func TestBase64PKCS12SecretPreservesTrailingNull(t *testing.T) {
-    original := []byte{0xde, 0xad, 0xbe, 0xef, 0x00}
-    encoded := make([]byte, base64.StdEncoding.EncodedLen(len(original)))
-    base64.StdEncoding.Encode(encoded, original)
+	original := []byte{0xde, 0xad, 0xbe, 0xef, 0x00}
+	encoded := make([]byte, base64.StdEncoding.EncodedLen(len(original)))
+	base64.StdEncoding.Encode(encoded, original)
 
-    provider := K8sProvider{
-        secretsState: k8sSecretsState{
-            updateDestinations: map[string][]updateDestination{
-                "pkcs12var": {{
-                    k8sSecretName: "pkcs12secret",
-                    secretName:    "pkcs12file",
-                    contentType:   "base64",
-                }},
-            },
-        },
-    }
+	provider := K8sProvider{
+		secretsState: k8sSecretsState{
+			updateDestinations: map[string][]updateDestination{
+				"pkcs12var": {{
+					k8sSecretName: "pkcs12secret",
+					secretName:    "pkcs12file",
+					contentType:   "base64",
+				}},
+			},
+		},
+	}
 
-    conjurSecrets := map[string][]byte{
-        "pkcs12var": encoded,
-    }
+	conjurSecrets := map[string][]byte{
+		"pkcs12var": encoded,
+	}
 
-    secretData := provider.createSecretData(conjurSecrets)
-    got := secretData["pkcs12secret"]["pkcs12file"]
+	secretData := provider.createSecretData(conjurSecrets)
+	got := secretData["pkcs12secret"]["pkcs12file"]
 
-    assert.Equal(t, original, got, "decoded PKCS#12 secret should match original including trailing null bytes")
-}
+	assert.Equal(t, original, got, "decoded PKCS#12 secret should match original including trailing null bytes")
+}