Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Remove agent-to-controller calls #245

Merged
merged 1 commit into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,14 @@
import hudson.FilePath;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.remoting.VirtualChannel;
import io.jenkins.plugins.coverage.adapter.CoverageAdapterDescriptor;
import io.jenkins.plugins.coverage.exception.CoverageException;
import jenkins.MasterToSlaveFileCallable;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.File;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class AntPathReportDetector extends ReportDetector {

Expand All @@ -30,12 +26,7 @@ public AntPathReportDetector(String path) {
@Override
public List<FilePath> findFiles(Run<?, ?> run, FilePath workspace, TaskListener listener) throws CoverageException {
try {
return Arrays.stream(workspace.act(new MasterToSlaveFileCallable<FilePath[]>() {
@Override
public FilePath[] invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
return workspace.list(path);
}
})).collect(Collectors.toList());
return Arrays.asList(workspace.list(path));
} catch (IOException | InterruptedException e) {
throw new CoverageException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public void resolveSourceFiles(Run<?, ?> run, FilePath workspace, TaskListener l
final Map<String, FilePath> sourceFileMapping = createSourceFileMapping(workspace, listener);

paints.forEach((sourceFilePath, paint) -> {
final FilePath buildDirSourceFile = new FilePath(new File(runRootDir, DEFAULT_SOURCE_CODE_STORE_DIRECTORY + sanitizeFilename(sourceFilePath)));
final File buildDirSourceFile = new File(runRootDir, DEFAULT_SOURCE_CODE_STORE_DIRECTORY + sanitizeFilename(sourceFilePath));

try {
listener.getLogger().printf("Starting copy source file %s. %n", sourceFilePath);
Expand All @@ -92,17 +92,13 @@ public void resolveSourceFiles(Run<?, ?> run, FilePath workspace, TaskListener l
possibleParentPaths = Collections.emptySet();
}

final boolean copiedSucceed = workspace.act(new SourceFilePainter(
FileUtils.write(buildDirSourceFile, workspace.act(new SourceFilePainter(
sourceFilePath,
paint,
buildDirSourceFile,
possibleParentPaths,
sourceFileMapping
));
if (copiedSucceed) {
listener.getLogger().printf("Copied %s. %n", sourceFilePath);

}
)), StandardCharsets.UTF_8);
listener.getLogger().printf("Copied %s. %n", sourceFilePath);

} catch (IOException | InterruptedException e) {
listener.getLogger().println(e.getMessage());
Expand Down Expand Up @@ -152,44 +148,39 @@ public ListBoxModel doFillLevelItems() {
}
}

private static class SourceFilePainter extends MasterToSlaveFileCallable<Boolean> {
private static class SourceFilePainter extends MasterToSlaveFileCallable<String> {
private static final long serialVersionUID = 6548573019315830249L;

private final String sourceFilePath;
private final Set<String> possiblePaths;
private final CoveragePaint paint;
private final FilePath destination;
private final Map<String, FilePath> sourceFileMapping;

SourceFilePainter(
@NonNull String sourceFilePath,
@NonNull CoveragePaint paint,
@NonNull FilePath destination,
@NonNull Set<String> possiblePaths,
@NonNull Map<String, FilePath> sourceFileMapping
) {
this.sourceFilePath = sourceFilePath;
this.paint = paint;
this.destination = destination;
this.possiblePaths = possiblePaths;
this.sourceFileMapping = sourceFileMapping;
}

@Override
public Boolean invoke(File workspace, VirtualChannel channel) throws IOException {
public String invoke(File workspace, VirtualChannel channel) throws IOException {
FilePath sourceFile = tryFindSourceFile(workspace);
if (sourceFile == null) {
throw new IOException(
String.format("Unable to find source file %s in workspace %s", sourceFilePath, workspace.getAbsolutePath()));
}

try {
paintSourceCode(sourceFile, paint, destination);
return paintSourceCode(sourceFile, paint);
} catch (CoverageException e) {
throw new IOException(e);
}

return true;
}

private FilePath tryFindSourceFile(File workspace) {
Expand Down Expand Up @@ -239,8 +230,8 @@ private boolean isValidSourceFile(File sourceFile) {
return sourceFile.exists() && sourceFile.isFile() && sourceFile.canRead();
}

private void paintSourceCode(FilePath source, CoveragePaint paint, FilePath canvas) throws CoverageException {
try (BufferedWriter output = new BufferedWriter(new OutputStreamWriter(canvas.write(), StandardCharsets.UTF_8));
private String paintSourceCode(FilePath source, CoveragePaint paint) throws CoverageException {
try (StringWriter output = new StringWriter();
BufferedReader input = new BufferedReader(new InputStreamReader(source.read(), StandardCharsets.UTF_8))) {
int line = 0;
String content;
Expand Down Expand Up @@ -276,6 +267,7 @@ private void paintSourceCode(FilePath source, CoveragePaint paint, FilePath canv
}

paint.setTotalLines(line);
return output.toString();
} catch (IOException | InterruptedException e) {
throw new CoverageException(e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* The MIT License
*
* Copyright 2021 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package io.jenkins.plugins.coverage;

import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.Extension;
import hudson.remoting.ChannelBuilder;
import java.io.File;
import jenkins.ReflectiveFilePathFilter;
import jenkins.SlaveToMasterFileCallable;
import jenkins.security.ChannelConfigurator;

/**
* Prevents all {@link SlaveToMasterFileCallable}s from running during tests, to make sure we do not rely on them.
*/
public class BlockSlaveToMasterFileCallable extends ReflectiveFilePathFilter {

@Override protected boolean op(String name, File path) throws SecurityException {
throw new SecurityException("refusing to " + name + " on " + path);
}

@Extension public static class ChannelConfiguratorImpl extends ChannelConfigurator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a TestExtension?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because that would only apply within a single test. This deliberately gets loaded in all tests in the plugin.


@Override public void onChannelBuilding(ChannelBuilder builder, @Nullable Object context) {
new BlockSlaveToMasterFileCallable().installTo(builder, 1000); // higher priority than, say, AdminFilePathFilter or even DefaultFilePathFilter
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import java.util.Objects;

import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.JenkinsRule;

import com.google.common.collect.Lists;
Expand All @@ -15,6 +17,7 @@
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
import hudson.FilePath;
import hudson.model.Result;
import hudson.slaves.DumbSlave;

import io.jenkins.plugins.coverage.adapter.CoberturaReportAdapter;
import io.jenkins.plugins.coverage.adapter.JacocoReportAdapter;
Expand All @@ -24,6 +27,8 @@

public class CoveragePublisherPipelineTest {

@ClassRule
public static BuildWatcher bw = new BuildWatcher();

@Rule
public JenkinsRule j = new JenkinsRule();
Expand Down Expand Up @@ -317,14 +322,17 @@ public void testAbsolutePathSourceFile() throws Exception {

@Test
public void testRelativePathSourceFile() throws Exception {
DumbSlave agent = j.createOnlineSlave();

CoverageScriptedPipelineScriptBuilder builder = CoverageScriptedPipelineScriptBuilder.builder()
.onAgent(agent)
.setEnableSourceFileResolver(true);

CoberturaReportAdapter adapter = new CoberturaReportAdapter("cobertura-coverage.xml");
builder.addAdapter(adapter);

WorkflowJob project = j.createProject(WorkflowJob.class, "coverage-pipeline-test");
FilePath workspace = j.jenkins.getWorkspaceFor(project);
FilePath workspace = agent.getWorkspaceFor(project);

Objects.requireNonNull(workspace)
.child("cobertura-coverage.xml")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.jenkins.plugins.coverage;

import hudson.model.Descriptor;
import hudson.model.Slave;
import io.jenkins.plugins.coverage.adapter.CoverageAdapter;
import io.jenkins.plugins.coverage.adapter.CoverageReportAdapter;
import io.jenkins.plugins.coverage.detector.AntPathReportDetector;
Expand All @@ -21,6 +22,7 @@ public class CoverageScriptedPipelineScriptBuilder {
private boolean applyThresholdRecursively;

private boolean enableSourceFileResolver;
private Slave agent;

private CoverageScriptedPipelineScriptBuilder() {
}
Expand All @@ -40,10 +42,18 @@ public CoverageScriptedPipelineScriptBuilder addGlobalThreshold(Threshold thresh
return this;
}

public CoverageScriptedPipelineScriptBuilder onAgent(Slave slave) {
this.agent = slave;
return this;
}

public String build() {
StringBuilder sb = new StringBuilder();
sb.append("node {")
sb.append("node");
if (agent != null) {
sb.append("('").append(agent.getNodeName()).append("')");
}
sb.append(" {")
.append("publishCoverage(");

sb.append("failUnhealthy:").append(failUnhealthy);
Expand Down