Security policies file + cache key migrated from string to Settings o…

…bject * * security policies file refactored, * cache key migrated from string to settings, * cache cleanup in case of any exceptions Signed-off-by: pusranja <[email protected]> * Removing uninteded changes. Signed-off-by: Dmitri Goroh <[email protected]> --------- Signed-off-by: pusranja <[email protected]> Signed-off-by: Dmitri Goroh <[email protected]> Co-authored-by: Dmitri Goroh <[email protected]>
opensearch-project · Jan 30, 2025 · b62b260 · b62b260
1 parent fccb710
commit b62b260
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 19 deletions.
diff --git a/build.gradle b/build.gradle
@@ -41,7 +41,7 @@ subprojects {
 
     ext {
         opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT")
-        sdk_version = "3.24.0"
+        sdk_version = "3.30.0"
         jackson_version = "2.18.2"
     }
 

diff --git a/...itory-plugin/src/main/java/org/opensearch/repositories/oci/OciObjectStorageBlobStore.java b/...itory-plugin/src/main/java/org/opensearch/repositories/oci/OciObjectStorageBlobStore.java
@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -43,6 +44,7 @@
 import org.opensearch.common.blobstore.DeleteResult;
 import org.opensearch.common.blobstore.support.PlainBlobMetadata;
 import org.opensearch.common.collect.MapBuilder;
+import org.opensearch.common.settings.Settings;
 import org.opensearch.core.common.unit.ByteSizeUnit;
 import org.opensearch.core.common.unit.ByteSizeValue;
 import org.opensearch.repositories.oci.sdk.com.oracle.bmc.model.BmcException;
@@ -105,15 +107,17 @@ class OciObjectStorageBlobStore implements BlobStore {
 
     private final String bucketName;
     private final String namespace;
-    private final String clientName;
+    private final Settings cacheKey;
     private final OciObjectStorageService storageService;
     private final OciObjectStorageClientSettings clientSettings;
+    final Map<Settings, OciObjectStorageClientSettings> clientSettingsMap =
+            new ConcurrentHashMap<>();
 
     OciObjectStorageBlobStore(
             final OciObjectStorageService storageService, final RepositoryMetadata metadata) {
         this.storageService = storageService;
         this.bucketName = getSetting(BUCKET_SETTING, metadata);
-        this.clientName = CLIENT_NAME_SETTINGS.get(metadata.settings());
+        this.cacheKey = metadata.settings();
         this.namespace = getSetting(NAMESPACE_SETTING, metadata);
         this.clientSettings = new OciObjectStorageClientSettings(metadata);
         final String bucketCompartmentId =
@@ -134,7 +138,11 @@ class OciObjectStorageBlobStore implements BlobStore {
     }
 
     private ObjectStorageAsync client() throws IOException {
-        return storageService.client(clientName, clientSettings);
+        if (storageService.client(cacheKey) == null) {
+            clientSettingsMap.put(cacheKey, clientSettings);
+            storageService.refreshWithoutClearingCache(clientSettingsMap);
+        }
+        return storageService.client(cacheKey);
     }
 
     @Override
@@ -189,6 +197,8 @@ public void onError(
 
             return getBucketResponse.getBucket() != null;
         } catch (final Exception e) {
+            log.info("Evicting client for cacheKey [{}] due to exception", cacheKey);
+            storageService.evictCache(cacheKey);
             if (e.getCause() instanceof BmcException) {
                 final BmcException bmcEx = (BmcException) e.getCause();
                 if (bmcEx.getStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {

diff --git a/...ository-plugin/src/main/java/org/opensearch/repositories/oci/OciObjectStorageService.java b/...ository-plugin/src/main/java/org/opensearch/repositories/oci/OciObjectStorageService.java
@@ -11,12 +11,17 @@
 
 package org.opensearch.repositories.oci;
 
+import static java.util.Collections.emptyMap;
+
 import java.io.Closeable;
 import java.io.IOException;
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
 import lombok.extern.log4j.Log4j2;
 import org.apache.logging.log4j.message.ParameterizedMessage;
+import org.opensearch.common.collect.MapBuilder;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.common.util.LazyInitializable;
 import org.opensearch.repositories.oci.sdk.com.oracle.bmc.ClientConfiguration;
 import org.opensearch.repositories.oci.sdk.com.oracle.bmc.objectstorage.ObjectStorageAsync;
 import org.opensearch.repositories.oci.sdk.com.oracle.bmc.objectstorage.ObjectStorageAsyncClient;
@@ -27,7 +32,8 @@ public class OciObjectStorageService implements Closeable {
     /**
      * Dictionary of client instances. Client instances are built lazily from the latest settings.
      */
-    private final Map<String, ObjectStorageAsync> clientsCache = new HashMap<>();
+    private final AtomicReference<Map<Settings, LazyInitializable<ObjectStorageAsync, IOException>>>
+            clientsCache = new AtomicReference<>(emptyMap());
 
     /**
      * Attempts to retrieve a client from the cache. If the client does not exist it will be created
@@ -38,34 +44,34 @@ public class OciObjectStorageService implements Closeable {
      * @param clientName name of the client settings used to create the client
      * @return a cached client storage instance that can be used to manage objects (blobs)
      */
-    public synchronized ObjectStorageAsync client(
-            final String clientName, OciObjectStorageClientSettings clientSettings)
-            throws IOException {
+    public synchronized ObjectStorageAsync client(final Settings clientName) throws IOException {
 
-        ObjectStorageAsync client = clientsCache.get(clientName);
-        if (client == null) {
-            client = createClientAsync(clientName, clientSettings);
-            clientsCache.put(clientName, client);
+        final LazyInitializable<ObjectStorageAsync, IOException> lazyClient =
+                clientsCache.get().get(clientName);
+        if (lazyClient == null) {
+            log.warn("No client found for client name");
+            return null;
         }
-        return client;
+        return lazyClient.getOrCompute();
     }
 
     /**
      * Creates a client that can be used to manage OCI Object Storage objects. The client is
      * thread-safe.
      *
-     * @param clientName name of client settings to use, including secure settings
+     * @param clientCacheKey name of client settings to use, including secure settings
      * @param clientSettings name of client settings to use, including secure settings
      * @return a new client storage instance that can be used to manage objects (blobs)
      */
     static ObjectStorageAsync createClientAsync(
-            String clientName, OciObjectStorageClientSettings clientSettings) throws IOException {
+            Settings clientCacheKey, OciObjectStorageClientSettings clientSettings)
+            throws IOException {
         log.debug(
                 () ->
                         new ParameterizedMessage(
                                 "creating OCI object store client with client_name [{}], endpoint"
                                         + " [{}]",
-                                clientName,
+                                clientCacheKey,
                                 clientSettings.getEndpoint()));
 
         final ObjectStorageAsync objectStorageClient =
@@ -80,14 +86,67 @@ static ObjectStorageAsync createClientAsync(
         return objectStorageClient;
     }
 
+    /**
+     * Refreshes the client settings of existing and new clients. Will not clear the cache of other
+     * clients. Subsequent calls to {@code OciObjectStorageService#client} will return new clients
+     * constructed using the parameter settings.
+     *
+     * @param clientsSettings the new settings used for building clients for subsequent requests
+     */
+    public synchronized void refreshWithoutClearingCache(
+            Map<Settings, OciObjectStorageClientSettings> clientsSettings) {
+
+        // build the new lazy clients
+        final Map<Settings, LazyInitializable<ObjectStorageAsync, IOException>> oldClientCache =
+                clientsCache.get();
+        final MapBuilder<Settings, LazyInitializable<ObjectStorageAsync, IOException>>
+                newClientsCache = MapBuilder.newMapBuilder();
+
+        // replace or add new clients
+        newClientsCache.putAll(oldClientCache);
+        for (final Map.Entry<Settings, OciObjectStorageClientSettings> entry :
+                clientsSettings.entrySet()) {
+            final LazyInitializable<ObjectStorageAsync, IOException> previousClient =
+                    oldClientCache.get(entry.getKey());
+            newClientsCache.put(
+                    entry.getKey(),
+                    new LazyInitializable<>(
+                            () -> createClientAsync(entry.getKey(), entry.getValue())));
+            // we will release the previous client for this entry if existed
+            if (previousClient != null) {
+                previousClient.reset();
+            }
+        }
+        clientsCache.getAndSet(newClientsCache.immutableMap());
+    }
+
+    /**
+     * @param cacheKey
+     */
+    public synchronized void evictCache(Settings cacheKey) {
+
+        final Map<Settings, LazyInitializable<ObjectStorageAsync, IOException>> oldClientCache =
+                clientsCache.get();
+        final MapBuilder<Settings, LazyInitializable<ObjectStorageAsync, IOException>>
+                newClientsCache = MapBuilder.newMapBuilder();
+
+        for (Map.Entry<Settings, LazyInitializable<ObjectStorageAsync, IOException>> entry :
+                oldClientCache.entrySet()) {
+            if (!entry.getKey().equals(cacheKey)) {
+                newClientsCache.put(entry.getKey(), entry.getValue());
+            }
+        }
+        clientsCache.getAndSet(newClientsCache.immutableMap());
+    }
+
     @Override
     public void close() throws IOException {
         log.info("Shutting down all clients");
-        clientsCache.values().stream()
+        clientsCache.get().values().stream()
                 .forEach(
                         lazyClient -> {
                             try {
-                                lazyClient.close();
+                                lazyClient.getOrCompute().close();
                             } catch (Exception e) {
                                 log.error("unable to close client");
                             }

diff --git a/...src/main/resources/plugin-security.policy → ...in/plugin-metadata/plugin-security.policy b/...src/main/resources/plugin-security.policy → ...in/plugin-metadata/plugin-security.policy
@@ -3,6 +3,8 @@ grant {
   // TODO: get these fixed in oci sdk
   // Required by Jersey
   permission java.lang.RuntimePermission "getClassLoader";
+  permission java.lang.RuntimePermission "setSecurityManager";
+  permission java.lang.RuntimePermission "createSecurityManager";
   permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
   permission java.lang.RuntimePermission "accessDeclaredMembers";
   permission java.util.PropertyPermission "*", "read,write";