Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
n1v0lg committed Dec 8, 2023
1 parent c865955 commit cd95799
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ public RemoteClusterCredentialsManager(Settings settings) {

public void updateClusterCredentials(Settings settings) {
clusterCredentials = REMOTE_CLUSTER_CREDENTIALS.getAsMap(settings);
logger.debug(() -> Strings.format("Updated remote cluster credentials: %s", clusterCredentials.keySet()));
logger.debug(
() -> Strings.format(
"Updated remote cluster credentials for clusters: [%s]",
Strings.collectionToCommaDelimitedString(clusterCredentials.keySet())
)
);
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,10 @@ private void reloadSharedSecretsForJwtRealms(Settings settingsWithKeystore) {
});
}

/**
* This method uses a transport action internally to access classes that are injectable but are not part of the plugin contract.
* See {@link TransportReloadRemoteClusterCredentialsAction} for more context.
*/
private void reloadRemoteClusterCredentials(Settings settingsWithKeystore) {
// Accepting a blocking call here since the underlying action is local-only and only performs fast in-memory ops
// (extracts a subset of passed in `settingsWithKeystore` and stores them in a map)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,23 @@
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.transport.RemoteClusterService;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.security.action.settings.ReloadRemoteClusterCredentialsAction;
import org.elasticsearch.xpack.security.Security;

/**
* This is a local-only action which updates remote cluster credentials for remote cluster connections, from keystore settings reloaded via
* a call to {@link org.elasticsearch.rest.action.admin.cluster.RestReloadSecureSettingsAction}.
*
* It's invoked as part of the `reload` call in the Security plugin {@link Security#reload(Settings)}.
*
* This action is largely an implementation detail to work around the fact that Security is a plugin without direct access to many core
* classes, including the {@link RemoteClusterService} which is required for credentials update. A transport action gives us access to
* the {@link RemoteClusterService} which is injectable but not part of the plugin contract.
*/
public class TransportReloadRemoteClusterCredentialsAction extends TransportAction<
ReloadRemoteClusterCredentialsAction.Request,
ActionResponse.Empty> {
Expand All @@ -35,7 +47,7 @@ protected void doExecute(
ReloadRemoteClusterCredentialsAction.Request request,
ActionListener<ActionResponse.Empty> listener
) {
// We could stash and mark context as system, but avoiding this to keep action as minimal as possible (i.e., avoid copying context)
// We avoid stashing and marking context as system to keep the action as minimal as possible (i.e., avoid copying context)
remoteClusterService.updateRemoteClusterCredentials(request.getSettings());
listener.onResponse(ActionResponse.Empty.INSTANCE);
}
Expand Down

0 comments on commit cd95799

Please sign in to comment.