-
Notifications
You must be signed in to change notification settings - Fork 3
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
Simplify finding external plugins by using PYTHONPATH #19
Comments
If we do this, we should probably make sure we're adding to |
Good point, and thank you for bringing it up! I agree this would work. I'm on the fence about whether it would be better though, but it's certainly worth considering. For example, if a certbot plugin snap accidentally drops unrelated things into that path that breaks things, and that breaks certbot, then would restricting the plugin to plugin module loading only limit the damage, or make it more obvious to the user that the plugin is causing the problem? I'm excluding anything malicious here - for an attacker it'll make no difference either way I think. I'm just wondering about whether there would be any UX downsides should anything go wrong. |
That's a good question and is something someone should probably look into more before we change things here. I also think we can change how this works after snaps are released. As long as we're careful to make sure the core snap's paths are searched before any plugin paths, the only thing I can immediately think of that a plugin could do is include broken modules that Certbot or its dependencies try to load before Certbot tries to load its plugins and the modules are not required or included in the Certbot snap. Some projects definitely do this, but I think it's somewhat uncommon. I also think that Certbot currently doesn't do much to prevent a broken plugin from crashing it. I think if a plugin fails to load at all, Certbot will crash. This is something we could improve in the future though and limiting the scope where the plugins modules are in Certbot's |
I hackily moved this conversation to certbot/certbot#7945. Please add any new discussion there. |
While I fine making minor modifications to Certbot to make snaps work, I think we may not have to.
Instead of using a separate
CERTBOT_PLUGIN_PATH
environment variable and making the changes to Certbot in f9ba645 to use it, I think we can just set the PYTHONPATH environment variable in the Certbot shell wrapper.I hackily made these changes locally and it seemed to work.
The text was updated successfully, but these errors were encountered: