From 2e703467a752f0bf4e899e7954a5c019b2a53210 Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Wed, 25 Jul 2018 13:16:51 -0700 Subject: [PATCH 1/5] handlers/v1: add json tags to response structs --- agent/handlers/v1/response.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/handlers/v1/response.go b/agent/handlers/v1/response.go index 9ec79acc845..94125cd5864 100644 --- a/agent/handlers/v1/response.go +++ b/agent/handlers/v1/response.go @@ -46,7 +46,7 @@ type TasksResponse struct { // ContainerResponse is the schema for the container response JSON object type ContainerResponse struct { - DockerID string + DockerID string `json:"DockerId"` DockerName string Name string Ports []PortResponse `json:",omitempty"` From b041a7808341e16a200ecb70d7d40bc4de25d6c6 Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Wed, 25 Jul 2018 17:34:14 -0700 Subject: [PATCH 2/5] test: add unit tests for v1 handler --- agent/handlers/v1/response_test.go | 171 +++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 agent/handlers/v1/response_test.go diff --git a/agent/handlers/v1/response_test.go b/agent/handlers/v1/response_test.go new file mode 100644 index 00000000000..b955452f816 --- /dev/null +++ b/agent/handlers/v1/response_test.go @@ -0,0 +1,171 @@ +// Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package v1 + +import ( + "encoding/json" + "testing" + + apicontainer "github.com/aws/amazon-ecs-agent/agent/api/container" + apieni "github.com/aws/amazon-ecs-agent/agent/api/eni" + apitask "github.com/aws/amazon-ecs-agent/agent/api/task" + apitaskstatus "github.com/aws/amazon-ecs-agent/agent/api/task/status" + "github.com/stretchr/testify/assert" +) + +const ( + taskARN = "t1" + family = "sleep" + version = "1" + containerID = "cid" + containerName = "sleepy" + eniIPv4Address = "10.0.0.2" +) + +func TestTaskResponse(t *testing.T) { + expectedTaskResponseJSONString := + `{` + + `"Arn":"t1",` + + `"DesiredStatus":"RUNNING",` + + `"KnownStatus":"RUNNING",` + + `"Family":"sleep",` + + `"Version":"1",` + + `"Containers":[{` + + `"DockerId":"cid",` + + `"DockerName":"sleepy",` + + `"Name":"sleepy",` + + `"Ports":[{` + + `"ContainerPort":80,` + + `"Protocol":"tcp",` + + `"HostPort":80` + + `}],` + + `"Networks":[{` + + `"NetworkMode":"awsvpc",` + + `"IPv4Addresses":["10.0.0.2"]` + + `}]` + + `}]` + + `}` + + task := &apitask.Task{ + Arn: taskARN, + Family: family, + Version: version, + DesiredStatusUnsafe: apitaskstatus.TaskRunning, + KnownStatusUnsafe: apitaskstatus.TaskRunning, + ENI: &apieni.ENI{ + IPV4Addresses: []*apieni.ENIIPV4Address{ + { + Address: eniIPv4Address, + }, + }, + }, + } + + container := &apicontainer.Container{ + Name: containerName, + Ports: []apicontainer.PortBinding{ + { + ContainerPort: 80, + Protocol: apicontainer.TransportProtocolTCP, + }, + }, + } + + containerNameToDockerContainer := map[string]*apicontainer.DockerContainer{ + taskARN: &apicontainer.DockerContainer{ + DockerID: containerID, + DockerName: containerName, + Container: container, + }, + } + + taskResponse := NewTaskResponse(task, containerNameToDockerContainer) + + taskResponseJSON, err := json.Marshal(taskResponse) + + assert.NoError(t, err) + assert.Equal(t, expectedTaskResponseJSONString, string(taskResponseJSON)) +} + + +func TestContainerResponse(t *testing.T) { + expectedContainerResponseJSONString := + `{` + + `"DockerId":"cid",` + + `"DockerName":"sleepy",` + + `"Name":"sleepy",` + + `"Ports":[{` + + `"ContainerPort":80,` + + `"Protocol":"tcp",` + + `"HostPort":80` + + `}],` + + `"Networks":[{` + + `"NetworkMode":"awsvpc",` + + `"IPv4Addresses":["10.0.0.2"]` + + `}]` + + `}` + + container := &apicontainer.Container{ + Name: containerName, + Ports: []apicontainer.PortBinding{ + { + ContainerPort: 80, + Protocol: apicontainer.TransportProtocolTCP, + }, + }, + } + + dockerContainer := &apicontainer.DockerContainer{ + DockerID: containerID, + DockerName: containerName, + Container: container, + } + + eni := &apieni.ENI{ + IPV4Addresses: []*apieni.ENIIPV4Address{ + { + Address: eniIPv4Address, + }, + }, + } + + containerResponse := NewContainerResponse(dockerContainer, eni) + containerResponseJSON, err := json.Marshal(containerResponse) + + assert.NoError(t, err) + assert.Equal(t, expectedContainerResponseJSONString, string(containerResponseJSON)) +} + +func TestPortBindingsResponse(t *testing.T) { + container := &apicontainer.Container{ + Name: containerName, + Ports: []apicontainer.PortBinding{ + { + ContainerPort: 80, + HostPort: 80, + Protocol: apicontainer.TransportProtocolTCP, + }, + }, + } + + dockerContainer := &apicontainer.DockerContainer{ + Container: container, + } + + PortBindingsResponse := NewPortBindingsResponse(dockerContainer, nil) + + assert.Equal(t, uint16(80), PortBindingsResponse[0].ContainerPort) + assert.Equal(t, uint16(80), PortBindingsResponse[0].HostPort) + assert.Equal(t, "tcp", PortBindingsResponse[0].Protocol) +} From 593009c9a72534f6e2355f52b4233b1d8c0125fd Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Wed, 25 Jul 2018 18:01:41 -0700 Subject: [PATCH 3/5] handlers: add json tags for response structs --- agent/handlers/v1/response.go | 36 ++++++++++----------- agent/handlers/v2/response.go | 60 +++++++++++++++++------------------ 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/agent/handlers/v1/response.go b/agent/handlers/v1/response.go index 94125cd5864..591200341c4 100644 --- a/agent/handlers/v1/response.go +++ b/agent/handlers/v1/response.go @@ -24,41 +24,41 @@ import ( // MetadataResponse is the schema for the metadata response JSON object type MetadataResponse struct { - Cluster string - ContainerInstanceArn *string - Version string + Cluster string `json:"Cluster"` + ContainerInstanceArn *string `json:"ContainerInstanceArn"` + Version string `json:"Version"` } // TaskResponse is the schema for the task response JSON object type TaskResponse struct { - Arn string - DesiredStatus string `json:",omitempty"` - KnownStatus string - Family string - Version string - Containers []ContainerResponse + Arn string `json:"Arn"` + DesiredStatus string `json:"DesiredStatus,omitempty"` + KnownStatus string `json:"KnownStatus"` + Family string `json:"Family"` + Version string `json:"Version"` + Containers []ContainerResponse `json:"Containers"` } // TasksResponse is the schema for the tasks response JSON object type TasksResponse struct { - Tasks []*TaskResponse + Tasks []*TaskResponse `json:"Tasks"` } // ContainerResponse is the schema for the container response JSON object type ContainerResponse struct { - DockerID string `json:"DockerId"` - DockerName string - Name string - Ports []PortResponse `json:",omitempty"` - Networks []containermetadata.Network `json:",omitempty"` + DockerID string `json:"DockerId"` + DockerName string `json:"DockerName"` + Name string `json:"Name"` + Ports []PortResponse `json:"Ports,omitempty"` + Networks []containermetadata.Network `json:"Networks,omitempty"` } // PortResponse defines the schema for portmapping response JSON // object. type PortResponse struct { - ContainerPort uint16 - Protocol string - HostPort uint16 `json:",omitempty"` + ContainerPort uint16 `json:"ContainerPort,omitempty"` + Protocol string `json:"Protocol,omitempty"` + HostPort uint16 `json:"HostPort,omitempty"` } // NewTaskResponse creates a TaskResponse for a task. diff --git a/agent/handlers/v2/response.go b/agent/handlers/v2/response.go index 2762b0d8790..bfe18abc71c 100644 --- a/agent/handlers/v2/response.go +++ b/agent/handlers/v2/response.go @@ -29,46 +29,46 @@ import ( // TaskResponse defines the schema for the task response JSON object type TaskResponse struct { - Cluster string - TaskARN string - Family string - Revision string - DesiredStatus string `json:",omitempty"` - KnownStatus string - Containers []ContainerResponse `json:",omitempty"` - Limits *LimitsResponse `json:",omitempty"` - PullStartedAt *time.Time `json:",omitempty"` - PullStoppedAt *time.Time `json:",omitempty"` - ExecutionStoppedAt *time.Time `json:",omitempty"` + Cluster string `json:"Cluster"` + TaskARN string `json:"TaskARN"` + Family string `json:"Family"` + Revision string `json:"Revision"` + DesiredStatus string `json:"DesiredStatus,omitempty"` + KnownStatus string `json:"KnownStatus"` + Containers []ContainerResponse `json:"Containers,omitempty"` + Limits *LimitsResponse `json:"Limits,omitempty"` + PullStartedAt *time.Time `json:"PullStartedAt,omitempty"` + PullStoppedAt *time.Time `json:"PullStoppedAt,omitempty"` + ExecutionStoppedAt *time.Time `json:"ExecutionStoppedAt,omitempty"` } // ContainerResponse defines the schema for the container response // JSON object type ContainerResponse struct { - ID string `json:"DockerId"` - Name string - DockerName string - Image string - ImageID string - Ports []v1.PortResponse `json:",omitempty"` - Labels map[string]string `json:",omitempty"` - DesiredStatus string - KnownStatus string - ExitCode *int `json:",omitempty"` - Limits LimitsResponse - CreatedAt *time.Time `json:",omitempty"` - StartedAt *time.Time `json:",omitempty"` - FinishedAt *time.Time `json:",omitempty"` - Type string - Networks []containermetadata.Network `json:",omitempty"` - Health *apicontainer.HealthStatus `json:",omitempty"` + ID string `json:"DockerId"` + Name string `json:"Name"` + DockerName string `json:"DockerName"` + Image string `json:"Image"` + ImageID string `json:"ImageID"` + Ports []v1.PortResponse `json:"Ports,omitempty"` + Labels map[string]string `json:"Labels,omitempty"` + DesiredStatus string `json:"DesiredStatus"` + KnownStatus string `json:"KnownStatus"` + ExitCode *int `json:"ExitCode,omitempty"` + Limits LimitsResponse `json:"Limits"` + CreatedAt *time.Time `json:"CreatedAt,omitempty"` + StartedAt *time.Time `json:"StartedAt,omitempty"` + FinishedAt *time.Time `json:"FinishedAt,omitempty"` + Type string `json:"Type"` + Networks []containermetadata.Network `json:"Networks,omitempty"` + Health *apicontainer.HealthStatus `json:"Health,omitempty"` } // LimitsResponse defines the schema for task/cpu limits response // JSON object type LimitsResponse struct { - CPU *float64 `json:",omitempty"` - Memory *int64 `json:",omitempty"` + CPU *float64 `json:"CPU,omitempty"` + Memory *int64 `json:"Memory,omitempty"` } // NewTaskResponse creates a new response object for the task From a644d61e50f352e4e82a9c08a937036984364ae9 Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Thu, 26 Jul 2018 11:29:05 -0700 Subject: [PATCH 4/5] test: use map for comparison in v1 handler --- agent/handlers/v1/response_test.go | 103 +++++++++++++++++------------ 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/agent/handlers/v1/response_test.go b/agent/handlers/v1/response_test.go index b955452f816..db664c2332e 100644 --- a/agent/handlers/v1/response_test.go +++ b/agent/handlers/v1/response_test.go @@ -34,28 +34,36 @@ const ( ) func TestTaskResponse(t *testing.T) { - expectedTaskResponseJSONString := - `{` + - `"Arn":"t1",` + - `"DesiredStatus":"RUNNING",` + - `"KnownStatus":"RUNNING",` + - `"Family":"sleep",` + - `"Version":"1",` + - `"Containers":[{` + - `"DockerId":"cid",` + - `"DockerName":"sleepy",` + - `"Name":"sleepy",` + - `"Ports":[{` + - `"ContainerPort":80,` + - `"Protocol":"tcp",` + - `"HostPort":80` + - `}],` + - `"Networks":[{` + - `"NetworkMode":"awsvpc",` + - `"IPv4Addresses":["10.0.0.2"]` + - `}]` + - `}]` + - `}` + expectedTaskResponseMap := map[string]interface{}{ + "Arn": "t1", + "DesiredStatus": "RUNNING", + "KnownStatus": "RUNNING", + "Family": "sleep", + "Version": "1", + "Containers": []interface{}{ + map[string]interface{}{ + "DockerId": "cid", + "DockerName": "sleepy", + "Name": "sleepy", + "Ports": []interface{}{ + map[string]interface{}{ + // The number should be float here, because when we unmarshal + // something and we don't specify the number type, it will be + // set to float. + "ContainerPort": float64(80), + "Protocol": "tcp", + "HostPort": float64(80), + }, + }, + "Networks": []interface{}{ + map[string]interface{}{ + "NetworkMode": "awsvpc", + "IPv4Addresses": []interface{}{"10.0.0.2"}, + }, + }, + }, + }, + } task := &apitask.Task{ Arn: taskARN, @@ -93,28 +101,34 @@ func TestTaskResponse(t *testing.T) { taskResponse := NewTaskResponse(task, containerNameToDockerContainer) taskResponseJSON, err := json.Marshal(taskResponse) - assert.NoError(t, err) - assert.Equal(t, expectedTaskResponseJSONString, string(taskResponseJSON)) -} + taskResponseMap := make(map[string]interface{}) + + json.Unmarshal(taskResponseJSON, &taskResponseMap) + + assert.Equal(t, expectedTaskResponseMap, taskResponseMap) +} func TestContainerResponse(t *testing.T) { - expectedContainerResponseJSONString := - `{` + - `"DockerId":"cid",` + - `"DockerName":"sleepy",` + - `"Name":"sleepy",` + - `"Ports":[{` + - `"ContainerPort":80,` + - `"Protocol":"tcp",` + - `"HostPort":80` + - `}],` + - `"Networks":[{` + - `"NetworkMode":"awsvpc",` + - `"IPv4Addresses":["10.0.0.2"]` + - `}]` + - `}` + expectedContainerResponseMap := map[string]interface{}{ + "DockerId": "cid", + "DockerName": "sleepy", + "Name": "sleepy", + "Ports": []interface{}{ + map[string]interface{}{ + "ContainerPort": float64(80), + "Protocol": "tcp", + "HostPort": float64(80), + }, + }, + "Networks": []interface{}{ + map[string]interface{}{ + "NetworkMode": "awsvpc", + "IPv4Addresses": []interface{}{"10.0.0.2"}, + }, + }, + } container := &apicontainer.Container{ Name: containerName, @@ -141,10 +155,15 @@ func TestContainerResponse(t *testing.T) { } containerResponse := NewContainerResponse(dockerContainer, eni) - containerResponseJSON, err := json.Marshal(containerResponse) + containerResponseJSON, err := json.Marshal(containerResponse) assert.NoError(t, err) - assert.Equal(t, expectedContainerResponseJSONString, string(containerResponseJSON)) + + containerResponseMap := make(map[string]interface{}) + + json.Unmarshal(containerResponseJSON, &containerResponseMap) + + assert.Equal(t, expectedContainerResponseMap, containerResponseMap) } func TestPortBindingsResponse(t *testing.T) { From 98d92d8c84be1a4025863004b31b78baaf711c3a Mon Sep 17 00:00:00 2001 From: haikuoliu Date: Thu, 26 Jul 2018 11:36:24 -0700 Subject: [PATCH 5/5] changelog: add entry for PR #1473 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 165e3989b1e..aa7c655ddce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 1.19.1 +* Bug - Fixed a bug where responses of introspection API break backward compatibility [#1473](https://github.com/aws/amazon-ecs-agent/pull/1473) + ## 1.19.0 * Feature - Private registry can be authenticated through task definition using AWS Secrets Manager [#1427](https://github.com/aws/amazon-ecs-agent/pull/1427)