[Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern.

[DarshitChanpura]

Description

After trying to implement @Inject pattern to inject the extension and use the Guice injection to retrieve client from extension in plugin's transport actions, we discovered that cluster bootstrap would always fails with NoClasDefFoundError whenever SPI is not present in the classpath (i.e. security is not present).

We experimented with this annotation as discussed in comments of #5541, but were not successful.
Hence, we are reverting to client accessor singleton pattern.

Category: Bug fix

Issues Resolved

Unblocks plugin CIs and allows clusters without security plugin to bootstrap.

Testing

Automated tests

Check List

• New functionality includes testing
• New functionality has been documented
• New Roles/Permissions have a corresponding security dashboards plugin PR
• API changes companion pull request created
• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[DarshitChanpura]

This changes reverts back to the changes made here: #5541

The only other alternative is to have plugins implement their own version of:

ResourceExtensionWrapper.java

/*
 * SPDX-License-Identifier: Apache-2.0
 *
 * The OpenSearch Contributors require contributions made to
 * this file be licensed under the Apache-2.0 license or a
 * compatible open source license.
 *
 * Modifications Copyright OpenSearch Contributors. See
 * GitHub history for details.
 */

package org.opensearch.sample;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.inject.Injector;

import java.lang.reflect.Method;

public final class ResourceExtensionWrapper {
    private static final Logger log = LogManager.getLogger(ResourceExtensionWrapper.class);

    // Class names kept as strings to avoid compile-time coupling
    private static final String SPI_IFACE =
            "org.opensearch.security.spi.resources.ResourceSharingExtension";
    private static final String IMPL_CLASS =
            "org.opensearch.sample.SampleResourceExtension";

    @Inject
    public Injector injector;

    // Init state
    private volatile boolean initialized;          // set to true when we have a definitive state
    private volatile boolean spiAbsentForever;     // true iff SPI not on classpath
    private volatile Object extension;             // the extension instance or null
    private volatile Method getClientMethod;       // cached "getResourceSharingClient" Method

    /** Visible for tests: allow explicit instance injection. */
    public void setExtension(Object extension) {
        this.extension = extension;
        // cache method if possible
        if (extension != null) {
            try {
                this.getClientMethod = extension.getClass().getMethod("getResourceSharingClient");
            } catch (ReflectiveOperationException e) {
                log.debug("Timeseries RS extension present but no getResourceSharingClient()", e);
                this.getClientMethod = null;
            }
        } else {
            this.getClientMethod = null;
        }
        this.initialized = true;
    }

    /** Returns true if SPI is on the classpath AND an extension instance is available. */
    public boolean isPresent() {
        ensureInitialized();
        return extension != null && getClientMethod != null;
    }

    /**
     * Returns the ResourceSharingClient (as Object) or null if SPI/extension is not available.
     * Intentionally returns Object to avoid compile-time coupling to SPI types.
     */
    public Object getResourceSharingClient() {
        ensureInitialized();
        Object ext = extension;
        Method m = getClientMethod;
        if (ext == null || m == null) return null;
        try {
            return m.invoke(ext);
        } catch (ReflectiveOperationException e) {
            // treat as absent at runtime; don't flip initialized so future retries could still happen if state changes
            log.debug("Invoking getResourceSharingClient() failed; treating as absent.", e);
            return null;
        }
    }

    private void ensureInitialized() {
        if (initialized) return;

        synchronized (this) {
            if (initialized) return;

            // 0) If we previously discovered SPI is missing, we can finalize immediately.
            if (spiAbsentForever) {
                initialized = true;
                return;
            }

            // 1) Check SPI presence using TCCL first, then our loader.
            ClassLoader tccl = Thread.currentThread().getContextClassLoader();
            ClassLoader self = getClass().getClassLoader();
            if (classAbsent(tccl) && classAbsent(self)) {
                // SPI truly not installed — no need to ever retry
                spiAbsentForever = true;
                extension = null;
                getClientMethod = null;
                initialized = true;
                log.debug("Security SPI not present on classpath; RS extension unavailable.");
                return;
            }

            // 2) SPI present; try to obtain our implementation instance from Guice.
            //    If this fails due to binding timing, leave 'initialized'=false so we retry on next call.
            if (injector == null) {
                // This shouldn't happen with ctor injection, but be defensive if refactored.
                log.debug("Injector not available yet; deferring RS extension initialization.");
                return; // DO NOT set initialized — we want to retry later
            }

            try {
                Class<?> implClazz = Class.forName(IMPL_CLASS, /* initialize */ false, self);
                Object instance = injector.getInstance((Class<?>) implClazz);
                Method m = implClazz.getMethod("getResourceSharingClient");

                this.extension = instance;
                this.getClientMethod = m;
                this.initialized = true; // success — we can finalize
                log.debug("Initialized Timeseries RS extension via Guice: {}", IMPL_CLASS);
            } catch (ClassNotFoundException e) {
                // Our impl isn't on the classpath (shouldn't happen since it's our own class) — finalize as absent
                log.debug("RS extension impl class not found; treating as unavailable.", e);
                this.extension = null;
                this.getClientMethod = null;
                this.initialized = true;
            } catch (Throwable t) {
                // Provisioning/binding not ready yet or other runtime issue — DO NOT finalize; allow retrial.
                log.debug("RS extension not ready yet (will retry on next call).", t);
                // leave initialized=false to allow a later retry
            }
        }
    }

    private static boolean classAbsent(ClassLoader cl) {
        try {
            if (cl == null) return true;
            Class.forName(ResourceExtensionWrapper.SPI_IFACE, /* initialize */ false, cl);
            return false;
        } catch (ClassNotFoundException e) {
            return true;
        } catch (LinkageError e) {
            // present but broken — treat as absent to be safe
            return true;
        }
    }
}

and then inject it in their createComponents like:

 try {
            // This loader will throw class not found exception if SPI doesn't exist (feature disabled or security not installed)
            Class.forName("org.opensearch.security.spi.resources.ResourceSharingExtension");
            // if no exception was thrown we make the wrapper an injectable component
            components.add(new ResourceExtensionWrapper());
        } catch (ClassNotFoundException e) {
            // spi is not loaded into classpath, i.e. the feature is disabled or security plugin is absent, do nothing
        }

I'm not in favor of that approach.

[cwperks]

Thank you for trying @DarshitChanpura . Unfortunately, it seems like our fork of Guice from 10 years ago doesn't have the optional bindings necessary for such a use-case.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.07%. Comparing base (82b9d9e) to head (420874e).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5576   +/-   ##
=======================================
  Coverage   73.06%   73.07%           
=======================================
  Files         407      408    +1     
  Lines       25255    25259    +4     
  Branches     3842     3842           
=======================================
+ Hits        18452    18457    +5     
- Misses       4931     4934    +3     
+ Partials     1872     1868    -4     

Files with missing lines	Coverage Δ
...org/opensearch/sample/SampleResourceExtension.java	100.00% <100.00%> (ø)
...va/org/opensearch/sample/SampleResourcePlugin.java	96.29% <ø> (ø)
...h/sample/client/ResourceSharingClientAccessor.java	100.00% <100.00%> (ø)
.../actions/transport/GetResourceTransportAction.java	88.67% <100.00%> (+1.88%)	⬆️
...transport/RevokeResourceAccessTransportAction.java	100.00% <100.00%> (+6.66%)	⬆️
...tions/transport/SearchResourceTransportAction.java	63.63% <100.00%> (+2.27%)	⬆️
...ctions/transport/ShareResourceTransportAction.java	78.94% <100.00%> (+5.26%)	⬆️
.../opensearch/security/OpenSearchSecurityPlugin.java	85.11% <ø> (-0.02%)	⬇️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.