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

Create change on behalf of initiator #74

Conversation

BobSilent
Copy link
Collaborator

Provide possibility to apply aggregator changes on behalf of event initiator.

The provided PAT needs to have the appropriate permission to bypass rule validation.

the configuration can be made either

  • in rule code via directive for every mapping or
  • during map.rule command for a dedicated invocation.

Provides solution for #62

@giuliov
Copy link
Member

giuliov commented Sep 5, 2019

I will look at this more deeply, but I am surprised at the mixed design. Impersonation is a mapping property or a rule property?

Looks like you can have more than one mapping to the same rule, some with some without, the impersonate flag. One can set the flag at rule level using a directive and this latter wins over the mapping. On the other hand the add.rule has no option to set this flag.
I would summarize: it is a property at both levels and rule-level wins.

I would opt for a property defined at rule-level: one can upload the same source twice to create two rules, one with, the other without impersonation.

A stretch goal would be to highlight the impersonation configuration in list.rules output. This is easy if, instead of a directive, you put this flag in the rule configuration (i.e. the application settings). It also enables adding the flag on the add.rule command.

@BobSilent
Copy link
Collaborator Author

I will look at this more deeply, but I am surprised at the mixed design. Impersonation is a mapping property or a rule property?

you are right, it came during development and thats why I wanted to show the code early to start the discussion here with you about the solution direction. 😃

Looks like you can have more than one mapping to the same rule, some with some without, the impersonate flag. One can set the flag at rule level using a directive and this latter wins over the mapping. On the other hand the add.rule has no option to set this flag.
I would summarize: it is a property at both levels and rule-level wins.

Yes, that is currently.
I started with the Idea to decide it when mapping the rule, because then I know the identity permission for the rule instance and I know that I want impersonation for a rule. Therefore I implemented it for map.rule, to decide per mapping (i have multiple mappings to same rule from different projects - yes larger organization).

Afterwards I faced the situation for one rule: This rule should always run impersonated (on behalf of the event trigger identity).
First Idea was to add it during add.rule, but then I need to know it everytime when adding the rule to an instance. For development I have the rules in source control, and it is also not always me who writes or adapts or deploys a rule.

So it made sense to me to have the information in rule code (directive) as well, to show it everyone looking at the rule code: This rule is an (always) impersonated one.

To summarize, in current code you can decide per mapping if rule should be executed impersonated, or it is in general decided in rule code (which always wins).

I would opt for a property defined at rule-level: one can upload the same source twice to create two rules, one with, the other without impersonation.

This would either be an additional Option or as replacement for the rule directive.
If we go this way, i would loose the information in my rule code and move it to my "release stage" where i do the rollout of rule files.

A stretch goal would be to highlight the impersonation configuration in list.rules output. This is easy if, instead of a directive, you put this flag in the rule configuration (i.e. the application settings). It also enables adding the flag on the add.rule command.

Yes, I am not finished, I also wanted to show this information in the lists commands (either list.rules or list.mappings or both - depending on solution)

What do you think?

@BobSilent BobSilent marked this pull request as ready for review September 21, 2019 10:41
@BobSilent BobSilent force-pushed the create-change-on-behalf-of-initiator branch from ef37cd7 to f7bae18 Compare September 22, 2019 23:17
@stale
Copy link

stale bot commented Nov 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Nov 21, 2019
@BobSilent BobSilent removed the wontfix This will not be worked on label Nov 23, 2019
@BobSilent BobSilent self-assigned this Nov 23, 2019
@BobSilent BobSilent force-pushed the create-change-on-behalf-of-initiator branch from f7bae18 to d584ae5 Compare December 20, 2019 14:06
…ignature of ListRules from KuduFunction to RuleOutputData
RuleCode allows setting default via directive
Configure Impersonation via rule.configure
remove impersonation option from  add.rule, update.rule
reduce compiler messages
Extract some types into separate files and beautify code
@BobSilent BobSilent force-pushed the create-change-on-behalf-of-initiator branch from d584ae5 to cc0a905 Compare December 20, 2019 14:09
giuliov added a commit that referenced this pull request Dec 22, 2019
giuliov added a commit that referenced this pull request Dec 22, 2019
@giuliov
Copy link
Member

giuliov commented Dec 22, 2019

Merged in 84212cb and 4d4a0eb.

@giuliov giuliov closed this Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants