-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
DataSources is now common to Secrets and ConfigMaps. #714
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: monopole The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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
@@ -206,15 +206,15 @@ type GeneratorArgs struct { | |||
// 'replace': replace the existing one | |||
// 'merge': merge with the existing one | |||
Behavior string `json:"behavior,omitempty" yaml:"behavior,omitempty"` | |||
|
|||
// DataSources for the generator. | |||
DataSources `json:",inline,omitempty" yaml:",inline,omitempty"` | |||
} | |||
|
|||
// ConfigMapArgs contains the metadata of how to generate a configmap. | |||
type ConfigMapArgs struct { | |||
// GeneratorArgs for the configmap. | |||
GeneratorArgs `json:",inline,omitempty" yaml:",inline,omitempty"` |
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.
At the moment, I prefer this PR to a PR that does an even stronger coupling:
type SecretArgs {
ConfigMapArgs
Type string
}
but might change my mind.
The current PR draws a stronger distinction between the two types, for what that's worth,
leaving an obvious opening as to how they could further differ.
Maybe that option isn't worth much.
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 like the option for them to further differ. I'm really hoping we can find a way to get secret generation back in a way that is compatible with kubectl
Code reduction. Code still settling after #692