[CORE-182] Move listRuntimes over to new Sam permissions model #4810
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4810 +/- ##
===========================================
- Coverage 74.77% 74.63% -0.15%
===========================================
Files 165 165
Lines 14954 14850 -104
Branches 1187 1187
===========================================
- Hits 11182 11083 -99
+ Misses 3772 3767 -5
Continue to review full report in Codecov by Sentry.
|
marctalbott
force-pushed
the
mtalbott-sam-list-runtimes
branch
from
85895eb to
d95c38b
Compare
| @@ -2179,6 +2017,7 @@ class RuntimeV2ServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite with | |||
| .save() | |||
| ) | |||
|
|
|||
| // todo: Don't really understand this scenario. If I don't have permission then I shouldn't be able to see it even if I created it | |||
There was a problem hiding this comment.
I believe this is an outdated scenario in the new permissions model, but would love if someone could confirm that for me.
There was a problem hiding this comment.
yeah this is mainly a use case for Azure runtimes governed by WSM
| @@ -1740,171 +1781,6 @@ class RuntimeV2ServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite with | |||
| res.unsafeRunSync()(cats.effect.unsafe.IORuntime.global) | |||
| } | |||
|
|
|||
| it should "list runtimes, omitting runtimes for workspaces and projects user cannot read" in isolatedDbTest { | |||
There was a problem hiding this comment.
This is out of date IMO. If Sam says the user has access to a runtime, then the user has access to the runtime. There is no need for Leo to check workspace and project access anymore.
| res.unsafeRunSync()(cats.effect.unsafe.IORuntime.global) | ||
| } | ||
|
|
||
| it should "list runtimes given different user permissions" in isolatedDbTest { |
There was a problem hiding this comment.
Out of date. As I said above, if Sam says the user has access to a runtime, then they have access to that runtime. There's no need for Leo to test or check workspace and project permissions for the user.
marctalbott
changed the title
| val runtimesFiltered = runtimesAuthorized | ||
| // Filter by params | ||
| val runtimes = clusterQuery | ||
| .filter(_.internalId inSetBind runtimeIds.map(_.asString)) |
There was a problem hiding this comment.
wow this is a huge improvement, thanks!
| @@ -2179,6 +2017,7 @@ class RuntimeV2ServiceInterpSpec extends AnyFlatSpec with LeonardoTestSuite with | |||
| .save() | |||
| ) | |||
|
|
|||
| // todo: Don't really understand this scenario. If I don't have permission then I shouldn't be able to see it even if I created it | |||
There was a problem hiding this comment.
yeah this is mainly a use case for Azure runtimes governed by WSM
Jira ticket: https://broadworkbench.atlassian.net/browse/CORE-182
Summary of changes
Change how
listRuntimesgathers user runtimes to rely on the new permissions model in Sam. See https://broadworkbench.atlassian.net/wiki/spaces/IA/pages/3362848772/Leonardo+Access+Control+-+Desired+State for more details.What
Why
Testing these changes
What to test
Who tested and where
jenkins retestorjenkins multi-test.