From b7eb89b00b76f67c2c415d086db1f97bab9196f2 Mon Sep 17 00:00:00 2001 From: Mythri Garaga Manjunatha Date: Fri, 18 Sep 2020 12:28:35 -0700 Subject: [PATCH] Fix response codes for container metadata and task metadata requests in v3 and v4 --- agent/handlers/task_server_setup_test.go | 49 +++++++++++++++---- .../handlers/v3/container_metadata_handler.go | 6 ++- agent/handlers/v3/task_metadata_handler.go | 6 +-- .../handlers/v4/container_metadata_handler.go | 6 ++- agent/handlers/v4/task_metadata_handler.go | 6 +-- 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/agent/handlers/task_server_setup_test.go b/agent/handlers/task_server_setup_test.go index 02ff13b3daf..b65771096bc 100644 --- a/agent/handlers/task_server_setup_test.go +++ b/agent/handlers/task_server_setup_test.go @@ -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) @@ -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) }) } } diff --git a/agent/handlers/v3/container_metadata_handler.go b/agent/handlers/v3/container_metadata_handler.go index 65ac5d5e0f3..32724dbded7 100644 --- a/agent/handlers/v3/container_metadata_handler.go +++ b/agent/handlers/v3/container_metadata_handler.go @@ -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) @@ -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) @@ -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 @@ -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 diff --git a/agent/handlers/v3/task_metadata_handler.go b/agent/handlers/v3/task_metadata_handler.go index f07c33986d1..8126935d548 100644 --- a/agent/handlers/v3/task_metadata_handler.go +++ b/agent/handlers/v3/task_metadata_handler.go @@ -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 } @@ -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 } @@ -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 diff --git a/agent/handlers/v4/container_metadata_handler.go b/agent/handlers/v4/container_metadata_handler.go index 8f59ff46e92..0e36fe9de47 100644 --- a/agent/handlers/v4/container_metadata_handler.go +++ b/agent/handlers/v4/container_metadata_handler.go @@ -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) @@ -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) @@ -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) } @@ -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 diff --git a/agent/handlers/v4/task_metadata_handler.go b/agent/handlers/v4/task_metadata_handler.go index 55d1f2cfbf2..275986e6481 100644 --- a/agent/handlers/v4/task_metadata_handler.go +++ b/agent/handlers/v4/task_metadata_handler.go @@ -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 } @@ -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 } @@ -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