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

feat: configuration annotations #112

Open
wants to merge 30 commits into
base: development
Choose a base branch
from

Conversation

gentlementlegen
Copy link
Member

@gentlementlegen gentlementlegen commented Sep 19, 2024

Resolves #80

QA: https://github.com/Meniole/ubiquibot-config/commit/7aaa773aaf3eb93ea1bf1e15706bdc20d2c3e988#commitcomment-147051448

Changes

The kernel is now capable to check the validity of a given configuration. On push event, if the commits contain the .github/.ubiquibot-config.yml, the kernel will proceed to verify its content. It does the current checks:

  • YAML is properly formed
  • YAML is decoded successfully
  • plugins are reachable (URL / Action)
  • plugin with sends the proper arguments

If any of these checks fail, an annotation is made within the commit, pointing to the faulty line.

Side tasks

  • Actions will need a validate-workflow.yml
  • Workers will need a /manifest endpoint
  • The plugin template should be updated

Copy link
Collaborator

@0x4007 0x4007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain to me how this works:

plugin with sends the proper arguments

@gentlementlegen
Copy link
Member Author

Can you explain to me how this works:

plugin with sends the proper arguments

For Actions, it triggers a Validate Schema workflow run, like here https://github.com/Meniole/automated-merging/actions/runs/10969158970 and gets a workflow dispatched event in return
For Workers, it sends a POST to /manifest endpoint and gets a JSON as a reponse with eventual errors

@gentlementlegen
Copy link
Member Author

@whilefoo Please let me know if the logic makes sense, I'll be working on the changes required in the plugins if you give it your go.

@whilefoo
Copy link
Contributor

Looks good, just wondering if it would be better for plugin to provide the schema for the with object in the manifest instead of doing the check. This way we could avoid calling a workflow and async stuff. What do you think?

@gentlementlegen
Copy link
Member Author

gentlementlegen commented Sep 23, 2024

@whilefoo I thought about it but the dangerous thing is that it would allow for possible code injection, and also in the case of plugins like conversation-rewards it would be a very complex schema to provide, so I didn't think it would be reliable.

@gentlementlegen gentlementlegen marked this pull request as ready for review September 23, 2024 03:05
src/github/handlers/push-event.ts Outdated Show resolved Hide resolved
src/github/handlers/push-event.ts Outdated Show resolved Hide resolved
src/github/types/state-validation-payload.ts Show resolved Hide resolved
src/github/handlers/push-event.ts Show resolved Hide resolved
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.

Configuration validation and annotations on errors
3 participants