[
https://issues.apache.org/jira/browse/MRESOLVER-301?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17819752#comment-17819752
]
ASF GitHub Bot commented on MRESOLVER-301:
------------------------------------------
gnodet commented on code in PR #432:
URL: https://github.com/apache/maven-resolver/pull/432#discussion_r1499592777
##########
maven-resolver-generator-signer/src/main/java/org/eclipse/aether/generator/signer/SignerArtifactGenerator.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.eclipse.aether.generator.signer;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.spi.artifact.generator.ArtifactGenerator;
+
+import static java.util.Objects.requireNonNull;
+
+final class SignerArtifactGenerator implements ArtifactGenerator {
Review Comment:
Why not `public` instead of `final` with a public constructor and protected
fields ? Why would not people be allowed to extend it ?
##########
maven-resolver-generator-signer/src/main/java/org/eclipse/aether/generator/signer/gpg/GpgSigner.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.eclipse.aether.generator.signer.gpg;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+
+import org.bouncycastle.bcpg.ArmoredOutputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
+import org.bouncycastle.bcpg.HashAlgorithmTags;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPrivateKey;
+import org.bouncycastle.openpgp.PGPSecretKey;
+import org.bouncycastle.openpgp.PGPSignature;
+import org.bouncycastle.openpgp.PGPSignatureGenerator;
+import org.bouncycastle.openpgp.PGPSignatureSubpacketVector;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.generator.signer.Signer;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.util.artifact.SubArtifact;
+
+/**
+ * GnuPG {@link Signer} implementation that is kept and reused across session.
+ */
+@SuppressWarnings("checkstyle:magicnumber")
+public final class GpgSigner implements Signer {
+ private static final String ARTIFACT_EXTENSION = "asc";
+ private final RepositoryLayout repositoryLayout;
+ private final PGPSecretKey secretKey;
+ private final PGPPrivateKey privateKey;
+ private final PGPSignatureSubpacketVector hashSubPackets;
+ private final ArrayList<Path> signatureTempFiles;
+
+ public GpgSigner(
+ RepositoryLayout repositoryLayout,
+ PGPSecretKey secretKey,
+ PGPPrivateKey privateKey,
+ PGPSignatureSubpacketVector hashSubPackets) {
+ this.repositoryLayout = repositoryLayout;
+ this.secretKey = secretKey;
+ this.privateKey = privateKey;
+ this.hashSubPackets = hashSubPackets;
+ this.signatureTempFiles = new ArrayList<>();
+ }
+
+ @Override
+ public String signerId() {
+ return GpgConfigurationKeys.NAME;
+ }
+
+ @Override
+ public Collection<Artifact> sign(Collection<Artifact> artifacts) throws
IOException {
+ // back out if PGP signatures found among artifacts
+ if (artifacts.stream().anyMatch(a -> a.getExtension().endsWith("." +
ARTIFACT_EXTENSION))) {
+ return Collections.emptyList();
+ }
+
+ // sign relevant artifacts
+ ArrayList<Artifact> result = new ArrayList<>();
+ for (Artifact artifact : artifacts) {
+ if (!repositoryLayout.hasChecksums(artifact)) {
+ continue;
+ }
+
+ Path signatureTempFile = Files.createTempFile("signer-pgp", "tmp");
+ signatureTempFiles.add(signatureTempFile);
+ try (InputStream artifactContent =
Files.newInputStream(artifact.getPath());
+ OutputStream signatureContent =
Files.newOutputStream(signatureTempFile)) {
+ sign(artifactContent, signatureContent);
+ }
+ result.add(new SubArtifact(
+ artifact, null, artifact.getExtension() + "." +
ARTIFACT_EXTENSION, signatureTempFile.toFile()));
+ }
+ return result;
+ }
+
+ /**
+ * Clean up all temp files when install/deploy request is processed, files
are copied/uploaded to their
+ * final place.
+ */
+ public void close() {
+ signatureTempFiles.forEach(p -> {
+ try {
+ Files.deleteIfExists(p);
+ } catch (IOException e) {
+ p.toFile().deleteOnExit();
+ }
+ });
+ }
+
+ private void sign(InputStream content, OutputStream signature) throws
IOException {
Review Comment:
`protected` ?
##########
maven-resolver-generator-signer/src/main/java/org/eclipse/aether/generator/signer/gpg/GpgEnvPasswordLoader.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * 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.eclipse.aether.generator.signer.gpg;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.sisu.Priority;
+
+@Singleton
+@Named(GpgEnvPasswordLoader.NAME)
+@Priority(50)
+@SuppressWarnings("checkstyle:magicnumber")
+public final class GpgEnvPasswordLoader implements
GpgSignerFactory.KeyPasswordLoader {
Review Comment:
Is the `final` keyword needed ?
##########
maven-resolver-generator-signer/src/main/java/org/eclipse/aether/generator/signer/gpg/GpgSigner.java:
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.eclipse.aether.generator.signer.gpg;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+
+import org.bouncycastle.bcpg.ArmoredOutputStream;
+import org.bouncycastle.bcpg.BCPGOutputStream;
+import org.bouncycastle.bcpg.HashAlgorithmTags;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPrivateKey;
+import org.bouncycastle.openpgp.PGPSecretKey;
+import org.bouncycastle.openpgp.PGPSignature;
+import org.bouncycastle.openpgp.PGPSignatureGenerator;
+import org.bouncycastle.openpgp.PGPSignatureSubpacketVector;
+import org.bouncycastle.openpgp.operator.bc.BcPGPContentSignerBuilder;
+import org.eclipse.aether.artifact.Artifact;
+import org.eclipse.aether.generator.signer.Signer;
+import org.eclipse.aether.spi.connector.layout.RepositoryLayout;
+import org.eclipse.aether.util.artifact.SubArtifact;
+
+/**
+ * GnuPG {@link Signer} implementation that is kept and reused across session.
+ */
+@SuppressWarnings("checkstyle:magicnumber")
+public final class GpgSigner implements Signer {
Review Comment:
Same...
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultDeployer.java:
##########
@@ -151,7 +162,23 @@ private DeployResult deploy(SyncContext syncContext,
RepositorySystemSession ses
throw new DeploymentException("Failed to deploy
artifacts/metadata: " + e.getMessage(), e);
}
+ final List<Artifact> artifacts = new
ArrayList<>(request.getArtifacts());
Review Comment:
I don't see how `final` is useful. The static analysis very well knows if a
variable is final or not.
##########
maven-resolver-generator-signer/src/main/java/org/eclipse/aether/generator/signer/gpg/GpgFileRingMaterialLoader.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.eclipse.aether.generator.signer.gpg;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.sisu.Priority;
+
+@Singleton
+@Named(GpgFileRingMaterialLoader.NAME)
+@Priority(10)
+public final class GpgFileRingMaterialLoader implements
GpgSignerFactory.KeyRingMaterialLoader {
Review Comment:
Same here
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultInstaller.java:
##########
@@ -95,53 +106,96 @@ private InstallResult install(SyncContext syncContext,
RepositorySystemSession s
RequestTrace trace = RequestTrace.newChild(request.getTrace(),
request);
- List<? extends MetadataGenerator> generators =
getMetadataGenerators(session, request);
+ final List<Artifact> artifacts = new
ArrayList<>(request.getArtifacts());
Review Comment:
Once again ?
> Artifact Generators
> -------------------
>
> Key: MRESOLVER-301
> URL: https://issues.apache.org/jira/browse/MRESOLVER-301
> Project: Maven Resolver
> Issue Type: New Feature
> Components: Resolver
> Reporter: Tamas Cservenak
> Assignee: Tamas Cservenak
> Priority: Major
> Fix For: 2.0.0, 2.0.0-alpha-9
>
>
> Resolver should provide extension point for "generators". Typical use case
> for these are for example "signing" of artifacts.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
