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-12190: Fix setting of file permissions on non-POSIX filesystems #9968

Merged
merged 1 commit into from
Jan 26, 2021

Conversation

wilkinsona
Copy link
Contributor

This pull request back-ports to 2.6 the changes I contributed in #9947.

Previously, StateDirectory used PosixFilePermissions to configure its directories' permissions which fails on Windows as its file system is not POSIX-compliant. This PR updates StateDirectory to fall back to the File API on non-POSIX-compliant file systems. The File API doesn't allow as much control over the permissions so they're as close as the API permits.

The unit tests have been updated to also verify the behaviour on non-POSIX-compliant file systems.

Committer Checklist (excluded from commit message)

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

Previously, StateDirectory used PosixFilePermissions to configure its
directories' permissions which fails on Windows as its file system is
not POSIX-compliant. This commit updates StateDirectory to fall back to
the File API on non-POSIX-compliant file systems.
Copy link
Contributor

@ableegoldman ableegoldman left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for porting this to the 2.6 branch. LGTM. Will merge once the build completes

@ableegoldman
Copy link
Contributor

Huh, the Java 8 & 14 builds both passed but the Java 11 build has been still in progress for a while now. Seems to be environmental -- in fact if I look at the output it actually seems all tests have passed, and it's just hanging somewhere in the shutdown 🤔

Will test out this patch locally to be safe before merging

@ableegoldman ableegoldman merged commit fab75b7 into apache:2.6 Jan 26, 2021
@ableegoldman
Copy link
Contributor

Merged to 2.6

@wilkinsona
Copy link
Contributor Author

Thanks again.

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.

2 participants