Use PR base branch when checking transport version modifications

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionValidationFuncTest.groovy

@@ -311,4 +311,43 @@ class TransportVersionValidationFuncTest extends AbstractTransportVersionFuncTes
         then:
         result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
     }
+
+    def "modification checks use PR base branch in CI"() {
+        given:
+        // create 9.1 branch
+        execute("git checkout -b 9.1")
+        // setup 9.1 upper bound as if 9.2 was just branched, so no patches yet
+        unreferableTransportVersion("initial_9.1.0", "8013000")
+        transportVersionUpperBound("9.1", "initial_9.1.0", "8013000")
+        execute("git add .")
+        execute("git commit -m initial")
+
+        // advance main branch
+        execute("git checkout main")
+        unreferableTransportVersion("initial_9.1.0", "8013000")
+        referableAndReferencedTransportVersion("new_tv", "8124000,8013001")
+        transportVersionUpperBound("9.2", "new_tv", "8124000")
+        transportVersionUpperBound("9.1", "new_tv", "8013001")
+        execute("git add .")
+        execute("git commit -m update")
+
+        // create a 9.1 backport
+        execute("git checkout 9.1")
+        execute("git checkout -b test_9.1")
+        referableAndReferencedTransportVersion("new_tv", "8124000,8013001")
+        transportVersionUpperBound("9.2", "new_tv", "8124000")
+        transportVersionUpperBound("9.1", "new_tv", "8013001")
+        file("myserver/build.gradle") << """
+            tasks.named('validateTransportVersionResources') {
+                currentUpperBoundName = '9.1'
+            }
+        """
+
+        when:
+        def result = gradleRunner("validateTransportVersionResources")
+            .withEnvironment(Map.of("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "9.1")).build()
+
+        then:
+        result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
+    }
 }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesPlugin.java

@@ -14,18 +14,23 @@
 import org.elasticsearch.gradle.internal.conventions.VersionPropertiesPlugin;
 import org.elasticsearch.gradle.internal.conventions.precommit.PrecommitPlugin;
 import org.elasticsearch.gradle.internal.conventions.precommit.PrecommitTaskPlugin;
+import org.elasticsearch.gradle.internal.info.BuildParameterExtension;
+import org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin;
 import org.gradle.api.Action;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
 import org.gradle.api.file.Directory;
 import org.gradle.api.plugins.JavaPlugin;
+import org.gradle.api.provider.Property;
 import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.Copy;
 import org.gradle.language.base.plugins.LifecycleBasePlugin;
 
 import java.util.Map;
 import java.util.Properties;
 
+import static org.elasticsearch.gradle.internal.util.ParamsUtils.loadBuildParams;
+
 public class TransportVersionResourcesPlugin implements Plugin<Project> {
 
     public static final String TRANSPORT_REFERENCES_TOPIC = "transportReferences";
@@ -37,6 +42,9 @@ public void apply(Project project) {
         project.getPluginManager().apply(PrecommitTaskPlugin.class);
         var psService = project.getPlugins().apply(ProjectSubscribeServicePlugin.class).getService();
 
+        project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
+        Property<BuildParameterExtension> buildParams = loadBuildParams(project);
+
         Properties versions = (Properties) project.getExtensions().getByName(VersionPropertiesPlugin.VERSIONS_EXT);
         Version currentVersion = Version.fromString(versions.getProperty("elasticsearch"));
 
@@ -76,6 +84,7 @@ public void apply(Project project) {
                 t.getShouldValidateDensity().convention(true);
                 t.getShouldValidatePrimaryIdNotPatch().convention(true);
                 t.getCurrentUpperBoundName().convention(currentVersion.getMajor() + "." + currentVersion.getMinor());
+                t.getCI().set(buildParams.get().getCi());
             });
         project.getTasks().named(PrecommitPlugin.PRECOMMIT_TASK_NAME).configure(t -> t.dependsOn(validateTask));
 

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java

@@ -284,6 +284,11 @@ private String findUpstreamRef() {
             logger.warn("No remotes found. Using 'main' branch as upstream ref for transport version resources");
             return "main";
         }
+        // default the branch name to look at to that which a PR in CI is targeting
+        String branchName = System.getenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH");
+        if (branchName == null || branchName.strip().isEmpty()) {
+            branchName = "main";
+        }
         List<String> remoteNames = List.of(remotesOutput.split("\n"));
         if (remoteNames.contains(UPSTREAM_REMOTE_NAME) == false) {
             // our special remote doesn't exist yet, so create it
@@ -299,14 +304,14 @@ private String findUpstreamRef() {
                 gitCommand("remote", "add", UPSTREAM_REMOTE_NAME, upstreamUrl);
             } else {
                 logger.warn("No elastic github remotes found to copy. Using 'main' branch as upstream ref for transport version resources");
-                return "main";
+                return branchName;
             }
         }
 
         // make sure the remote main ref is up to date
-        gitCommand("fetch", UPSTREAM_REMOTE_NAME, "main");
+        gitCommand("fetch", UPSTREAM_REMOTE_NAME, branchName);
 
-        return UPSTREAM_REMOTE_NAME + "/main";
+        return UPSTREAM_REMOTE_NAME + "/" + branchName;
     }
 
     // Return the transport version resources paths that exist in upstream

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/ValidateTransportVersionResourcesTask.java

@@ -66,6 +66,9 @@ public Path getResourcesDir() {
     @Input
     public abstract Property<String> getCurrentUpperBoundName();
 
+    @Input
+    public abstract Property<Boolean> getCI();
+
     private static final Pattern NAME_FORMAT = Pattern.compile("[a-z0-9_]+");
 
     @ServiceReference("transportVersionResources")
@@ -82,13 +85,14 @@ public void validateTransportVersions() throws IOException {
         Map<String, TransportVersionUpperBound> upperBounds = resources.getUpperBounds();
         TransportVersionUpperBound currentUpperBound = upperBounds.get(getCurrentUpperBoundName().get());
         boolean onReleaseBranch = checkIfDefinitelyOnReleaseBranch(upperBounds);
+        boolean validateModifications = onReleaseBranch == false || getCI().get();
 
         for (var definition : referableDefinitions.values()) {
-            validateNamedDefinition(definition, referencedNames);
+            validateNamedDefinition(definition, referencedNames, validateModifications);
         }
 
         for (var definition : unreferableDefinitions.values()) {
-            validateUnreferableDefinition(definition);
+            validateUnreferableDefinition(definition, validateModifications);
         }
 
         for (var entry : idsByBase.entrySet()) {
@@ -101,10 +105,10 @@ public void validateTransportVersions() throws IOException {
 
         if (onReleaseBranch) {
             // on release branches we only check the current upper bound, others may be inaccurate
-            validateUpperBound(currentUpperBound, allDefinitions, idsByBase);
+            validateUpperBound(currentUpperBound, allDefinitions, idsByBase, validateModifications);
         } else {
             for (var upperBound : upperBounds.values()) {
-                validateUpperBound(upperBound, allDefinitions, idsByBase);
+                validateUpperBound(upperBound, allDefinitions, idsByBase, validateModifications);
             }
 
             validatePrimaryIds(resources, upperBounds, allDefinitions);
@@ -126,7 +130,11 @@ private Map<String, TransportVersionDefinition> collectAllDefinitions(
         return allDefinitions;
     }
 
-    private void validateNamedDefinition(TransportVersionDefinition definition, Set<String> referencedNames) {
+    private void validateNamedDefinition(
+        TransportVersionDefinition definition,
+        Set<String> referencedNames,
+        boolean validateModifications
+    ) {
         if (referencedNames.contains(definition.name()) == false) {
             throwDefinitionFailure(definition, "is not referenced");
         }
@@ -152,35 +160,39 @@ private void validateNamedDefinition(TransportVersionDefinition definition, Set<
                 }
             }
         }
-        // validate any modifications
-        TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromGitBase(definition.name());
-        if (originalDefinition != null) {
-            validateIdenticalPrimaryId(definition, originalDefinition);
-            for (int i = 1; i < originalDefinition.ids().size(); ++i) {
-                TransportVersionId originalId = originalDefinition.ids().get(i);
-
-                // we have a very small number of ids in a definition, so just search linearly
-                boolean found = false;
-                for (int j = 1; j < definition.ids().size(); ++j) {
-                    TransportVersionId id = definition.ids().get(j);
-                    if (id.base() == originalId.base()) {
-                        found = true;
-                        if (id.complete() != originalId.complete()) {
-                            throwDefinitionFailure(definition, "has modified patch id from " + originalId + " to " + id);
+
+        if (validateModifications) {
+            TransportVersionDefinition originalDefinition = getResources().get().getReferableDefinitionFromGitBase(definition.name());
+            if (originalDefinition != null) {
+                validateIdenticalPrimaryId(definition, originalDefinition);
+                for (int i = 1; i < originalDefinition.ids().size(); ++i) {
+                    TransportVersionId originalId = originalDefinition.ids().get(i);
+
+                    // we have a very small number of ids in a definition, so just search linearly
+                    boolean found = false;
+                    for (int j = 1; j < definition.ids().size(); ++j) {
+                        TransportVersionId id = definition.ids().get(j);
+                        if (id.base() == originalId.base()) {
+                            found = true;
+                            if (id.complete() != originalId.complete()) {
+                                throwDefinitionFailure(definition, "has modified patch id from " + originalId + " to " + id);
+                            }
                         }
                     }
-                }
-                if (found == false) {
-                    throwDefinitionFailure(definition, "has removed id " + originalId);
+                    if (found == false) {
+                        throwDefinitionFailure(definition, "has removed id " + originalId);
+                    }
                 }
             }
         }
     }
 
-    private void validateUnreferableDefinition(TransportVersionDefinition definition) {
-        TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromGitBase(definition.name());
-        if (originalDefinition != null) {
-            validateIdenticalPrimaryId(definition, originalDefinition);
+    private void validateUnreferableDefinition(TransportVersionDefinition definition, boolean validateModifications) {
+        if (validateModifications) {
+            TransportVersionDefinition originalDefinition = getResources().get().getUnreferableDefinitionFromGitBase(definition.name());
+            if (originalDefinition != null) {
+                validateIdenticalPrimaryId(definition, originalDefinition);
+            }
         }
         if (definition.ids().isEmpty()) {
             throwDefinitionFailure(definition, "does not contain any ids");
@@ -204,7 +216,8 @@ private void validateIdenticalPrimaryId(TransportVersionDefinition definition, T
     private void validateUpperBound(
         TransportVersionUpperBound upperBound,
         Map<String, TransportVersionDefinition> definitions,
-        Map<Integer, List<IdAndDefinition>> idsByBase
+        Map<Integer, List<IdAndDefinition>> idsByBase,
+        boolean validateModifications
     ) {
         TransportVersionDefinition upperBoundDefinition = definitions.get(upperBound.definitionName());
         if (upperBoundDefinition == null) {
@@ -240,13 +253,16 @@ private void validateUpperBound(
             );
         }
 
-        TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromGitBase(upperBound.name());
-        if (existingUpperBound != null && getShouldValidatePrimaryIdNotPatch().get()) {
-            if (upperBound.definitionId().patch() != 0 && upperBound.definitionId().base() != existingUpperBound.definitionId().base()) {
-                throwUpperBoundFailure(
-                    upperBound,
-                    "modifies base id from " + existingUpperBound.definitionId().base() + " to " + upperBound.definitionId().base()
-                );
+        if (validateModifications) {
+            TransportVersionUpperBound existingUpperBound = getResources().get().getUpperBoundFromGitBase(upperBound.name());
+            if (existingUpperBound != null && getShouldValidatePrimaryIdNotPatch().get()) {
+                if (upperBound.definitionId().patch() != 0
+                    && upperBound.definitionId().base() != existingUpperBound.definitionId().base()) {
+                    throwUpperBoundFailure(
+                        upperBound,
+                        "modifies base id from " + existingUpperBound.definitionId().base() + " to " + upperBound.definitionId().base()
+                    );
+                }
             }
         }
     }

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java

@@ -51,7 +51,6 @@
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.sameInstance;