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

KAFKA-13337: fix of possible java.nio.file.AccessDeniedException during Con… #11371

Closed
wants to merge 1 commit into from

Conversation

heritamas
Copy link

…nect plugin directory scan

org.apache.kafka.connect.runtime.isolation.PluginUtils.pluginUrls scans a path and collects plugin candidates from there. However, if a directory is not readable, it fails with AccessDeniedException instead of skipping it. This commit fixes this minor problem with the change of plugin path filter used during scanning.

I've also added a unit test for proof.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@akatona84
Copy link
Contributor

@heritamas pls change the title MINOR part to be KAFKA-13337

Hey @kkonstantine , could you take a look, pls?
Connect dies during startup if it encounters unreadable plugin dir, minor fix, but we created a ticket for that, maybe others would hit it too and they could find jira about it.

@heritamas heritamas changed the title MINOR: fix of possible java.nio.file.AccessDeniedException during Con… KAFKA-13337: fix of possible java.nio.file.AccessDeniedException during Con… Oct 1, 2021
…ng Connect plugin directory scan

org.apache.kafka.connect.runtime.isolation.PluginUtils.pluginUrls scans a path and collects plugin candidates from there. However, if a directory is not readable, it fails with AccessDeniedException instead of skipping it. This commit fixes this minor problem with the change of plugin path filter used during scanning.
Copy link
Contributor

@viktorsomogyi viktorsomogyi left a comment

Choose a reason for hiding this comment

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

Looks good to me. Ran the failed tests locally and it seems like they're flaky (ran successfully locally). @kkonstantine would you please review this too?

@mimaison
Copy link
Member

Thanks for the PR!

I played a bit locally and while Connect logs a AccessDeniedException exception if a file/directory is not readable, Connect seems to work fine and in the logs I can see Connect is ignoring the directory:

[2021-10-25 11:43:59,745] ERROR Could not get listing for plugin path: /tmp/connect. Ignoring. (org.apache.kafka.connect.runtime.isolation.DelegatingClassLoader:257)
java.nio.file.AccessDeniedException: /tmp/connect/private

KAFKA-13337 mentions Connect will fail with an AccessDeniedException., is there another issue or is the ticket just about the exception/stacktrace? (It's still valuable to address the stacktrace but I just want to be sure there's not another issue)

@akatona84
Copy link
Contributor

@mimaison , correct, connect could work, but the plugins are not loaded. My connect stopped because an extension was supposed to be loaded and a ConnectException was thrown and connect died.

@mimaison
Copy link
Member

mimaison commented Nov 3, 2021

Sorry I'm still not quite sure I fully understand the issue you're describing.
If the path is not readable, yes Connect won't load plugins in it. But it's printing an error message Could not get listing for plugin path: <PATH>. Ignoring. so it's easy to spot there's an issue with permissions.

As far as I can tell, with this PR, we don't get a message anymore. I think this could make debugger harder.

When there's an unreadable folder in my plugin.path, I don't see any issues with Connect. It just skipped that folder but otherwise works just fine. Can you explain why yours failed/stopped?

@akatona84
Copy link
Contributor

akatona84 commented Nov 3, 2021

IMHO we could enhance the code to log those which are not eligible for the conditions (readable, directory or jar, whatever) if that helps.
The problem is that if it's not readable, it'll fail to load ANY plugins. Not just skipping the problematic, no plugin will be available/loaded at all.

@mimaison
Copy link
Member

@akatona84 Sorry this PR felt through the cracks. If I remember correctly, last time I was looking at it I had trouble replicating the issue. Can you provide steps to reproduce?

@akatona84
Copy link
Contributor

@mimaison , np, thx for following up :)

steps to fail:

  1. create - for example - an environment variable config provider, create a jar from it
  2. put it into the plugins directory
  3. configure this config provider in connect-distributed.properties and also use it at another config entry. e.g.
something=${env:MY_ENVIRONMENT_VARIABLE}
  1. create a directory within the plugins dir and make it unreadable via chmod (chmod 0000 would do the trick)
  2. try to start connect

It proves that connect won't load any plugin not just skipping the non-readable one. (And the server dies because of the missing config provider.)

I hope it helps.

@viktorsomogyi
Copy link
Contributor

Since #13733 is the continuation of this task, I'll close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants