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

Fix response codes for container metadata and task metadata requests #2643

Merged
merged 1 commit into from
Sep 19, 2020
Merged

Fix response codes for container metadata and task metadata requests #2643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 40 additions & 9 deletions agent/handlers/task_server_setup_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1518,24 +1518,55 @@ func TestTaskHTTPEndpointErrorCode400(t *testing.T) {
"/v2/stats/",
"/v2/stats/wrong-container-id",
"/v2/stats/container-id/other-path",
"/v3/wrong-v3-endpoint-id/stats",
"/v3/wrong-v3-endpoint-id/task/stats",
"/v3/task/stats",
"/v4/wrong-v3-endpoint-id/stats",
"/v4/wrong-v3-endpoint-id/task/stats",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference/dev1",
}

ctrl := gomock.NewController(t)
defer ctrl.Finish()

state := mock_dockerstate.NewMockTaskEngineState(ctrl)
auditLog := mock_audit.NewMockAuditLogger(ctrl)
statsEngine := mock_stats.NewMockEngine(ctrl)
ecsClient := mock_api.NewMockECSClient(ctrl)

server := taskServerSetup(credentials.NewManager(), auditLog, state, ecsClient, clusterName, statsEngine,
config.DefaultTaskMetadataSteadyStateRate, config.DefaultTaskMetadataBurstRate, "", containerInstanceArn)

for _, testPath := range testPaths {
t.Run(fmt.Sprintf("Test path: %s", testPath), func(t *testing.T) {
// Make every possible call to state fail
state.EXPECT().TaskARNByV3EndpointID(gomock.Any()).Return("", false).AnyTimes()
state.EXPECT().DockerIDByV3EndpointID(gomock.Any()).Return("", false).AnyTimes()
state.EXPECT().TaskARNByV3EndpointID(gomock.Any()).Return("", false).AnyTimes()
state.EXPECT().GetTaskByIPAddress(gomock.Any()).Return("", false).AnyTimes()

recorder := httptest.NewRecorder()
req, _ := http.NewRequest("GET", testPath, nil)
req.RemoteAddr = remoteIP + ":" + remotePort
server.Handler.ServeHTTP(recorder, req)
assert.Equal(t, http.StatusBadRequest, recorder.Code)
})
}
}

func TestTaskHTTPEndpointErrorCode500(t *testing.T) {
testPaths := []string{
"/v3/wrong-v3-endpoint-id",
"/v3/",
"/v3/wrong-v3-endpoint-id/stats",
"/v3/stats",
"/v3/wrong-v3-endpoint-id/task",
"/v3/task",
"/v3/wrong-v3-endpoint-id/task/stats",
"/v3/task/stats",
"/v4/wrong-v3-endpoint-id",
"/v4/",
"/v4/wrong-v3-endpoint-id/stats",
"/v4/stats",
"/v4/wrong-v3-endpoint-id/task",
"/v4/task",
"/v4/wrong-v3-endpoint-id/task/stats",
"/v4/task/stats",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference/dev1",
}

ctrl := gomock.NewController(t)
Expand All @@ -1561,7 +1592,7 @@ func TestTaskHTTPEndpointErrorCode400(t *testing.T) {
req, _ := http.NewRequest("GET", testPath, nil)
req.RemoteAddr = remoteIP + ":" + remotePort
server.Handler.ServeHTTP(recorder, req)
assert.Equal(t, http.StatusBadRequest, recorder.Code)
assert.Equal(t, http.StatusInternalServerError, recorder.Code)
})
}
}
6 changes: 4 additions & 2 deletions agent/handlers/v3/container_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, responseJSON, utils.RequestTypeContainerMetadata)
return
}
containerResponse, err := GetContainerResponse(containerID, state)
Expand All @@ -48,7 +48,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}
seelog.Infof("V3 container metadata handler: writing response for container '%s'", containerID)
Expand All @@ -65,6 +65,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
func GetContainerResponse(containerID string, state dockerstate.TaskEngineState) (*v2.ContainerResponse, error) {
containerResponse, err := v2.NewContainerResponse(containerID, state)
if err != nil {
seelog.Errorf("Unable to get container metadata for container '%s'", containerID)
return nil, errors.Errorf("Unable to generate metadata for container '%s'", containerID)
}
// fill in network details if not set
Expand All @@ -86,6 +87,7 @@ func GetContainerNetworkMetadata(containerID string, state dockerstate.TaskEngin
// https://github.com/aws/amazon-ecs-agent/blob/0c8913ba33965cf6ffdd6253fad422458d9346bd/agent/containermetadata/parse_metadata.go#L123
settings := dockerContainer.Container.GetNetworkSettings()
if settings == nil {
seelog.Errorf("unable to get container network response for container '%s'", containerID)
return nil, errors.Errorf("Unable to generate network response for container '%s'", containerID)
}
// This metadata is the information provided in older versions of the API
Expand Down
6 changes: 3 additions & 3 deletions agent/handlers/v3/task_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeTaskMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, responseJSON, utils.RequestTypeTaskMetadata)
return
}

Expand All @@ -57,7 +57,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeTaskMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJSON, utils.RequestTypeTaskMetadata)
return
}

Expand All @@ -72,7 +72,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}
containerResponse.Networks = networks
Expand Down
6 changes: 4 additions & 2 deletions agent/handlers/v4/container_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, responseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, responseJSON, utils.RequestTypeContainerMetadata)
return
}
containerResponse, err := GetContainerResponse(containerID, state)
Expand All @@ -48,7 +48,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}
seelog.Infof("V4 container metadata handler: writing response for container '%s'", containerID)
Expand All @@ -65,6 +65,7 @@ func ContainerMetadataHandler(state dockerstate.TaskEngineState) func(http.Respo
func GetContainerResponse(containerID string, state dockerstate.TaskEngineState) (*ContainerResponse, error) {
containerResponse, err := NewContainerResponse(containerID, state)
if err != nil {
seelog.Errorf("Unable to get container metadata for container '%s'", containerID)
return nil, errors.Errorf("unable to generate metadata for container '%s'", containerID)
}

Expand All @@ -87,6 +88,7 @@ func GetContainerNetworkMetadata(containerID string, state dockerstate.TaskEngin
// https://github.com/aws/amazon-ecs-agent/blob/0c8913ba33965cf6ffdd6253fad422458d9346bd/agent/containermetadata/parse_metadata.go#L123
settings := dockerContainer.Container.GetNetworkSettings()
if settings == nil {
seelog.Errorf("Unable to get container network response for container '%s'", containerID)
return nil, errors.Errorf("unable to generate network response for container '%s'", containerID)
}
// This metadata is the information provided in older versions of the API
Expand Down
6 changes: 3 additions & 3 deletions agent/handlers/v4/task_metadata_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, ResponseJSON, utils.RequestTypeTaskMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, ResponseJSON, utils.RequestTypeTaskMetadata)
return
}

Expand All @@ -53,7 +53,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJson, utils.RequestTypeTaskMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJson, utils.RequestTypeTaskMetadata)
return
}

Expand All @@ -69,7 +69,7 @@ func TaskMetadataHandler(state dockerstate.TaskEngineState, ecsClient api.ECSCli
if e := utils.WriteResponseIfMarshalError(w, err); e != nil {
return
}
utils.WriteJSONToResponse(w, http.StatusBadRequest, errResponseJSON, utils.RequestTypeContainerMetadata)
utils.WriteJSONToResponse(w, http.StatusInternalServerError, errResponseJSON, utils.RequestTypeContainerMetadata)
return
}
containerResponse.Networks = networks
Expand Down