Deterministic response order of _security/stats (#135540)

Small quality-of-life readability improvement for humans.

Author: szybia

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCache.java

@@ -38,7 +38,6 @@
 
 import java.io.Closeable;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -316,7 +315,7 @@ public Map<String, Object> usageStats() {
         stats.put("evictions", cacheStats.getEvictions());
         stats.put("hits_time_in_millis", TimeValue.nsecToMSec(hitsTimeInNanos.sum()));
         stats.put("misses_time_in_millis", TimeValue.nsecToMSec(missesTimeInNanos.sum()));
-        return Collections.unmodifiableMap(stats);
+        return stats;
     }
 
     private static final class BitsetCacheKey {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/stats/GetSecurityStatsNodeResponseTests.java

@@ -9,12 +9,15 @@
 
 import org.elasticsearch.cluster.node.DiscoveryNodeUtils;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
 
 import java.io.IOException;
 import java.util.Map;
 import java.util.Objects;
 
+import static org.hamcrest.Matchers.equalTo;
+
 public class GetSecurityStatsNodeResponseTests extends AbstractWireSerializingTestCase<GetSecurityStatsNodeResponse> {
 
     @Override
@@ -46,4 +49,20 @@ protected GetSecurityStatsNodeResponse mutateInstance(GetSecurityStatsNodeRespon
             default -> throw new IllegalStateException("Unexpected value");
         };
     }
+
+    public void testRolesStatsInOrderSerialization() throws IOException {
+        final Map<String, Object> rolesStats = Maps.newLinkedHashMapWithExpectedSize(3);
+        rolesStats.put("one", "value");
+        rolesStats.put("two", "value");
+        rolesStats.put("three", "value");
+        final GetSecurityStatsNodeResponse in = new GetSecurityStatsNodeResponse(
+            DiscoveryNodeUtils.create(randomAlphaOfLength(10)),
+            rolesStats
+        );
+
+        final GetSecurityStatsNodeResponse out = copyInstance(in);
+        assertThat(in, equalTo(out));
+
+        assertThat(out.getRolesStoreStats().keySet().toArray(new String[0]), equalTo(new String[] { "one", "two", "three" }));
+    }
 }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/DocumentSubsetBitsetCacheTests.java

@@ -65,6 +65,7 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.not;
@@ -529,6 +530,11 @@ public void testHitsMissesAndEvictionsStats() throws Exception {
         assertThat(cache.usageStats(), equalTo(finalStats));
     }
 
+    public void testUsageStatsAreOrdered() {
+        final Map<String, Object> stats = newCache(Settings.EMPTY).usageStats();
+        assertThat("needs to be LinkedHashMap for order in transport", stats, instanceOf(LinkedHashMap.class));
+    }
+
     private void runTestOnIndex(CheckedBiConsumer<SearchExecutionContext, LeafReaderContext, Exception> body) throws Exception {
         runTestOnIndices(1, ctx -> {
             final TestIndexContext indexContext = ctx.get(0);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

@@ -22,6 +22,7 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.util.concurrent.ReleasableLock;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.set.Sets;
@@ -670,7 +671,9 @@ Iterable<ProjectScoped<RoleKey>> cachedRoles() {
     }
 
     public Map<String, Object> usageStatsWithJustDls() {
-        return Map.of("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
+        final Map<String, Object> usageStats = Maps.newLinkedHashMapWithExpectedSize(1);
+        usageStats.put("dls", Map.of("bit_set_cache", dlsBitsetCache.usageStats()));
+        return usageStats; // return LinkedHashMap for deterministic order in transport
     }
 
     public void usageStats(ActionListener<Map<String, Object>> listener) {

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java

@@ -141,6 +141,7 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -3754,6 +3755,27 @@ public void testGetRoleDescriptorsListUsesRoleStoreToResolveRoleWithInternalRole
         assertThat(future.actionGet(), equalTo(Set.of(expectedRoleDescriptor)));
     }
 
+    public void testOrderedUsageStatsWithJustDls() {
+        final CompositeRolesStore compositeRolesStore = buildCompositeRolesStore(
+            SECURITY_ENABLED_SETTINGS,
+            null,
+            null,
+            null,
+            null,
+            null,
+            null,
+            null,
+            null,
+            null
+        );
+
+        assertThat(
+            "needs LinkedHashMap to be ordered in transport",
+            compositeRolesStore.usageStatsWithJustDls(),
+            instanceOf(LinkedHashMap.class)
+        );
+    }
+
     private Role getRoleForRoleNames(CompositeRolesStore store, String... roleNames) {
         final PlainActionFuture<Role> future = new PlainActionFuture<>();
         getRoleForRoleNames(store, Set.of(roleNames), future);