Use centralized environemnt variable + label getting function

Author: danehlim

agent/api/task/task.go

@@ -166,7 +166,7 @@ const (
 
 	ipv6LoopbackAddress = "::1"
 
-	PAUSE_LABELS_ENV_VAR = "ECS_PAUSE_LABELS"
+	pauseLabelsEnvVar = "ECS_PAUSE_LABELS"
 )
 
 // TaskOverrides are the overrides applied to a task
@@ -1817,7 +1817,7 @@ func (task *Task) dockerConfig(container *apicontainer.Container, apiVersion doc
 
 	switch container.Type {
 	case apicontainer.ContainerCNIPause, apicontainer.ContainerNamespacePause:
-		if pauseLabels := os.Getenv(PAUSE_LABELS_ENV_VAR); pauseLabels != "" {
+		if pauseLabels := os.Getenv(pauseLabelsEnvVar); pauseLabels != "" {
 			// Set labels to pause container if it's provieded as env var.
 			setLabelsFromJsonString(containerConfig, pauseLabels)
 		}
@@ -1834,7 +1834,7 @@ func (task *Task) dockerConfig(container *apicontainer.Container, apiVersion doc
 // Parse label string and set them to the given container configuration.
 func setLabelsFromJsonString(config *dockercontainer.Config, labelsString string) {
 	if len(labelsString) > 0 {
-		labels, err := toLabelMap(labelsString)
+		labels, err := commonutils.JsonBlockToStringToStringMap(labelsString)
 		if err != nil {
 			log.Errorf("Skipped setting labels because of failed to decode. Error: %s", err)
 			return
@@ -1845,12 +1845,6 @@ func setLabelsFromJsonString(config *dockercontainer.Config, labelsString string
 	}
 }
 
-func toLabelMap(jsonBlock string) (map[string]string, error) {
-	out := map[string]string{}
-	err := json.Unmarshal([]byte(jsonBlock), &out)
-	return out, err
-}
-
 // dockerExposedPorts returns the container ports that need to be exposed for a container
 //  1. For bridge-mode ServiceConnect-enabled tasks:
 //     1a. Pause containers need to expose the port(s) for their associated task container. In particular, SC pause container

agent/api/task/task_test.go

@@ -5959,7 +5959,7 @@ func TestDockerConfigPauseContainerLabelsWithEnvVar_ECS_PAUSE_LABELS(t *testing.
 		},
 	}
 	labelsString := "{\"test.label.1\":\"test_a\",\"test.label.2\":\"test_b\"}"
-	t.Setenv("ECS_PAUSE_LABELS", labelsString)
+	t.Setenv(pauseLabelsEnvVar, labelsString)
 
 	config, configErr := testTask.DockerConfig(testTask.Containers[0], defaultDockerClientAPIVersion)
 	if configErr != nil {
@@ -5984,7 +5984,7 @@ func TestDockerConfigNamespacePauseContainerLabelsWithEnvVar_ECS_PAUSE_LABELS(t
 		},
 	}
 	labelsString := "{\"test.label.1\":\"test_a\",\"test.label.2\":\"test_b\"}"
-	t.Setenv("ECS_PAUSE_LABELS", labelsString)
+	t.Setenv(pauseLabelsEnvVar, labelsString)
 
 	config, configErr := testTask.DockerConfig(testTask.Containers[0], defaultDockerClientAPIVersion)
 	if configErr != nil {
@@ -6010,7 +6010,7 @@ func TestDockerConfigPauseContainerLabelsWithInvalidEnvVar_ECS_PAUSE_LABELS(t *t
 	}
 	// Invalid format.
 	labelsString := "{\"test.label\":\"test\""
-	t.Setenv("ECS_PAUSE_LABELS", labelsString)
+	t.Setenv(pauseLabelsEnvVar, labelsString)
 
 	config, configErr := testTask.DockerConfig(testTask.Containers[0], defaultDockerClientAPIVersion)
 	if configErr != nil {

agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/utils/utils.go

@@ -13,6 +13,7 @@
 package utils
 
 import (
+	"encoding/json"
 	"math"
 	"reflect"
 	"regexp"
@@ -158,3 +159,10 @@ func ToPtrSlice[V any](xs []V) []*V {
 	}
 	return result
 }
+
+// JsonBlockToStringToStringMap converts a JSON block string to a string-to-string map.
+func JsonBlockToStringToStringMap(jsonBlock string) (map[string]string, error) {
+	out := map[string]string{}
+	err := json.Unmarshal([]byte(jsonBlock), &out)
+	return out, err
+}

ecs-agent/utils/utils.go

@@ -393,3 +393,20 @@ func TestToPtrSlice(t *testing.T) {
 		}
 	})
 }
+
+func TestJsonBlockToStringToStringMap(t *testing.T) {
+	testData := `{"test.label.1":"value1","test.label.2":"value2"}`
+	out, err := JsonBlockToStringToStringMap(testData)
+	require.NoError(t, err, "Expected no error while decoding JSON block")
+	require.Equal(t, "value1", out["test.label.1"])
+
+	for key, value := range out {
+		t.Logf("Key: %s %T | Value: %s %T", key, key, value, value)
+	}
+}
+
+func TestJsonBlockToStringToStringMapBadData(t *testing.T) {
+	testData := `{"something":[{"test.label.1":"value1"},{"test.label.2":"value2"}]}`
+	_, err := JsonBlockToStringToStringMap(testData)
+	require.Error(t, err, "Expected an error on badly formatted data as input")
+}

ecs-agent/utils/utils_test.go

@@ -15,7 +15,6 @@ package docker
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"io"
 	"os"
@@ -25,6 +24,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/aws/amazon-ecs-agent/ecs-agent/utils"
 	"github.com/aws/amazon-ecs-agent/ecs-init/backoff"
 	"github.com/aws/amazon-ecs-agent/ecs-init/config"
 	"github.com/aws/amazon-ecs-agent/ecs-init/gpu"
@@ -384,7 +384,7 @@ func (c *client) getContainerConfig(envVarsFromFiles map[string]string) *godocke
 func setLabels(cfg *godocker.Config, labelsStringRaw string) {
 	// Is there labels to add?
 	if len(labelsStringRaw) > 0 {
-		labels, err := generateLabelMap(labelsStringRaw)
+		labels, err := utils.JsonBlockToStringToStringMap(labelsStringRaw)
 		if err != nil {
 			// Are the labels valid?
 			log.Errorf("Failed to decode the container labels, skipping labels. Error: %s", err)
@@ -455,12 +455,6 @@ func (c *client) getEnvVars(filename string) map[string]string {
 	return envVariables
 }
 
-func generateLabelMap(jsonBlock string) (map[string]string, error) {
-	out := map[string]string{}
-	err := json.Unmarshal([]byte(jsonBlock), &out)
-	return out, err
-}
-
 func (c *client) getHostConfig(envVarsFromFiles map[string]string) *godocker.HostConfig {
 	dockerSocketBind := getDockerSocketBind(envVarsFromFiles)
 

ecs-init/docker/docker.go

@@ -754,23 +754,6 @@ func TestStopAgent(t *testing.T) {
 	}
 }
 
-func TestContainerLabels(t *testing.T) {
-	testData := `{"test.label.1":"value1","test.label.2":"value2"}`
-	out, err := generateLabelMap(testData)
-	if err != nil {
-		t.Logf("Got an error while decoding labels, error: %s", err)
-		t.Fail()
-	}
-	if out["test.label.1"] != "value1" {
-		t.Logf("Label did not give the correct value out.")
-		t.Fail()
-	}
-
-	for key, value := range out {
-		t.Logf("Key: %s %T | Value: %s %T", key, key, value, value)
-	}
-}
-
 func TestContainerLabelsNoData(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -800,15 +783,6 @@ func TestContainerLabelsNoData(t *testing.T) {
 	}
 }
 
-func TestContainerLabelsBadData(t *testing.T) {
-	testData := `{"something":[{"test.label.1":"value1"},{"test.label.2":"value2"}]}`
-	_, err := generateLabelMap(testData)
-	if err == nil {
-		t.Logf("Didn't get a error while getting lables on badly formatted data, error: %s", err)
-		t.Fail()
-	}
-}
-
 func TestGetDockerSocketBind(t *testing.T) {
 	testCases := []struct {
 		name                     string

ecs-init/docker/docker_test.go

@@ -26,6 +26,7 @@ require (
 require (
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/Microsoft/go-winio v0.6.2 // indirect
+	github.com/aws/aws-sdk-go v1.55.7 // indirect
 	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.5 // indirect
 	github.com/aws/aws-sdk-go-v2/credentials v1.17.42 // indirect
 	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect
@@ -46,6 +47,7 @@ require (
 	github.com/docker/go-connections v0.4.0 // indirect
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
+	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/klauspost/compress v1.16.7 // indirect
 	github.com/kr/text v0.2.0 // indirect
 	github.com/moby/patternmatcher v0.6.0 // indirect
@@ -60,6 +62,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
+	golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
 	golang.org/x/net v0.38.0 // indirect
 	golang.org/x/sys v0.31.0 // indirect
 	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect

ecs-init/go.mod

@@ -8,6 +8,8 @@ github.com/Microsoft/hcsshim v0.12.0 h1:rbICA+XZFwrBef2Odk++0LjFvClNCJGRK+fsrP25
 github.com/Microsoft/hcsshim v0.12.0/go.mod h1:RZV12pcHCXQ42XnlQ3pz6FZfmrC1C+R4gaOHhRNML1g=
 github.com/NVIDIA/go-nvml v0.12.4-0 h1:4tkbB3pT1O77JGr0gQ6uD8FrsUPqP1A/EOEm2wI1TUg=
 github.com/NVIDIA/go-nvml v0.12.4-0/go.mod h1:8Llmj+1Rr+9VGGwZuRer5N/aCjxGuR5nPb/9ebBiIEQ=
+github.com/aws/aws-sdk-go v1.55.7 h1:UJrkFq7es5CShfBwlWAC8DA077vp8PyVbQd3lqLiztE=
+github.com/aws/aws-sdk-go v1.55.7/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU=
 github.com/aws/aws-sdk-go-v2 v1.36.6 h1:zJqGjVbRdTPojeCGWn5IR5pbJwSQSBh5RWFTQcEQGdU=
 github.com/aws/aws-sdk-go-v2 v1.36.6/go.mod h1:EYrzvCCN9CMUTa5+6lf6MM4tq3Zjp8UhSGR/cBsjai0=
 github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.5 h1:xDAuZTn4IMm8o1LnBZvmrL8JA1io4o3YWNXgohbf20g=
@@ -79,6 +81,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af h1:pmfjZENx5imkbgOkpRUYLnmbU7UEFbjtDA2hxJ1ichM=
+github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.16.7 h1:2mk3MPGNzKyxErAw8YaohYh69+pa4sIQSC0fPGCFR9I=
@@ -128,6 +132,8 @@ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
+golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=