HomeMesosphereNo notifications. 4 unresolved issues.
  • Search
Differential D289

Fixes 4790 - POD instances should not be killable via /v2/tasks
ClosedAll Users
Actions

Authored by unterstein on Dec 7 2016, 2:22 PM.

Details

Reviewers
aquamatthias
zen-dog
meichstedt
jeschkies
Commits
rMARATHON89740d036d21: Fixes 4790 - POD instances should not be killable via /v2/tasks
rMARATHON059e852cfb6b: Fixes 4790 - POD instances should not be killable via /v2/tasks
rMARATHONe9e650dc88cc: Fixes 4790 - POD instances should not be killable via /v2/tasks
Summary

Currently it is possible to trigger a POD kill via the /v2/tasks api. This should be fixed.

Test Plan

sbt test

Diff Detail

Repository
rMARATHON marathon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
unterstein retitled this revision from to Fixes 4790 - POD instances should not be killable via /v2/tasks.Dec 7 2016, 2:22 PM
unterstein updated this object.
unterstein edited the test plan for this revision. (Show Details)
unterstein added reviewers: aquamatthias, meichstedt, jeschkies, zen-dog.
unterstein added a subscriber: marathon-team.
aquamatthias accepted this revision.Dec 7 2016, 3:26 PM
This revision is now accepted and ready to land.Dec 7 2016, 3:26 PM
meichstedt accepted this revision.Dec 7 2016, 4:32 PM
Closed by commit rMARATHON89740d036d21: Fixes 4790 - POD instances should not be killable via /v2/tasks (authored by unterstein, committed by meichstedt). · Explain WhyDec 7 2016, 4:33 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1138

View Options

src/main/scala/mesosphere/marathon/api/v2/TasksResource.scala

Show First 20 Lines • Show All 124 Lines • ▼ Show 20 Line(s)
125125 }
126126
127127 def killTasks(toKill: Map[PathId, Seq[Instance]]): Response = {
128128 val affectedApps = tasksToAppId.values.flatMap(appId => result(groupManager.app(appId))).toSeq
129129 // FIXME (gkleiman): taskKiller.kill a few lines below also checks authorization, but we need to check ALL before
130130 // starting to kill tasks
131131 affectedApps.foreach(checkAuthorization(UpdateRunSpec, _))
132132
133 val killed = result(Future.sequence(toKill.map {
133 val killed = result(Future.sequence(toKill
134 .filter { case (appId, instances) => affectedApps.exists(app => app.id == appId) }
135 .map {
134136 case (appId, instances) => taskKiller.kill(appId, _ => instances, wipe)
135137 })).flatten
136138 ok(jsonObjString("tasks" -> killed.flatMap { instance =>
137139 instance.tasksMap.valuesIterator.map { task =>
138140 EnrichedTask(task.runSpecId, task, instance.agentInfo, Seq.empty)
139141 }
140142 }))
141143 }
Show All 16 Lines
View Options

src/test/scala/mesosphere/marathon/api/v2/TasksResourceTest.scala

Show First 20 Lines • Show All 78 Lines • ▼ Show 20 Line(s)
7979 And("Both tasks should be requested to be killed")
8080 verify(taskKiller).kill(eq(app1), any, any)(any)
8181 verify(taskKiller).kill(eq(app2), any, any)(any)
8282
8383 And("nothing else should be called on the TaskKiller")
8484 noMoreInteractions(taskKiller)
8585 }
8686
87 test("try to kill pod instances") {
88 Given("two apps and 1 task each")
89 val pod1 = "/pod".toRootPath
90
91 val instance = TestInstanceBuilder.newBuilder(pod1).addTaskRunning(Some("container1")).getInstance()
92
93 val (container, _) = instance.tasksMap.head
94
95 val body = s"""{"ids": ["${container.idString}"]}"""
96 val bodyBytes = body.toCharArray.map(_.toByte)
97
98 config.zkTimeoutDuration returns 5.seconds
99 taskTracker.instancesBySpecSync returns InstanceTracker.InstancesBySpec.forInstances(instance)
100 taskKiller.kill(any, any, any)(any) returns Future.successful(Seq.empty[Instance])
101 groupManager.app(any) returns Future.successful(None)
102
103 When("we ask to kill the pod container")
104 val response = taskResource.killTasks(scale = false, force = false, wipe = false, body = bodyBytes, auth.request)
105
106 Then("The response should be OK")
107 response.getStatus shouldEqual 200
108
109 And("No task should be called on the TaskKiller")
110 noMoreInteractions(taskKiller)
111 }
112
87113 test("killTasks with force") {
88114 Given("two apps and 1 task each")
89115 val app1 = "/my/app-1".toRootPath
90116 val app2 = "/my/app-2".toRootPath
91117
92118 val instance1 = TestInstanceBuilder.newBuilder(app1).addTaskRunning().getInstance()
93119 val instance2 = TestInstanceBuilder.newBuilder(app2).addTaskStaged().getInstance()
94120
▲ Show 20 Lines • Show All 213 Lines • Show Last 20 Lines