You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2014/08/14 12:50:51 UTC

[2/3] git commit: Incorporate review comments (PRs #115 and #114)

Incorporate review comments (PRs #115 and #114)

Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/b10da7ef
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/b10da7ef
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/b10da7ef

Branch: refs/heads/master
Commit: b10da7effb1e6f9e874008993f5dfa0eadb38d31
Parents: 266a2ef
Author: Aled Sage <al...@gmail.com>
Authored: Thu Aug 14 11:47:09 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Thu Aug 14 11:47:09 2014 +0100

----------------------------------------------------------------------
 .../brooklyn/launcher/BrooklynLauncher.java     | 21 +++++++-----------
 .../main/java/brooklyn/util/io/FileUtil.java    | 23 ++++++++++++++------
 2 files changed, 24 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b10da7ef/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
----------------------------------------------------------------------
diff --git a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
index d3faeea..083763d 100644
--- a/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
+++ b/usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java
@@ -27,7 +27,6 @@ import io.brooklyn.camp.spi.instantiate.AssemblyTemplateInstantiator;
 
 import java.io.Closeable;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.StringReader;
 import java.net.InetAddress;
 import java.net.URI;
@@ -447,6 +446,7 @@ public class BrooklynLauncher {
                 if (globalBrooklynPropertiesFile != null) {
                     // brooklyn.properties stores passwords (web-console and cloud credentials), 
                     // so ensure it has sensible permissions
+                    checkFileReadable(globalBrooklynPropertiesFile);
                     checkFilePermissionsX00(globalBrooklynPropertiesFile);
                     builder.globalPropertiesFile(globalBrooklynPropertiesFile);
                 }
@@ -513,18 +513,13 @@ public class BrooklynLauncher {
     
     private void checkFilePermissionsX00(String file) {
         File f = new File(Os.tidyPath(file));
-        if (f.exists() && f.isFile() && f.canRead()) {
-            try {
-                Maybe<String> permission = FileUtil.getFilePermissions(f);
-                if (permission.isAbsent()) {
-                    LOG.debug("Could not determine permissions of global brooklyn properties file; assuming ok: "+f);
-                } else {
-                    if (!permission.get().substring(4).equals("------")) {
-                        throw new FatalRuntimeException("Invalid permissions for file "+file+"; expected 600 but was "+permission.get());
-                    }
-                }
-            } catch (FileNotFoundException e) {
-                throw Exceptions.propagate(e); // should not happen; did f.exists() above
+        
+        Maybe<String> permission = FileUtil.getFilePermissions(f);
+        if (permission.isAbsent()) {
+            LOG.debug("Could not determine permissions of file; assuming ok: "+f);
+        } else {
+            if (!permission.get().substring(4).equals("------")) {
+                throw new FatalRuntimeException("Invalid permissions for file "+file+"; expected ?00 but was "+permission.get());
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/b10da7ef/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
index 208717a..bf8c2ab 100644
--- a/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
+++ b/utils/common/src/main/java/brooklyn/util/io/FileUtil.java
@@ -20,7 +20,6 @@ package brooklyn.util.io;
 
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -59,8 +58,13 @@ public class FileUtil {
                 if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}",
                         new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
             } else {
-                LOG.warn("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
-                        new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                if (Os.isMicrosoftWindows()) {
+                    if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 700 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+                            new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                } else {
+                    LOG.warn("Failed to set permissions to 700 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+                            new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                }
                 loggedSetFilePermissionsWarning = true;
             }
         }
@@ -86,8 +90,13 @@ public class FileUtil {
                 if (LOG.isTraceEnabled()) LOG.trace("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}",
                         new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
             } else {
-                LOG.warn("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
-                        new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                if (Os.isMicrosoftWindows()) {
+                    if (LOG.isDebugEnabled()) LOG.debug("Failed to set permissions to 600 for file {}; expected behaviour on Windows; setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+                            new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                } else {
+                    LOG.warn("Failed to set permissions to 600 for file {}: setRead={}, setWrite={}, setExecutable={}; subsequent failures (on any file) will be logged at trace",
+                            new Object[] {file.getAbsolutePath(), setRead, setWrite, setExec});
+                }
                 loggedSetFilePermissionsWarning = true;
             }
         }
@@ -121,8 +130,8 @@ public class FileUtil {
      * @return The file permission (in a form like "-rwxr--r--"), or null if the permissions could not be determined.
      */
     @Beta
-    public static Maybe<String> getFilePermissions(File file) throws FileNotFoundException {
-        if (!file.exists()) throw new FileNotFoundException();
+    public static Maybe<String> getFilePermissions(File file) {
+        if (!file.exists()) return Maybe.absent("File "+file+" does not exist");
         
         if (Os.isMicrosoftWindows()) {
             return Maybe.absent("Cannot determine permissions on windows");