You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "smolnar82 (via GitHub)" <gi...@apache.org> on 2023/12/14 09:49:38 UTC
[PR] KNOX-2990 - Using InMemoryDerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
smolnar82 opened a new pull request, #826:
URL: https://github.com/apache/knox/pull/826
## What changes were proposed in this pull request?
Implemented the changes I listed in [KNOX-2990](https://issues.apache.org/jira/browse/KNOX-2990):
- deprecated the following TSS implementations:
- AliasBasedTokenStateService
- ZookeeperTokenStateService
- JournalBasedTokenStateService
- implemented a DerbyDB storage that stores tokens in `$DATA_DIR/security/tokens` (this time it's not yet encrypted)
- file permissions are set on that folder to `700` (only the owner can access it)
- changed the default implementation in `TokenStateServiceFactory` to the new DerbyDatabaseTSS
- implemenedt a new KnoxCLI command that migrates existing tokens from credential stores to any JDBC-based TSS backend (tested it with the new DerbyDB TSS; see below)
- integrated this new KnoxCLI command in a way such that it runs when Knox Gateway is started: if token management is enabled, and the configured TSS implementation is a migration target (currently it's true for any JDBC-based TSS implementation)
- added some new `gateway-site.xml` properties:
- `gateway.knox.token.migration.skip `: ensures that the previous automated step can be controlled (E.g. in case of unforeseen errors it can be turned off). Defaults
- `gateway.knox.token.migration.archive.tokens `: indicates if migrated tokesn should be archived in another credential store called `__tokens`. Defaults to `false`.
- `gateway.knox.token.migration.include.expired.tokens`: whether expired tokens should be migrated or skipped. Defaults to `false`.
- `gateway.knox.token.migration.verbose`: if true, migrated/skipped tokens are added in the `[gateway|knoxcli].log` and, optionally, on the STDOUT (when running the KnoxCLI tool manually). Defaults to `true`, because it's very useful to have the chancee to cross-reference token IDs in case of error debugging.
- `gateway.knox.token.migration.progress.count`: the number of tokens after the token migration tool displays progress in the logs and, optionally, on the STDOUT.
- modified the token generation page to accept the new DerbyDB TSS.
## How was this patch tested?
Configured Knox to have the `AliasBasedTSS` as the token state backend and to allow unlimited token creation:
```
<property>
<name>gateway.service.tokenstate.impl</name>
<value>org.apache.knox.gateway.services.token.impl.AliasBasedTokenStateService</value>
</property>
<property>
<name>gateway.knox.token.limit.per.user</name>
<value>-1</value>
</property>
```
Generated 456 tokens with random expiration times (456x4=1824 aliases) then stopped the Knox GW (to avoid the reaper thread removing expired tokens).
```
$ bin/knoxcli.sh list-alias | grep "items"
1827
```
Executed the new KnoxCLI command to confirm it only migrates anything if the configured backend allows token migration:
```
$ bin/knoxcli.sh migrate-tokens --progressCount 15 --archiveMigrated true
This tool is meant to migrate tokens into a JDBC TokenStateService backend. However, the currently configured one (org.apache.knox.gateway.services.token.impl.AliasBasedTokenStateService) does not fulfill this requirement!
```
Before running the new KnoxLCI command again, I commented out the `gateway.service.tokenstate.impl` param in `gateway-site.xml` => the new default, DerbyDBTSS, was in place.
Executed the command again:
```
$ bin/knoxcli.sh migrate-tokens --progressCount 15 --archiveMigrated true > ~/migrationResultWithArchival.txt
$ bin/knoxcli.sh list-alias | grep items
3 items.
$ bin/knoxcli.sh list-alias --cluster __tokens | grep items
1824 items.
$ cat ~/migrationResultWithArchival.txt
Migrating tokens from __gateway credential store into the configured TokenStateService backend...
Loading token aliases from the __gateway credential store. This could take a while.
Token aliased loaded in 178741 milliseconds
Processed 15 tokens in 102 milliseconds
Processed 30 tokens in 174 milliseconds
...
Processed 450 tokens in 2191 milliseconds
Processed 456 tokens in 2202 milliseconds
Archiving token aliases in the __tokens credential store...
Archived token related aliases in the __tokens credential store in 141849 millsieconds
Removing token aliases from the __gateway credential store...
Removed token related aliases from the __gateway credential store in 38 milliseconds
```
---
Repeated the generate/migration step, but this time without token archival:
```
$ bin/knoxcli.sh migrate-tokens --progressCount 15 > ~/migrationResultWithoutArchival.txt
$ cat ~/migrationResultWithoutArchival.txt
Migrating tokens from __gateway credential store into the configured TokenStateService backend...
Loading token aliases from the __gateway credential store. This could take a while.
Token aliased loaded in 182497 milliseconds
Processed 15 tokens in 160 milliseconds
Processed 30 tokens in 271 milliseconds
...
Processed 456 tokens in 1677 milliseconds
Removing token aliases from the __gateway credential store...
Removed token related aliases from the __gateway credential store in 61 milliseconds
```
I also tested the token migration tool integration during the Knox Gateway startup. I removed the previously created data/security/tokens folder, switched to AliasBasedTSS and created another 456 tokens. Then switched back to the default DerbyDBTSS and started the Knox GW:
```
2023-12-14 10:14:18,653 INFO knox.gateway (GatewayServer.java:logSysProp(227)) - System Property: user.name=sandormolnar
2023-12-14 10:14:18,659 INFO knox.gateway (GatewayServer.java:logSysProp(227)) - System Property: user.dir=/Users/sandormolnar/test/knoxGateway
2023-12-14 10:14:18,659 INFO knox.gateway (GatewayServer.java:logSysProp(227)) - System Property: java.runtime.name=OpenJDK Runtime Environment
2023-12-14 10:14:18,659 INFO knox.gateway (GatewayServer.java:logSysProp(227)) - System Property: java.runtime.version=1.8.0_282-bre_2021_01_20_16_37-b00
2023-12-14 10:14:18,659 INFO knox.gateway (GatewayServer.java:logSysProp(227)) - System Property: java.home=/usr/local/Cellar/openjdk@8/1.8.0+282/libexec/openjdk.jdk/Contents/Home/jre
...
2023-12-14 10:14:22,001 INFO knox.gateway (AbstractServiceFactory.java:logServiceUsage(103)) - Using org.apache.knox.gateway.services.token.impl.DerbyDBTokenStateService implementation for TokenStateService
...
2023-12-14 10:14:24,932 INFO knox.gateway (AbstractGatewayServices.java:start(60)) - Starting service: org.apache.knox.gateway.services.token.impl.DerbyDBTokenStateService
2023-12-14 10:14:24,937 INFO token.state (TokenMigrationTool.java:log(114)) - Loading token aliases from the __gateway credential store. This could take a while.
2023-12-14 10:17:35,276 INFO token.state (TokenMigrationTool.java:log(114)) - Token aliases loaded in 190342 milliseconds
2023-12-14 10:17:35,514 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token cdb286ff-2037-41ea-8918-c338675f92eb into the configured TokenStateService backend.
2023-12-14 10:17:35,515 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = bd11f6bf-6ee4-4322-98ab-6797f8ac7d00
2023-12-14 10:17:35,562 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 5b8a754f-78c7-428a-95d6-ff6cff586b87 into the configured TokenStateService backend.
2023-12-14 10:17:35,601 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token d0281d9c-a652-4d79-824e-7b53f4476ea2 into the configured TokenStateService backend.
2023-12-14 10:17:35,634 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token a4d506d8-28aa-41be-8a45-ee9e3f7d109c into the configured TokenStateService backend.
2023-12-14 10:17:35,634 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 7d7a24b1-fcba-4828-9bc4-a22623a78357
2023-12-14 10:17:35,660 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 0c52b28b-b6d3-411a-a8af-d148a05aac0a into the configured TokenStateService backend.
2023-12-14 10:17:35,684 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token d94263f1-5e5b-4fe3-9d78-74a26caee446 into the configured TokenStateService backend.
2023-12-14 10:17:35,684 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 72e7bb73-d53b-4e89-9a8b-d5c4a17a93b3
2023-12-14 10:17:35,684 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 7f89e3c5-4567-40c7-b713-17e54695cb6c
2023-12-14 10:17:35,685 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 10 tokens in 342 milliseconds
2023-12-14 10:17:35,708 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 3dbe9224-ac82-43f1-b383-efd930dcfb09 into the configured TokenStateService backend.
2023-12-14 10:17:35,731 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 760c1ff9-49f6-4ba0-9114-2c0dd3cdc9d2 into the configured TokenStateService backend.
2023-12-14 10:17:35,753 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token d67cc4ab-4de0-4f2d-8664-3e9dd721b634 into the configured TokenStateService backend.
2023-12-14 10:17:35,775 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 49a55ad7-44f6-48cc-bd4a-3db5a2108029 into the configured TokenStateService backend.
2023-12-14 10:17:35,775 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 853527ab-6c52-4d8c-8dd6-f7cf5c4f3b27
2023-12-14 10:17:35,776 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 681206a9-3335-4129-90c3-50f996f194c1
2023-12-14 10:17:35,798 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token f424005f-2ae0-498c-949b-760c880e4662 into the configured TokenStateService backend.
2023-12-14 10:17:35,799 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 8ab5fc0f-02d6-44d6-89ec-d41b4825c29d
2023-12-14 10:17:35,829 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 772e0455-4913-48ff-b66d-422e7d624f43 into the configured TokenStateService backend.
2023-12-14 10:17:35,856 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 8b24070d-11f6-47e9-8832-faaee12faed0 into the configured TokenStateService backend.
2023-12-14 10:17:35,857 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 20 tokens in 515 milliseconds
2023-12-14 10:17:35,857 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 1fc77b80-c96d-4f3e-9841-cd2a2703e2db
2023-12-14 10:17:35,857 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 4dd4955e-1380-4f12-9b5d-49052e4c794b
2023-12-14 10:17:35,882 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token dfa2ca51-2f9c-4ebf-95c1-760c1ed23d89 into the configured TokenStateService backend.
2023-12-14 10:17:35,908 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 48a363c5-7abc-4914-ab11-84bd8caf7423 into the configured TokenStateService backend.
2023-12-14 10:17:35,909 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 3a5888a1-d44a-4f72-b100-bbeeccc11f2b
2023-12-14 10:17:35,909 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = ca972a16-0458-4a07-b547-26d2e8361a0e
2023-12-14 10:17:35,909 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 6d132cf8-9307-4b53-9931-7ba950529850
2023-12-14 10:17:35,909 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = cdbba2df-8884-4caf-a693-a1ea5268f48f
2023-12-14 10:17:35,934 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 326e575a-6fea-4cc8-957c-e5829d76f517 into the configured TokenStateService backend.
2023-12-14 10:17:35,954 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 31f7daee-e134-49e8-a633-4ec9e38bce57 into the configured TokenStateService backend.
2023-12-14 10:17:35,954 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 30 tokens in 612 milliseconds
...
2023-12-14 10:17:40,306 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 440 tokens in 4964 milliseconds
2023-12-14 10:17:40,306 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 50439317-05a7-42ef-8dbc-0f4a648d4c46
2023-12-14 10:17:40,306 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 95332d90-60db-4d54-9b3f-202d2de3f7b4
2023-12-14 10:17:40,306 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = bca3afa2-f1f0-44ae-8766-c938a624e949
2023-12-14 10:17:40,317 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 7d3347c9-c059-42f6-b663-ab8964654010 into the configured TokenStateService backend.
2023-12-14 10:17:40,328 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 6a879abc-03c8-42d6-b990-d2d61d45291c into the configured TokenStateService backend.
2023-12-14 10:17:40,339 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token c7a298bd-b3df-418c-b235-3ae90dcfd1b4 into the configured TokenStateService backend.
2023-12-14 10:17:40,358 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 05a4d692-0618-4008-a7b2-981bcb75a5fe into the configured TokenStateService backend.
2023-12-14 10:17:40,374 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 84143f53-11a2-4d94-8f86-84971a2d1905 into the configured TokenStateService backend.
2023-12-14 10:17:40,388 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 69787bc1-2361-4e90-a219-bf132bea6cce into the configured TokenStateService backend.
2023-12-14 10:17:40,403 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 7c855052-9a69-4f52-8ee0-f8b952ecb656 into the configured TokenStateService backend.
2023-12-14 10:17:40,403 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 450 tokens in 5061 milliseconds
2023-12-14 10:17:40,418 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 9b1ee5aa-9600-4fac-9f48-f152c01dd94b into the configured TokenStateService backend.
2023-12-14 10:17:40,438 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token c03c4748-98c6-4e83-a399-f520e8d579b3 into the configured TokenStateService backend.
2023-12-14 10:17:40,455 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 5bbb60ba-7608-4437-b4cc-72c81097bf8a into the configured TokenStateService backend.
2023-12-14 10:17:40,455 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = 6da78f70-22d2-4f52-b9e7-f274d2028468
2023-12-14 10:17:40,455 INFO token.state (TokenMigrationTool.java:log(114)) - Skipping the migration of expired token with ID = e7b5e8a2-829a-40a2-8cd4-150943d69788
2023-12-14 10:17:40,474 INFO token.state (TokenMigrationTool.java:log(114)) - Migrated token 1419101b-3714-4e32-9a3b-98ffa46a0e3e into the configured TokenStateService backend.
2023-12-14 10:17:40,474 INFO token.state (TokenMigrationTool.java:log(114)) - Processed 456 tokens in 5132 milliseconds
2023-12-14 10:17:40,474 INFO token.state (TokenMigrationTool.java:log(114)) - Removing token aliases from the __gateway credential store...
2023-12-14 10:17:40,536 INFO token.state (TokenMigrationTool.java:log(114)) - Removed token related aliases from the __gateway credential store in 62 milliseconds
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474128445
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java:
##########
@@ -457,4 +457,9 @@ private Collection<KnoxToken> fetchTokens(String userName, boolean createdBy) {
});
return tokens;
}
+
+ @Override
+ public boolean isMigrationTarget() {
Review Comment:
Done.
##########
gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.util;
+
+import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.token.impl.TokenStateServiceMessages;
+
+public class TokenMigrationTool {
+
+ private static final String TOKEN_ALIAS_SUFFIX_DELIM = "--";
+ private static final String TOKEN_ISSUE_TIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "iss";
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473959716
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java:
##########
@@ -65,12 +76,33 @@ public void init(GatewayConfig config, Map<String, String> options) throws Servi
} catch (Exception e) {
throw new ServiceLifecycleException("Error while initiating JDBCTokenStateService: " + e, e);
}
+
+ this.skipTokenMigration = config.skipTokenMigration();
+ this.archiveMigratedTokens = config.archiveMigratedTokens();
+ this.migrateExpiredTokens = config.migrateExpiredTokens();
+ this.verboseTokenMigration = config.printVerboseTokenMigrationMessages();
+ this.tokenMigrationProgressCount = config.getTokenMigrationProgressCount();
} finally {
initLock.unlock();
}
}
}
+ @Override
+ public void start() throws ServiceLifecycleException {
+ super.start();
+ if (skipTokenMigration) {
+ log.skipTokenMigration();
+ } else {
+ final TokenMigrationTool tokenMigrationTool = new TokenMigrationTool(aliasService, this, null);
Review Comment:
Yes, that was the final agreement with @lmccay on the [[DISCUSS]](https://lists.apache.org/thread/fs9nkl6l45o330ttvgvqxj3jnxt63bcs) thread about this change:
```
- the migration tool will be run automatically when the Knox Gateway starts, and it will run on the main thread (i.e. not in the background).
- there will be a config to control this step: in case of an error/bug, this automated migration could be turned off
```
In the 2.1.0 release, we will add proper documentation on best practices about this change: the recommended way of upgrading to that version is to run the new KnoxCLI code before the upgraded Knox Gateway starts. If admins do that, this implicit step will finish in a matter of (milli)seconds during Knox Gateway startup. However, if they failed to do that before, we agreed that it's better to have tokens auto-migrated than leave them in the dark and guess why token authentication isn't working. I believe the detailed entries in `gateway.log` (see above in the description) make it crystal clear why the KG startup takes such time for the first time.
Sample startup after token migration executed before:
```
2024-02-01 10:36:46,854 INFO token.state (TokenMigrationTool.java:log(115)) - Loading token aliases from the __gateway credential store. This could take a while.
2024-02-01 10:36:46,960 INFO token.state (TokenMigrationTool.java:log(115)) - Token aliases loaded in 107 milliseconds
2024-02-01 10:36:46,962 INFO token.state (TokenMigrationTool.java:log(115)) - Processed 0 tokens in 1 milliseconds
2024-02-01 10:36:46,962 INFO token.state (TokenMigrationTool.java:log(115)) - Removing token aliases from the __gateway credential store...
2024-02-01 10:36:46,964 INFO token.state (TokenMigrationTool.java:log(115)) - Removed token related aliases from the __gateway credential store in 2 milliseconds
```
It took only 110 milliseconds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473959716
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java:
##########
@@ -65,12 +76,33 @@ public void init(GatewayConfig config, Map<String, String> options) throws Servi
} catch (Exception e) {
throw new ServiceLifecycleException("Error while initiating JDBCTokenStateService: " + e, e);
}
+
+ this.skipTokenMigration = config.skipTokenMigration();
+ this.archiveMigratedTokens = config.archiveMigratedTokens();
+ this.migrateExpiredTokens = config.migrateExpiredTokens();
+ this.verboseTokenMigration = config.printVerboseTokenMigrationMessages();
+ this.tokenMigrationProgressCount = config.getTokenMigrationProgressCount();
} finally {
initLock.unlock();
}
}
}
+ @Override
+ public void start() throws ServiceLifecycleException {
+ super.start();
+ if (skipTokenMigration) {
+ log.skipTokenMigration();
+ } else {
+ final TokenMigrationTool tokenMigrationTool = new TokenMigrationTool(aliasService, this, null);
Review Comment:
Yes, that was the final agreement with @lmccay on the [[DISCUSS]](https://lists.apache.org/thread/fs9nkl6l45o330ttvgvqxj3jnxt63bcs) thread about this change:
```
- the migration tool will be run automatically when the Knox Gateway starts, and it will run on the main thread (i.e. not in the background).
- there will be a config to control this step: in case of an error/bug, this automated migration could be turned off
```
In the 2.1.0 release, we will add proper documentation on best practices about this change: the recommended way of upgrading to that version is to run the new KnoxCLI code before the upgraded Knox Gateway starts. If admins do that, this implicit step will finish in a matter of (milli)seconds during Knox Gateway startup. However, if they failed to do that before, we agreed that it's better to have tokens auto-migrated than leave them in the dark and guess why token authentication isn't working. I believe the detailed entries in `gateway.log` (see above in the description) make it crystal clear why the KG startup takes such time for the first time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1472744505
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ ((DerbyDBTokenStateService) service).setMasterService(getMasterService(gatewayServices));
+ service.init(gatewayConfig, options);
+ } catch (ServiceLifecycleException e) {
+ LOG.errorInitializingService(DerbyDBTokenStateService.class.getName(), e.getMessage(), e);
+ service = new DefaultTokenStateService();
Review Comment:
If we fail to initialize `DerbyDBTokenStateService` should we just fail so that the user knows there was an issue? looks like we fall back on `DefaultTokenStateService` . Basically, should we fail fast? i'd say so.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
I am thinking if this is a default impl then in almost all the cases this will be used. Do you think it will speed up knox startup if we move this to the top, it might be a sub milli-sec improvement :) so feel free to ignore this comment.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ throw new ServiceLifecycleException("Error while initiating DerbyDBTokenStateService: " + e, e);
+ }
+ }
+
+ private void startDerby() throws Exception {
Review Comment:
Can we start Derby on a separate async thread? DB init can be done in parallel.
##########
gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java:
##########
@@ -282,6 +282,11 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
private static final String TOKEN_STATE_SERVER_MANAGED = GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.exp.server-managed";
private static final String USERS_CAN_SEE_ALL_TOKENS = GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.management.users.can.see.all.tokens";
+ private static final String SKIP_TOKEN_MIGRATION= GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.migration.skip";
Review Comment:
Low Priority: If you are making other changes, it might be better to create a variable for `knox.token` for readability.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
need to remove print stacktrace
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java:
##########
@@ -52,6 +52,8 @@
/**
* A TokenStateService implementation based on the AliasService.
+ *
+ * @deprecated Since 2.1.0
*/
public class AliasBasedTokenStateService extends AbstractPersistentTokenStateService implements TokenStatePeristerMonitorListener {
Review Comment:
Low Priority: Do you think we should also put a log entry (INFO) stating this impl is deprecated?
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JournalBasedTokenStateService.java:
##########
@@ -32,6 +32,9 @@
import java.util.Map;
import java.util.Set;
+/**
+ * @deprecated Since 2.1.0
+ */
public class JournalBasedTokenStateService extends AbstractPersistentTokenStateService {
Review Comment:
Perhaps put a log entry too about deprecation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473653395
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
@pzampino good point, service loader.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "pzampino (via GitHub)" <gi...@apache.org>.
pzampino commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474682395
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
Let's create a JIRA improvement issue to address the extensibility conversation. With that, I agree we can exclude that from the scope of this specific work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "zeroflag (via GitHub)" <gi...@apache.org>.
zeroflag commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1439521871
##########
gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.util;
+
+import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.token.impl.TokenStateServiceMessages;
+
+public class TokenMigrationTool {
+
+ private static final String TOKEN_ALIAS_SUFFIX_DELIM = "--";
+ private static final String TOKEN_ISSUE_TIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "iss";
+ private static final String TOKEN_MAX_LIFETIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "max";
+ private static final String TOKEN_META_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "meta";
+ private static final TokenStateServiceMessages LOG = MessagesFactory.get(TokenStateServiceMessages.class);
+
+ private final AliasService aliasService;
+ private final TokenStateService tokenStateService;
+ private final PrintStream out;
+
+ private int progressCount = 10;
+ private boolean archiveMigratedTokens;
+ private boolean migrateExpiredTokens;
+ private boolean verbose;
+
+ public TokenMigrationTool(AliasService aliasService, TokenStateService tokenStateService, PrintStream out) {
+ this.aliasService = aliasService;
+ this.tokenStateService = tokenStateService;
+ this.out = out;
+ }
+
+ public void setProgressCount(int progressCount) {
+ this.progressCount = progressCount;
+ }
+
+ public void setArchiveMigratedTokens(boolean archiveMigratedTokens) {
+ this.archiveMigratedTokens = archiveMigratedTokens;
+ }
+
+ public void setMigrateExpiredTokens(boolean migrateExpiredTokens) {
+ this.migrateExpiredTokens = migrateExpiredTokens;
+ }
+
+ public void setVerbose(boolean verbose) {
+ this.verbose = verbose;
+ }
+
+ public void migrateTokensFromGatewayCredentialStore() {
+ try {
+ final Map<String, TokenData> tokenDataMap = new ConcurrentHashMap<>();
+ final long start = System.currentTimeMillis();
+ String logMessage = "Loading token aliases from the __gateway credential store. This could take a while.";
+ log(logMessage);
+ final Map<String, char[]> passwordAliasMap = aliasService.getPasswordsForGateway();
+ log("Token aliases loaded in " + (System.currentTimeMillis() - start) + " milliseconds");
+ String alias;
+ for (Map.Entry<String, char[]> passwordAliasMapEntry : passwordAliasMap.entrySet()) {
+ alias = passwordAliasMapEntry.getKey();
+ processAlias(passwordAliasMap, passwordAliasMapEntry, alias, tokenDataMap);
+ }
+
+ final long migrationStart = System.currentTimeMillis();
+ final AtomicInteger count = new AtomicInteger(0);
Review Comment:
Is there a reason why we use AtomicInteger + ConcurrentHashMap here? Is this method called from multiple threads?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 merged PR #826:
URL: https://github.com/apache/knox/pull/826
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473960005
##########
gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.util;
+
+import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.token.impl.TokenStateServiceMessages;
+
+public class TokenMigrationTool {
+
+ private static final String TOKEN_ALIAS_SUFFIX_DELIM = "--";
+ private static final String TOKEN_ISSUE_TIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "iss";
Review Comment:
Ack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473933619
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
The service loader approach is already in use by `org.apache.knox.gateway.services.GatewayServiceFactory`. Thus, if someone wants to add their own implementation without contributing here, in the Knox project, they can do that by overriding the existing `TokenStateServiceFactory` and having that custom factory inserted into the `org.apache.knox.gateway.services.ServiceFactory` file within their custom JAR.
However, I see that's a different discussion we should start in Knox's DEV e-mail list. As of now, all service implementations work that way and I would rather not do this work in the scope of this JIRA.
Any objection?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473924934
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
Review Comment:
That's a different discussion we should start in Knox's DEV e-mail list. As of now, all service implementations work that way and I would rather not do this work in the scope of this JIRA.
Any objection?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474126537
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java:
##########
@@ -52,6 +52,8 @@
/**
* A TokenStateService implementation based on the AliasService.
+ *
+ * @deprecated Since 2.1.0
*/
public class AliasBasedTokenStateService extends AbstractPersistentTokenStateService implements TokenStatePeristerMonitorListener {
Review Comment:
Fixed: added a `WARN` message. Sample:
```
2024-02-01T10:03:54,725 WARN token.state (AliasBasedTokenStateService.java:init(85)) - The configured TokenStateService implementation, org.apache.knox.gateway.services.token.impl.AliasBasedTokenStateService, is deprecated!
```
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474127882
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JournalBasedTokenStateService.java:
##########
@@ -32,6 +32,9 @@
import java.util.Map;
import java.util.Set;
+/**
+ * @deprecated Since 2.1.0
+ */
public class JournalBasedTokenStateService extends AbstractPersistentTokenStateService {
Review Comment:
Fixed: added a WARN message. Sample:
```
2024-02-01T10:03:54,725 WARN token.state (AliasBasedTokenStateService.java:init(85)) - The configured TokenStateService implementation, org.apache.knox.gateway.services.token.impl.AliasBasedTokenStateService, is deprecated!```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473936489
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ ((DerbyDBTokenStateService) service).setMasterService(getMasterService(gatewayServices));
+ service.init(gatewayConfig, options);
+ } catch (ServiceLifecycleException e) {
+ LOG.errorInitializingService(DerbyDBTokenStateService.class.getName(), e.getMessage(), e);
+ service = new DefaultTokenStateService();
Review Comment:
We have a clear indication about this issue
- on the Token Generation UI
- in the logs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1440274597
##########
gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.util;
+
+import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.token.impl.TokenStateServiceMessages;
+
+public class TokenMigrationTool {
+
+ private static final String TOKEN_ALIAS_SUFFIX_DELIM = "--";
+ private static final String TOKEN_ISSUE_TIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "iss";
+ private static final String TOKEN_MAX_LIFETIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "max";
+ private static final String TOKEN_META_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "meta";
+ private static final TokenStateServiceMessages LOG = MessagesFactory.get(TokenStateServiceMessages.class);
+
+ private final AliasService aliasService;
+ private final TokenStateService tokenStateService;
+ private final PrintStream out;
+
+ private int progressCount = 10;
+ private boolean archiveMigratedTokens;
+ private boolean migrateExpiredTokens;
+ private boolean verbose;
+
+ public TokenMigrationTool(AliasService aliasService, TokenStateService tokenStateService, PrintStream out) {
+ this.aliasService = aliasService;
+ this.tokenStateService = tokenStateService;
+ this.out = out;
+ }
+
+ public void setProgressCount(int progressCount) {
+ this.progressCount = progressCount;
+ }
+
+ public void setArchiveMigratedTokens(boolean archiveMigratedTokens) {
+ this.archiveMigratedTokens = archiveMigratedTokens;
+ }
+
+ public void setMigrateExpiredTokens(boolean migrateExpiredTokens) {
+ this.migrateExpiredTokens = migrateExpiredTokens;
+ }
+
+ public void setVerbose(boolean verbose) {
+ this.verbose = verbose;
+ }
+
+ public void migrateTokensFromGatewayCredentialStore() {
+ try {
+ final Map<String, TokenData> tokenDataMap = new ConcurrentHashMap<>();
+ final long start = System.currentTimeMillis();
+ String logMessage = "Loading token aliases from the __gateway credential store. This could take a while.";
+ log(logMessage);
+ final Map<String, char[]> passwordAliasMap = aliasService.getPasswordsForGateway();
+ log("Token aliases loaded in " + (System.currentTimeMillis() - start) + " milliseconds");
+ String alias;
+ for (Map.Entry<String, char[]> passwordAliasMapEntry : passwordAliasMap.entrySet()) {
+ alias = passwordAliasMapEntry.getKey();
+ processAlias(passwordAliasMap, passwordAliasMapEntry, alias, tokenDataMap);
+ }
+
+ final long migrationStart = System.currentTimeMillis();
+ final AtomicInteger count = new AtomicInteger(0);
Review Comment:
No, there is no multi-threading here. I needed to use AtomicInteger because of the stream below accepts a final `count` variable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "pzampino (via GitHub)" <gi...@apache.org>.
pzampino commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473463201
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
Review Comment:
If the constructors accept GatewayServices, GatewayConfig, and options, the implementations can do all their own initialization in a self-contained manner, simplifying the factory implementation.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java:
##########
@@ -65,12 +76,33 @@ public void init(GatewayConfig config, Map<String, String> options) throws Servi
} catch (Exception e) {
throw new ServiceLifecycleException("Error while initiating JDBCTokenStateService: " + e, e);
}
+
+ this.skipTokenMigration = config.skipTokenMigration();
+ this.archiveMigratedTokens = config.archiveMigratedTokens();
+ this.migrateExpiredTokens = config.migrateExpiredTokens();
+ this.verboseTokenMigration = config.printVerboseTokenMigrationMessages();
+ this.tokenMigrationProgressCount = config.getTokenMigrationProgressCount();
} finally {
initLock.unlock();
}
}
}
+ @Override
+ public void start() throws ServiceLifecycleException {
+ super.start();
+ if (skipTokenMigration) {
+ log.skipTokenMigration();
+ } else {
+ final TokenMigrationTool tokenMigrationTool = new TokenMigrationTool(aliasService, this, null);
Review Comment:
Token migration is an implicit action when a JDBC-based impl starts? I missed that part when I first read the PR description. Should skipTokenMigration default to true, such that an admin has to explicitly enable this implicit migration?
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java:
##########
@@ -457,4 +457,9 @@ private Collection<KnoxToken> fetchTokens(String userName, boolean createdBy) {
});
return tokens;
}
+
+ @Override
+ public boolean isMigrationTarget() {
Review Comment:
Instead of a method to override in every impl, you could have those implementations that support migration implement a MigrationTarget interface.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
There is something about this logic that smells bad to me. From my point of view, someone should be able to implement TokenStateService with whatever impl they need, configure it in gateway-site, and drop their JAR in the ext dir. With this logic, that won't work. Can we consider refactoring this to keep Knox extensible?
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ ((DerbyDBTokenStateService) service).setMasterService(getMasterService(gatewayServices));
+ service.init(gatewayConfig, options);
+ } catch (ServiceLifecycleException e) {
+ LOG.errorInitializingService(DerbyDBTokenStateService.class.getName(), e.getMessage(), e);
+ service = new DefaultTokenStateService();
Review Comment:
Falling back to in-memory is ok with me, so long as we log some indication about that fact. If a user thinks their Knox is running against the DB they configured, but it's only in-memory, that could be a real problem.
##########
gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java:
##########
@@ -0,0 +1,232 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.util;
+
+import java.io.PrintStream;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.security.AliasService;
+import org.apache.knox.gateway.services.security.AliasServiceException;
+import org.apache.knox.gateway.services.security.token.TokenMetadata;
+import org.apache.knox.gateway.services.security.token.TokenStateService;
+import org.apache.knox.gateway.services.token.impl.TokenStateServiceMessages;
+
+public class TokenMigrationTool {
+
+ private static final String TOKEN_ALIAS_SUFFIX_DELIM = "--";
+ private static final String TOKEN_ISSUE_TIME_POSTFIX = TOKEN_ALIAS_SUFFIX_DELIM + "iss";
Review Comment:
It would be ideal if these were referenced from the AliasBasedTokenStateService where they're defined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473202890
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
Ack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473202455
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
- logServiceUsage(isEmptyDefaultImplementation(implementation) ? AliasBasedTokenStateService.class.getName() : implementation, serviceType);
+ logServiceUsage(service.getClass().getName(), serviceType);
}
return service;
}
+ private Service useDerbyDatabaseTokenStateService(GatewayServices gatewayServices, GatewayConfig gatewayConfig, Map<String, String> options) {
+ Service service;
+ try {
+ service = new DerbyDBTokenStateService();
+ ((DerbyDBTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ ((DerbyDBTokenStateService) service).setMasterService(getMasterService(gatewayServices));
+ service.init(gatewayConfig, options);
+ } catch (ServiceLifecycleException e) {
+ LOG.errorInitializingService(DerbyDBTokenStateService.class.getName(), e.getMessage(), e);
+ service = new DefaultTokenStateService();
Review Comment:
Nope, I don't think that's a good idea. The aim here is to have a TSS working OOTB even if there were some initialization issues. The same as we had in previous versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473200586
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java:
##########
@@ -61,17 +62,32 @@ protected Service createService(GatewayServices gatewayServices, ServiceType ser
service.init(gatewayConfig, options);
} catch (ServiceLifecycleException e) {
LOG.errorInitializingService(implementation, e.getMessage(), e);
- service = new AliasBasedTokenStateService();
- ((AliasBasedTokenStateService) service).setAliasService(getAliasService(gatewayServices));
+ service = useDerbyDatabaseTokenStateService(gatewayServices, gatewayConfig, options);
}
+ } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {
Review Comment:
I'll make that change anyway and move the`DerbyDBTokenStateService` so that the list starts with the `default` implementation (naming is not our friend here :) ). I may also rename `DefaultTSS` to `InMemoryTSS`...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473947362
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java:
##########
@@ -457,4 +457,9 @@ private Collection<KnoxToken> fetchTokens(String userName, boolean createdBy) {
});
return tokens;
}
+
+ @Override
+ public boolean isMigrationTarget() {
Review Comment:
Since the `DefaultTokenStateService` implements that method already, there is no need to implement it everywhere. The only override of this method is needed when you want to make that implementation a migration target. That's what I did in `JDBCTokenStateService`.
From this point of view, making that class implement an interface or override this method, doesn't make any difference, therefore I don't mind changing to the interface approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474129338
##########
gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java:
##########
@@ -282,6 +282,11 @@ public class GatewayConfigImpl extends Configuration implements GatewayConfig {
private static final String TOKEN_STATE_SERVER_MANAGED = GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.exp.server-managed";
private static final String USERS_CAN_SEE_ALL_TOKENS = GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.management.users.can.see.all.tokens";
+ private static final String SKIP_TOKEN_MIGRATION= GATEWAY_CONFIG_FILE_PREFIX + ".knox.token.migration.skip";
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "moresandeep (via GitHub)" <gi...@apache.org>.
moresandeep commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1474340901
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ throw new ServiceLifecycleException("Error while initiating DerbyDBTokenStateService: " + e, e);
+ }
+ }
+
+ private void startDerby() throws Exception {
Review Comment:
It might be but then we wait 1 sec below for it to start up, depending on how much tokens there are it will take time for DB to start up and we will then have to update the sleep times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default [knox]
Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on code in PR #826:
URL: https://github.com/apache/knox/pull/826#discussion_r1473214839
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ throw new ServiceLifecycleException("Error while initiating DerbyDBTokenStateService: " + e, e);
+ }
+ }
+
+ private void startDerby() throws Exception {
Review Comment:
I don't think the gain here exceeds what we lose with the extra complexity. What reasons do you have for this action? The `startDerby` call is a really fast operation.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.knox.gateway.services.token.impl;
+
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_NAME;
+import static org.apache.knox.gateway.config.impl.GatewayConfigImpl.GATEWAY_DATABASE_TYPE;
+import static org.apache.knox.gateway.services.security.AliasService.NO_CLUSTER_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_PASSWORD_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DATABASE_USER_ALIAS_NAME;
+import static org.apache.knox.gateway.util.JDBCUtils.DERBY_DB_TYPE;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.security.MasterService;
+import org.apache.knox.gateway.shell.jdbc.derby.DerbyDatabase;
+import org.apache.knox.gateway.util.FileUtils;
+
+public class DerbyDBTokenStateService extends JDBCTokenStateService {
+
+ public static final String DEFAULT_TOKEN_DB_USER_NAME = "knox";
+ public static final String DB_NAME = "tokens";
+
+ private DerbyDatabase derbyDatabase;
+ private Path derbyDatabaseFolder;
+ private MasterService masterService;
+
+ public void setMasterService(MasterService masterService) {
+ this.masterService = masterService;
+ }
+
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws ServiceLifecycleException {
+ try {
+ derbyDatabaseFolder = Paths.get(config.getGatewaySecurityDir(), DB_NAME);
+ startDerby();
+ ((Configuration) config).set(GATEWAY_DATABASE_TYPE, DERBY_DB_TYPE);
+ ((Configuration) config).set(GATEWAY_DATABASE_NAME, derbyDatabaseFolder.toString());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_USER_ALIAS_NAME, getDatabaseUserName());
+ getAliasService().addAliasForCluster(NO_CLUSTER_NAME, DATABASE_PASSWORD_ALIAS_NAME, getDatabasePassword());
+ super.init(config, options);
+
+ // we need the "x" permission too to be able to browse that folder (600 is not enough)
+ if (Files.exists(derbyDatabaseFolder)) {
+ FileUtils.chmod("700", derbyDatabaseFolder.toFile());
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ throw new ServiceLifecycleException("Error while initiating DerbyDBTokenStateService: " + e, e);
+ }
+ }
+
+ private void startDerby() throws Exception {
Review Comment:
@moresandeep - I don't think the gain here exceeds what we lose with the extra complexity. What reasons do you have for this action? The `startDerby` call is a really fast operation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@knox.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org