Skip to content

Commit

Permalink
Set router skip clean to false.
Browse files Browse the repository at this point in the history
This will  redirect request like //v2/credential to /v2/credential for the correct handler to handle it
  • Loading branch information
suneyz committed Feb 14, 2019
1 parent 6c02fa8 commit fc92bf4
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"family": "task-server-endpoint-validator",
"taskRoleArn": "$$$TASK_ROLE$$$",
"networkMode": "host",
"containerDefinitions": [{
"image": "amazonlinux:2",
"name": "task_server_endpoint_validator_container",
"memory": 256,
"command": ["sh", "-c", "curl -L -o /dev/null -s -w \"%{http_code}\n\" http://169.254.170.2/$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI | grep \"200\" && exit 42 || exit 1"]
}]
}
42 changes: 42 additions & 0 deletions agent/functional_tests/tests/functionaltests_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1427,3 +1427,45 @@ func TestElasticInferenceValidator(t *testing.T) {
exitCode, _ := task.ContainerExitcode("container_1")
assert.Equal(t, 42, exitCode, fmt.Sprintf("Expected exit code of 42 for container; got %d", exitCode))
}

// TestServerEndpointValidator tests the workflow of task server endpoint
func TestServerEndpointValidator(t *testing.T) {
// The test runs only when the environment TEST_IAM_ROLE was set
if os.Getenv("TEST_DISABLE_TASK_IAM_ROLE_NET_HOST") == "true" {
t.Skip("Skipping test TaskIamRole in host network mode, as TEST_DISABLE_TASK_IAM_ROLE_NET_HOST is set true")
}
agentOptions := &AgentOptions{
ExtraEnvironment: map[string]string{
"ECS_ENABLE_TASK_IAM_ROLE": "true",
"ECS_ENABLE_TASK_IAM_ROLE_NETWORK_HOST": "true",
},
PortBindings: map[nat.Port]map[string]string{
"51679/tcp": {
"HostIP": "0.0.0.0",
"HostPort": "51679",
},
},
}

agent := RunAgent(t, agentOptions)
defer agent.Cleanup()

roleArn := os.Getenv("TASK_IAM_ROLE_ARN")
if utils.ZeroOrNil(roleArn) {
t.Logf("TASK_IAM_ROLE_ARN not set, will try to use the role attached to instance profile")
role, err := GetInstanceIAMRole()
require.NoError(t, err, "Error getting IAM Roles from instance profile")
roleArn = *role.Arn
}
tdOverride := make(map[string]string)
tdOverride["$$$TASK_ROLE$$$"] = roleArn

task, err := agent.StartTaskWithTaskDefinitionOverrides(t, "task-server-endpoint-validator", tdOverride)
require.NoError(t, err, "Error start task-server-endpoint-validator task")

err = task.WaitStopped(10 * time.Minute)
assert.NoError(t, err)
code, ok := task.ContainerExitcode("task_server_endpoint_validator_container")
assert.True(t, ok, "Get exit code failed")
assert.Equal(t, 42, code, "Wrong exit code")
}
8 changes: 4 additions & 4 deletions agent/handlers/task_server_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ func taskServerSetup(credentialsManager credentials.Manager,
containerInstanceArn string) *http.Server {
muxRouter := mux.NewRouter()

// Set this so that for request like "/v3//metadata/task", the Agent will pass
// it to task metadata handler instead of returning a 301 error.
muxRouter.SkipClean(true)
// Set this to false so that for request like "//v3//metadata/task"
// to permanently redirect(301) to "/v3/metadata/task" handler
muxRouter.SkipClean(false)

muxRouter.HandleFunc(v1.CredentialsPath,
v1.CredentialsHandler(credentialsManager, auditLogger))
Expand All @@ -79,7 +79,7 @@ func taskServerSetup(credentialsManager credentials.Manager,
loggingMuxRouter.Handle(rootPath, tollbooth.LimitHandler(
limiter, NewLoggingHandler(muxRouter)))

loggingMuxRouter.SkipClean(true)
loggingMuxRouter.SkipClean(false)

server := http.Server{
Addr: ":" + strconv.Itoa(config.AgentCredentialsPort),
Expand Down
35 changes: 31 additions & 4 deletions agent/handlers/task_server_setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,34 @@ func TestV3ContainerAssociation(t *testing.T) {
assert.Equal(t, expectedAssociationResponse, string(res))
}

func TestTaskHTTPEndpoint301Redirect(t *testing.T) {
testPathsMap := map[string]string{
"http://127.0.0.1/v3///task/": "http://127.0.0.1/v3/task/",
"http://127.0.0.1//v2/credentials/test": "http://127.0.0.1/v2/credentials/test",
}

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, expectedPath := range testPathsMap {
t.Run(fmt.Sprintf("Test path: %s", testPath), func(t *testing.T) {
recorder := httptest.NewRecorder()
req, _ := http.NewRequest("GET", testPath, nil)
server.Handler.ServeHTTP(recorder, req)
assert.Equal(t, http.StatusMovedPermanently, recorder.Code)
assert.Equal(t, expectedPath, recorder.Header().Get("Location"))
})
}
}

func TestTaskHTTPEndpointErrorCode404(t *testing.T) {
testPaths := []string{
"/",
Expand All @@ -1061,7 +1089,6 @@ func TestTaskHTTPEndpointErrorCode404(t *testing.T) {
"/v3/v3-endpoint-id/",
"/v3/v3-endpoint-id/wrong-path",
"/v3/v3-endpoint-id/task/",
"/v3///task/",
"/v3/v3-endpoint-id/task/stats/",
"/v3/v3-endpoint-id/task/stats/wrong-path",
"/v3/v3-endpoint-id/associtions-with-typo/elastic-inference/dev1",
Expand Down Expand Up @@ -1101,11 +1128,11 @@ func TestTaskHTTPEndpointErrorCode400(t *testing.T) {
"/v3/wrong-v3-endpoint-id",
"/v3/",
"/v3/wrong-v3-endpoint-id/stats",
"/v3//stats",
"/v3/stats",
"/v3/wrong-v3-endpoint-id/task",
"/v3//task",
"/v3/task",
"/v3/wrong-v3-endpoint-id/task/stats",
"/v3//task/stats",
"/v3/task/stats",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference",
"/v3/wrong-v3-endpoint-id/associations/elastic-inference/dev1",
}
Expand Down

0 comments on commit fc92bf4

Please sign in to comment.