[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258084689
 
 

 ##
 File path: gateway-service-remoteconfig/pom.xml
 ##
 @@ -38,6 +38,10 @@
 org.apache.knox
 gateway-spi
 
+
 
 Review comment:
   Yea just checking that we didn't pull in a new dependency for another reason.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258078897
 
 

 ##
 File path: 
gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
 ##
 @@ -34,13 +34,13 @@
 public interface GatewayMessages {
 
   @Message( level = MessageLevel.FATAL, text = "Failed to parse command line: 
{0}" )
-  void failedToParseCommandLine( @StackTrace( level = MessageLevel.DEBUG ) 
ParseException e );
+  void failedToParseCommandLine( @StackTrace( level = MessageLevel.FATAL ) 
ParseException e );
 
 Review comment:
   Ok makes sense. Didn't tie the two pieces together. The stacktrace level is 
basically opposite of what I expected. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258078572
 
 

 ##
 File path: 
gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java
 ##
 @@ -49,17 +51,26 @@
 private static final RemoteConfigurationMessages log = 
MessagesFactory.get(RemoteConfigurationMessages.class);
 
 // Cache the current JAAS configuration
-private Configuration delegate = Configuration.getConfiguration();
+private final Configuration delegate;
 
-private AliasService aliasService;
+private final AliasService aliasService;
 
-private Map contextEntries =  new 
HashMap<>();
+private final Map contextEntries =  new 
HashMap<>();
 
 static RemoteConfigurationRegistryJAASConfig 
configure(List configs, AliasService 
aliasService) {
 return new RemoteConfigurationRegistryJAASConfig(configs, 
aliasService);
 }
 
 private 
RemoteConfigurationRegistryJAASConfig(List 
configs, AliasService aliasService) {
+try {
+  delegate = Configuration.getConfiguration();
+} catch(Exception e) {
+  //populate the original error with a meaningful message; logging 
will happen later in the call hierarchy
+  final String message = String.format(Locale.ROOT, "Error while 
getting secure configuration. This error usually indicates an issue within the 
supplied JAAS configuration: %s",
+  System.getProperty(GatewayConfig.KRB5_LOGIN_CONFIG, 
"Undefined"));
 
 Review comment:
   Ah thanks


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258078180
 
 

 ##
 File path: 
gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java
 ##
 @@ -796,7 +796,7 @@ public void testMultipleDoubleStarPathMatching() throws 
URISyntaxException {
 Template template;
 Template input;
 Matcher stringMatcher;
-Matcher.Match match;
 
 Review comment:
   Yea do you by chance have the Java level not set to 7+ or something where it 
doesn't like the generics?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258069718
 
 

 ##
 File path: 
gateway-service-remoteconfig/src/test/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfigTest.java
 ##
 @@ -36,6 +47,14 @@
 
 public class RemoteConfigurationRegistryJAASConfigTest {
 
+@Rule
+public final TemporaryFolder testFolder = new TemporaryFolder();
+
+@Rule
+public final ExpectedException expectedException = 
ExpectedException.none();
+
+private static final String JAAS_CONFIG_ERRROR_PREFIX = "Error while 
getting secure configuration. This error usually indicates an issue within the 
supplied JAAS configuration";
 
 Review comment:
   This should be in the class `RemoteConfigurationRegistryJAASConfig` so it 
can be used in the test and in the actual throwing of the exception.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258068872
 
 

 ##
 File path: 
gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java
 ##
 @@ -34,13 +34,13 @@
 public interface GatewayMessages {
 
   @Message( level = MessageLevel.FATAL, text = "Failed to parse command line: 
{0}" )
-  void failedToParseCommandLine( @StackTrace( level = MessageLevel.DEBUG ) 
ParseException e );
+  void failedToParseCommandLine( @StackTrace( level = MessageLevel.FATAL ) 
ParseException e );
 
 Review comment:
   Is this change necessary? I would have assumed that the MessageLevel was a 
minimum level to expose not the exact level. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258069209
 
 

 ##
 File path: gateway-service-remoteconfig/pom.xml
 ##
 @@ -38,6 +38,10 @@
 org.apache.knox
 gateway-spi
 
+
 
 Review comment:
   Is this just for `ConfigurationException`?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258066039
 
 

 ##
 File path: 
gateway-service-remoteconfig/src/main/java/org/apache/knox/gateway/service/config/remote/zk/RemoteConfigurationRegistryJAASConfig.java
 ##
 @@ -49,17 +51,26 @@
 private static final RemoteConfigurationMessages log = 
MessagesFactory.get(RemoteConfigurationMessages.class);
 
 // Cache the current JAAS configuration
-private Configuration delegate = Configuration.getConfiguration();
+private final Configuration delegate;
 
-private AliasService aliasService;
+private final AliasService aliasService;
 
-private Map contextEntries =  new 
HashMap<>();
+private final Map contextEntries =  new 
HashMap<>();
 
 static RemoteConfigurationRegistryJAASConfig 
configure(List configs, AliasService 
aliasService) {
 return new RemoteConfigurationRegistryJAASConfig(configs, 
aliasService);
 }
 
 private 
RemoteConfigurationRegistryJAASConfig(List 
configs, AliasService aliasService) {
+try {
+  delegate = Configuration.getConfiguration();
+} catch(Exception e) {
+  //populate the original error with a meaningful message; logging 
will happen later in the call hierarchy
+  final String message = String.format(Locale.ROOT, "Error while 
getting secure configuration. This error usually indicates an issue within the 
supplied JAAS configuration: %s",
+  System.getProperty(GatewayConfig.KRB5_LOGIN_CONFIG, 
"Undefined"));
 
 Review comment:
   Is this always a system property?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] risdenk commented on a change in pull request #55: KNOX-1162 - Logging stacktrace for FATAL messages and displaying a meaningful error message in case of missing/non-parsable JAAS configurati

2019-02-19 Thread GitBox 
risdenk commented on a change in pull request #55: KNOX-1162 - Logging 
stacktrace for FATAL messages and displaying a meaningful error message in case 
of missing/non-parsable JAAS configuration
URL: https://github.com/apache/knox/pull/55#discussion_r258069908
 
 

 ##
 File path: 
gateway-util-urltemplate/src/test/java/org/apache/knox/gateway/util/urltemplate/MatcherTest.java
 ##
 @@ -796,7 +796,7 @@ public void testMultipleDoubleStarPathMatching() throws 
URISyntaxException {
 Template template;
 Template input;
 Matcher stringMatcher;
-Matcher.Match match;
 
 Review comment:
   Is this change necessary?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services