Skip to content
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

Appeal approval outside Guardian #217

Open
singhvikash11 opened this issue Jul 12, 2022 · 11 comments
Open

Appeal approval outside Guardian #217

singhvikash11 opened this issue Jul 12, 2022 · 11 comments

Comments

@singhvikash11
Copy link
Member

Summary
Complex appeal approval flow can't be modelled using policy config YAML file so in that case, Guardian should be able to integrate with existing complex approval flow like bpmn.

Proposed solution
Guardian can integrate with exiting approval flow either by webhook or subscribe to events.

@rahmatrhd
Copy link
Member

my rough thought for now is, we can introduce type field in the approval step configuration. If the approach for this is to fetch to external service, we can set the type as fetch and provide the host, creds, payload etc. While the existing human/auto approval step would be the normal type

@ravisuhag
Copy link
Member

ravisuhag commented Jul 13, 2022

Hmm. This is interesting. I can think of a couple of types.

  • manual: By Humans. (I think this is what you refer as normal)
  • http: Guardian will call an http API to check. (Referred as fetch)
  • auto: Approve automatically if a certain condition is met.

Thoughts?

@rahmatrhd
Copy link
Member

pardon me, I just realized that we already have strategy field with possible values auto and manual, we can simply introduce http for this 😁

@rahmatrhd
Copy link
Member

@singhvikash11 we need to define a contract for the http api, can you help with that?

@ravisuhag ravisuhag moved this to 2022 Q4 in Roadmap Jul 13, 2022
@rahmatrhd
Copy link
Member

rahmatrhd commented Jul 21, 2022

my proposal for this:

# policy
id: my-policy
steps:
- name: owner-approval
  strategy: manual
  approvers:
  - $appeal.resource.details.owner
- name: external-approval
  strategy: auto
  http:
    url: http://example.com/approval/$appeal.account_id
    headers:
      X-Foo-Bar: baz
    authorization:
      type: basic
      username: user
      password: pass
  approve_if: '$response.body.foo == "bar"' 
  1. can just use existing auto strategy with the same resolution using existing approve_if.
  2. approve_if will get evaluated only if response status is 200 OK
  3. add $response param to approve_if evaluation if http config is present and response status is 200 OK
  4. one field addition which is http containing target API info (GET only)
  5. no API contract required with guardian

@ravisuhag
Copy link
Member

@rahmatrhd any reason to use auto and not http?

@rahmatrhd
Copy link
Member

@ravisuhag because http fetch approval is essentially an automatic approval that doesn't require human approval

@ravisuhag
Copy link
Member

ravisuhag commented Jul 21, 2022

@rahmatrhd One concern I have is that lot of params will become conditional. If the strategy is this then set this and this and so on. So it might get tricky for policy users to figure out for which strategy which params I should use. Same on the implementation side and combinations start to increase. So, would it make sense to actually segregate it as per strategy? thoughts?

@bsushmith
Copy link
Member

I think in addition to the above and detailed documentation, we should have also a command/api similar to guardian policy validate or guardian policy lint so that one can run it and check if the policy file that they have created is valid. and if valid, detailed error output to describe what is incorrect.

@ravisuhag
Copy link
Member

@bsushmith can you create an issue for this please?

@bsushmith
Copy link
Member

Created issue #237

lifosmin pushed a commit to lifosmin/guardian that referenced this issue Aug 31, 2023
* fix; remove old un supported APIs

* fix: remove old un supported APIs

* fix: add uploadToSCheduler api for optimus

* fix: remove redundant event names from jobEvent proto enum

* fix: import cycles

* fix: optimus reserve old used enums

* fix: optimus: event type name fix

* fix: reserved an old used feild
@ravisuhag ravisuhag moved this from 2023 to 2024 in Roadmap Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 2024
Development

No branches or pull requests

4 participants