Adds json struct tags to mirror yaml ones#297
Conversation
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
|
Please add the marshal json to hide the secrets. |
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
|
Before I get fancy and try to align the underlying json/yaml {un,}marshaling code, I pushed up some support for synchronizing secret, url, and http config validations. Let me know if I should continue or if this is sufficient, thanks. |
|
|
||
| // MarshalJSON implements the json.Marshaler interface for Secret. | ||
| func (s Secret) MarshalJSON() ([]byte, error) { | ||
| return json.Marshal(secretToken) |
There was a problem hiding this comment.
Can we use the same as in marshal yaml, return nil when empty?
There was a problem hiding this comment.
I originally copied this from Alertmanager (although these types should probably be defined solely in common and imported there) and wanted to keep parity with the implementation there.
As for returning nil, we cannot actually do this as it's unsupported for the MarshalJSON method, see example
I can, however, replace it with something like the following if you think it's better to show "" for an empty secret when the field is not specified as omitempty.
// MarshalJSON implements the json.Marshaler interface for Secret.
func (s Secret) MarshalJSON() ([]byte, error) {
if len(s) == 0 {
return json.Marshal("")
}
return json.Marshal(secretToken)
}
How would you like me to proceed?
There was a problem hiding this comment.
marshalling to "" seems OK
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" |
There was a problem hiding this comment.
let's not depend on testify here, even for tests. Thanks.
Signed-off-by: Owen Diehl <ow.diehl@gmail.com>
|
Thanks! |
This PR includes json struct tags to mirror the yaml ones in config objects. This allows downstream projects to consume these with a consistent key structure regardless of format. I'm hoping this isn't too controversial :).
For context: I'd like to consolidate these so Grafana can more easily present a JSON API over the Alertmanager structs and seamlessly transition json<>yaml. The lack of tag synchronization combined with the
<secret>redaction in marshaling code makes this a bit more difficult than I'd like.Interestingly, this has already been done with Alertmanager but not with Prometheus.
Please let me know if this is reasonable, and if so, if you'd like me to add some marshaling testware.
/cc @roidelapluie