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

CTFd Plugins conflict too much #2511

Open
1 of 6 tasks
miyoyo opened this issue Mar 28, 2024 · 9 comments
Open
1 of 6 tasks

CTFd Plugins conflict too much #2511

miyoyo opened this issue Mar 28, 2024 · 9 comments

Comments

@miyoyo
Copy link
Contributor

miyoyo commented Mar 28, 2024

CTFd's plugin system bases itself on python features, notably, the fact that functions can be modified, wrapped, and replaced entirely at any time.

While this is a good thing for a total conversion, this causes issues when trying to operate multiple plugins, or to keep plugins up to date, as this means that plugins MUST depend on internal implementation details of CTFd.

Some plugins, like challenge, are even depended upon by internal code, via the CHALLENGE_CLASSES global.

I'm opening this issue as more of a tracking issue than anything, as I aim to make the CTFd plugin architecture better, with the objectives of:

  • Being able to run with zero plugins
  • Make plugins behave like they are native parts of CTFd, without having to actually inject itself in native parts
  • Allowing plugins to extend the listed classes (AKA these in a table) without having to override templates, hack through the DB, or other similar behavior
  • Ultimately, move something big, such as MLC, to the plugin architecture.

I'd gladly take suggestions for things to change, but currently, here's what I've thought of:

  • Allow plugins to show up in the config page
  • Add either an extension field on User, or a method to get all attributes from one user that are stored by the plugins
  • Flexibilize the table pages, to allow for easier extension without having to manually support everything in the theme
  • Add an internal event system to allow plugins to react to actions (such as flag submitted, user logged in, etc) without having to rely on database listening
  • Allow plug-ins to depend on other plug-ins being loaded
  • Support translations for plugins

Pull requests:

If you have more ideas, lmk

@pve
Copy link

pve commented Mar 28, 2024

Related, possibly, is how to deploy them consistently while tracking upstream. Here is our approach for that: https://gitlab.com/jointcyberrange.nl/ctfd-docker-with-plugins/-/blob/main/Dockerfile?ref_type=heads
comments welcome.

@pl4nty
Copy link

pl4nty commented Mar 28, 2024

I did something similar, loading plugins from GitHub
https://github.com/pl4nty/containers/blob/main/ctfd%2FDockerfile

@miyoyo miyoyo changed the title CTFd Plugins cannot cooperate CTFd Plugins conflict too much Mar 28, 2024
@ColdHeat
Copy link
Member

I am happy to hear your ideas but try to keep in mind that the plugins are this way for a reason. Pretty much everyone has their own ideas and visions of what they want to achieve on top of CTFd and by offering a high level of flexibility, generally most people's ideas have been possible.

I am a consumer of the plugin system myself so I know it's not perfect but in many situations it has been good to me.

@miyoyo
Copy link
Contributor Author

miyoyo commented Mar 29, 2024

I don't disagree with the ability to hotpatch anything, it's a powerful feature that python lets us use.

The issues I see are mostly related to the ability to combine plug-ins, as two plug-ins that replace the same piece of code can have different interactions depending on load order, some may not load at all, partially load, or crash ctfd.

Hot patching also has the problem of relying on implementation details, which makes most plug-ins version dependent.

My vision with this issue is to expose some explicit hooks to plug-ins, defining a strict API for it, so that the surface area of what has to be hotpatched is minimized, and therefore the risk of conflict is minimized.

@frankli0324
Copy link
Contributor

frankli0324 commented Apr 13, 2024

I attempted implementing plugin dependencies (1. dependencies between plugins, 2. managing plugin dependencies with pip by packaging plugins into python packages) earlier but I ended up in a solution I too am not satisfied with
#2225
just for your reference

to add to your list, plugins should also have the ability to provide their own translations

@frankli0324
Copy link
Contributor

I'm personally not in favor with this (#2509) change since the plugin mechanism allows registering admin pages and many could be achieved through extending templates

https://github.com/frankli0324/ctfd-whale/blob/master/templates/whale_base.html
https://github.com/frankli0324/ctfd-whale/blob/master/__init__.py#L41

I think having a dedicated admin page for configuring plugins not only provides more flexibility and also less chance to be affected by CTFd changes.

@ColdHeat
Copy link
Member

ColdHeat commented Apr 13, 2024 via email

@miyoyo
Copy link
Contributor Author

miyoyo commented Apr 13, 2024

I'm personally not in favor with this (#2509)...

This is exactly against the idea I'm having for plugins, they're currently aliens that modify the CTFd code via hot patching, and they either have to live outside of the rest of CTFd, or they have to modify something and hope nothing else modifies them.

Why do the settings exist for each page in a single place, but plugins would have to register their own page for it?
That's just another place where inconsistency existed, and that change brings them closer to being native citizens of CTFd.

I don't see how this lowers flexibility, or even risk being affected by changes, if for some reason they change in the future, then yeah, it can just be refactored out to their own page.

I attempted implementing plugin dependencies...

I think dependencies are a good idea, however I don't believe they should exist within the context of the absolute horror that is pip, maybe as a later option, maybe as individual plugins, but CTFd shouldn't depend on it.

I drafted down quick notes for a dependency system a while ago which should be fairly quick to implement and easy to use, without having to use external file definitions or draft up an entire dependency resolution system:

  • Each plugin can depend on another plugin by calling required_dependency("plugin_name") (can also have optional dependencies with optional_dependency("plugin_name"))
    • This function returns True if the plugin loads, False if it fails (only if optional=True, otherwise it exits)
  • When this function is called, the current plugin is marked as "building dependencies"
  • The next plugin is then loaded, which can, in turn, depend on other plugins
  • If this plugin is attempted to be loaded while it's building dependencies, we have a dependency loop, FATAL error, exit
  • Once the plugin's call to load() is finished, unmark it as "building dependencies"
  • If the plugin is depended upon after it's been initialized, skip initialization and return OK

to add to your list, plugins should also have the ability to provide their own translations

Good idea!

@frankli0324
Copy link
Contributor

I don't believe they should exist within

well, 1 and 2 are not dependent on each other in my previous impl, the pip is originally involved for unit test coverage, the idea is that we could achieve both mandatory and optional dependencies with a exception type, plugin load could decide wether a plugin is required or optional and choose whether to throw that exception

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

No branches or pull requests

5 participants