Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Commit

Permalink
A group is accessible, if the group is selected, a subgroup is select…
Browse files Browse the repository at this point in the history
…ed or a pod/app in that group is selected.

Summary:
The logic of group selection was incomplete, since it did not take viewable apps/pods into account.
If a pod/app is viewable, all parent groups are implicitly viewable.
The content of all parent nodes is still filtered.

Fixes #4659 (MARATHON-1266)

Test Plan: sbt test

Reviewers: jeschkies, meichstedt

Reviewed By: jeschkies, meichstedt

Subscribers: marathon-team

JIRA Issues: MARATHON-1266

Differential Revision: https://phabricator.mesosphere.com/D191

(cherry picked from commit ff78c10)

# Conflicts:
#	src/main/scala/mesosphere/marathon/core/appinfo/impl/DefaultInfoService.scala
#	src/test/scala/mesosphere/marathon/integration/setup/SingleMarathonIntegrationTest.scala
  • Loading branch information
aquamatthias committed Feb 3, 2017
1 parent 4528f88 commit 1027968
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ private[appinfo] class DefaultInfoService(
None
//if a subgroup is allowed, we also have to allow all parents implicitly
def groupMatches(group: Group): Boolean = {
alreadyMatched.getOrElseUpdate(group.id, groupSelector.matches(group) || group.groups.exists(groupMatches))
alreadyMatched.getOrElseUpdate(
group.id,
groupSelector.matches(group) ||
group.groups.exists(groupMatches) ||
group.apps.keys.exists(infoById.contains))
}
if (groupMatches(ref)) Some(GroupInfo(ref, apps, groups)) else None
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,35 @@ class DefaultInfoServiceTest extends MarathonSpec with GivenWhenThen with Mockit
result.futureValue.get.transitiveGroups.get should have size 2
}

test("Selecting with App Selector implicitly gives access to parent groups") {
Given("a nested group with access to only nested app /group/app1")
val f = new Fixture
val rootId = PathId.empty
val rootApp = AppDefinition(PathId("/app"))
val nestedApp1 = AppDefinition(PathId("/group/app1"))
val nestedApp2 = AppDefinition(PathId("/group/app2"))
val nestedGroup = Group(PathId("/group"), Map(nestedApp1.id -> nestedApp1, nestedApp2.id -> nestedApp2), Set.empty[Group])
val rootGroup = Group(rootId, Map(rootApp.id -> rootApp), Set(nestedGroup))

f.baseData.appInfoFuture(any, any) answers { args =>
Future.successful(AppInfo(args.head.asInstanceOf[AppDefinition]))
}
f.groupManager.group(rootId) returns Future.successful(Some(rootGroup))
val selector = GroupSelector(
_.id.toString.startsWith("/group/app1"),
_ => false // no group
)

When("querying extending group information with selector")
val result = f.infoService.selectGroup(rootId, selector, Set.empty, Set(GroupInfo.Embed.Apps, GroupInfo.Embed.Groups))

Then("The result is filtered by the selector")
result.futureValue.get.transitiveGroups.get should have size 1
result.futureValue.get.transitiveGroups.get.head.group should be(nestedGroup)
result.futureValue.get.transitiveApps.get should have size 1
result.futureValue.get.transitiveApps.get.head.app should be(nestedApp1)
}

class Fixture {
lazy val groupManager = mock[GroupManager]
lazy val appRepo = mock[AppRepository]
Expand Down

0 comments on commit 1027968

Please sign in to comment.