[MNG-7789] Dependency validation rules used wrong data (#1115)

Refactored and now dropped the unused pluginDescriptor/dependencies validation. Now we have "plugin dependency" (using POM) components to perform dependency validation.

Also, two "inlined" validation from plugin dependencies resolver factored out to components, along with 3 existing moved to newly added (they were wrongly validating descriptor dependencies while intent was to validate "real" dependencies) validators.

Ported from
6c562a46ee

---

https://issues.apache.org/jira/browse/MNG-7789
This commit is contained in:
Tamas Cservenak 2023-05-24 08:28:24 +02:00
parent a27086a6f5
commit 12ccaacf63
11 changed files with 208 additions and 165 deletions

View File

@ -18,9 +18,10 @@
*/
package org.apache.maven.plugin.internal;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.PluginValidationManager;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
import static java.util.Objects.requireNonNull;
@ -38,12 +39,17 @@ abstract class AbstractMavenPluginDependenciesValidator implements MavenPluginDe
}
@Override
public void validate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
if (mojoDescriptor.getPluginDescriptor() != null
&& mojoDescriptor.getPluginDescriptor().getDependencies() != null) {
doValidate(mavenSession, mojoDescriptor);
public void validate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
if (artifactDescriptorResult.getDependencies() != null) {
doValidate(session, pluginArtifact, artifactDescriptorResult);
}
}
protected abstract void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor);
protected abstract void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult);
}

View File

@ -148,7 +148,6 @@ public class DefaultMavenPluginManager implements MavenPluginManager {
private PluginArtifactsCache pluginArtifactsCache;
private MavenPluginValidator pluginValidator;
private List<MavenPluginConfigurationValidator> configurationValidators;
private List<MavenPluginDependenciesValidator> dependenciesValidators;
private PluginValidationManager pluginValidationManager;
private List<MavenPluginPrerequisitesChecker> prerequisitesCheckers;
private final ExtensionDescriptorBuilder extensionDescriptorBuilder = new ExtensionDescriptorBuilder();
@ -168,7 +167,6 @@ public class DefaultMavenPluginManager implements MavenPluginManager {
PluginArtifactsCache pluginArtifactsCache,
MavenPluginValidator pluginValidator,
List<MavenPluginConfigurationValidator> configurationValidators,
List<MavenPluginDependenciesValidator> dependencyValidators,
PluginValidationManager pluginValidationManager,
List<MavenPluginPrerequisitesChecker> prerequisitesCheckers) {
this.container = container;
@ -182,7 +180,6 @@ public class DefaultMavenPluginManager implements MavenPluginManager {
this.pluginArtifactsCache = pluginArtifactsCache;
this.pluginValidator = pluginValidator;
this.configurationValidators = configurationValidators;
this.dependenciesValidators = dependencyValidators;
this.pluginValidationManager = pluginValidationManager;
this.prerequisitesCheckers = prerequisitesCheckers;
}
@ -574,10 +571,6 @@ public class DefaultMavenPluginManager implements MavenPluginManager {
"Mojo implements `Contextualizable` interface from Plexus Container, which is EOL.");
}
for (MavenPluginDependenciesValidator validator : dependenciesValidators) {
validator.validate(session, mojoDescriptor);
}
XmlNode dom = mojoExecution.getConfiguration() != null
? mojoExecution.getConfiguration().getDom()
: null;

View File

@ -27,8 +27,6 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.maven.RepositoryUtils;
import org.apache.maven.model.Dependency;
@ -82,11 +80,16 @@ public class DefaultPluginDependenciesResolver implements PluginDependenciesReso
private final PluginValidationManager pluginValidationManager;
private final List<MavenPluginDependenciesValidator> dependenciesValidators;
@Inject
public DefaultPluginDependenciesResolver(
RepositorySystem repoSystem, PluginValidationManager pluginValidationManager) {
RepositorySystem repoSystem,
PluginValidationManager pluginValidationManager,
List<MavenPluginDependenciesValidator> dependenciesValidators) {
this.repoSystem = repoSystem;
this.pluginValidationManager = pluginValidationManager;
this.dependenciesValidators = dependenciesValidators;
}
private Artifact toArtifact(Plugin plugin, RepositorySystemSession session) {
@ -114,34 +117,8 @@ public class DefaultPluginDependenciesResolver implements PluginDependenciesReso
request.setTrace(trace);
ArtifactDescriptorResult result = repoSystem.readArtifactDescriptor(pluginSession, request);
if (result.getDependencies() != null) {
for (org.eclipse.aether.graph.Dependency dependency : result.getDependencies()) {
if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
&& "maven-compat".equals(dependency.getArtifact().getArtifactId())
&& !JavaScopes.TEST.equals(dependency.getScope())) {
pluginValidationManager.reportPluginValidationIssue(
session,
pluginArtifact,
"Plugin depends on the deprecated Maven 2.x compatibility layer, which may not be supported in Maven 4.x");
}
}
Set<String> mavenArtifacts = result.getDependencies().stream()
.filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
.map(org.eclipse.aether.graph.Dependency::getArtifact)
.filter(a -> "org.apache.maven".equals(a.getGroupId()))
.filter(a -> !MavenPluginDependenciesValidator.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
a.getGroupId() + ":" + a.getArtifactId()))
.filter(a -> a.getVersion().startsWith("3."))
.map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
.collect(Collectors.toSet());
if (!mavenArtifacts.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
session,
pluginArtifact,
"Plugin should declare these Maven artifacts in `provided` scope: " + mavenArtifacts);
}
for (MavenPluginDependenciesValidator dependenciesValidator : dependenciesValidators) {
dependenciesValidator.validate(session, pluginArtifact, result);
}
pluginArtifact = result.getArtifact();

View File

@ -22,11 +22,7 @@ import javax.inject.Named;
import javax.inject.Singleton;
import java.io.File;
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Locale;
import java.util.Map;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.maven.eventspy.AbstractEventSpy;
@ -46,6 +42,13 @@ import org.slf4j.LoggerFactory;
@Singleton
@Named
public final class DefaultPluginValidationManager extends AbstractEventSpy implements PluginValidationManager {
/**
* The collection of "G:A" combinations that do NOT belong to Maven Core, hence, should be excluded from
* "expected in provided scope" type of checks.
*/
static final Collection<String> EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA =
Collections.unmodifiableCollection(Arrays.asList(
"org.apache.maven:maven-archiver", "org.apache.maven:maven-jxr", "org.apache.maven:plexus-utils"));
private static final String ISSUES_KEY = DefaultPluginValidationManager.class.getName() + ".issues";

View File

@ -25,10 +25,11 @@ import javax.inject.Singleton;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.PluginValidationManager;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.codehaus.plexus.component.repository.ComponentDependency;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.graph.Dependency;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
/**
* Detects Maven2 plugins.
@ -45,19 +46,22 @@ class Maven2DependenciesValidator extends AbstractMavenPluginDependenciesValidat
}
@Override
protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
Set<String> maven2Versions = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
protected void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
Set<String> maven2Versions = artifactDescriptorResult.getDependencies().stream()
.map(Dependency::getArtifact)
.filter(d -> "org.apache.maven".equals(d.getGroupId()))
.filter(d -> !EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(d.getGroupId() + ":" + d.getArtifactId()))
.map(ComponentDependency::getVersion)
.filter(d -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
d.getGroupId() + ":" + d.getArtifactId()))
.map(Artifact::getVersion)
.filter(v -> v.startsWith("2."))
.collect(Collectors.toSet());
if (!maven2Versions.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
mavenSession,
mojoDescriptor,
"Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
session, pluginArtifact, "Plugin is a Maven 2.x plugin, which will be not supported in Maven 4.x");
}
}
}

View File

@ -0,0 +1,61 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.plugin.internal;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import org.apache.maven.plugin.PluginValidationManager;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
import org.eclipse.aether.util.artifact.JavaScopes;
/**
* Detects Maven3 plugins using maven-compat Maven2 compatibility layer.
*
* @since 3.9.3
*/
@Singleton
@Named
class Maven3CompatDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
@Inject
Maven3CompatDependenciesValidator(PluginValidationManager pluginValidationManager) {
super(pluginValidationManager);
}
@Override
protected void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
for (org.eclipse.aether.graph.Dependency dependency : artifactDescriptorResult.getDependencies()) {
if ("org.apache.maven".equals(dependency.getArtifact().getGroupId())
&& "maven-compat".equals(dependency.getArtifact().getArtifactId())
&& !JavaScopes.TEST.equals(dependency.getScope())) {
pluginValidationManager.reportPluginValidationIssue(
session,
pluginArtifact,
"Plugin depends on the deprecated Maven 2.x compatibility layer, which will be not supported in Maven 4.x");
}
}
}
}

View File

@ -25,10 +25,11 @@ import javax.inject.Singleton;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.PluginValidationManager;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.codehaus.plexus.component.repository.ComponentDependency;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.graph.Dependency;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
/**
* Detects mixed Maven versions in plugins.
@ -45,16 +46,21 @@ class MavenMixedDependenciesValidator extends AbstractMavenPluginDependenciesVal
}
@Override
protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
Set<String> mavenVersions = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
protected void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
Set<String> mavenVersions = artifactDescriptorResult.getDependencies().stream()
.map(Dependency::getArtifact)
.filter(d -> "org.apache.maven".equals(d.getGroupId()))
.filter(d -> !EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(d.getGroupId() + ":" + d.getArtifactId()))
.map(ComponentDependency::getVersion)
.filter(d -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
d.getGroupId() + ":" + d.getArtifactId()))
.map(Artifact::getVersion)
.collect(Collectors.toSet());
if (mavenVersions.size() > 1) {
pluginValidationManager.reportPluginValidationIssue(
mavenSession, mojoDescriptor, "Plugin mixes multiple Maven versions: " + mavenVersions);
session, pluginArtifact, "Plugin mixes multiple Maven versions: " + mavenVersions);
}
}
}

View File

@ -18,31 +18,21 @@
*/
package org.apache.maven.plugin.internal;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
/**
* Service responsible for validating plugin dependencies.
*
* @since 3.9.2
* @since 3.9.3
*/
interface MavenPluginDependenciesValidator {
/**
* The collection of "G:A" combinations that do NOT belong to Maven Core, hence, should be excluded from
* "expected in provided scope" type of checks.
*
* @since 3.9.3
*/
Collection<String> EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA = Collections.unmodifiableCollection(Arrays.asList(
"org.apache.maven:maven-archiver", "org.apache.maven:maven-jxr", "org.apache.maven:plexus-utils"));
/**
* Checks mojo dependency issues.
*/
void validate(MavenSession mavenSession, MojoDescriptor mojoDescriptor);
void validate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult);
}

View File

@ -1,72 +0,0 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.plugin.internal;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.PluginValidationManager;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
/**
* Detects presence of unwanted Maven3 artifacts in plugin descriptor, possibly caused by multitude of reasons, among
* them is "wrong scope" dependency declaration as well.
* <p>
* Historically, this class was named as "MavenScopeDependenciesValidator" due original intent to check "wrong Maven
* Artifact scopes". Since then, it turned out that the values validated (the plugin descriptor dependencies, that is
* produced at plugin build time by maven-plugin-plugin) may be off (for example due maven-plugin-plugin bug), and
* is potentially not inline with "reality" (actual plugin dependencies).
* <p>
* The original intent related check is moved to
* {@link DefaultPluginDependenciesResolver#resolve(org.apache.maven.model.Plugin, java.util.List, org.eclipse.aether.RepositorySystemSession)}
* method instead.
*
* @since 3.9.3
*/
@Singleton
@Named
class MavenPluginDescriptorDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
@Inject
MavenPluginDescriptorDependenciesValidator(PluginValidationManager pluginValidationManager) {
super(pluginValidationManager);
}
@Override
protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
Set<String> mavenArtifacts = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
.filter(d -> "org.apache.maven".equals(d.getGroupId()))
.filter(d -> !EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(d.getGroupId() + ":" + d.getArtifactId()))
.filter(d -> d.getVersion().startsWith("3."))
.map(d -> d.getGroupId() + ":" + d.getArtifactId() + ":" + d.getVersion())
.collect(Collectors.toSet());
if (!mavenArtifacts.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
mavenSession,
mojoDescriptor,
"Plugin descriptor should not contain these Maven artifacts: " + mavenArtifacts);
}
}
}

View File

@ -0,0 +1,71 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.plugin.internal;
import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import java.util.Set;
import java.util.stream.Collectors;
import org.apache.maven.plugin.PluginValidationManager;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
import org.eclipse.aether.util.artifact.JavaScopes;
/**
* Detects Maven3 dependencies scope.
*
* @since 3.9.3
*/
@Singleton
@Named
class MavenScopeDependenciesValidator extends AbstractMavenPluginDependenciesValidator {
@Inject
MavenScopeDependenciesValidator(PluginValidationManager pluginValidationManager) {
super(pluginValidationManager);
}
@Override
protected void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
Set<String> mavenArtifacts = artifactDescriptorResult.getDependencies().stream()
.filter(d -> !JavaScopes.PROVIDED.equals(d.getScope()) && !JavaScopes.TEST.equals(d.getScope()))
.map(org.eclipse.aether.graph.Dependency::getArtifact)
.filter(a -> "org.apache.maven".equals(a.getGroupId()))
.filter(a -> !DefaultPluginValidationManager.EXPECTED_PROVIDED_SCOPE_EXCLUSIONS_GA.contains(
a.getGroupId() + ":" + a.getArtifactId()))
.filter(a -> a.getVersion().startsWith("3."))
.map(a -> a.getGroupId() + ":" + a.getArtifactId() + ":" + a.getVersion())
.collect(Collectors.toSet());
if (!mavenArtifacts.isEmpty()) {
pluginValidationManager.reportPluginValidationIssue(
session,
pluginArtifact,
"Plugin should declare Maven artifacts in `provided` scope. If the plugin already declares them in `provided` scope, update the maven-plugin-plugin to latest version. Artifacts found with wrong scope: "
+ mavenArtifacts);
}
}
}

View File

@ -22,9 +22,10 @@ import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.PluginValidationManager;
import org.apache.maven.plugin.descriptor.MojoDescriptor;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.resolution.ArtifactDescriptorResult;
/**
* Detects Plexus Container Default in plugins.
@ -40,14 +41,17 @@ class PlexusContainerDefaultDependenciesValidator extends AbstractMavenPluginDep
super(pluginValidationManager);
}
protected void doValidate(MavenSession mavenSession, MojoDescriptor mojoDescriptor) {
boolean pcdPresent = mojoDescriptor.getPluginDescriptor().getDependencies().stream()
.filter(d -> "org.codehaus.plexus".equals(d.getGroupId()))
.anyMatch(d -> "plexus-container-default".equals(d.getArtifactId()));
protected void doValidate(
RepositorySystemSession session,
Artifact pluginArtifact,
ArtifactDescriptorResult artifactDescriptorResult) {
boolean pcdPresent = artifactDescriptorResult.getDependencies().stream()
.filter(d -> "org.codehaus.plexus".equals(d.getArtifact().getGroupId()))
.anyMatch(d -> "plexus-container-default".equals(d.getArtifact().getArtifactId()));
if (pcdPresent) {
pluginValidationManager.reportPluginValidationIssue(
mavenSession, mojoDescriptor, "Plugin depends on plexus-container-default, which is EOL");
session, pluginArtifact, "Plugin depends on plexus-container-default, which is EOL");
}
}
}