-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ID-1328 system resource access policy creation on boot #1482
Conversation
@@ -80,6 +81,20 @@ object Boot extends IOApp with LazyLogging { | |||
|
|||
_ <- dependencies.policyEvaluatorService.initPolicy() | |||
|
|||
// make sure all users referenced by resourceAccessPolicies exist | |||
_ <- appConfig.resourceAccessPolicies.flatMap { case (_, policy) => policy.memberEmails }.toList.traverse { email => | |||
dependencies.samApplication.userService.inviteUser(email, SamRequestContext()).attempt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want to silently do this? Since this is on-boot, I'm wondering if it would be better to hard-fail if the users don't exist, with some descriptive message saying there's a configuration error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a BEE, the user's probably won't exist because our services register their service account users on startup. Any new live environment will also have this issue. The accounts exist in our live environments because they were registered by the same process many moons ago.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point about BEEs!
src/main/scala/org/broadinstitute/dsde/workbench/sam/Boot.scala
Outdated
Show resolved
Hide resolved
samRequestContext | ||
) | ||
} else { | ||
overwritePolicy( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we expect to be setting up non resource_type_admin policies with this functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no current expectation, but it seemed just as easy to support it as to throw an exception
@@ -243,3 +243,52 @@ janitor { | |||
trackResourceTopicId = ${?JANITOR_TRACK_RESOURCE_TOPIC_ID} | |||
} | |||
|
|||
terra.support.emails = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this new config should replace this script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quality Gate passedIssues Measures |
] | ||
|
||
resourceAccessPolicies { | ||
resource_type_admin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's an example for doing this for services (Leo, Rawls)? Wondering if our existing roles will work, or if it would be better to create some new ones for service management of resources..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my intent was to create new roles with hand picked actions. The workspace application role is probably ok for Leo but every other resource type probably needs other roles.
Ticket: https://broadworkbench.atlassian.net/browse/ID-1328
What:
create resource access policies from config at creation tome
Why:
Terra requires some system access policies for it's services, e.g. rawls, leo, etc. To date, there have only been a couple and they have been trivial. We expect more and more complicated policies soon. Better to manage them in code.
How:
A new config stanza called
resourceAccessPolicies
is read at boot time and fed into existing code to overwrite policies. The config has the same structure as the api to create policies.PR checklist