-
Notifications
You must be signed in to change notification settings - Fork 77
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
[AAP-13408] Add support for custom actions #532
base: main
Are you sure you want to change the base?
Conversation
0df9175
to
c5dfbf0
Compare
@@ -435,6 +447,46 @@ async def _call_action( | |||
except BaseException as e: | |||
logger.error(e) | |||
raise | |||
elif action_plugin := self.find_action(action): |
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.
@benthomasson are we going to have special names for the custom actions so they don't conflict with our builtin action names. If the name conflict exists we will only run the builtin and not run the custom actions.
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.
@benthomasson Since the code is the same for the 2 find_actions call can we move it into a separate function
def run_action_plugin(self,....):
action_plugin = self.find_action(action) or find_action(*split_collection_name(action))
if not action_plugin:
return
try:
await action_plugin["main"](
event_log=self.event_log,
inventory=inventory,
hosts=hosts,
variables=variables,
project_data_file=self.project_data_file,
source_ruleset_name=ruleset,
source_ruleset_uuid=ruleset_uuid,
source_rule_name=rule,
source_rule_uuid=rule_uuid,
rule_run_at=rule_run_at,
**action_args,
)
except Exception as e:
logger.error("Error calling action %s, err %s", action, str(e))
except BaseException as e:
logger.error(e)
raise
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.
@benthomasson doing this search with every action call might not make sense, we could load all the actions in when we read the ruleset. Which also allows us to ensure that all the actions are valid before we start accepting the events and running the rule engine. if the actions are missing we can do an early termination to indicate that actions are missing.
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.
@benthomasson are we going to have special names for the custom actions so they don't conflict with our builtin action names. If the name conflict exists we will only run the builtin and not run the custom actions.
We should support FQCN for actions. Names without collections prefixes are either builtin or loaded from a local directory. The precedence should be builtin and then local directory for non-FQCN.
@@ -134,6 +137,24 @@ def load_rulebook(collection, rulebook): | |||
return yaml.safe_load(f.read()) | |||
|
|||
|
|||
def has_action(collection, action): |
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.
@benthomasson if we can take in an additional parameter additional_dirs, here which is the directory names we can search for the files in the collection location and an additional directory
def has_action(collection, action, additional_dirs = []):
dirs_to_search = additional_dirs
dirs_to_search.append(EDA_ACTION_PATHS)
return has_object(
collection,
action,
dirs_to_search,
".py",
)
source_rule_uuid=rule_uuid, | ||
rule_run_at=rule_run_at, | ||
**action_args, | ||
) |
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.
@benthomasson do we have to send the result of the action_plugin to the AAP server, after it has finished execution. Will there be a post_event/set_fact at the end of this action. Would it make sense to extract out some of the logic from builtin which does the reporting for us.
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.
We probably should send the result. Moving the reporting code out of builtin into a different module is a good idea.
What is it the story/ticket related? |
AAP-13408 |
94ca0e2
to
5cd07b2
Compare
Note to self: this needs a way to dynamically extend the rulebook schema or allow for arbitrary actions in the schema. |
We could establish that each action has to be hosted in a folder and that it should contain |
5cd07b2
to
a02b525
Compare
3456e39
to
1d55262
Compare
1d55262
to
d0afb16
Compare
@@ -92,6 +92,8 @@ async def run(parsed_args: argparse.Namespace) -> None: | |||
startup_args.controller_url = parsed_args.controller_url | |||
startup_args.controller_token = parsed_args.controller_token | |||
startup_args.controller_ssl_verify = parsed_args.controller_ssl_verify | |||
startup_args.source_dir = parsed_args.source_dir |
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.
@benthomasson Can these go into the settings and it can be accessed from anywhere. We have been moving most of the passed in args into the settings.
inventory, | ||
hosts, | ||
) | ||
await action_plugin.main(metadata, control, **action_args) |
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.
@benthomasson Should we pass the metadata, control which are data classes as dictionaries into the action plugin so they can be tested outside of ansible rulebook otherwise they would have to know about the Metadata,Control data class definitions and if we change it it will effect the plugin. Also for the feedback should we be passing in a function that can they call to send the feedback to the eda-server. We should have a very loose coupling between action plugin and ansible-rulebook so each can be tested separately.
Add action loading from a directory