Skip to content
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.

Sign up for GitHub

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

Jump to bottom

adding sidecarEnvJson annotation #726

Merged
merged 4 commits into from
Sep 11, 2023
Merged

adding sidecarEnvJson annotation #726

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e41bbf0
adding sidecarEnvJson annotation
dileepng Aug 30, 2023
8a7a639
adding customSidecarEnvJson annotation
dileepng Aug 30, 2023
349aa72
adding sidecarEnvJson annotation
dileepng Sep 11, 2023
1502972
Merge branch 'master' into master
dileepng Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pkg/inject/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ const (
//
AppMeshEnvAnnotation = "appmesh.k8s.aws/sidecarEnv"

// AppMeshEnvJsonAnnotation which is similar AppMeshEnvAnnotation, but it is used to specify the list Jsons of environment variables that need to be programmed on Envoy sidecars
// Here's how a sample annotations will be like
//
// e.g. appmesh.k8s.aws/sidecarEnvJson: '[{"DD_ENV":"prod","TEST_ENV":"env_val"}]'
// e.g. appmesh.k8s.aws/sidecarEnvJson: '[{"DD_ENV":"prod"}]'
//
AppMeshEnvJsonAnnotation = "appmesh.k8s.aws/sidecarEnvJson"

// === begin xray daemon annotations ===

// AppMeshXrayAgentConfigAnnotation specifies the mount path for the Xray daemon's configuration file.
Expand Down
45 changes: 45 additions & 0 deletions pkg/inject/envoy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package inject

import (
"fmt"
"k8s.io/apimachinery/pkg/util/json"
"strconv"
"strings"

Expand Down Expand Up @@ -92,6 +93,15 @@ func (m *envoyMutator) mutate(pod *corev1.Pod) error {
return err
}

customEnvJson, err := m.getCustomEnvJson(pod)
if err != nil {
return err
}
if customEnvJson != nil {
for k, v := range customEnvJson {
customEnv[k] = v
}
}
container, err := buildEnvoySidecar(variables, customEnv)
if err != nil {
return err
Expand Down Expand Up @@ -249,6 +259,41 @@ func (m *envoyMutator) getCustomEnv(pod *corev1.Pod) (map[string]string, error)
return customEnv, nil
}

type customEnvJsonType map[string]string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this custom type still need to exist, or can getCustomEnvJson just deserialize to map[string]string directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


func (c *customEnvJsonType) UnmarshalJSON(data []byte) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need we make a valid check for bytes before unmarshal if the input is empty or nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will throw malformed error. so, I didn't make a check for that condition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Good to know

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

var temp []map[string]interface{}
if err := json.Unmarshal(data, &temp); err != nil {
return err
}
*c = make(customEnvJsonType)
for _, item := range temp {
for key, value := range item {
if strValue, isString := value.(string); isString {
(*c)[key] = strValue
} else {
return errors.Errorf("nested json isn't supported with this annotation %s, expected format: %s", AppMeshEnvJsonAnnotation, `[{"DD_ENV":"prod","TEST_ENV":"env_val"}]`)
}
}
}
return nil
}

func (m *envoyMutator) getCustomEnvJson(pod *corev1.Pod) (map[string]string, error) {
var customEnvJson customEnvJsonType
if v, ok := pod.ObjectMeta.Annotations[AppMeshEnvJsonAnnotation]; ok {
err := json.Unmarshal([]byte(v), &customEnvJson)
if err != nil {
if strings.Contains(err.Error(), "nested json") {
return nil, err
}
err = errors.Errorf("malformed annotation %s, expected format: %s", AppMeshEnvJsonAnnotation, `[{"DD_ENV":"prod","TEST_ENV":"env_val"}]`)
return nil, err
}
}
return customEnvJson, nil
}

func (m *envoyMutator) mutateVolumeMounts(pod *corev1.Pod, envoyContainer *corev1.Container, volumeMounts map[string]string) {
for volumeName, mountPath := range volumeMounts {
volumeMount := corev1.VolumeMount{
Expand Down
92 changes: 92 additions & 0 deletions pkg/inject/envoy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3510,6 +3510,98 @@ func Test_envoyMutator_getCustomEnv(t *testing.T) {
})
}
}
func Test_envoyMutator_getCustomEnvJson(t *testing.T) {
type args struct {
pod *corev1.Pod
}
tests := []struct {
name string
args args
want map[string]string
wantErr error
}{
{
name: "pods with valid appmesh.k8s.aws/sidecarEnvJson annotation",
args: args{
pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"appmesh.k8s.aws/sidecarEnvJson": `[{"DD_ENV":"prod","TEST_ENV":"env_val"}]`,
},
},
},
},
want: map[string]string{
"DD_ENV": "prod",
"TEST_ENV": "env_val",
},
wantErr: nil,
},
{
name: "pods with no appmesh.k8s.aws/sidecarEnvJson annotation",
args: args{
pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{},
},
},
},
want: nil,
wantErr: nil,
},
{
name: "pods with invalid appmesh.k8s.aws/sidecarEnvJson annotation",
args: args{
pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"appmesh.k8s.aws/sidecarEnvJson": `[{"DD_ENV":"{PROD_ENV: "prod"}","TEST_ENV":"env_val"}]`,
},
},
},
},
wantErr: errors.New("malformed annotation appmesh.k8s.aws/sidecarEnvJson, expected format: [{\"DD_ENV\":\"prod\",\"TEST_ENV\":\"env_val\"}]"),
},
{
name: "pods with Nested Json appmesh.k8s.aws/sidecarEnvJson annotation",
args: args{
pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"appmesh.k8s.aws/sidecarEnvJson": `[{"DD_ENV":{"PROD_ENV": "prod"},"TEST_ENV":"env_val"}]`,
},
},
},
},
wantErr: errors.New("nested json isn't supported with this annotation appmesh.k8s.aws/sidecarEnvJson, expected format: [{\"DD_ENV\":\"prod\",\"TEST_ENV\":\"env_val\"}]"),
},
{
name: "pods with no_input appmesh.k8s.aws/sidecarEnvJson annotation",
args: args{
pod: &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"appmesh.k8s.aws/sidecarEnvJson": ``,
},
},
},
},
wantErr: errors.New("malformed annotation appmesh.k8s.aws/sidecarEnvJson, expected format: [{\"DD_ENV\":\"prod\",\"TEST_ENV\":\"env_val\"}]"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := &envoyMutator{}
got, err := m.getCustomEnvJson(tt.args.pod)
if tt.wantErr != nil {
assert.EqualError(t, err, tt.wantErr.Error())
} else {
assert.NoError(t, err)
assert.Equal(t, got, tt.want)
}
})
}
}

func Test_envoyMutator_getAugmentedMeshName(t *testing.T) {
type fields struct {
Expand Down
Loading