From f66e9673396a044cff6a5068acd79d71a99a951e Mon Sep 17 00:00:00 2001 From: Lukas Krejci Date: Wed, 6 Feb 2019 14:40:30 +0100 Subject: [PATCH] Make variable expansion work for environment variables in k8s. K8s does the expansion only if it already knows about the variable being expanded. This means we have to sort the environment variable list prior to sending it to k8s in such a way that vars that reference others always follow the referenced ones. Signed-off-by: Lukas Krejci --- .../che/commons/lang/TopologicalSort.java | 247 ++++++++++++++++++ .../che/commons/lang/TopologicalSortTest.java | 155 +++++++++++ .../provision/env/EnvVarsConverter.java | 41 ++- .../kubernetes/util/EnvVars.java | 63 +++++ .../provision/env/EnvVarsConverterTest.java | 108 ++++++++ .../kubernetes/util/EnvVarsTest.java | 73 ++++++ 6 files changed, 680 insertions(+), 7 deletions(-) create mode 100644 core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/TopologicalSort.java create mode 100644 core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/TopologicalSortTest.java create mode 100644 infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverterTest.java create mode 100644 infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVarsTest.java diff --git a/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/TopologicalSort.java b/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/TopologicalSort.java new file mode 100644 index 0000000000..a84a6aeb6b --- /dev/null +++ b/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/TopologicalSort.java @@ -0,0 +1,247 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.commons.lang; + +import static java.util.Collections.emptyList; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.Function; +import java.util.stream.Collectors; + +/** + * This is an implementation of a stable topological sort on a directed graph. + * + *

The sorting does not pose any requirements on the types being sorted. Instead, the + * implementation merely requires a function to provide it with a set of predecessors of a certain + * "node". The implementation of the function is completely in the hands of the caller. + * + *

Additionally, a function to extract an "ID" from a node is required. The reasoning behind this + * is that it usually is easier to work with identifiers when establishing the predecessors than + * with the full node instances. That said, nothing prevents the caller from using the actual node + * instance as its ID if the caller so wishes. The consequence of this is that, as a side-effect of + * the sorting, the duplicates, as determined by the equality of {@code ID} instances, are removed + * from the resulting sorted list. + * + * @param the type of nodes + * @param the type of a node ID + */ +public final class TopologicalSort { + + private final Function identityExtractor; + private final Function> directPredecessorsExtractor; + + /** + * @param identityExtractor a function to extract some kind of value uniquely identifying a node + * amongst the others. + * @param directPredecessorsExtractor a function returning a list of ids of direct predecessors of + * a node + */ + public TopologicalSort( + Function identityExtractor, Function> directPredecessorsExtractor) { + this.identityExtractor = identityExtractor; + this.directPredecessorsExtractor = directPredecessorsExtractor; + } + + /** + * Given the function for determining the predecessors of the nodes, return the list of the nodes + * in topological order. I.e. all predecessors will be placed sooner in the list than their + * successors. Note that the input collection is assumed to contain no duplicate entries as + * determined by the equality of the {@code ID} type. If such duplicates are present in the input + * collection, the output list will only contain the first instance of the duplicates from the + * input collection. + * + *

The implemented sort algorithm is stable. If there is no relationship between 2 nodes, they + * retain the relative position to each other as they had in the provided collection (e.g. if "a" + * preceded "b" in the original collection and there is no relationship between them (as + * determined by the predecessor function), the "a" will still precede "b" in the resulting list. + * Other nodes may be inserted in between them though in the result). + * + *

The cycles in the graph determined by the predecessor function are ignored and nodes in the + * cycle are placed into the output list in the source order. + * + * @param nodes the collection of nodes + * @return the list of nodes sorted in topological order + */ + public List sort(Collection nodes) { + // the linked hashmap is important to retain the original order of elements unless required + // by the dependencies between nodes + LinkedHashMap> nodeInfos = new LinkedHashMap<>(nodes.size()); + List> results = new ArrayList<>(nodes.size()); + + int pos = 0; + boolean needsSorting = false; + for (N node : nodes) { + ID nodeID = identityExtractor.apply(node); + // we need the set to be modifiable, so let's make our own + Set preds = new HashSet<>(directPredecessorsExtractor.apply(node)); + needsSorting |= !preds.isEmpty(); + + NodeInfo nodeInfo = nodeInfos.computeIfAbsent(nodeID, __ -> new NodeInfo<>()); + nodeInfo.id = nodeID; + nodeInfo.predecessors = preds; + nodeInfo.sourcePosition = pos++; + nodeInfo.node = node; + + for (ID pred : preds) { + // note that this means that we're inserting the nodeinfos into the map in an incorrect + // order and will have to sort them in the source order before we do the actual topo sort. + // We take that cost because we gamble on there being no dependencies in the nodes as a + // common case. + NodeInfo predNode = nodeInfos.computeIfAbsent(pred, __ -> new NodeInfo<>()); + if (predNode.successors == null) { + predNode.successors = new HashSet<>(); + } + predNode.successors.add(nodeID); + } + } + + if (needsSorting) { + // because of the predecessors, we have put the nodeinfos in the map in an incorrect order. + // we need to correct that before we try to sort... + TreeSet> tmp = new TreeSet<>(Comparator.comparingInt(a -> a.sourcePosition)); + tmp.addAll(nodeInfos.values()); + nodeInfos.clear(); + tmp.forEach(ni -> nodeInfos.put(ni.id, ni)); + + // now we're ready to produce the results + sort(nodeInfos, results); + } else { + // we don't need to sort, but we need to keep the expected behavior of removing the duplicates + results = new ArrayList<>(nodeInfos.values()); + } + + return results.stream().map(ni -> ni.node).collect(Collectors.toList()); + } + + private void sort(LinkedHashMap> nodes, List> results) { + + while (!nodes.isEmpty()) { + NodeInfo curr = removeFirstIndependent(nodes); + if (curr != null) { + // yay, simple. Just add the found independent node to the results. + results.add(curr); + } else { + // ok, there is a cycle in the graph. Let's remove all the nodes in the first cycle we find + // from our predecessors map, add them to the result in their original order and try to + // continue normally + + // find the first cycle in the predecessors (in the original list order) + Iterator> nexts = nodes.values().iterator(); + List> cycle; + do { + curr = nexts.next(); + cycle = findCycle(curr, nodes); + } while (cycle.isEmpty() && nexts.hasNext()); + + // If we ever find a graph that doesn't have any independent node, yet we fail to find a + // cycle in it, the universe must be broken. + if (cycle.isEmpty()) { + throw new IllegalStateException( + String.format( + "Failed to find a cycle in a graph that doesn't seem to have any independent" + + " node. This should never happen. Please file a bug. Current state of the" + + " sorting is: nodes=%s, results=%s", + nodes.toString(), results.toString())); + } + + cycle.sort(Comparator.comparingInt(a -> a.sourcePosition)); + + for (NodeInfo n : cycle) { + removePredecessorMapping(nodes, n); + results.add(n); + } + } + } + } + + private void removePredecessorMapping(Map> nodes, NodeInfo node) { + if (node.successors != null) { + for (ID succ : node.successors) { + NodeInfo succNode = nodes.get(succ); + if (succNode != null) { + succNode.predecessors.remove(node.id); + } + } + } + nodes.remove(node.id); + } + + private List> findCycle(NodeInfo node, Map> nodes) { + // bail out quickly if there are no preds - should be fairly common occurrence hopefully + Set preds = node.predecessors; + if (preds == null || preds.isEmpty()) { + return emptyList(); + } + + List> ret = new ArrayList<>(); + List todo = new ArrayList<>(preds); + + Set visited = new HashSet<>(); + + while (!todo.isEmpty()) { + ID n = todo.remove(0); + + if (visited.contains(n)) { + continue; + } + + visited.add(n); + + NodeInfo predNode = nodes.get(n); + if (predNode != null) { + todo.addAll(predNode.predecessors); + ret.add(predNode); + + if (predNode.equals(node)) { + // we found the cycle to our original node + return ret; + } + } + } + + return emptyList(); + } + + private NodeInfo removeFirstIndependent(Map> nodes) { + Iterator>> it = nodes.entrySet().iterator(); + + while (it.hasNext()) { + Entry> e = it.next(); + if (e.getValue().predecessors.isEmpty()) { + it.remove(); + NodeInfo ret = e.getValue(); + removePredecessorMapping(nodes, ret); + return ret; + } + } + + return null; + } + + private static final class NodeInfo { + private ID id; + private int sourcePosition; + private Set predecessors; + private Set successors; + private N node; + } +} diff --git a/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/TopologicalSortTest.java b/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/TopologicalSortTest.java new file mode 100644 index 0000000000..4c803494fa --- /dev/null +++ b/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/TopologicalSortTest.java @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.commons.lang; + +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; +import static org.testng.Assert.assertEquals; + +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class TopologicalSortTest { + + @Test(dataProvider = "sortingCases") + public void shouldApplySorting(List list, List expectedSorting) throws Exception { + // when + sort(list); + + // then + assertEquals(list, expectedSorting); + } + + @DataProvider + public static Object[][] sortingCases() { + return new Object[][] { + // keep order if no dependencies + new Object[] { + vars(var('b', "."), var('a', "."), var('c', ".")), + vars(var('b', "."), var('a', "."), var('c', ".")) + }, + + // sort dependents after dependencies + new Object[] { + vars(var('a', "b"), var('b', "."), var('c', ".")), + vars(var('b', "."), var('a', "b"), var('c', ".")) + }, + + // sort dependents after dependencies, multiple dependencies + new Object[] { + vars(var('a', "bc"), var('b', "."), var('c', ".")), + vars(var('b', "."), var('c', "."), var('a', "bc")) + }, + + // sort dependents after dependencies, check dependee moved after dependencies + new Object[] { + vars(var('d', "."), var('a', "c"), var('b', "."), var('c', ".")), + vars(var('d', "."), var('b', "."), var('c', "."), var('a', "c")) + }, + + // test the robustness against cycles + + // dependent directly on itself + new Object[] {vars(var('a', "a")), vars(var('a', "a"))}, + + // two mutually dependent nodes + new Object[] {vars(var('a', "b"), var('b', "a")), vars(var('a', "b"), var('b', "a"))}, + + // independent node mixed inside a cycle + new Object[] { + vars(var('b', "c"), var('d', "."), var('a', "b"), var('c', "a")), + vars(var('d', "."), var('b', "c"), var('a', "b"), var('c', "a")) + }, + + // cycle (a-b-c) with one of the nodes also depending on another node (a-d), + // mixed with a "chain" (f-e-b) + new Object[] { + vars( + var('f', "e"), + var('a', "bd"), + var('b', "c"), + var('e', "b"), + var('c', "a"), + var('d', ".")), + vars( + var('d', "."), + var('a', "bd"), + var('b', "c"), + var('c', "a"), + var('e', "b"), + var('f', "e")) + }, + + // removes duplicates + new Object[] { + vars(var('a', "."), var('a', "."), var('b', ".")), vars(var('a', "."), var('b', ".")) + } + }; + } + + private static Var var(char name, String value) { + return new Var(name, value.chars().mapToObj(c -> (char) c).collect(toSet())); + } + + private static List vars(Var... vars) { + return Stream.of(vars).collect(toList()); + } + + private static void sort(List vars) { + Function idFunction = v -> v.name; + Function> predecessors = + v -> v.dependencies.stream().filter(Character::isAlphabetic).collect(toSet()); + + TopologicalSort sort = new TopologicalSort<>(idFunction, predecessors); + List newVars = sort.sort(vars); + vars.clear(); + vars.addAll(newVars); + } + + private static final class Var { + + private final char name; + private final Set dependencies; + + private Var(char name, Set dependencies) { + this.name = name; + this.dependencies = dependencies; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Var var = (Var) o; + return name == var.name && dependencies.equals(var.dependencies); + } + + @Override + public int hashCode() { + return Objects.hash(name, dependencies); + } + + @Override + public String toString() { + return "Var{" + "name='" + name + '\'' + ", deps='" + dependencies + '\'' + '}'; + } + } +} diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverter.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverter.java index ecb5a7777b..989c79d624 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverter.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverter.java @@ -11,19 +11,26 @@ */ package org.eclipse.che.workspace.infrastructure.kubernetes.provision.env; +import static java.util.function.Function.identity; +import static java.util.stream.Collectors.toMap; + import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.EnvVar; +import java.util.List; +import java.util.Map; import javax.inject.Singleton; import org.eclipse.che.api.core.model.workspace.config.MachineConfig; import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; import org.eclipse.che.api.workspace.server.spi.InfrastructureException; import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; import org.eclipse.che.commons.annotation.Traced; +import org.eclipse.che.commons.lang.TopologicalSort; import org.eclipse.che.commons.tracing.TracingTags; import org.eclipse.che.workspace.infrastructure.kubernetes.Names; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment.PodData; import org.eclipse.che.workspace.infrastructure.kubernetes.provision.ConfigurationProvisioner; +import org.eclipse.che.workspace.infrastructure.kubernetes.util.EnvVars; /** * Converts environment variables in {@link MachineConfig} to Kubernetes environment variables. @@ -32,6 +39,10 @@ import org.eclipse.che.workspace.infrastructure.kubernetes.provision.Configurati */ @Singleton public class EnvVarsConverter implements ConfigurationProvisioner { + + private final TopologicalSort topoSort = + new TopologicalSort<>(EnvVar::getName, EnvVars::extractReferencedVariables); + @Override @Traced public void provision(KubernetesEnvironment k8sEnv, RuntimeIdentity identity) @@ -43,13 +54,29 @@ public class EnvVarsConverter implements ConfigurationProvisioner { for (Container container : pod.getSpec().getContainers()) { String machineName = Names.machineName(pod, container); InternalMachineConfig machineConf = k8sEnv.getMachines().get(machineName); - machineConf - .getEnv() - .forEach( - (key, value) -> { - container.getEnv().removeIf(env -> key.equals(env.getName())); - container.getEnv().add(new EnvVar(key, value, null)); - }); + + // we need to combine the env vars from the machine config with the variables already + // present in the container. Let's key the variables by name and use the map for merging + Map envVars = + machineConf + .getEnv() + .entrySet() + .stream() + .map(e -> new EnvVar(e.getKey(), e.getValue(), null)) + .collect(toMap(EnvVar::getName, identity())); + + // the env vars defined in our machine config take precedence over the ones already defined + // in the container, if any + container.getEnv().forEach(v -> envVars.putIfAbsent(v.getName(), v)); + + // The environment variable interpolation only works if a variable that is referenced + // is already defined earlier in the list of environment variables. + // We need to produce a list where variables that reference others always appear later + // in the list. + + List sorted = topoSort.sort(envVars.values()); + + container.setEnv(sorted); } } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java new file mode 100644 index 0000000000..a1fb9ffcc9 --- /dev/null +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVars.java @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.util; + +import io.fabric8.kubernetes.api.model.EnvVar; +import java.util.HashSet; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** Utility class for dealing with environment variables */ +public class EnvVars { + + private static final Pattern REFERENCE_PATTERN = Pattern.compile("\\$\\(\\w+\\)"); + + private EnvVars() {} + + /** + * Looks at the value of the provided environment variable and returns a set of environment + * variable references in the Kubernetes convention of {@literal $(VAR_NAME)}. + * + *

See API + * docs and/or documentation. + * + * @param var the environment variable to analyze + * @return a set of variable references, never null + */ + public static Set extractReferencedVariables(EnvVar var) { + String val = var.getValue(); + + Matcher matcher = REFERENCE_PATTERN.matcher(val); + + // let's just keep the initial size small, because usually there are not that many references + // present. + Set ret = new HashSet<>(2); + + while (matcher.find()) { + int start = matcher.start(); + + // the variable reference can be escaped using a double $, e.g. $$(VAR) is not a reference + if (start > 0 && val.charAt(start - 1) == '$') { + continue; + } + + // extract the variable name out of the reference $(NAME) -> NAME + String refName = matcher.group().substring(2, matcher.group().length() - 1); + ret.add(refName); + } + + return ret; + } +} diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverterTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverterTest.java new file mode 100644 index 0000000000..753586fbcb --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/provision/env/EnvVarsConverterTest.java @@ -0,0 +1,108 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.provision.env; + +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static org.testng.Assert.assertEquals; + +import io.fabric8.kubernetes.api.model.Container; +import io.fabric8.kubernetes.api.model.EnvVar; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.api.model.Pod; +import io.fabric8.kubernetes.api.model.PodSpec; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity; +import org.eclipse.che.api.workspace.server.model.impl.RuntimeIdentityImpl; +import org.eclipse.che.api.workspace.server.spi.InfrastructureException; +import org.eclipse.che.api.workspace.server.spi.environment.InternalMachineConfig; +import org.eclipse.che.workspace.infrastructure.kubernetes.Names; +import org.eclipse.che.workspace.infrastructure.kubernetes.environment.KubernetesEnvironment; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +public class EnvVarsConverterTest { + + private static final String PRE_EXISTING_VAR = "VAR_THAT_EXISTS"; + private static final String PRE_EXISTING_VAR_VALUE = "jmenuju se VAR"; + private static final String PRE_EXISTING_VAR_NEW_VALUE = "my name is VAR"; + private static final String A_VAR = "A"; + private static final String A_VAL = "$(C)"; + private static final String B_VAR = "B"; + private static final String B_VAL = "b"; + private static final String C_VAR = "C"; + private static final String C_VAL = "c"; + + private KubernetesEnvironment environment; + private RuntimeIdentity identity; + private Container testContainer; + private InternalMachineConfig machine; + + @BeforeMethod + public void setUp() { + testContainer = new Container(); + + PodSpec podSpec = new PodSpec(); + podSpec.setContainers(singletonList(testContainer)); + + ObjectMeta podMeta = new ObjectMeta(); + podMeta.setName("pod"); + + Pod pod = new Pod(); + pod.setSpec(podSpec); + pod.setMetadata(podMeta); + + Map pods = new HashMap<>(); + pods.put("pod", pod); + + environment = KubernetesEnvironment.builder().setPods(pods).build(); + + machine = new InternalMachineConfig(); + + environment.setMachines( + Collections.singletonMap(Names.machineName(podMeta, testContainer), machine)); + + identity = new RuntimeIdentityImpl("wsId", "blah", "bleh"); + } + + @Test + public void shouldProvisionEnvironmentVariablesSorted() throws InfrastructureException { + // given + List preExistingEnvironment = new ArrayList<>(); + preExistingEnvironment.add(new EnvVar(PRE_EXISTING_VAR, PRE_EXISTING_VAR_VALUE, null)); + testContainer.setEnv(preExistingEnvironment); + + machine.getEnv().put(PRE_EXISTING_VAR, PRE_EXISTING_VAR_NEW_VALUE); + machine.getEnv().put(A_VAR, A_VAL); + machine.getEnv().put(B_VAR, B_VAL); + machine.getEnv().put(C_VAR, C_VAL); + + // when + EnvVarsConverter converter = new EnvVarsConverter(); + converter.provision(environment, identity); + + // then + EnvVar expectedA = new EnvVar(A_VAR, A_VAL, null); + EnvVar expectedB = new EnvVar(B_VAR, B_VAL, null); + EnvVar expectedC = new EnvVar(C_VAR, C_VAL, null); + EnvVar expectedPreExisting = new EnvVar(PRE_EXISTING_VAR, PRE_EXISTING_VAR_NEW_VALUE, null); + + List expectedOrder = asList(expectedB, expectedC, expectedA, expectedPreExisting); + + assertEquals(4, testContainer.getEnv().size()); + assertEquals(expectedOrder, testContainer.getEnv()); + } +} diff --git a/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVarsTest.java b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVarsTest.java new file mode 100644 index 0000000000..129e7c8eff --- /dev/null +++ b/infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/util/EnvVarsTest.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2012-2018 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.workspace.infrastructure.kubernetes.util; + +import static java.util.Collections.emptySet; +import static java.util.Collections.singleton; +import static org.eclipse.che.workspace.infrastructure.kubernetes.util.EnvVars.extractReferencedVariables; +import static org.testng.Assert.assertEquals; + +import io.fabric8.kubernetes.api.model.EnvVar; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class EnvVarsTest { + + @Test(dataProvider = "detectReferencesTestValues") + public void shouldDetectReferences(String value, Set expected, String caseName) { + assertEquals( + extractReferencedVariables(var("name", value)), expected, caseName + ": just value"); + assertEquals( + extractReferencedVariables(var("name", "v" + value)), + expected, + caseName + ": value with prefix"); + assertEquals( + extractReferencedVariables(var("name", "v" + value + "v")), + expected, + caseName + ": value with prefix and postfix"); + assertEquals( + extractReferencedVariables(var("name", value + "v")), + expected, + caseName + ": value with postfix"); + } + + @DataProvider + public static Object[][] detectReferencesTestValues() { + return new Object[][] { + new Object[] {"value", emptySet(), "no refs"}, + new Object[] {"$(NO_REF", emptySet(), "unclosed ref"}, + new Object[] {"$$(NO_REF)", emptySet(), "escaped ref"}, + new Object[] {"$NO_REF)", emptySet(), "invalid start ref"}, + new Object[] {"$(NO REF)", emptySet(), "invalid name ref"}, + new Object[] {"$(REF)", singleton("REF"), "valid ref"} + }; + } + + @Test + public void shouldDetectMultipleReferences() throws Exception { + // given + EnvVar envVar = var("a", "$(b) $(c) $$(d)"); + + // when + Set refs = extractReferencedVariables(envVar); + + // then + assertEquals(refs, new HashSet<>(Arrays.asList("b", "c"))); + } + + private static EnvVar var(String name, String value) { + return new EnvVar(name, value, null); + } +}