mike-jumper commented on code in PR #739:
URL: https://github.com/apache/guacamole-client/pull/739#discussion_r915147424
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,51 @@ 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.
+ * A distinct KSM client will exist for every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new
ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM client 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 client for the provided KSM config if not already present in
the
+ * cache map, otherwise the existing cache entry.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while creating the KSM client.
+ */
+ private KsmClient getClient(@Nonnull String ksmConfig)
+ throws GuacamoleException {
+
+ // If a cache already exists for the provided config, use it
+ KsmClient ksmClient = ksmClientMap.get(ksmConfig);
+ if (ksmClient != null)
+ return ksmClient;
+
+ // Create and store a new KSM cache instance for the provided KSM
config blob
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,51 @@ 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.
+ * A distinct KSM client will exist for every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new
ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM client 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 client for the provided KSM config if not already present in
the
+ * cache map, otherwise the existing cache entry.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while creating the KSM client.
+ */
+ private KsmClient getClient(@Nonnull String ksmConfig)
+ throws GuacamoleException {
+
+ // If a cache already exists for the provided config, use it
+ KsmClient ksmClient = ksmClientMap.get(ksmConfig);
+ if (ksmClient != null)
+ return ksmClient;
+
+ // Create and store a new KSM cache instance for the provided KSM
config blob
+ SecretsManagerOptions options =
confService.getSecretsManagerOptions(ksmConfig);
+ ksmClient = ksmClientFactory.create(options);
+ KsmClient prevClient = ksmClientMap.putIfAbsent(ksmConfig, ksmClient);
+
+ // If the cache was already set before this thread got there, use the
existing one
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/conf/VaultAttributeService.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * 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.guacamole.vault.conf;
+
+import java.util.Collection;
+
+import org.apache.guacamole.form.Form;
+
+/**
+ * A service that exposes attributes for the admin UI, specific to the vault
+ * implementation. Any vault implementation will need to expose the attributes
+ * necessary for that implementation.
+ */
+public interface VaultAttributeService {
+
+ /**
+ * Return all custom connection group attributes to be exposed through the
+ * admin UI for the current vault implementation.
+ *
+ * @return
+ * All custom connection group attributes to be exposed through the
+ * admin UI for the current vault implementation.
+ *
Review Comment:
Please remove this trailing blank line.
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,51 @@ 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.
+ * A distinct KSM client will exist for every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new
ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM client 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 client for the provided KSM config if not already present in
the
+ * cache map, otherwise the existing cache entry.
+ *
+ * @throws GuacamoleException
+ * If an error occurs while creating the KSM client.
+ */
+ private KsmClient getClient(@Nonnull String ksmConfig)
+ throws GuacamoleException {
+
+ // If a cache already exists for the provided config, use it
Review Comment:
client*
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java:
##########
@@ -60,6 +76,51 @@ 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.
+ * A distinct KSM client will exist for every KSM config.
+ */
+ private final ConcurrentMap<String, KsmClient> ksmClientMap = new
ConcurrentHashMap<>();
+
+ /**
+ * Create and return a KSM client 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 client for the provided KSM config if not already present in
the
+ * cache map, otherwise the existing cache entry.
Review Comment:
Should each of these references to the cache map, entry, etc. be
s/cache/client/?
##########
extensions/guacamole-vault/modules/guacamole-vault-base/src/main/java/org/apache/guacamole/vault/user/VaultUserContext.java:
##########
@@ -398,9 +421,19 @@ 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() {
+ // Add any custom attributes to any previously defined attributes
+ return Stream.concat(
+ super.getConnectionGroupAttributes().stream(),
+ attributeService.getConnectionGroupAttributes().stream()
+ ).collect(Collectors.toUnmodifiableList());
Review Comment:
`Collectors.toUnmodifiableList()` is only available in JDK 10+.
##########
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:
Shouldn't this be client map and client entry?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
