template: Indicate which configs contain secret data#2286
template: Indicate which configs contain secret data#2286diogomonica merged 1 commit intomoby:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2286 +/- ##
==========================================
+ Coverage 60.11% 60.25% +0.13%
==========================================
Files 128 128
Lines 25891 25895 +4
==========================================
+ Hits 15565 15603 +38
+ Misses 8947 8920 -27
+ Partials 1379 1372 -7 |
|
This seems like an ok solution. We're doing this to avoid not doing tmps for all configs. |
cyli
left a comment
There was a problem hiding this comment.
Minor nitpicks, but other than that LGTM!
| } | ||
|
|
||
| func (ctx PayloadContext) secretGetter(target string) (string, error) { | ||
| func (ctx *PayloadContext) secretGetter(target string) (string, error) { |
There was a problem hiding this comment.
Non-blocking nitpick: If this is a pointer receiver, should all the other methods on PayloadContext also be pointer receivers as per https://golang.org/doc/faq#methods_on_values_or_pointers's point about consistency?
There was a problem hiding this comment.
I hadn't seen this recommendation before. I've changed all the methods to use pointer receivers.
| return &configCopy, nil | ||
| } | ||
|
|
||
| func (t templatedConfigGetter) GetAndFlagSecretData(configID string) (*api.Config, bool, error) { |
There was a problem hiding this comment.
Non-blocking: Would it make sense to basically replace the Get implementation with a call to GetAndFlagSecretData, and just leave out the flag in the return value? Since they otherwise seem to be exactly the same function.
01aa80e to
9a750dc
Compare
This changes the templated ConfigGetter to include an extra method, GetAndFlagSecretData, which returns an interpolated config and also a flag indicating whether it contains any data from a secret. This can be used downstream to avoid storing the config to disk. Ideally, there would be some kind of Sensitive method on a Config object itself, but since that type is automatically generated by protoc, including a private field is awkward to do. Thus this approach of returning the flag along with the config. Note that I opted for a method that returns the config and the flag at the same time instead of a method that just returns the flag, since a method that returns the flag still needs to expand the config, and it seems most robust and performant to just return the config and the flag at the same time instead of relying on different code paths to generate consistent results. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
9a750dc to
4f8fb2b
Compare
|
ping @diogomonica |
This changes the templated ConfigGetter to include an extra method,
GetAndFlagSecretData, which returns an interpolated config and also a
flag indicating whether it contains any data from a secret. This can be
used downstream to avoid storing the config to disk.
Ideally, there would be some kind of Sensitive method on a Config object
itself, but since that type is automatically generated by protoc,
including a private field is awkward to do. Thus this approach of
returning the flag along with the config. Note that I opted for a method
that returns the config and the flag at the same time instead of a
method that just returns the flag, since a method that returns the flag
still needs to expand the config, and it seems most robust and
performant to just return the config and the flag at the same time
instead of relying on different code paths to generate consistent
results.
cc @diogomonica @cyli