[GitHub] smolnar82 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 configura
smolnar82 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_r258088484
##
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:
Fixed; 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] smolnar82 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 configura
smolnar82 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_r258079051 ## File path: gateway-service-remoteconfig/pom.xml ## @@ -38,6 +38,10 @@ org.apache.knox gateway-spi + Review comment: Yes. If there is an issue with the supplied JAAS config it is a `ConfigurationException`, right? Moreover `ConfigurationException` is not a checked one so that I did not have to modify the API. I think it would be an exaggeration to create a new type of exception just for this purpose if we already have to proper one. Agreed? 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] smolnar82 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 configura
smolnar82 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_r258076018
##
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:
Like I indicated in the description without this change I was not able to
run tests locally. I'm not sure how it was working before in Travis CI..
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] smolnar82 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 configura
smolnar82 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_r258075546
##
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:
Agree; will change it soon...
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] smolnar82 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 configura
smolnar82 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_r258075247
##
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:
The JIRA requested that there should be no FATAL messages displayed on DEBUG
level (which I agree with); so I believe this is necessary. Any objection?
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] smolnar82 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 configura
smolnar82 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_r258072346
##
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:
Yepp; this is set before we try to start the gateway in
`org.apache.knox.gateway.GatewayServer.configureKerberosSecurity(GatewayConfig)`
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] smolnar82 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 configura
smolnar82 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_r257807719
##
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 upper in the call hierarchy
Review comment:
Fixed; thanks for the review!
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
