Merge remote-tracking branch 'dakrone/vagrant-umask'

core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -51,6 +51,7 @@ import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -134,6 +135,30 @@ class InstallPluginCommand extends Command {
     private final OptionSpec<Void> batchOption;
     private final OptionSpec<String> arguments;
 
+    public static final Set<PosixFilePermission> DIR_AND_EXECUTABLE_PERMS;
+    public static final Set<PosixFilePermission> FILE_PERMS;
+
+    static {
+        Set<PosixFilePermission> dirAndExecutablePerms = new HashSet<>(7);
+        // Directories and executables get chmod 755
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_EXECUTE);
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OWNER_WRITE);
+        dirAndExecutablePerms.add(PosixFilePermission.GROUP_EXECUTE);
+        dirAndExecutablePerms.add(PosixFilePermission.GROUP_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OTHERS_READ);
+        dirAndExecutablePerms.add(PosixFilePermission.OTHERS_EXECUTE);
+        DIR_AND_EXECUTABLE_PERMS = Collections.unmodifiableSet(dirAndExecutablePerms);
+
+        Set<PosixFilePermission> filePerms = new HashSet<>(4);
+        // Files get chmod 644
+        filePerms.add(PosixFilePermission.OWNER_READ);
+        filePerms.add(PosixFilePermission.OWNER_WRITE);
+        filePerms.add(PosixFilePermission.GROUP_READ);
+        filePerms.add(PosixFilePermission.OTHERS_READ);
+        FILE_PERMS = Collections.unmodifiableSet(filePerms);
+    }
+
     InstallPluginCommand(Environment env) {
         super("Install a plugin");
         this.env = env;
@@ -298,15 +323,7 @@ class InstallPluginCommand extends Command {
 
     private Path stagingDirectory(Path pluginsDir) throws IOException {
         try {
-            Set<PosixFilePermission> perms = new HashSet<>();
+            return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(DIR_AND_EXECUTABLE_PERMS));
-            perms.add(PosixFilePermission.OWNER_EXECUTE);
-            perms.add(PosixFilePermission.OWNER_READ);
-            perms.add(PosixFilePermission.OWNER_WRITE);
-            perms.add(PosixFilePermission.GROUP_READ);
-            perms.add(PosixFilePermission.GROUP_EXECUTE);
-            perms.add(PosixFilePermission.OTHERS_READ);
-            perms.add(PosixFilePermission.OTHERS_EXECUTE);
-            return Files.createTempDirectory(pluginsDir, ".installing-", PosixFilePermissions.asFileAttribute(perms));
         } catch (IllegalArgumentException e) {
             // Jimfs throws an IAE where it should throw an UOE
             // remove when google/jimfs#30 is integrated into Jimfs
@@ -409,6 +426,15 @@ class InstallPluginCommand extends Command {
             }
 
             Files.move(tmpRoot, destination, StandardCopyOption.ATOMIC_MOVE);
+            try (DirectoryStream<Path> stream = Files.newDirectoryStream(destination)) {
+                for (Path pluginFile : stream) {
+                    if (Files.isDirectory(pluginFile)) {
+                        setFileAttributes(pluginFile, DIR_AND_EXECUTABLE_PERMS);
+                    } else {
+                        setFileAttributes(pluginFile, FILE_PERMS);
+                    }
+                }
+            }
             terminal.println("-> Installed " + info.getName());
 
         } catch (Exception installProblem) {
@@ -427,17 +453,7 @@ class InstallPluginCommand extends Command {
             throw new UserError(ExitCodes.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
         }
         Files.createDirectory(destBinDir);
-
+        setFileAttributes(destBinDir, DIR_AND_EXECUTABLE_PERMS);
-        // setup file attributes for the installed files to those of the parent dir
-        final Set<PosixFilePermission> perms = new HashSet<>();
-        final PosixFileAttributeView binAttributeView = Files.getFileAttributeView(destBinDir.getParent(), PosixFileAttributeView.class);
-        if (binAttributeView != null) {
-            perms.addAll(binAttributeView.readAttributes().permissions());
-            // setting execute bits, since this just means "the file is executable", and actual execution requires read
-            perms.add(PosixFilePermission.OWNER_EXECUTE);
-            perms.add(PosixFilePermission.GROUP_EXECUTE);
-            perms.add(PosixFilePermission.OTHERS_EXECUTE);
-        }
 
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpBinDir)) {
             for (Path srcFile : stream) {
@@ -449,11 +465,7 @@ class InstallPluginCommand extends Command {
 
                 Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
                 Files.copy(srcFile, destFile);
-
+                setFileAttributes(destFile, DIR_AND_EXECUTABLE_PERMS);
-                final PosixFileAttributeView view = Files.getFileAttributeView(destFile, PosixFileAttributeView.class);
-                if (view != null) {
-                    view.setPermissions(perms);
-                }
             }
         }
         IOUtils.rm(tmpBinDir); // clean up what we just copied
@@ -468,8 +480,8 @@ class InstallPluginCommand extends Command {
             throw new UserError(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
         }
 
-        // create the plugin's config dir "if necessary"
         Files.createDirectories(destConfigDir);
+        setFileAttributes(destConfigDir, DIR_AND_EXECUTABLE_PERMS);
         final PosixFileAttributeView destConfigDirAttributesView =
             Files.getFileAttributeView(destConfigDir.getParent(), PosixFileAttributeView.class);
         final PosixFileAttributes destConfigDirAttributes =
@@ -487,6 +499,7 @@ class InstallPluginCommand extends Command {
                 Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));
                 if (Files.exists(destFile) == false) {
                     Files.copy(srcFile, destFile);
+                    setFileAttributes(destFile, FILE_PERMS);
                     if (destConfigDirAttributes != null) {
                         setOwnerGroup(destFile, destConfigDirAttributes);
                     }
@@ -504,4 +517,13 @@ class InstallPluginCommand extends Command {
         fileAttributeView.setGroup(attributes.group());
     }
 
+    /**
+     * Sets the attributes for a path iff posix attributes are supported
+     */
+    private static void setFileAttributes(final Path path, final Set<PosixFilePermission> permissions) throws IOException {
+        PosixFileAttributeView fileAttributeView = Files.getFileAttributeView(path, PosixFileAttributeView.class);
+        if (fileAttributeView != null) {
+            Files.setPosixFilePermissions(path, permissions);
+        }
+    }
 }

distribution/build.gradle

@@ -239,6 +239,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     dependsOn createEtc, createEtcScripts
     with configFiles
     into "${packagingFiles}/etc/elasticsearch"
+    fileMode 0640
+    dirMode 0750
     /* Explicitly declare the output files so this task doesn't consider itself
       up to date when the directory is created, which it would by default. And
       that'll happen when createEtc runs. */
@@ -303,6 +305,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     postUninstall file("${scripts}/postrm")
 
     into '/usr/share/elasticsearch'
+    fileMode 0644
+    dirMode 0755
     user 'root'
     permissionGroup 'root'
     with libFiles
@@ -344,6 +348,8 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
     configurationFile project.expansions['path.env']
     into(new File(project.expansions['path.env']).getParent()) {
       from "${project.packagingFiles}/env/elasticsearch"
+      fileMode 0644
+      dirMode 0755
     }
 
     /**
@@ -351,6 +357,7 @@ configure(subprojects.findAll { ['deb', 'rpm'].contains(it.name) }) {
      */
     Closure suckUpEmptyDirectories = { path, u, g ->
       into(path) {
+        fileMode 0755
         from "${packagingFiles}/${path}"
         includeEmptyDirs true
         createDirectoryEntry true

distribution/deb/build.gradle

@@ -31,6 +31,7 @@ task buildDeb(type: Deb) {
   }
   into('/usr/share/doc/elasticsearch') {
     from "${project.packagingFiles}/copyright"
+    fileMode 0644
   }
 }
 

distribution/rpm/build.gradle

@@ -38,6 +38,8 @@ task buildRpm(type: Rpm) {
   license '2009'
   distribution 'Elasticsearch'
   vendor 'Elasticsearch'
+  dirMode 0755
+  fileMode 0644
   // TODO ospackage doesn't support icon but we used to have one
 }
 

distribution/src/main/packaging/scripts/postinst

@@ -99,5 +99,11 @@ fi
 chown -R $ES_USER:$ES_GROUP /var/lib/elasticsearch
 chown -R $ES_USER:$ES_GROUP /var/log/elasticsearch
 chown -R root:$ES_GROUP /etc/elasticsearch
+chmod 0750 /etc/elasticsearch
+chmod 0750 /etc/elasticsearch/scripts
+
+if [ -f /etc/sysconfig/elasticsearch ]; then
+   chmod 0644 /etc/sysconfig/elasticsearch
+fi
 
 ${scripts.footer}

distribution/tar/build.gradle

@@ -22,6 +22,8 @@ task buildTar(type: Tar) {
   extension = 'tar.gz'
   with archivesFiles
   compression = Compression.GZIP
+  dirMode 0755
+  fileMode 0644
 }
 
 artifacts {

qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -236,11 +236,7 @@ public class InstallPluginCommandTests extends ESTestCase {
                     assertFalse("not a dir", Files.isDirectory(file));
                     if (isPosix) {
                         PosixFileAttributes attributes = Files.readAttributes(file, PosixFileAttributes.class);
-                        Set<PosixFilePermission> expectedPermissions = new HashSet<>(binAttributes.permissions());
+                        assertEquals(InstallPluginCommand.DIR_AND_EXECUTABLE_PERMS, attributes.permissions());
-                        expectedPermissions.add(PosixFilePermission.OWNER_EXECUTE);
-                        expectedPermissions.add(PosixFilePermission.GROUP_EXECUTE);
-                        expectedPermissions.add(PosixFilePermission.OTHERS_EXECUTE);
-                        assertEquals(expectedPermissions, attributes.permissions());
                     }
                 }
             }

qa/vagrant/src/test/resources/packaging/scripts/packaging_test_utils.bash

@@ -134,14 +134,18 @@ skip_not_zip() {
 
 assert_file_exist() {
     local file="$1"
-    echo "Should exist: ${file}"
+    if [ ! -e "$file" ]; then
+        echo "Should exist: ${file} but does not"
+    fi
     local file=$(readlink -m "${file}")
     [ -e "$file" ]
 }
 
 assert_file_not_exist() {
     local file="$1"
-    echo "Should not exist: ${file}"
+    if [ -e "$file" ]; then
+        echo "Should not exist: ${file} but does"
+    fi
     local file=$(readlink -m "${file}")
     [ ! -e "$file" ]
 }
@@ -156,28 +160,38 @@ assert_file() {
     assert_file_exist "$file"
 
     if [ "$type" = "d" ]; then
-        echo "And be a directory...."
+        if [ ! -d "$file" ]; then
+            echo "[$file] should be a directory but is not"
+        fi
         [ -d "$file" ]
     else
-        echo "And be a regular file...."
+        if [ ! -f "$file" ]; then
+            echo "[$file] should be a regular file but is not"
+        fi
         [ -f "$file" ]
     fi
 
     if [ "x$user" != "x" ]; then
         realuser=$(find "$file" -maxdepth 0 -printf "%u")
-        echo "Expected user: $user, found $realuser"
+        if [ "$realuser" != "$user" ]; then
+            echo "Expected user: $user, found $realuser [$file]"
+        fi
         [ "$realuser" = "$user" ]
     fi
 
     if [ "x$group" != "x" ]; then
         realgroup=$(find "$file" -maxdepth 0 -printf "%g")
-        echo "Expected group: $group, found $realgroup"
+        if [ "$realgroup" != "$group" ]; then
+            echo "Expected group: $group, found $realgroup [$file]"
+        fi
         [ "$realgroup" = "$group" ]
     fi
 
     if [ "x$privileges" != "x" ]; then
         realprivileges=$(find "$file" -maxdepth 0 -printf "%m")
-        echo "Expected privileges: $privileges, found $realprivileges"
+        if [ "$realprivileges" != "$privileges" ]; then
+            echo "Expected privileges: $privileges, found $realprivileges [$file]"
+        fi
         [ "$realprivileges" = "$privileges" ]
     fi
 }
@@ -190,10 +204,8 @@ assert_module_or_plugin_directory() {
     #just make sure that everything is the same as $CONFIG_DIR, which was properly set up during install
     config_user=$(find "$ESHOME" -maxdepth 0 -printf "%u")
     config_owner=$(find "$ESHOME" -maxdepth 0 -printf "%g")
-    # directories should use the user file-creation mask
-    config_privileges=$(executable_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
 
-    assert_file $directory d $config_user $config_owner $(printf "%o" $config_privileges)
+    assert_file $directory d $config_user $config_owner 755
 }
 
 assert_module_or_plugin_file() {
@@ -201,11 +213,7 @@ assert_module_or_plugin_file() {
     shift
 
     assert_file_exist "$(readlink -m $file)"
-
+    assert_file $file f $config_user $config_owner 644
-    # config files should not be executable and otherwise use the user
-    # file-creation mask
-    expected_file_privileges=$(file_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
-    assert_file $file f $config_user $config_owner $(printf "%o" $expected_file_privileges)
 }
 
 assert_output() {

qa/vagrant/src/test/resources/packaging/scripts/plugins.bash

@@ -84,20 +84,16 @@ install_jvm_example() {
     bin_user=$(find "$ESHOME/bin" -maxdepth 0 -printf "%u")
     bin_owner=$(find "$ESHOME/bin" -maxdepth 0 -printf "%g")
     bin_privileges=$(find "$ESHOME/bin" -maxdepth 0 -printf "%m")
-    assert_file "$ESHOME/bin/jvm-example" d $bin_user $bin_owner $bin_privileges
+    assert_file "$ESHOME/bin/jvm-example" d $bin_user $bin_owner 755
-    assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner $bin_privileges
+    assert_file "$ESHOME/bin/jvm-example/test" f $bin_user $bin_owner 755
 
     #owner group and permissions vary depending on how es was installed
     #just make sure that everything is the same as $CONFIG_DIR, which was properly set up during install
     config_user=$(find "$ESCONFIG" -maxdepth 0 -printf "%u")
     config_owner=$(find "$ESCONFIG" -maxdepth 0 -printf "%g")
     # directories should user the user file-creation mask
-    config_privileges=$(executable_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
+    assert_file "$ESCONFIG/jvm-example" d $config_user $config_owner 755
-    assert_file "$ESCONFIG/jvm-example" d $config_user $config_owner $(printf "%o" $config_privileges)
+    assert_file "$ESCONFIG/jvm-example/example.yaml" f $config_user $config_owner 644
-    # config files should not be executable and otherwise use the user
-    # file-creation mask
-    expected_file_privileges=$(file_privileges_for_user_from_umask $ESPLUGIN_COMMAND_USER)
-    assert_file "$ESCONFIG/jvm-example/example.yaml" f $config_user $config_owner $(printf "%o" $expected_file_privileges)
 
     echo "Running jvm-example's bin script...."
     "$ESHOME/bin/jvm-example/test" | grep test

qa/vagrant/src/test/resources/packaging/scripts/tar.bash

@@ -33,7 +33,7 @@ install_archive() {
     echo "Unpacking tarball to $ESHOME"
     rm -rf /tmp/untar
     mkdir -p /tmp/untar
-    tar -xzf elasticsearch*.tar.gz -C /tmp/untar
+    tar -xzpf elasticsearch*.tar.gz -C /tmp/untar
 
     find /tmp/untar -depth -type d -name 'elasticsearch*' -exec mv {} "$ESHOME" \; > /dev/null