-
Notifications
You must be signed in to change notification settings - Fork 4
feat: dynamic role for operations #252
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| configRequest.EnvVars = obj.EnvVarMap | ||
|
|
||
| // Add operation roles if present |
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.
theres ton of these unnecessary comments, can we clean these out?
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.
Yepp, left them in for later will clean before merge, mostly claude, got lot of labour work done with it
pkg/plans/types/composite_plan.go
Outdated
| FetchImageMetadataPlan *FetchImageMetadataPlan `json:"fetch_image_metadata_plan,omitempty"` | ||
| SandboxRunPlan *SandboxRunPlan `json:"sandbox_run_plan,omitempty"` | ||
|
|
||
| // Auth for cloud providera |
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.
typo
| ) | ||
|
|
||
| type OperationRoleRuleRequest struct { | ||
| Principal string `json:"principal" validate:"required"` // "nuon::component:name", "nuon::sandbox", "nuon::action:name" |
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 up with these comments? are they needed? we wouldn't want to keep these up to date to different versions.
also, we probably can solve for that making operation and principal typed so the type describes the options.
|
|
||
| type EntityOperationRoleMap map[app.OperationType]string | ||
|
|
||
| func EntityOperationRoleMapFromHstore(hstore map[string]*string) EntityOperationRoleMap { |
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.
Recommend using our pkg/db/generics and just working with map[string]string here
|
This PR was marked as stale, and will be closed after 3 more days. Add the #keep-open label to prevent this from being closed. |
f3f25a0 to
711b7b7
Compare
|
This PR was marked as stale, and will be closed after 3 more days. Add the #keep-open label to prevent this from being closed. |
- removes unwanted comments - fixes missing entity role assignment for composite plans
711b7b7 to
213229b
Compare
|
This PR was marked as stale, and will be closed after 3 more days. Add the #keep-open label to prevent this from being closed. |
feat: temporary dynamic role selection
feat: add role selector to component sandbox and actions workflow on runtimne
feat: add ability to pass in role via api and store them into respective run tables
add azure credentials to composite plan auth
make sure break glass workflow works