Uploaded image for project: 'Log4j 2'
  1. Log4j 2
  2. LOG4J2-1699

Configurable Log File Permissions with PosixFilePermission

    Details

    • Type: Question
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0
    • Component/s: Appenders
    • Labels:
    • Environment:

      Linux

      Description

      We would like to hear the communities thoughts on being able to configure the permissions log files are created with. We don't want to rely on UMASK because we have managed services who's process should generate logs with a 644 yet deployed applications by users should default to a 640 because the logs may contain sensitive information.

      We will make the modification and set this in the properties file. Now we are looking to see what the community position would be on accepting such a patch, we don't want to be patching our own distribution indefinitely.

      I searched all the JIRAs and was not able to find any matching requirements recently. All I could find was something dated in 2006: https://bz.apache.org/bugzilla/show_bug.cgi?id=40407

      1. LOG4J2-1699.patch
        53 kB
        Pierrick HYMBERT
      2. LOG4J2-1699-2.patch
        51 kB
        Pierrick HYMBERT

        Issue Links

          Activity

          Hide
          garydgregory Gary Gregory added a comment -

          What do you plan more specifically? Allow the user to specify a combo from https://docs.oracle.com/javase/7/docs/api/java/nio/file/attribute/PosixFilePermission.html ?

          Show
          garydgregory Gary Gregory added a comment - What do you plan more specifically? Allow the user to specify a combo from https://docs.oracle.com/javase/7/docs/api/java/nio/file/attribute/PosixFilePermission.html ?
          Hide
          ddimatos Demetrios Dimatos added a comment -

          That is exactly what i was thinking. I would allow for the properties to be set for the file appender so that when a new log file is created these file permissions are honored.

          Show
          ddimatos Demetrios Dimatos added a comment - That is exactly what i was thinking. I would allow for the properties to be set for the file appender so that when a new log file is created these file permissions are honored.
          Hide
          garydgregory Gary Gregory added a comment - - edited

          So this is about calling java.nio.file.Files.setPosixFilePermissions(Path, Set<PosixFilePermission>) at just the right time.

          It might also be interesting to have a separate ticket for java.nio.file.Files.setOwner(Path, UserPrincipal).

          Give it a go

          Show
          garydgregory Gary Gregory added a comment - - edited So this is about calling java.nio.file.Files.setPosixFilePermissions(Path, Set<PosixFilePermission>) at just the right time. It might also be interesting to have a separate ticket for java.nio.file.Files.setOwner(Path, UserPrincipal) . Give it a go
          Hide
          ddimatos Demetrios Dimatos added a comment -

          Thanks Gary!!!

          Show
          ddimatos Demetrios Dimatos added a comment - Thanks Gary!!!
          Hide
          garydgregory Gary Gregory added a comment -

          Demetrios Dimatos,

          Do you plan on providing a patch with unit tests?

          Gary

          Show
          garydgregory Gary Gregory added a comment - Demetrios Dimatos , Do you plan on providing a patch with unit tests? Gary
          Hide
          ddimatos Demetrios Dimatos added a comment -

          Hi Gary,
          I do plan on submitting this in the coming month. I had some other work taking priorities. Should we change this from type question to improvement ?
          I apologize for the lack of communication , my employer does want this in place when the Hadoop stack adopts Log4j2.

          Sent from my phone

          Show
          ddimatos Demetrios Dimatos added a comment - Hi Gary, I do plan on submitting this in the coming month. I had some other work taking priorities. Should we change this from type question to improvement ? I apologize for the lack of communication , my employer does want this in place when the Hadoop stack adopts Log4j2. Sent from my phone
          Hide
          jvz Matt Sicker added a comment -

          Improvement would work. We keep a manual changelog that is more of a master list rather than relying on the generated JIRA changelog for each release, so really, the type doesn't matter all that much in the end.

          Show
          jvz Matt Sicker added a comment - Improvement would work. We keep a manual changelog that is more of a master list rather than relying on the generated JIRA changelog for each release, so really, the type doesn't matter all that much in the end.
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Hello Team,

          I have tried to implement posix user, group and permissions in the attached patch within the FileManager hierarchy.

          Please review and correct me if something wrong.
          Done a simple test for permissions on posix systems only although I dont know how to unit test user and group.
          Also I am not sure if FileOutputStream create the file on all jvm implementation, tested on .

          NB: this feature was also required in a stackoverflow question

          Show
          phymbert Pierrick HYMBERT added a comment - Hello Team, I have tried to implement posix user, group and permissions in the attached patch within the FileManager hierarchy. Please review and correct me if something wrong. Done a simple test for permissions on posix systems only although I dont know how to unit test user and group. Also I am not sure if FileOutputStream create the file on all jvm implementation, tested on . NB: this feature was also required in a stackoverflow question
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 8318a3856ac6fedb2374fa894b48f5e54ff9d7d9 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=8318a38 ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Patch applied with some minor tweaks.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 8318a3856ac6fedb2374fa894b48f5e54ff9d7d9 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=8318a38 ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Patch applied with some minor tweaks.
          Hide
          garydgregory Gary Gregory added a comment -

          Patch applied with some minor tweaks. Thank you!

          Please verify and close.

          Gary

          Show
          garydgregory Gary Gregory added a comment - Patch applied with some minor tweaks. Thank you! Please verify and close. Gary
          Hide
          ralph.goers@dslextreme.com Ralph Goers added a comment - - edited

          The Jenkins build now has a unit test failing.

          org.junit.ComparisonFailure: expected:<rw-r[w]-r--> but was:<rw-r[-]-r-->
          	at org.apache.logging.log4j.core.appender.FilePermissionsTest.testFilePermissions(FilePermissionsTest.java:126)
          

          Gary, did you run the tests before you merged it?

          Show
          ralph.goers@dslextreme.com Ralph Goers added a comment - - edited The Jenkins build now has a unit test failing. org.junit.ComparisonFailure: expected:<rw-r[w]-r--> but was:<rw-r[-]-r--> at org.apache.logging.log4j.core.appender.FilePermissionsTest.testFilePermissions(FilePermissionsTest.java:126) Gary, did you run the tests before you merged it?
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b1356fa16e8ae19eba8bcd117212d489f041f5a7 in logging-log4j2's branch refs/heads/master from Gary Gregory
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b1356fa ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Update condition for test exit with a JUnit Assume.

          Show
          jira-bot ASF subversion and git services added a comment - Commit b1356fa16e8ae19eba8bcd117212d489f041f5a7 in logging-log4j2's branch refs/heads/master from Gary Gregory [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b1356fa ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Update condition for test exit with a JUnit Assume.
          Hide
          garydgregory Gary Gregory added a comment -

          Yes, I ran the tests on Windows 10 from both Maven and Eclipse and they passed.

          Show
          garydgregory Gary Gregory added a comment - Yes, I ran the tests on Windows 10 from both Maven and Eclipse and they passed.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b92934bc671ef990d371ccb72fbf474f09a680ad in logging-log4j2's branch refs/heads/master from Gary Gregory
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b92934b ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Reformat the new method definePathAttributeView().

          Show
          jira-bot ASF subversion and git services added a comment - Commit b92934bc671ef990d371ccb72fbf474f09a680ad in logging-log4j2's branch refs/heads/master from Gary Gregory [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b92934b ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Reformat the new method definePathAttributeView().
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 95f089fe73d40ec786a8df514fdc13bc9eef2169 in logging-log4j2's branch refs/heads/master from Gary Gregory
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=95f089f ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Tweak one of the test fixtures to try to make it
          run on Jenkins.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 95f089fe73d40ec786a8df514fdc13bc9eef2169 in logging-log4j2's branch refs/heads/master from Gary Gregory [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=95f089f ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Tweak one of the test fixtures to try to make it run on Jenkins.
          Hide
          garydgregory Gary Gregory added a comment -

          I tweaked the test fixture a bit and now the test passes on Jenkins: https://builds.apache.org/job/Log4j%202.x/2816/org.apache.logging.log4j$log4j-core/

          So I think we are OK.

          Show
          garydgregory Gary Gregory added a comment - I tweaked the test fixture a bit and now the test passes on Jenkins: https://builds.apache.org/job/Log4j%202.x/2816/org.apache.logging.log4j$log4j-core/ So I think we are OK.
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Thank you Gary for effort to integrate this patch.
          Actually I faced the exact opposite error on a redhadt6 + oraclejdk7: rw-rw-r-- instead of rw-r-r-. I do think this test maybe unstable on other posix Os. Now we know it works somehow on posix systems but better to ignore this test for alternate manual build ?
          Nb: noted for indentation rules

          Show
          phymbert Pierrick HYMBERT added a comment - Thank you Gary for effort to integrate this patch. Actually I faced the exact opposite error on a redhadt6 + oraclejdk7: rw-rw-r-- instead of rw-r- r -. I do think this test maybe unstable on other posix Os. Now we know it works somehow on posix systems but better to ignore this test for alternate manual build ? Nb: noted for indentation rules
          Hide
          garydgregory Gary Gregory added a comment -

          The test feels brittle sadly but Jenkins passes now so that's good and Windows passes as well. Any way you can think of improving the test is appreciated

          Show
          garydgregory Gary Gregory added a comment - The test feels brittle sadly but Jenkins passes now so that's good and Windows passes as well. Any way you can think of improving the test is appreciated
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Hello Gary Gregory,

          Please consider this new patch, rebased on master:

          1. Finally understood the issue with attributes on jenkins, when createOnDemand=false, definition of posix file attributes was not called and user umask was applied as before (that's why it differed from my redhat for example)
          2. As far as most of log4j users are using RollingFileAppender, this fix propagates by default the log file defined permissions to rolled files, compressed or not
          3. It is also now possible to define a filePermissions action within the rolling strategy, ie have different permissions for current file and rolled files
          4. Moved defineFilePosixAttributeView to FileUtils class
          5. Add a test for fileOwner and fileGroup (that's fixed fileGroup)
          6. Add test for rolling files both scenarios (propagation of permissions to rolled file + test of PosixViewAttributeAction)

          Example of configurations:

           <RollingFile name="RollingFile2" fileName="target/rollingpermissions1/test2.log"
                           filePattern="target/rollingpermissions1/test2-$${date:MM-dd-yyyy}-%i.log.gz"
                           filePermissions="rw-------">
                <PatternLayout>
                  <Pattern>%m%n</Pattern>
                </PatternLayout>
                <SizeBasedTriggeringPolicy size="500" />
                <DefaultRolloverStrategy stopCustomActionsOnError="true">
                  <PosixViewAttribute basePath="target/rollingpermissions1" filePermissions="r--r--r--">
                  	<IfFileName glob="*.gz" /> 
                  </PosixViewAttribute>
                </DefaultRolloverStrategy>
              </RollingFile>
          

          mvn clean install tested on redhat, ubuntu and windows, hope it will not break the build

          Show
          phymbert Pierrick HYMBERT added a comment - Hello Gary Gregory , Please consider this new patch, rebased on master: Finally understood the issue with attributes on jenkins, when createOnDemand =false, definition of posix file attributes was not called and user umask was applied as before (that's why it differed from my redhat for example) As far as most of log4j users are using RollingFileAppender, this fix propagates by default the log file defined permissions to rolled files, compressed or not It is also now possible to define a filePermissions action within the rolling strategy, ie have different permissions for current file and rolled files Moved defineFilePosixAttributeView to FileUtils class Add a test for fileOwner and fileGroup (that's fixed fileGroup) Add test for rolling files both scenarios (propagation of permissions to rolled file + test of PosixViewAttributeAction) Example of configurations: <RollingFile name= "RollingFile2" fileName= "target/rollingpermissions1/test2.log" filePattern= "target/rollingpermissions1/test2-$${date:MM-dd-yyyy}-%i.log.gz" filePermissions= "rw-------" > <PatternLayout> <Pattern> %m%n </Pattern> </PatternLayout> <SizeBasedTriggeringPolicy size= "500" /> <DefaultRolloverStrategy stopCustomActionsOnError= "true" > <PosixViewAttribute basePath= "target/rollingpermissions1" filePermissions= "r--r--r--" > <IfFileName glob= "*.gz" /> </PosixViewAttribute> </DefaultRolloverStrategy> </RollingFile> mvn clean install tested on redhat, ubuntu and windows, hope it will not break the build
          Hide
          ralph.goers@dslextreme.com Ralph Goers added a comment -

          I haven't looked at the patch but I just wanted to say that we definitely appreciate your hard work on this!

          Show
          ralph.goers@dslextreme.com Ralph Goers added a comment - I haven't looked at the patch but I just wanted to say that we definitely appreciate your hard work on this!
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Thank you for your encouragement Ralph Goers, let's agree this is just a minor enhancement within a big project that make our every day life easier

          Show
          phymbert Pierrick HYMBERT added a comment - Thank you for your encouragement Ralph Goers , let's agree this is just a minor enhancement within a big project that make our every day life easier
          Hide
          jvz Matt Sicker added a comment -

          In the future, note that there's a docker build included so you can cross compile for Linux on Windows and macOS. However, I believe it has to be updated with the Java 9 toolchain stuff now.

          Show
          jvz Matt Sicker added a comment - In the future, note that there's a docker build included so you can cross compile for Linux on Windows and macOS. However, I believe it has to be updated with the Java 9 toolchain stuff now.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5b2df230d9809962687fd792d70c1f4a46961719 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=5b2df23 ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Apply patch with some formatting tweaks to
          RollingAppenderSizeCompressPermissionsTest.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5b2df230d9809962687fd792d70c1f4a46961719 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=5b2df23 ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Apply patch with some formatting tweaks to RollingAppenderSizeCompressPermissionsTest.
          Hide
          garydgregory Gary Gregory added a comment -

          Thank you Pierrick HYMBERT.

          I applied patch 2 (see previous comment).

          My only concern is that the new class PosixViewAttributeAction uses a factory method instead of a Builder. As soon as we'll want to add anything, we will have to deprecate the method in favor of a new one (this happens time and time again).

          How do you feel about updating the class to use a Builder?

          Gary

          Show
          garydgregory Gary Gregory added a comment - Thank you Pierrick HYMBERT . I applied patch 2 (see previous comment). My only concern is that the new class PosixViewAttributeAction uses a factory method instead of a Builder. As soon as we'll want to add anything, we will have to deprecate the method in favor of a new one (this happens time and time again). How do you feel about updating the class to use a Builder? Gary
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user phymbert opened a pull request:

          https://github.com/apache/logging-log4j2/pull/83

          LOG4J2-1699 Refactor PosixFileAttributeViewAction with Builder. Fix posix tests on jenkins.

          Refactor PosixFileAttributeViewAction with Builder rather than method factory. Split posix tests inner static classes to 2 differents test class files because it doesnt run on jenkins. Add a static method in FileUtils to check if posix file attribute view is supported in order to log if posix attributes are defined
          but the filesystem doesnt support it. Fix when user has only one group in the test case.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/logging-log4j2/pull/83.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #83


          commit 64c55f2b60d1a22e948969582c0779e630b728c1
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-22T12:13:54Z

          LOG4J2-1699 Refactor PosixFileAttributeViewAction with Builder rather
          than method factory. Split posix tests inner static classes to 2
          differents test class file because it doesnt run on jenkins. Add a
          static method in FileUtils to check if posix file attribute view is
          supported. It will allow later to log if posix attributes are defined
          but the file system doesnt support them. Fix when user has only one
          group in the test case.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user phymbert opened a pull request: https://github.com/apache/logging-log4j2/pull/83 LOG4J2-1699 Refactor PosixFileAttributeViewAction with Builder. Fix posix tests on jenkins. Refactor PosixFileAttributeViewAction with Builder rather than method factory. Split posix tests inner static classes to 2 differents test class files because it doesnt run on jenkins. Add a static method in FileUtils to check if posix file attribute view is supported in order to log if posix attributes are defined but the filesystem doesnt support it. Fix when user has only one group in the test case. Signed-off-by: phymbert <pierrick.hymbert@gmail.com> You can merge this pull request into a Git repository by running: $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/logging-log4j2/pull/83.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #83 commit 64c55f2b60d1a22e948969582c0779e630b728c1 Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-22T12:13:54Z LOG4J2-1699 Refactor PosixFileAttributeViewAction with Builder rather than method factory. Split posix tests inner static classes to 2 differents test class file because it doesnt run on jenkins. Add a static method in FileUtils to check if posix file attribute view is supported. It will allow later to log if posix attributes are defined but the file system doesnt support them. Fix when user has only one group in the test case. Signed-off-by: phymbert <pierrick.hymbert@gmail.com>
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Hello,

          Well I did it this way - Builder - in this new patch, I have tried PR approach on github (see above), hope it is ok for you.

          Let me know if we are good.

          Show
          phymbert Pierrick HYMBERT added a comment - Hello, Well I did it this way - Builder - in this new patch, I have tried PR approach on github (see above), hope it is ok for you. Let me know if we are good.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 9d32793b12e2fe9b7a77519e59f5027942db4917 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=9d32793 ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Apply patch.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 9d32793b12e2fe9b7a77519e59f5027942db4917 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=9d32793 ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Apply patch.
          Hide
          garydgregory Gary Gregory added a comment -

          Pierrick HYMBERT:

          Thank you for that last PR. Now for icing on top, would you mind updating the docs?

          Thank you,
          Gary

          Show
          garydgregory Gary Gregory added a comment - Pierrick HYMBERT : Thank you for that last PR. Now for icing on top, would you mind updating the docs? Thank you, Gary
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user phymbert commented on the issue:

          https://github.com/apache/logging-log4j2/pull/83

          Merged in 9d32793b12e2fe9b7a77519e59f5027942db4917

          Show
          githubbot ASF GitHub Bot added a comment - Github user phymbert commented on the issue: https://github.com/apache/logging-log4j2/pull/83 Merged in 9d32793b12e2fe9b7a77519e59f5027942db4917
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user phymbert closed the pull request at:

          https://github.com/apache/logging-log4j2/pull/83

          Show
          githubbot ASF GitHub Bot added a comment - Github user phymbert closed the pull request at: https://github.com/apache/logging-log4j2/pull/83
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user phymbert opened a pull request:

          https://github.com/apache/logging-log4j2/pull/85

          LOG4J2-1699 Documentation updated for Posix File Attribute view feature

          I have run site:site goal, looks not bad. Please double check my english

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699-DOC

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/logging-log4j2/pull/85.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #85


          commit 3b4bc474b2d3fd7de615d9970a678bcdeceb564a
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-23T10:46:03Z

          LOG4J2-1699 Added documentation for new file attribute view parameters
          in FileManager hierarchy.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          commit 9e6223d352a16e5161b786c6237dd4576ff2875e
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-23T10:47:20Z

          LOG4J2-1699 Fix wrong link RollingFileAppender documentation
          introduction.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          commit adcca77da380d57c2c3011413383fb9745e89b15
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-23T11:12:34Z

          LOG4J2-1699 Documentation for PosixViewAttributeAction.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user phymbert opened a pull request: https://github.com/apache/logging-log4j2/pull/85 LOG4J2-1699 Documentation updated for Posix File Attribute view feature I have run site:site goal, looks not bad. Please double check my english You can merge this pull request into a Git repository by running: $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699 -DOC Alternatively you can review and apply these changes as the patch at: https://github.com/apache/logging-log4j2/pull/85.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #85 commit 3b4bc474b2d3fd7de615d9970a678bcdeceb564a Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-23T10:46:03Z LOG4J2-1699 Added documentation for new file attribute view parameters in FileManager hierarchy. Signed-off-by: phymbert <pierrick.hymbert@gmail.com> commit 9e6223d352a16e5161b786c6237dd4576ff2875e Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-23T10:47:20Z LOG4J2-1699 Fix wrong link RollingFileAppender documentation introduction. Signed-off-by: phymbert <pierrick.hymbert@gmail.com> commit adcca77da380d57c2c3011413383fb9745e89b15 Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-23T11:12:34Z LOG4J2-1699 Documentation for PosixViewAttributeAction. Signed-off-by: phymbert <pierrick.hymbert@gmail.com>
          Hide
          phymbert Pierrick HYMBERT added a comment -

          Hello Gary Gregory, I have tried to update documentation in this PR #85, but this is my first attempt please correct any mistake I have done. Thank you!

          Show
          phymbert Pierrick HYMBERT added a comment - Hello Gary Gregory , I have tried to update documentation in this PR #85 , but this is my first attempt please correct any mistake I have done. Thank you!
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b96e13342dc4cd514d75253212decb9ec1188c18 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b96e133 ]

          LOG4J2-1699 Configurable Log File Permissions with
          PosixFilePermission. Apply doc patch.

          Show
          jira-bot ASF subversion and git services added a comment - Commit b96e13342dc4cd514d75253212decb9ec1188c18 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=b96e133 ] LOG4J2-1699 Configurable Log File Permissions with PosixFilePermission. Apply doc patch.
          Hide
          garydgregory Gary Gregory added a comment -

          Pierrick HYMBERT,

          Thank you for your doc patch.

          Shouldn't the RollingFileAppender also include the same documentation? What about other appenders?

          Gary

          Show
          garydgregory Gary Gregory added a comment - Pierrick HYMBERT , Thank you for your doc patch. Shouldn't the RollingFileAppender also include the same documentation? What about other appenders? Gary
          Hide
          phymbert Pierrick HYMBERT added a comment - - edited

          Hello Gary Gregory,
          I am not sure, I did it no ? Both FileAppender, RollingFileAppender, RollingRandomAccessFileAppender and even CustomPosixViewAttributeAction, can you please confirm what I should add ?

          Show
          phymbert Pierrick HYMBERT added a comment - - edited Hello Gary Gregory , I am not sure, I did it no ? Both FileAppender , RollingFileAppender , RollingRandomAccessFileAppender and even CustomPosixViewAttributeAction , can you please confirm what I should add ?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user phymbert commented on the issue:

          https://github.com/apache/logging-log4j2/pull/85

          Merged in b96e13342dc4cd514d75253212decb9ec1188c18

          Show
          githubbot ASF GitHub Bot added a comment - Github user phymbert commented on the issue: https://github.com/apache/logging-log4j2/pull/85 Merged in b96e13342dc4cd514d75253212decb9ec1188c18
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user phymbert closed the pull request at:

          https://github.com/apache/logging-log4j2/pull/85

          Show
          githubbot ASF GitHub Bot added a comment - Github user phymbert closed the pull request at: https://github.com/apache/logging-log4j2/pull/85
          Hide
          garydgregory Gary Gregory added a comment -

          Hi Pierrick HYMBERT:

          You are correct of course, my mistake, sorry.

          Can you think of anything else needed to complete or polish this issue?

          Thank you,
          Gary

          Show
          garydgregory Gary Gregory added a comment - Hi Pierrick HYMBERT : You are correct of course, my mistake, sorry. Can you think of anything else needed to complete or polish this issue? Thank you, Gary
          Hide
          phymbert Pierrick HYMBERT added a comment -

          I think the goal is reached with this first version, then we should wait users feedback if it matches requirements.

          Also before to freeze this new API, I would like to have feedback from the log4j team about following point:

          1. Is XML Configuration tag should be shortenen, filePermissions=>perm, fileOwner=>usr, fileGroup=>grp, PosixViewAttribute=>PosixAttr ?
          2. It is not clear what to do if posix attribute view are defined and the underlying files system does not support it ?
          3. Should we catch UnsupportedOperationException and OperationNotPermitted exception ?
          4. In org.apache.logging.log4j.core.appender.rolling.DirectWriteRolloverStrategy.rollover(RollingFileManager), I apply again file attribute, I think it is wrong because file is the same as the current file name, please confirm
          5. Should we imagine now an entry point for other FileAttributeView types: AclFileAttributeView, DosFileAttributeView ?
          Show
          phymbert Pierrick HYMBERT added a comment - I think the goal is reached with this first version, then we should wait users feedback if it matches requirements. Also before to freeze this new API, I would like to have feedback from the log4j team about following point: Is XML Configuration tag should be shortenen, filePermissions=>perm, fileOwner=>usr, fileGroup=>grp, PosixViewAttribute=>PosixAttr ? It is not clear what to do if posix attribute view are defined and the underlying files system does not support it ? Should we catch UnsupportedOperationException and OperationNotPermitted exception ? In org.apache.logging.log4j.core.appender.rolling.DirectWriteRolloverStrategy.rollover(RollingFileManager), I apply again file attribute, I think it is wrong because file is the same as the current file name, please confirm Should we imagine now an entry point for other FileAttributeView types: AclFileAttributeView, DosFileAttributeView ?
          Hide
          ralph.goers@dslextreme.com Ralph Goers added a comment -

          For item 2, If there is an error configuring an element standard practice is that an error or warning is logged using the StatusLogger and a null value is returned for that element. So in this case it would act like it wasn't specified, which I think is what you would want along with the logged message. Ideally, lack of support for this should be caught during configuration, not while logging an event or during rollover.
          For item 3, I would catch the exception, log the issue and return null.
          For item 4, every time a rollover occurs DirectWriteRolloverStrategy creates a new file and starts writing to it. If you want posix attributes specified the time to do it is when the file is created.

          Show
          ralph.goers@dslextreme.com Ralph Goers added a comment - For item 2, If there is an error configuring an element standard practice is that an error or warning is logged using the StatusLogger and a null value is returned for that element. So in this case it would act like it wasn't specified, which I think is what you would want along with the logged message. Ideally, lack of support for this should be caught during configuration, not while logging an event or during rollover. For item 3, I would catch the exception, log the issue and return null. For item 4, every time a rollover occurs DirectWriteRolloverStrategy creates a new file and starts writing to it. If you want posix attributes specified the time to do it is when the file is created.
          Hide
          garydgregory Gary Gregory added a comment - - edited

          1. Is XML Configuration tag should be shortenen, filePermissions=>perm, fileOwner=>usr, fileGroup=>grp, PosixViewAttribute=>PosixAttr ?

          I would not shorten.

          I'd go with what Ralph suggests.

          For (5), it's up to you, if you know there is a user case that is.

          Show
          garydgregory Gary Gregory added a comment - - edited 1. Is XML Configuration tag should be shortenen, filePermissions=>perm, fileOwner=>usr, fileGroup=>grp, PosixViewAttribute=>PosixAttr ? I would not shorten. I'd go with what Ralph suggests. For (5), it's up to you, if you know there is a user case that is.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user phymbert opened a pull request:

          https://github.com/apache/logging-log4j2/pull/89

          LOG4J2-1699 Log File Permissions with PosixFilePermission feedback from jira

          item 2: Better log at configuration step if file attribute view are defined but underlying files system doesnt support it
          item 3: Exception catch and logged if OperationNotSupported or Operation not permitted are thrown while changing file attribute permissions, user or group
          item 4: No need to apply file posix attribute if file is just rolled not compressed, both in DirectWriteRolloverStrategy and DefaultRolloverStrategy.

          Changed next release version in documentation and javadoc.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/logging-log4j2/pull/89.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #89


          commit 38e0becb1ac0ef01b8197931e08575ed1cb44387
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-27T09:18:26Z

          LOG4J2-1699 No need to apply file posix attribute if file is just
          renamed not compresed

          commit ecba42b570ff3451a5fbbf46b153f2fb8fe4886e
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-27T09:35:22Z

          LOG4J2-1699 Change wrong target version release, 2.8.3 to 2.9 in
          javadoc and documentation

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          commit 347db2a74532f37f498369da5c1e906a7a4ae7c1
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-27T09:51:47Z

          LOG4J2-1699 Use status logger to warn if posix file attribute view are
          defined but underlying files system doesnt support it.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          commit de82a825ca10f9c9314a18798a024fc71e5911ba
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-27T10:01:41Z

          LOG4J2-1699 Catch and log if applying attribute view throws exception.
          More comprehensible field/method name.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>

          commit b8a817047aed0432bf229558118587383e490187
          Author: phymbert <pierrick.hymbert@gmail.com>
          Date: 2017-06-27T10:08:49Z

          LOG4J2-1699 Update some comments and error message.

          Signed-off-by: phymbert <pierrick.hymbert@gmail.com>


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user phymbert opened a pull request: https://github.com/apache/logging-log4j2/pull/89 LOG4J2-1699 Log File Permissions with PosixFilePermission feedback from jira item 2: Better log at configuration step if file attribute view are defined but underlying files system doesnt support it item 3: Exception catch and logged if OperationNotSupported or Operation not permitted are thrown while changing file attribute permissions, user or group item 4: No need to apply file posix attribute if file is just rolled not compressed, both in DirectWriteRolloverStrategy and DefaultRolloverStrategy. Changed next release version in documentation and javadoc. You can merge this pull request into a Git repository by running: $ git pull https://github.com/phymbert/logging-log4j2 LOG4J2-1699 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/logging-log4j2/pull/89.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #89 commit 38e0becb1ac0ef01b8197931e08575ed1cb44387 Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-27T09:18:26Z LOG4J2-1699 No need to apply file posix attribute if file is just renamed not compresed commit ecba42b570ff3451a5fbbf46b153f2fb8fe4886e Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-27T09:35:22Z LOG4J2-1699 Change wrong target version release, 2.8.3 to 2.9 in javadoc and documentation Signed-off-by: phymbert <pierrick.hymbert@gmail.com> commit 347db2a74532f37f498369da5c1e906a7a4ae7c1 Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-27T09:51:47Z LOG4J2-1699 Use status logger to warn if posix file attribute view are defined but underlying files system doesnt support it. Signed-off-by: phymbert <pierrick.hymbert@gmail.com> commit de82a825ca10f9c9314a18798a024fc71e5911ba Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-27T10:01:41Z LOG4J2-1699 Catch and log if applying attribute view throws exception. More comprehensible field/method name. Signed-off-by: phymbert <pierrick.hymbert@gmail.com> commit b8a817047aed0432bf229558118587383e490187 Author: phymbert <pierrick.hymbert@gmail.com> Date: 2017-06-27T10:08:49Z LOG4J2-1699 Update some comments and error message. Signed-off-by: phymbert <pierrick.hymbert@gmail.com>
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit a5a95886b6912b31009a7c61f18396280e083cd6 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT
          [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=a5a9588 ]

          LOG4J2-1699 Log File Permissions with PosixFilePermission feedback
          from jira. Closes #89.

          • item 2: Better log at configuration step if file attribute view are
            defined but underlying files system doesnt support it
          • item 3: Exception catch and logged if OperationNotSupported or
            Operation not permitted are thrown while changing file attribute
            permissions, user or group
          • item 4: No need to apply file posix attribute if file is just rolled
            not compressed, both in DirectWriteRolloverStrategy and
            DefaultRolloverStrategy.
          • Changed next release version in documentation and javadoc.
          Show
          jira-bot ASF subversion and git services added a comment - Commit a5a95886b6912b31009a7c61f18396280e083cd6 in logging-log4j2's branch refs/heads/master from Pierrick HYMBERT [ https://git-wip-us.apache.org/repos/asf?p=logging-log4j2.git;h=a5a9588 ] LOG4J2-1699 Log File Permissions with PosixFilePermission feedback from jira. Closes #89. item 2: Better log at configuration step if file attribute view are defined but underlying files system doesnt support it item 3: Exception catch and logged if OperationNotSupported or Operation not permitted are thrown while changing file attribute permissions, user or group item 4: No need to apply file posix attribute if file is just rolled not compressed, both in DirectWriteRolloverStrategy and DefaultRolloverStrategy. Changed next release version in documentation and javadoc.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/logging-log4j2/pull/89

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/logging-log4j2/pull/89
          Hide
          garydgregory Gary Gregory added a comment -

          Patch applied from GitHub PR #89. Thank you!

          Show
          garydgregory Gary Gregory added a comment - Patch applied from GitHub PR #89. Thank you!
          Hide
          garydgregory Gary Gregory added a comment -

          Please verify and close.

          Show
          garydgregory Gary Gregory added a comment - Please verify and close.
          Hide
          garydgregory Gary Gregory added a comment -

          Changing priority from Critical to Major.

          Show
          garydgregory Gary Gregory added a comment - Changing priority from Critical to Major.
          Hide
          phymbert Pierrick HYMBERT added a comment -

          OK for me, thank you all! Dear Demetrios Dimatos, please confirm it is ok for you

          Show
          phymbert Pierrick HYMBERT added a comment - OK for me, thank you all! Dear Demetrios Dimatos , please confirm it is ok for you

            People

            • Assignee:
              Unassigned
              Reporter:
              ddimatos Demetrios Dimatos
            • Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 336h
                336h
                Remaining:
                Remaining Estimate - 336h
                336h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development