-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow P2F custom templates via ConfigMap #393
Conversation
8bb9ec0
to
cf9572a
Compare
19a36d8
to
727bc0d
Compare
@@ -297,16 +323,25 @@ func NewSecretGroups(secretsBasePath string, annotations map[string]string) ([]* | |||
return sgs, nil | |||
} | |||
|
|||
func newSecretGroup(groupName string, secretsBasePath string, annotations map[string]string) (*SecretGroup, []error) { | |||
func newSecretGroup(groupName string, annotations map[string]string, c Config) (*SecretGroup, []error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function newSecretGroup
has 5 return statements (exceeds 4 allowed).
if err != nil { | ||
return nil, []error{err} | ||
} | ||
|
||
return sg, nil | ||
} | ||
|
||
func collectTemplate(groupName string, annotations map[string]string, c Config) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function collectTemplate
has 5 return statements (exceeds 4 allowed).
727bc0d
to
be5cf8f
Compare
be5cf8f
to
6004b58
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!!! I'm thinking we should ignore the warnings about 5 returns in 2 functions. That seems too restrictive for Go.
I just have one question on whether UT covers custom template not being defined by either Annotation or ConfigMap.
}) | ||
}) | ||
|
||
t.Run("custom template file - annotation and configmap template provided", func(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this UT test case for custom template being provided by both annotation and configmap.
Is there a corresponding UT test case for secret file format being template
, but no custom template defined by either annotation or configmap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added a test case for the above context.
cd98d60
to
4d0dcf8
Compare
Code Climate has analyzed commit 4d0dcf8 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 94.2% (50% is the threshold). This pull request will bring the total coverage in the repository to 92.9% (0.1% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!! I don't know if there's a way to add a repository-specific CodeClimate config that would allow more then 4 returns for Golang functions. If not, I think we should just ignore the two instances of CodeClimate errors for 5 returns in a function.
Desired Outcome
From ONYX-14093:
Implemented Changes
Behavioral Changes
If a secret group's output file format, specified with the annotation
conjur.org/secret-file-format.{secret-group}
, is set totemplate
, then the functionnewSecretGroup
will look for either an annotation- or ConfigMap-based template string.The ConfigMap defining templates must be mounted to
/conjur/templates
, and the key for a given template value must be{secret-group}.tpl
. At secret group creation, Secrets Provider will only attempt to read from this file. If the file does not exist, it is assumed that a template was not provided via ConfigMap.If the annotation
conjur.org/secret-file-format.{secret-group}
is set totemplate
, and templates are provided through BOTH the group's annotation and volume-mounted ConfigMap, secret group creation will receive a failing error.If the annotation
conjur.org/secret-file-format.{secret-group}
is not set totemplate
, Secrets Provider will never look for a template in either form. If a template is provided by either/both method(s), it will be ignored.Explicit Changes
template
toconjur.org/secret-file-format.{secret-group}
accepted valuespkg/secrets/pushtofile/pull_from_reader.go
, a counterpart topush_to_writer.go
, to mock and dependency-inject reading content from the filesystem.secret_group.go
:func NewSecretGroups
into itself andfunc newSecretGroupsWithDeps
to dep-inject file reading operationscollectTemplate
andreadTemplateFromFile
PUSH_TO_FILE.md
updatesConnected Issue/Story
Resolves #[relevant GitHub issue(s), e.g. 76]
CyberArk internal issue link: ONYX-14903
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
README
s) were updated in this PRBehavior
Security