Second-person approval of a workflow

[kangyanzhou]

Summary

Record the user who creates a workflow, and the user who approves a step in the workflow using intermediate parameters, and make sure the user who approves the workflow is not the one who creates the workflow.

Currently it seems like we can record who approves a step using intermediate parameters in https://github.com/argoproj/argo-workflows/blob/master/workflow/util/util.go#L383-L386, and there's no way I can find it in the UI with the example in https://argoproj.github.io/argo-workflows/intermediate-inputs/#intermediate-parameters-approval-example and with SSO auth enabled.

Use Cases

This is needed for compliance use cases where we need a second person to approve some operations.

---

Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

[terrytangyuan]

Maybe just record the author information somewhere, e.g. in a configmap. And then load configmap as parameters to validate whether the approver is the same as the author.

[huynhsontung]

We should consider having a dedicated input type for approval so we can support more logic validation logic and show a better UI for this particular input. Picking from a list of YES/NO enum is not the best UI for this case.

Something like this perhaps:

- name: whalesay
  suspend: {}
  inputs:
    parameters:
    - name: approval
      approval:
        groups: "approvers"

[kangyanzhou]

Maybe just record the author information somewhere, e.g. in a configmap. And then load configmap as parameters to validate whether the approver is the same as the author.

@terrytangyuan One thing to note is that author is different from executor. What we want is to have different approvers from the executor of the workflow.

Also how do we surface the approver's info? I couldn't figure out how to retrieve the info based on the code link I shared earlier.

[kangyanzhou]

We should consider having a dedicated input type for approval so we can support more logic validation logic and show a better UI for this particular input. Picking from a list of YES/NO enum is not the best UI for this case.

Something like this perhaps:

- name: whalesay
  suspend: {}
  inputs:
    parameters:
    - name: approval
      approval:
        groups: "approvers"

+1 on this. Having a group of approvers is also what we are looking for.

I'm happy to implement both functionalities.

[alexcodelf]

Does there has any feedback regarding this enhancement?

[rvandernoort]

@kangyanzhou do you have any progress on this feature? I started an identification of what needs to be done as this sounds like a very neat addition to the current flow.

[kangyanzhou]

No I never started working on it

[agilgur5]

I think we need to design an approach of doing this securely if we want to support it in core. This may also be a good use-case for a plugin as approval is nuanced and use-case specific (compared to Argo's genericism)

As I outlined in https://github.com/argoproj/argo-workflows/discussions/13079#discussioncomment-9848829, simple metadata or status changes would not be sufficient, as any user with edit access to Workflows could make the same changes and also impersonate anyone. It requires some care to do with proper security attributes. In that comment, I mentioned that a CR for this purpose that only the Server could modify, similar to https://github.com/argoproj/argo-workflows/issues/6490#issuecomment-1961246329 would be more secure, but would still need some thought to make it function like an immutable audit log

If done as a UI plugin (vs core) #6945 , this could be more experimental at first and have custom implementations for specific auth providers etc. It wouldn't need the same security properties as a plugin as it would in core

[rvandernoort]

Hi thanks for your extensive comment.

I tried to translate your comments into a visual proposal, maybe you can give some feedback on this as well?

As for the plugin, I will look into this for the short term. Would this allow me to make this plugin persist the approval data within the plugin and checking the authentication from the dex server and contiueing the workflow from its server?