diff --git a/docs/changelog/99702.yaml b/docs/changelog/99702.yaml new file mode 100644 index 0000000000000..657ff34e045a8 --- /dev/null +++ b/docs/changelog/99702.yaml @@ -0,0 +1,6 @@ +pr: 99702 +summary: Making classname optional in Transport protocol +area: Infra/Plugins +type: bug +issues: + - 98584 diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index bd468e22a571a..373dab307f378 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -134,7 +134,8 @@ static TransportVersion def(int id) { public static final TransportVersion NODE_INFO_REQUEST_SIMPLIFIED = def(8_510_00_0); public static final TransportVersion NESTED_KNN_VECTOR_QUERY_V = def(8_511_00_0); public static final TransportVersion ML_PACKAGE_LOADER_PLATFORM_ADDED = def(8_512_00_0); - public static final TransportVersion UNIVERSAL_PROFILING_LICENSE_ADDED = def(8_513_00_0); + public static final TransportVersion PLUGIN_DESCRIPTOR_OPTIONAL_CLASSNAME = def(8_513_00_0); + public static final TransportVersion UNIVERSAL_PROFILING_LICENSE_ADDED = def(8_514_00_0); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java b/server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java index 46028ad36b66c..6d6089bf592f6 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java @@ -78,6 +78,7 @@ public class PluginDescriptor implements Writeable, ToXContentObject { * @param hasNativeController whether or not the plugin has a native controller * @param isLicensed whether is this a licensed plugin * @param isModular whether this plugin should be loaded in a module layer + * @param isStable whether this plugin is implemented using the stable plugin API */ public PluginDescriptor( String name, @@ -105,6 +106,8 @@ public PluginDescriptor( this.isLicensed = isLicensed; this.isModular = isModular; this.isStable = isStable; + + ensureCorrectArgumentsForPluginType(); } /** @@ -119,7 +122,11 @@ public PluginDescriptor(final StreamInput in) throws IOException { this.version = in.readString(); elasticsearchVersion = Version.readVersion(in); javaVersion = in.readString(); - this.classname = in.readString(); + if (in.getTransportVersion().onOrAfter(TransportVersions.PLUGIN_DESCRIPTOR_OPTIONAL_CLASSNAME)) { + this.classname = in.readOptionalString(); + } else { + this.classname = in.readString(); + } if (in.getTransportVersion().onOrAfter(MODULE_NAME_SUPPORT)) { this.moduleName = in.readOptionalString(); } else { @@ -145,6 +152,8 @@ public PluginDescriptor(final StreamInput in) throws IOException { isModular = moduleName != null; isStable = false; } + + ensureCorrectArgumentsForPluginType(); } @Override @@ -154,7 +163,11 @@ public void writeTo(final StreamOutput out) throws IOException { out.writeString(version); Version.writeVersion(elasticsearchVersion, out); out.writeString(javaVersion); - out.writeString(classname); + if (out.getTransportVersion().onOrAfter(TransportVersions.PLUGIN_DESCRIPTOR_OPTIONAL_CLASSNAME)) { + out.writeOptionalString(classname); + } else { + out.writeString(classname); + } if (out.getTransportVersion().onOrAfter(MODULE_NAME_SUPPORT)) { out.writeOptionalString(moduleName); } @@ -174,6 +187,18 @@ public void writeTo(final StreamOutput out) throws IOException { } } + private void ensureCorrectArgumentsForPluginType() { + if (classname == null && isStable == false) { + throw new IllegalArgumentException("Classname must be provided for classic plugins"); + } + if (classname != null && isStable) { + throw new IllegalArgumentException("Classname is not needed for stable plugins"); + } + if (moduleName != null && isStable) { + throw new IllegalArgumentException("ModuleName is not needed for stable plugins"); + } + } + /** * Reads the descriptor file for a plugin. * @@ -329,6 +354,9 @@ public String getDescription() { * @return the entry point to the plugin */ public String getClassname() { + if (isStable) { + throw new IllegalStateException("Stable plugins do not have an explicit entry point"); + } return classname; } diff --git a/server/src/test/java/org/elasticsearch/nodesinfo/NodeInfoStreamingTests.java b/server/src/test/java/org/elasticsearch/nodesinfo/NodeInfoStreamingTests.java index c92c9c5305777..a0accacc65eee 100644 --- a/server/src/test/java/org/elasticsearch/nodesinfo/NodeInfoStreamingTests.java +++ b/server/src/test/java/org/elasticsearch/nodesinfo/NodeInfoStreamingTests.java @@ -156,6 +156,8 @@ private static NodeInfo createNodeInfo() { int numPlugins = randomIntBetween(0, 5); List plugins = new ArrayList<>(); for (int i = 0; i < numPlugins; i++) { + var isStable = randomBoolean(); + var hasModuleName = randomBoolean(); plugins.add( new PluginDescriptor( randomAlphaOfLengthBetween(3, 10), @@ -163,19 +165,21 @@ private static NodeInfo createNodeInfo() { randomAlphaOfLengthBetween(3, 10), VersionUtils.randomVersion(random()), "1.8", - randomAlphaOfLengthBetween(3, 10), - randomBoolean() ? null : randomAlphaOfLengthBetween(3, 10), + isStable ? null : randomAlphaOfLengthBetween(3, 10), + isStable || hasModuleName == false ? null : randomAlphaOfLengthBetween(3, 10), Collections.emptyList(), randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + isStable ) ); } int numModules = randomIntBetween(0, 5); List modules = new ArrayList<>(); for (int i = 0; i < numModules; i++) { + var isStable = randomBoolean(); + var hasModuleName = randomBoolean(); modules.add( new PluginDescriptor( randomAlphaOfLengthBetween(3, 10), @@ -183,13 +187,13 @@ private static NodeInfo createNodeInfo() { randomAlphaOfLengthBetween(3, 10), VersionUtils.randomVersion(random()), "1.8", - randomAlphaOfLengthBetween(3, 10), - randomBoolean() ? null : randomAlphaOfLengthBetween(3, 10), + isStable ? null : randomAlphaOfLengthBetween(3, 10), + isStable || hasModuleName == false ? null : randomAlphaOfLengthBetween(3, 10), Collections.emptyList(), randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + isStable ) ); } diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginDescriptorTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginDescriptorTests.java index 10d5b3b9355d6..5e0e82cb18e20 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginDescriptorTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginDescriptorTests.java @@ -235,7 +235,7 @@ public void testSerialize() throws Exception { randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + false ); BytesStreamOutput output = new BytesStreamOutput(); info.writeTo(output); @@ -258,7 +258,7 @@ public void testSerializeWithModuleName() throws Exception { randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + false ); BytesStreamOutput output = new BytesStreamOutput(); info.writeTo(output); @@ -268,6 +268,16 @@ public void testSerializeWithModuleName() throws Exception { assertThat(info2.toString(), equalTo(info.toString())); } + public void testSerializeStablePluginDescriptor() throws Exception { + PluginDescriptor info = mockStableDescriptor(); + BytesStreamOutput output = new BytesStreamOutput(); + info.writeTo(output); + ByteBuffer buffer = ByteBuffer.wrap(output.bytes().toBytesRef().bytes); + ByteBufferStreamInput input = new ByteBufferStreamInput(buffer); + PluginDescriptor info2 = new PluginDescriptor(input); + assertThat(info2.toString(), equalTo(info.toString())); + } + PluginDescriptor newMockDescriptor(String name) { return new PluginDescriptor( name, @@ -281,7 +291,7 @@ PluginDescriptor newMockDescriptor(String name) { randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + false ); } @@ -311,19 +321,21 @@ public void testUnknownProperties() throws Exception { * use the hashcode to catch duplicate names */ public void testPluginEqualityAndHash() { + var isStable = randomBoolean(); + var classname = isStable ? null : "dummyclass"; PluginDescriptor descriptor1 = new PluginDescriptor( "c", "foo", "dummy", Version.CURRENT, "1.8", - "dummyclass", + classname, null, Collections.singletonList("foo"), randomBoolean(), randomBoolean(), randomBoolean(), - randomBoolean() + isStable ); // everything but name is different from descriptor1 PluginDescriptor descriptor2 = new PluginDescriptor( @@ -332,8 +344,8 @@ public void testPluginEqualityAndHash() { randomValueOtherThan(descriptor1.getVersion(), () -> randomAlphaOfLengthBetween(4, 12)), descriptor1.getElasticsearchVersion().previousMajor(), randomValueOtherThan(descriptor1.getJavaVersion(), () -> randomAlphaOfLengthBetween(4, 12)), - randomValueOtherThan(descriptor1.getClassname(), () -> randomAlphaOfLengthBetween(4, 12)), - randomAlphaOfLength(6), + descriptor1.isStable() ? randomAlphaOfLengthBetween(4, 12) : null, + descriptor1.isStable() ? randomAlphaOfLength(6) : null, Collections.singletonList( randomValueOtherThanMany(v -> descriptor1.getExtendedPlugins().contains(v), () -> randomAlphaOfLengthBetween(4, 12)) ), @@ -349,7 +361,7 @@ public void testPluginEqualityAndHash() { descriptor1.getVersion(), descriptor1.getElasticsearchVersion(), descriptor1.getJavaVersion(), - descriptor1.getClassname(), + classname, descriptor1.getModuleName().orElse(null), descriptor1.getExtendedPlugins(), descriptor1.hasNativeController(), diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 72aba521f1b79..eddc029dded6d 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -460,10 +460,11 @@ public void testPluginFromParentClassLoader() throws IOException { public void testExtensiblePlugin() { TestExtensiblePlugin extensiblePlugin = new TestExtensiblePlugin(); + var classname = "FakePlugin"; PluginsService.loadExtensions( List.of( new PluginsService.LoadedPlugin( - new PluginDescriptor("extensible", null, null, null, null, null, null, List.of(), false, false, false, false), + new PluginDescriptor("extensible", null, null, null, null, classname, null, List.of(), false, false, false, false), extensiblePlugin ) ) @@ -477,11 +478,24 @@ public void testExtensiblePlugin() { PluginsService.loadExtensions( List.of( new PluginsService.LoadedPlugin( - new PluginDescriptor("extensible", null, null, null, null, null, null, List.of(), false, false, false, false), + new PluginDescriptor("extensible", null, null, null, null, classname, null, List.of(), false, false, false, false), extensiblePlugin ), new PluginsService.LoadedPlugin( - new PluginDescriptor("test", null, null, null, null, null, null, List.of("extensible"), false, false, false, false), + new PluginDescriptor( + "test", + null, + null, + null, + null, + classname, + null, + List.of("extensible"), + false, + false, + false, + false + ), testPlugin ) ) diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java index 755f8dcf482b8..05ba122c2aaf0 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsUtilsTests.java @@ -436,7 +436,7 @@ private static PluginDescriptor getPluginDescriptorForVersion(Version id, String "1.0", id, javaVersion, - "FakePlugin", + isStable ? null : "FakePlugin", null, Collections.emptyList(), false, diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java index 1776b3bfd3c36..77ae4ab838585 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java @@ -119,6 +119,7 @@ private void doAssertXPackIsInstalled() { Collection pluginNames = nodeInfo.getInfo(PluginsAndModules.class) .getPluginInfos() .stream() + .filter(p -> p.descriptor().isStable() == false) .map(p -> p.descriptor().getClassname()) .collect(Collectors.toList()); assertThat( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java index 6057667fb575e..4e6ea34d3dc9e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java @@ -187,6 +187,7 @@ protected void doAssertXPackIsInstalled() { Collection pluginNames = nodeInfo.getInfo(PluginsAndModules.class) .getPluginInfos() .stream() + .filter(p -> p.descriptor().isStable() == false) .map(p -> p.descriptor().getClassname()) .collect(Collectors.toList()); assertThat(