Fixed permissions of shell scripts in binary builds

[tommaso-borgato]

In binary builds from sources, when the source is an already provisioned server (e.g. some "target/server" produced by the wildfly-maven-plugin), then execution permissions on ".sh" files are lost;

This causes the following error when the POD starts:

/opt/jboss/container/wildfly/run/run: line 74: /opt/server/bin/openshift-launch.sh: Permission denied

This MR just fixes permissions on ".sh" files which is the bare minimum required to have a working server installation;

Please make sure your PR meets the following requirements:

• Pull Request contains a description of the changes
• Pull Request does not include fixes for multiple issues/topics
• Code is formatted, imports ordered, code compiles and tests are passing
• Code is self-descriptive and/or documented

[mnovak1]

@tommaso-borgato I don't understand the details here, I just want to make sure that this is not workaround for something what is actually a bug. Shouldn't wildfly-maven-plugin preserve permission on .sh files?

[mnovak1]

Or is the executable flag lost when creating tar?

[tommaso-borgato]

@mnovak1 the issue here is that classes in org.apache.commons.compress.archivers.tar e.g. TarArchiveEntry do not preserve permissions when building a tar .... this is actually a bug in xtf

it's no surprise to me that the issue is in the only step of the process which is using Java library for creating a tar because I experienced the very same issue on another project (the solution was running zip directly there - which we cannot apply here)

this MR is a workaround for the one scenario that we use: creating a tar stream containing a server directory to be sent to the builder image

a more comprehensive solution would be try and find and alternative to org.apache.commons.compress.archivers.tar which preserves permissions (if any) and re-write BinaryBuildFromSources

perhaps we might trace the more comprehensive solution in a separate issue (with lower priority 😄 )

[fabiobrz]

@mnovak1 the issue here is that classes in org.apache.commons.compress.archivers.tar e.g. TarArchiveEntry do not preserve permissions when building a tar .... this is actually a bug in xtf

it's no surprise to me that the issue is in the only step of the process which is using Java library for creating a tar because I experienced the very same issue on another project (the solution was running zip directly there - which we cannot apply here)

this MR is a workaround for the one scenario that we use: creating a tar stream containing a server directory to be sent to the builder image

a more comprehensive solution would be try and find and alternative to org.apache.commons.compress.archivers.tar which preserves permissions (if any) and re-write BinaryBuildFromSources

perhaps we might trace the more comprehensive solution in a separate issue (with lower priority smile )

From what I can read around, this might be intentional, since permissions are dealt with at the OS level.
In such a case, it seems to me this is not a workaround for an external component bug, but rather a proper fix for the XTF (caller) behavior when dealing with org.apache.commons.compress.archivers.tar.TarArchiveEntry etc.

In such a case there's no issue that should be created externally, maybe an internal one fixed by this very PR.
Let me know in case I am missing something.

[mnovak1]

Ok, I think that correct fix should retrieve file permission from the file being added into tar archive and set on tar entry. Something like:

PosixFileAttributes attrs = Files.getFileAttributeView(tempFile, PosixFileAttributeView.class).readAttributes();
System.out.format("%s %s%n", attrs.owner().getName(), PosixFilePermissions.toString(attrs.permissions())); 
// prints OS: mnovak rw-------
// convert "rw-------" into "int mode" and then
tarArchiveEntry.setMode(intMode);

wdyt?

[tommaso-borgato]

@mnovak1 good idea, going to check

[mnovak1]

@mnovak1 good idea, going to check

👍 ...thanks for looking at it.

[tommaso-borgato]

@mnovak1 I applied your suggestion and the bug fix is now complete

[fabiobrz]

I am approving since this LGTM now. But leaving the last word to @mnovak1 of course :)

Maybe this could be extracted into some utils in the future and maybe we could add a comment identifying the source for such conversion.

[mnovak1]

LGTM, merging. Thanks @tommaso-borgato !