feat(server): Argo Server. Closes #1331 #1882
Conversation
alexec
changed the title
| .envrc | ||
| /.vendor-new | ||
| /workflow-controller |
There was a problem hiding this comment.
Yes you're right. I think the dist is excluded in the .dockerignore and so I had to put it under SRCROOT. Ignore this.
| allowed, err := authorizer.CanI("get", "workflow", wf.Namespace, wf.Name) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Theres a subtle bug/usability issue here:
If I request to list archived workflows with a small page size (lets say size 1), and the first workflow in the list, happens to be in a namespace which I have permissions to, then this call will return successfully.
On the other hand, if I make the same exact request, and the first workflow in the list is in a namespace which I do not have access to, then the API will return 403.
This means the API is inconsistent depending on how the order in which the database is returning values.
This could be simplified to:
-
is the request asking to list workflows at the cluster scope? If yes, perform the equivalent of:
kubectl auth can-i get wf --all-namespacesup front -
otherwise perform the equivalent of:
``kubectl auth can-i get wf --namespace NAMESPACE` up front
Both cases we can return earlier and perform authentication up front, instead of inline.
manifests/cluster-install/workflow-controller-rbac/workflow-controller-clusterrole.yaml
Outdated
Show resolved
Hide resolved
|
|
||
| wf, err := a.getWorkflowByUID(ctx, uid) | ||
| if err != nil { | ||
| a.serverInternalError(err, w) |
There was a problem hiding this comment.
we are clobbering the authz 403 error and always returning 500.
There was a problem hiding this comment.
Sure. I have an issue to revisit security for artifacts.
Checklist:
"fix(controller): Updates such and such. Fixes #1234". Why? for the release notes.See #1331 and #1879