Design updates based on questions/feedback

design/secrets_decoding_design.md

@@ -24,9 +24,11 @@
   * [Tasks](#tasks)
     + [Add a struct for storing content-types of individual Conjur secrets (K8s Secrets)](#add-a-struct-for-storing-content-types-of-individual-conjur-secrets--k8s-secrets-)
     + [Add/update a struct for storing content-types of individual Conjur secrets (P2F)](#add-update-a-struct-for-storing-content-types-of-individual-conjur-secrets--p2f-)
-    + [Add decoding of secrets with 'base64' content-types](#add-decoding-of-secrets-with--base64--content-types)
-    + [Update e2e tests and dev environment scripts/manifests](#update-e2e-tests-and-dev-environment-scripts-manifests)
+    + [Secrets with 'base64' content-types are decoded (K8s Secrets)](#secrets-with--base64--content-types-are-decoded--k8s-secrets-)
+    + [Secrets with 'base64' content-types are decoded (P2F)](#secrets-with--base64--content-types-are-decoded--p2f-)
+    + [Add e2e secrets decoding test and update manifests for the following cases:](#add-e2e-secrets-decoding-test-and-update-manifests-for-the-following-cases-)
     + [Create/update docs](#create-update-docs)
+    + [Create artifact for tech writers](#create-artifact-for-tech-writers)
     + [Docs walkthrough](#docs-walkthrough)
 
 ## Useful links
@@ -96,6 +98,8 @@ conjur-map: |-
     content-type: <content_type_value>    
 ```
 
+In the case where multiple Kubernetes secrets reference the same Conjur secret, we should support a different content-type definition for each instance of the secret. i.e. 'k8s-secret-decode' can reference `path/to/somevar` with a content-type of base64, and `k8s-secret-dont-decode` can reference `path/to/somevar` with a content-type of text, and each K8s secret will be updated with the correct decoded or non-decoded secret value.
+
 ### P2F Mode
 For P2F, we can update the existing `conjur.org/conjur-secrets.{secret-group}` annotations with the following allowed formats:
 ```
@@ -108,6 +112,8 @@ conjur.org/conjur-secrets.groupname: |
       content-type: <content_type_value>
 ```
 
+In the case where multiple secret groups reference the same Conjur secret, we should support a different content-type definition for each instance of the secret. i.e. `conjur.org/conjur-secrets.decode-this-group` can reference `path/to/somevar` with a content-type of base64, and `conjur.org/conjur-secrets.dont-decode-this-group` can reference `path/to/somevar` with a content-type of text, and each secret group's output file will be updated with the correct decoded or non-decoded secret value.
+
 ### Backwards compatibility
 This enhancement will not modify the default behavior of Secrets Provider. It will continue to assume that secrets should be delivered "as-is" unless it specifically finds a content-type annotation which indicates that the secret value should be decoded.
 
@@ -173,35 +179,63 @@ Unit tests for any helper functions added to assist with decoding secrets.
 Update integration tests in both [K8s secrets mode](https://github.com/cyberark/secrets-provider-for-k8s/tree/main/pkg/secrets/k8s_secrets_storage/provide_conjur_secrets_test.go) and [P2F mode](https://github.com/cyberark/secrets-provider-for-k8s/tree/main/pkg/secrets/pushtofile/provide_conjur_secrets_test.go) to validate that an encoded variable with base64 content-type is decoded.
 
 ### e2e Tests
-At least one e2e test should be added for validating that an encoded secret with valid annotations is provided and decoded to match the expected value.
+In addition to the unit/integration tests, we likely will want five e2e tests to cover the following cases with secrets decoding:
+* K8s Secrets
+* K8s Secrets with rotation
+* Push-to-file
+* Push-to-file with rotation
+* Decoding secrets > 65k characters
 
 ### Audit 
-The solution should produce an audit entry for each secret that was decoded as a result of the secrets configuration.
+The solution should produce an audit entry for each secret that was decoded as a result of the secrets configuration. It should also log errors/warnings on the following conditions:
+
+Failure on configuration:
+  - Malformed secret (alias/path)
+    - Log YAML parsing error
+    - Secrets Provider fails to start
+  - Malformed content-type
+    - If YAML can still be parsed, create a secret with default content type (text)
+    - Log (warning)
+
+Failure on providing secrets:
+- Fail to decode an encoded secret (invalid base64)
+   - Log (warning)
+   - Provide original value
+   - Secrets provider continues to run
 
 ## Documentation
 Since this feature is activated via the manifest, it would be reasonable to add a note to any sample manifests in existing documentation about the optional 'content-type' field for secret annotations. Both K8s Secrets and P2F documentation should have a brief mention of where and how configuration is needed to activate the feature respectively.
 
 ## Open questions
-* Conjur variables support a mime-type attribute and custom annotations. See: [Conjur policy variable attributes](https://docs.cyberark.com/Product-Doc/OnlineHelp/AAM-DAP/12.4/en/Content/Operations/Policy/statement-ref-variable.htm#Attributes). Either of these could potentially be used to determine that an individual Conjur secret is encoded. This likely wouldn't be compatible with the current implementation using Conjur API's batch retrieval endpoint, but it would allow the decoding to be handled in the API instead of Secrets Provider. Could also have compatility issues with secrets that come from synchronizer and/or require manual updating of secret metadata in Conjur.
+* (Out of scope) Conjur variables support a mime-type attribute and custom annotations. See: [Conjur policy variable attributes](https://docs.cyberark.com/Product-Doc/OnlineHelp/AAM-DAP/12.4/en/Content/Operations/Policy/statement-ref-variable.htm#Attributes). Either of these could be used to determine that an individual Conjur secret is encoded. This likely wouldn't be compatible with the current implementation using Conjur API's batch retrieval endpoint, but it would allow the decoding to be handled in the API instead of Secrets Provider. Could also have compatility issues with secrets that come from synchronizer and/or require manual updating of secret metadata in Conjur.
 
 
 ## Tasks
 * ### Add a struct for storing content-types of individual Conjur secrets (K8s Secrets)
-  * Add marshal/unmarshal logic to correctly parse YAML annotations into the new struct.
-  * Add/update tests for the new struct to include the expected content-type of secrets.
+  * Updated K8sProvider config which can store content-type of secrets
+  * Marshal/unmarshal logic to correctly parse new YAML annotations
+  * Add/update tests to check for the expected content-type of secrets
 * ### Add/update a struct for storing content-types of individual Conjur secrets (P2F)
-  * Can probably reuse the existing SecretSpec struct similar similar to the [POC branch](https://github.com/cyberark/secrets-provider-for-k8s/pull/502)
-  * Add marshal/unmarshal logic to correctly parse YAML annotations into the struct.
-  * Add/update tests for the struct to include the expected content-type of secrets.
-* ### Add decoding of secrets with 'base64' content-types
-  * Add logic to decode secrets based on content-type
-  * Likely want to handle this in or just beneath the Provide() function for each provider type (k8s_secrets and p2f)
-  * Unit tests for any helper functions added here
+  * Updated P2FProviderConfig which can store content-type of secrets for each secret group
+  * Marshal/unmarshal logic to correctly parse new YAML annotations
+  * Add/update tests to check for the expected content-type of secrets
+* ### Secrets with 'base64' content-types are decoded (K8s Secrets)
+  * Decode secrets based on content-type
+  * Update integration tests to include decoded variables and expected values
+  * Update dev environments to allow easy demonstration of base64 decoding
+* ### Secrets with 'base64' content-types are decoded (P2F)
+  * Decode secrets based on content-type
   * Update integration tests to include decoded variables and expected values
-* ### Update e2e tests and dev environment scripts/manifests
-  * We should have at least one e2e test verifying a secret being decoded via this feature
-  * Dev environments for both K8s secrets and P2F mode should include a decoded secret example
+  * Update dev environments to allow easy demonstration of base64 decoding
+* ### Add e2e secrets decoding test and update manifests for the following cases:
+  * K8s Secrets
+  * K8s Secrets with rotation
+  * Push-to-file
+  * Push-to-file with rotation
+  * Decoding secrets > 65k characters
 * ### Create/update docs
-  * Update existing docs and sample manifests where appropriate
+  * Update existing docs and sample manifests where appropriate.
+* ### Create artifact for tech writers
+  * Create doc artifact to hand over to TW for updating the docs.
 * ### Docs walkthrough
   * Manual UX testing