KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default #826

smolnar82 · 2023-12-14T09:49:35Z

What changes were proposed in this pull request?

Implemented the changes I listed in 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. I confirmed the $SECURITY_DIR/tokens was created with the proper file permissions:

ls -al data/security/
total 16
drwxr-xr-x   6 sandormolnar  staff   192 Dec 15 15:26 .
drwxr-xr-x   8 sandormolnar  staff   256 Dec 15 15:27 ..
drwxr-xr-x  12 sandormolnar  staff   384 Dec 14 19:43 keystores
-rw-------   1 sandormolnar  staff   125 Dec  5 16:05 master
-rw-r--r--   1 sandormolnar  staff  2746 Dec 15 15:27 registry
drwx------   9 sandormolnar  staff   288 Dec 15 15:27 tokens

I also checked if the automated token migration tool works as expected:

2023-12-15 15:32:35,742  INFO  knox.gateway (GatewayServer.java:logSysProp(226)) - System Property: user.name=sandormolnar
2023-12-15 15:32:35,749  INFO  knox.gateway (GatewayServer.java:logSysProp(226)) - System Property: user.dir=/Users/sandormolnar/test/knoxGateway
2023-12-15 15:32:35,749  INFO  knox.gateway (GatewayServer.java:logSysProp(226)) - System Property: java.runtime.name=OpenJDK Runtime Environment
2023-12-15 15:32:35,750  INFO  knox.gateway (GatewayServer.java:logSysProp(226)) - System Property: java.runtime.version=1.8.0_282-bre_2021_01_20_16_37-b00
2023-12-15 15:32:35,750  INFO  knox.gateway (GatewayServer.java:logSysProp(226)) - System Property: java.home=/usr/local/Cellar/openjdk@8/1.8.0+282/libexec/openjdk.jdk/Contents/Home/jre
...
2023-12-15 15:32:38,960  INFO  knox.gateway (AbstractServiceFactory.java:logServiceUsage(103)) - Using org.apache.knox.gateway.services.token.impl.DerbyDBTokenStateService implementation for TokenStateService
...
2023-12-15 15:32:41,930  INFO  knox.gateway (AbstractGatewayServices.java:start(60)) - Starting service: org.apache.knox.gateway.services.token.impl.DerbyDBTokenStateService
2023-12-15 15:32:41,935  INFO  token.state (TokenMigrationTool.java:log(115)) - Loading token aliases from the __gateway credential store. This could take a while.
2023-12-15 15:35:43,701  INFO  token.state (TokenMigrationTool.java:log(115)) - Token aliases loaded in 181768 milliseconds
2023-12-15 15:35:43,864  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token c9ec7cff...84d5a729b6ce into the configured TokenStateService backend.
2023-12-15 15:35:43,874  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 69072c33...9f110e2f1306 into the configured TokenStateService backend.
2023-12-15 15:35:43,884  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 16287741...34aa267014a7 into the configured TokenStateService backend.
2023-12-15 15:35:43,893  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 999a0d0d...5aabfb0fd559 into the configured TokenStateService backend.
2023-12-15 15:35:43,893  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = 5b997b0b...0675a4858596
2023-12-15 15:35:43,901  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token b7a77264...da2a6073e70e into the configured TokenStateService backend.
2023-12-15 15:35:43,909  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 2f633b54...c999f0b60de0 into the configured TokenStateService backend.
2023-12-15 15:35:43,918  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token da39a301...9cb261dd7ef2 into the configured TokenStateService backend.
2023-12-15 15:35:43,918  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = aaefe17d...8b12ee327d56
2023-12-15 15:35:43,930  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 1e4cd999...216cefe35978 into the configured TokenStateService backend.
2023-12-15 15:35:43,931  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 10 tokens in 180 milliseconds
2023-12-15 15:35:43,939  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token cf645daa...82f09c49d216 into the configured TokenStateService backend.
2023-12-15 15:35:43,947  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token ddb2e440...1d0703bf6c0e into the configured TokenStateService backend.
2023-12-15 15:35:43,955  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 6bcc713d...b29554301c07 into the configured TokenStateService backend.
2023-12-15 15:35:43,972  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 8e4702e2...49c69244fea1 into the configured TokenStateService backend.
2023-12-15 15:35:43,972  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = c32c416c...58c1da09412d
2023-12-15 15:35:43,981  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token b1df6f9e...3d9e2045d3c8 into the configured TokenStateService backend.
2023-12-15 15:35:43,990  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token c502de50...16fdf21fdbe5 into the configured TokenStateService backend.
2023-12-15 15:35:43,990  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = 300ed0b2...98edda2ba28c
2023-12-15 15:35:44,002  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 3615f73b...c6da9f9d00e9 into the configured TokenStateService backend.
2023-12-15 15:35:44,002  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = 651bbf46...2732ac3a9643
2023-12-15 15:35:44,002  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 20 tokens in 251 milliseconds
2023-12-15 15:35:44,002  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = bb59702e...6d81029ffeaf
2023-12-15 15:35:44,002  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = 8d31e3a4...80082f334ee8
2023-12-15 15:35:44,011  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 4a541cc9...476a8e220dea into the configured TokenStateService backend.
2023-12-15 15:35:44,020  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 338d9037...055fbded8510 into the configured TokenStateService backend.
2023-12-15 15:35:44,029  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token d86ae463...0ef110b55d19 into the configured TokenStateService backend.
2023-12-15 15:35:44,029  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = b5f3d372...2f312d032ebc
2023-12-15 15:35:44,039  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 25af1953...fe8f02118652 into the configured TokenStateService backend.
2023-12-15 15:35:44,047  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 2d619987...af6a546411f0 into the configured TokenStateService backend.
2023-12-15 15:35:44,060  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token fff03417...de1ad8382afb into the configured TokenStateService backend.
2023-12-15 15:35:44,068  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token f248d413...dc39dc1d8b3b into the configured TokenStateService backend.
2023-12-15 15:35:44,068  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 30 tokens in 317 milliseconds
...
2023-12-15 15:35:45,355  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 440 tokens in 1604 milliseconds
2023-12-15 15:35:45,358  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 9e1ab647...bcb4ea563af2 into the configured TokenStateService backend.
2023-12-15 15:35:45,362  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 160f3ee2...5cf694f6843c into the configured TokenStateService backend.
2023-12-15 15:35:45,366  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 3f9b9624...21922baeac30 into the configured TokenStateService backend.
2023-12-15 15:35:45,366  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = a40018f2...2d2e6305efa7
2023-12-15 15:35:45,366  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = d1b077d7...8a022f2dfc18
2023-12-15 15:35:45,370  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 85cfb49c...bcb97b55e7a5 into the configured TokenStateService backend.
2023-12-15 15:35:45,374  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 0ad9d321...9342d5e25e3e into the configured TokenStateService backend.
2023-12-15 15:35:45,377  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token cfaa61cc...bfce4b97b33e into the configured TokenStateService backend.
2023-12-15 15:35:45,377  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = fbc8e097...0099e87d7c31
2023-12-15 15:35:45,380  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token fd196a8f...99b776c356b2 into the configured TokenStateService backend.
2023-12-15 15:35:45,381  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 450 tokens in 1630 milliseconds
2023-12-15 15:35:45,384  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token 014cc90a...8c0bd9d2e5c9 into the configured TokenStateService backend.
2023-12-15 15:35:45,388  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token fab69348...8f56a484b0d0 into the configured TokenStateService backend.
2023-12-15 15:35:45,388  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = fe735825...e2164a8e41ff
2023-12-15 15:35:45,391  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token dea78625...17246bfe26b6 into the configured TokenStateService backend.
2023-12-15 15:35:45,395  INFO  token.state (TokenMigrationTool.java:log(115)) - Migrated token f5f96bf4...2243435a3a84 into the configured TokenStateService backend.
2023-12-15 15:35:45,395  INFO  token.state (TokenMigrationTool.java:log(115)) - Skipping the migration of expired token with ID = 35af7cec...de414e7e78d1
2023-12-15 15:35:45,395  INFO  token.state (TokenMigrationTool.java:log(115)) - Processed 456 tokens in 1644 milliseconds
2023-12-15 15:35:45,395  INFO  token.state (TokenMigrationTool.java:log(115)) - Removing token aliases from the __gateway credential store...
2023-12-15 15:35:45,454  INFO  token.state (TokenMigrationTool.java:log(115)) - Removed token related aliases from the __gateway credential store in 59 milliseconds

gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java

gateway-server/src/main/java/org/apache/knox/gateway/config/impl/GatewayConfigImpl.java

moresandeep · 2024-01-31T12:16:44Z

...-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java

        }
+      } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {


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.

I'll make that change anyway and move theDerbyDBTokenStateService so that the list starts with the default implementation (naming is not our friend here :) ). I may also rename DefaultTSS to InMemoryTSS...

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?

@pzampino good point, service loader.

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?

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.

...-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java

...r/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java

...rver/src/main/java/org/apache/knox/gateway/services/token/impl/DerbyDBTokenStateService.java

...src/main/java/org/apache/knox/gateway/services/token/impl/JournalBasedTokenStateService.java

pzampino · 2024-01-31T21:02:24Z

...-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java

        }
+      } else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {


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?

...-server/src/main/java/org/apache/knox/gateway/services/factory/TokenStateServiceFactory.java

...rver/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java

...-server/src/main/java/org/apache/knox/gateway/services/token/impl/JDBCTokenStateService.java

gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java

In addition to the new implementation I deprecated the AliasBased, Zookeeper and JournalBased TSS implementations in 2.1.0.

moresandeep

LGTM

pzampino

LGTM

smolnar82 requested review from zeroflag, moresandeep, lmccay and pzampino December 14, 2023 09:49

smolnar82 self-assigned this Dec 14, 2023

smolnar82 added knoxtoken KnoxCLI labels Dec 14, 2023

smolnar82 changed the title ~~KNOX-2990 - Using InMemoryDerbyDatabaseTSS instead of AliasBasedTSS by default~~ KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default Dec 14, 2023

smolnar82 force-pushed the KNOX-2990 branch 4 times, most recently from dd15602 to 263f4e2 Compare December 14, 2023 10:27

smolnar82 marked this pull request as draft December 15, 2023 11:36

smolnar82 force-pushed the KNOX-2990 branch from 263f4e2 to 91e9beb Compare December 15, 2023 14:40

smolnar82 marked this pull request as ready for review December 15, 2023 14:41

smolnar82 force-pushed the KNOX-2990 branch 2 times, most recently from 26c4012 to e1c54f0 Compare December 15, 2023 16:16

smolnar82 mentioned this pull request Jan 2, 2024

KNOX-2995 - Support json parsing NaN values #828

Merged

zeroflag reviewed Jan 2, 2024

View reviewed changes

gateway-server/src/main/java/org/apache/knox/gateway/util/TokenMigrationTool.java Show resolved Hide resolved

zeroflag approved these changes Jan 3, 2024

View reviewed changes

smolnar82 force-pushed the KNOX-2990 branch from e1c54f0 to 9b1aeee Compare January 29, 2024 08:27

moresandeep reviewed Jan 31, 2024

View reviewed changes

pzampino reviewed Jan 31, 2024

View reviewed changes

KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default

67d2e58

In addition to the new implementation I deprecated the AliasBased, Zookeeper and JournalBased TSS implementations in 2.1.0.

smolnar82 force-pushed the KNOX-2990 branch from 9b1aeee to 67d2e58 Compare February 1, 2024 09:39

smolnar82 requested review from moresandeep and pzampino February 1, 2024 09:43

moresandeep approved these changes Feb 1, 2024

View reviewed changes

pzampino approved these changes Feb 1, 2024

View reviewed changes

smolnar82 merged commit afdb4cc into apache:master Feb 2, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default #826

KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default #826

smolnar82 commented Dec 14, 2023 •

edited

Loading

moresandeep Jan 31, 2024

smolnar82 Jan 31, 2024

pzampino Jan 31, 2024

moresandeep Feb 1, 2024

smolnar82 Feb 1, 2024

pzampino Feb 1, 2024

pzampino Jan 31, 2024

moresandeep left a comment

pzampino left a comment

		}
		} else if (matchesImplementation(implementation, DerbyDBTokenStateService.class, true)) {

KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default #826

KNOX-2990 - Using DerbyDatabaseTSS instead of AliasBasedTSS by default #826

Conversation

smolnar82 commented Dec 14, 2023 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

moresandeep Jan 31, 2024

Choose a reason for hiding this comment

smolnar82 Jan 31, 2024

Choose a reason for hiding this comment

pzampino Jan 31, 2024

Choose a reason for hiding this comment

moresandeep Feb 1, 2024

Choose a reason for hiding this comment

smolnar82 Feb 1, 2024

Choose a reason for hiding this comment

pzampino Feb 1, 2024

Choose a reason for hiding this comment

pzampino Jan 31, 2024

Choose a reason for hiding this comment

moresandeep left a comment

Choose a reason for hiding this comment

pzampino left a comment

Choose a reason for hiding this comment

smolnar82 commented Dec 14, 2023 •

edited

Loading