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

executable permission is never set #794

Closed
tbutter opened this issue Aug 3, 2018 · 19 comments
Closed

executable permission is never set #794

tbutter opened this issue Aug 3, 2018 · 19 comments
Assignees
Milestone

Comments

@tbutter
Copy link

tbutter commented Aug 3, 2018

Description of the issue:

If you add a executable file to src/main/resources or src/main/jib the resulting docker image will include the files only with mode 0644.

Expected behavior:

The files should have a mode of 0744

Additional Information:

Creates a new TarArchiveEntry for each File. The TarArchiveEntry will always use TarArchiveEntry. DEFAULT_FILE_MODE for all files (0100644).

A possible change could be something like this:

if(java.nio.file.Files.isExecutable(sourceFile))
  tarArchiveEntry.setMode(tarArchiveEntry.getMode() | 0100);
@coollog
Copy link
Contributor

coollog commented Aug 3, 2018

Hi @tbutter , thanks for filing this issue and providing the recommended fix! This is related to #523 and #727. Will defer to @GoogleContainerTools/java-tools for some comments about the approach.

@coollog coollog added this to the 0.9.10 milestone Aug 3, 2018
@coollog
Copy link
Contributor

coollog commented Aug 3, 2018

We will most likely implement this fix by reading the file permissions set in src/main/jib and using those permissions explicitly over the defaults provided by TarArchiveEntry. We won't be doing any changes for classes/resources/dependencies files since those can be part of build output and vary in file permissions between different environments (and thus may break reproducibility if used). Therefore, this would only support executable files placed under src/main/jib such that the file permissions for those files would be generally committed into source control and reproducible across environments.

@chanseokoh
Copy link
Member

@chanseokoh
Copy link
Member

src/main/jib such that the file permissions for those files would be generally committed into source control and reproducible across environments.

Generally speaking, this is not true. For example,

$ umask
0022
$ git clone https://github.com/GoogleContainerTools/jib.git
$ ls -l jib/README.md 
-rw-r--r-- 1 chanseok chanseok 4691 Aug  3 14:38 jib/README.md
$ umask 077
$ rm -rf jib
$ git clone https://github.com/GoogleContainerTools/jib.git
$ ls -l jib/README.md 
-rw------- 1 chanseok chanseok 4691 Aug  3 14:38 jib/README.md  

@chanseokoh
Copy link
Member

And it's not uncommon to have the umask of 077 or 007. System administrators may force it, and I had such experience.

@coollog
Copy link
Contributor

coollog commented Aug 29, 2018

@tbutter After some discussions, we thought that since the entrypoint executable is the Java application, if your application needs to execute other files, it should set the executable bit for those files first. Is there any use case that this would not work for?

@coollog coollog removed this from the v0.9.10 milestone Aug 29, 2018
@tbutter
Copy link
Author

tbutter commented Aug 29, 2018

@coollog I think that is always possible, but in my understanding this is an expensive operation. Wouldn't it create a copy of the whole executable in the container instead of using the version from the image?

@coollog
Copy link
Contributor

coollog commented Sep 4, 2018

@tbutter Yes, that is correct. I've added the discuss label for further discussions about the best approach here, since just checking for executable bit is not a solution that can guarantee reproducibility (and also doesn't apply to Windows). If you don't mind answering, could you provide more details as to your use case (such as what the executable is intended for)?

@tbutter
Copy link
Author

tbutter commented Sep 4, 2018

One of the executables is ffmpeg (about 50MB) which is used to reencode a video thats generated in a Java application.

@tbutter
Copy link
Author

tbutter commented Sep 4, 2018

@coollog One approach could be to explicitly list executable files in the gradle/maven configuration. That would be some manual effort but still less than setting the executable flag in Java.

@danielpetisme
Copy link

danielpetisme commented Sep 27, 2018

I 👍 this issue.
I've integrated jib into jhipster generator and I must use a custom entrypoint.sh. This file is stored under src/main/jib and set as executable but once copied on the container image, the file is not executable anymore.
This problem forces me to run a dircty chmod +x before running the script 😢
It's not a blocking point but its a pitfall we could avoid.

I don't how I could help?

@coollog
Copy link
Contributor

coollog commented Sep 27, 2018

Hi @danielpetisme , thanks for the input! We currently have two methods we're considering for resolving this issue:

  1. Reading the file permissions for files only in src/main/jib (extra files directory) and set executable permission as found. This could break our reproducibility guarantee.
  2. As @tbutter suggested, add a <container><executables> configuration that allows the user to manually define a list of files (in the container) to set the executable bit on.

@GoogleContainerTools/java-tools

@coollog coollog added this to the v0.10.0 milestone Sep 27, 2018
@danielpetisme
Copy link

danielpetisme commented Sep 27, 2018

I assume that only files coming from the extraDirectories should be chmoded otherwise it should be part of the base image.

Reading the permission would be the more 'native/naive' way to propagate permissions
Are we sure +x is the only permission used? I guess it's the more represented but how could we propose a +w option too.

I like the declarative approach with <executables> but it forces to maintain the filename in both location. Plus, I guess you'll make the file executable during the file copy (not by running an actual chmod +x on the container) so it should be part of the <container> tag.

I would rather extend the extraDirectories concept:
Here's a proposal :

<container>
</container>
<copy>
  <directory>src/main/jib</directory>
  <defaultPermission>0755</defaultPermission>
  <files>
    <file permission="0600">my-file<file>
  </files
</copy>

WDYT?

@briandealwis
Copy link
Member

Explicitly invoke the shell? /bin/sh entrypoint.sh

@chanseokoh
Copy link
Member

Regarding #794 (comment), the default base image gcr.io/distroless/java doesn't have /bin/sh. The debug image gcr.io/distroless/java:debug has sh but at a different location: /busybox/sh.

@briandealwis
Copy link
Member

Specifying permissions in a pom/buildscript seems a bit icky. And sadly it's not very Mavenish to have XML attributes (though I think it can be done by accessing the Xpp3Dom?).

An alternative could be to allow specifying a tar file and extract and preserve its permissions?

@mfriedenhagen
Copy link

I think picking up the permissions from the source directory would be good. Altering permissions lateron even in an ENTRYPOINT may not be possible. E.g. we let all containers run with the read-only flag, they are only permitted to write in dedicated, mounted volumes.

@coollog
Copy link
Contributor

coollog commented Oct 2, 2018

@danielpetisme Thanks for the suggestion! That syntax looks nice, though we are trying to limit new configuration objects to reduce the keep at a minimum the complexity of Jib's configuration.

Based on your proposal, we came up with the following that changes the existing extraDirectory configuration to include permissions:

<extraDirectories>
  <paths>(defaults to src/main/jib)</paths>
  <permissions>
    <permission>
      <file>/file/in/container</file>
      <mode>755</mode>
    </permission>
  </permissions>
</extraDirectories>

Thoughts? @GoogleContainerTools/java-tools

@coollog
Copy link
Contributor

coollog commented Nov 9, 2018

Hi @tbutter, @danielpetisme, @mfriedenhagen, we've released version 0.10.0 with the ability to explicitly set file permissions on extra files. See documentation for Maven or Gradle.

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

No branches or pull requests

7 participants