adding sidecarEnvJson annotation #726
Conversation
pkg/inject/envoy.go
Outdated
| @@ -249,6 +259,45 @@ func (m *envoyMutator) getCustomEnv(pod *corev1.Pod) (map[string]string, error) | |||
| return customEnv, nil | |||
| } | |||
|
|
|||
| type customEnvJsonType []map[string]string | |||
|
|
|||
| func (c *customEnvJsonType) UnmarshalJSON(data []byte) error { | |||
There was a problem hiding this comment.
Need we make a valid check for bytes before unmarshal if the input is empty or nil?
There was a problem hiding this comment.
It will throw malformed error. so, I didn't make a check for that condition.
There was a problem hiding this comment.
Sort of a sidenote, but a nil array is functionally identical to a empty array in Go, so you generally never need to check for nil arrays. (the same is not always true for maps)
pkg/inject/envoy.go
Outdated
| @@ -249,6 +259,45 @@ func (m *envoyMutator) getCustomEnv(pod *corev1.Pod) (map[string]string, error) | |||
| return customEnv, nil | |||
| } | |||
|
|
|||
| type customEnvJsonType []map[string]string | |||
There was a problem hiding this comment.
why are you not just using a map[string]string?
There was a problem hiding this comment.
Initially, I thought of returning Array of maps, then I changed to returning only one map. I will change the logic.
pkg/inject/envoy.go
Outdated
| @@ -249,6 +259,41 @@ func (m *envoyMutator) getCustomEnv(pod *corev1.Pod) (map[string]string, error) | |||
| return customEnv, nil | |||
| } | |||
|
|
|||
| type customEnvJsonType map[string]string | |||
There was a problem hiding this comment.
Does this custom type still need to exist, or can getCustomEnvJson just deserialize to map[string]string directly?
There was a problem hiding this comment.
I think, it can't be deserialize directly to map[string]string.
we need to have a defined type for methods. Here this method needs some type to a variable to store the deserialized data.
There was a problem hiding this comment.
The only place I see this type used is in the getCustomEnvJson function, which immediately unwraps it to a map[string]string.
Generally, you only need to some serialization-time changes between the representative data type and the target data type (for instance, when converting strings to dates). I think that might have been true of the first draft of this where the input data was a list of strings, but now that it's just a map of string->string it's the same as what the target type is. Just passing map[string]string to unmarshal should accomplish this fine without the indirection.
Issue #, if available:
#724
Description of changes:
Adding Feature appmesh.k8s.aws/sidecarEnvJson that accepts json list as env for envoy.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.