You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/07/06 17:11:51 UTC
[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #739: GUACAMOLE-1629: Allow vault configurations per connection group
mike-jumper commented on code in PR #739:
URL: https://github.com/apache/guacamole-client/pull/739#discussion_r915076251
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,53 @@ public class KsmSecretService implements VaultSecretService {
@Inject
private KsmConfigurationService confService;
+ /**
+ * Factory for creating KSM client instances.
+ */
+ @Inject
+ private KsmClientFactory ksmClientFactory;
+
+ /**
+ * A map of base-64 encoded JSON KSM config blobs to associated KSM client instances.
+ * The `null` entry in this Map is associated with the KSM configuration parsed
+ * from the guacamole.properties config file. A distinct KSM client will exist for
+ * every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM cache for the provided KSM config if not already
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,53 @@ public class KsmSecretService implements VaultSecretService {
@Inject
private KsmConfigurationService confService;
+ /**
+ * Factory for creating KSM client instances.
+ */
+ @Inject
+ private KsmClientFactory ksmClientFactory;
+
+ /**
+ * A map of base-64 encoded JSON KSM config blobs to associated KSM client instances.
+ * The `null` entry in this Map is associated with the KSM configuration parsed
+ * from the guacamole.properties config file. A distinct KSM client will exist for
+ * every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM cache for the provided KSM config if not already
+ * present in the cache map, otherwise return the existing cache entry.
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java:
##########
@@ -246,7 +258,7 @@ private void validateCache() throws GuacamoleException {
});
- // Cache has been refreshed
+ // Client has been refreshed
Review Comment:
Cache*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,53 @@ public class KsmSecretService implements VaultSecretService {
@Inject
private KsmConfigurationService confService;
+ /**
+ * Factory for creating KSM client instances.
+ */
+ @Inject
+ private KsmClientFactory ksmClientFactory;
+
+ /**
+ * A map of base-64 encoded JSON KSM config blobs to associated KSM client instances.
+ * The `null` entry in this Map is associated with the KSM configuration parsed
+ * from the guacamole.properties config file. A distinct KSM client will exist for
+ * every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM cache for the provided KSM config if not already
+ * present in the cache map, otherwise return the existing cache entry.
+ *
+ * @param ksmConfig
+ * The base-64 encoded JSON KSM config blob associated with the cache entry.
+ * If an associated entry does not already exist, it will be created using
+ * this configuration.
+ *
+ * @return
+ * A KSM cache for the provided KSM config if not already present in the
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,53 @@ public class KsmSecretService implements VaultSecretService {
@Inject
private KsmConfigurationService confService;
+ /**
+ * Factory for creating KSM client instances.
+ */
+ @Inject
+ private KsmClientFactory ksmClientFactory;
+
+ /**
+ * A map of base-64 encoded JSON KSM config blobs to associated KSM client instances.
+ * The `null` entry in this Map is associated with the KSM configuration parsed
Review Comment:
I think this is incorrect / from an older version of this code: there can't be a `null` entry in this `Map` as it's a `ConcurrentHashMap`.
##########
extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java:
##########
@@ -398,9 +418,14 @@ protected void addTokens(Connection connection, Map<String, String> tokens)
// Substitute tokens producing secret names, retrieving and storing
// those secrets as parameter tokens
- tokens.putAll(resolve(getTokens(confService.getTokenMapping(), filter,
- config, new TokenFilter(tokens))));
+ tokens.putAll(resolve(getTokens(connection, confService.getTokenMapping(),
+ filter, config, new TokenFilter(tokens))));
}
+ @Override
+ public Collection<Form> getConnectionGroupAttributes() {
+ return attributeService.getConnectionGroupAttributes();
Review Comment:
This will need to use the result of invoking `super.getConnectionGroupAttributes()` as a basis or the connection group attributes of other extensions will be masked by these new attributes.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java:
##########
@@ -203,14 +215,14 @@ private void validateCache() throws GuacamoleException {
cacheLock.writeLock().lock();
try {
- // Cache may have been updated since the read-only check. Re-verify
+ // Client may have been updated since the read-only check. Re-verify
Review Comment:
Cache*
--
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@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org