Permalink
Browse files

[FIXED JENKINS-44497] Allow env vars and such in tools values.

We can't validate those, obviously, but such is life.
  • Loading branch information...
abayer committed Sep 1, 2017
1 parent a1fa5a5 commit 9b0ae5e67808bd5cc64bdb792e0e475ed833ce4d
@@ -40,15 +40,15 @@ import javax.annotation.Nonnull
* @author Andrew Bayer
*/
@SuppressFBWarnings(value="SE_NO_SERIALVERSIONID")
public class Tools extends MappedClosure<String,Tools> implements Serializable {
public class Tools extends MappedClosure<Closure,Tools> implements Serializable {

private static final Object CACHE_KEY = new Object()

private static final LoadingCache<Object,Map<String,String>> toolTypeCache =
Utils.generateTypeCache(ToolDescriptor.class, true)

@Whitelisted
Tools(Map<String,String> inMap) {
Tools(Map<String,Closure> inMap) {
resultMap = inMap
}

@@ -58,16 +58,13 @@ public class Tools extends MappedClosure<String,Tools> implements Serializable {
* @return A map of type/name
*/
@Nonnull
public Map<String,Object> mergeToolEntries(@CheckForNull Tools other) {
if (other == null) {
return getMap()
} else {
Map<String,Object> mergedMap = [:]
public Map<String,Closure> mergeToolEntries(@CheckForNull Tools other) {
Map<String,Closure> mergedMap = [:]
if (other != null) {
mergedMap.putAll(other.getMap())
mergedMap.putAll(getMap())

return mergedMap
}
mergedMap.putAll(getMap())
return mergedMap
}

/**
@@ -737,7 +737,11 @@ class RuntimeASTTransformer {
MapExpression toolsMap = new MapExpression()
original.tools.each { k, v ->
if (v.sourceLocation != null && v.sourceLocation instanceof Expression) {
toolsMap.addMapEntryExpression(constX(k.key), (Expression)v.sourceLocation)
if (v.sourceLocation instanceof ClosureExpression) {
toolsMap.addMapEntryExpression(constX(k.key), (ClosureExpression) v.sourceLocation)
} else {
toolsMap.addMapEntryExpression(constX(k.key), closureX(block(returnS((Expression) v.sourceLocation))))
}
}
}
return ctorX(ClassHelper.make(Tools.class), args(toolsMap))
@@ -163,8 +163,9 @@ class ModelValidatorImpl implements ModelValidator {
} else {
// Don't bother checking whether the tool exists in this Jenkins master if we know it isn't an allowed tool type.

// Can't do tools validation without a Jenkins instance, so move on if that's not available.
if (Jenkins.getInstance() != null) {
// Can't do tools validation without a Jenkins instance, so move on if that's not available, or if the tool value is a
// non-literal - we allow users to shoot themselves there.
if (Jenkins.getInstance() != null && v.isLiteral()) {
// Not bothering with a null check here since we could only get this far if the ToolDescriptor's available in the first place.
ToolDescriptor desc = ToolInstallation.all().find { it.getId().equals(Tools.typeForKey(k.key)) }
def installer = desc.getInstallations().find { it.name.equals((String) v.value) }
@@ -356,11 +356,11 @@ public class ModelInterpreter implements Serializable {
* @return The return of the resulting executed closure
*/
def toolsBlock(Agent agent, Tools tools, Root root = null, Closure body) {
def toolsMap = [:]
def toolsMap = new TreeMap<String,Closure>()
if (tools != null) {
toolsMap = tools.mergeToolEntries(root?.tools)
} else if (root?.tools != null) {
toolsMap = root.tools.getMap()
toolsMap = root.tools.mergeToolEntries(null)
}
// If there's no agent, don't install tools in the first place.
if (agent.hasAgent() && !toolsMap.isEmpty()) {
@@ -384,13 +384,15 @@ public class ModelInterpreter implements Serializable {
}
}

def actualToolsInstall(Map<String,String> toolsMap) {
def actualToolsInstall(Map<String,Closure> toolsMap) {
def toolEnv = []

toolsMap.each { k, v ->
String toolPath = script.tool(name: v, type: Tools.typeForKey(k))
String toolVer = v.call()

toolEnv.addAll(script.envVarsForTool(toolId: Tools.typeForKey(k), toolVersion: v))
String toolPath = script.tool(name: toolVer, type: Tools.typeForKey(k))

toolEnv.addAll(script.envVarsForTool(toolId: Tools.typeForKey(k), toolVersion: toolVer))
}

return toolEnv
@@ -58,7 +58,6 @@ public void simpleTools() throws Exception {
}

@Issue("JENKINS-44497")
@Ignore("Allowing env vars in tool version string breaks validation of version, and env vars aren't interpolated in tool string.")
@Test
public void envVarInTools() throws Exception {
expect("envVarInTools")

0 comments on commit 9b0ae5e

Please sign in to comment.