From 073c85cf47714bc6a1d08a63960079880374b4c5 Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Wed, 25 May 2022 09:53:16 +0200 Subject: [PATCH 1/6] Add variable substitution in ApplicationManifestUtils --- .../ApplicationManifestUtils.java | 136 +++++++++++------- .../ApplicationManifestUtilsTest.java | 16 +++ .../test/resources/fixtures/manifest-papa.yml | 6 + .../src/test/resources/fixtures/vars-papa.yml | 2 + 4 files changed, 109 insertions(+), 51 deletions(-) create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml diff --git a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java index b8f9360c01..24fc3879cc 100644 --- a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java +++ b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java @@ -42,6 +42,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static java.util.Collections.emptyMap; +import static java.util.stream.Collectors.toMap; + /** * Utilities for dealing with {@link ApplicationManifest}s. Includes the functionality to transform to and from standard CLI YAML files. */ @@ -70,7 +73,24 @@ private ApplicationManifestUtils() { * @return the resolved manifests */ public static List read(Path path) { - return doRead(path.toAbsolutePath()); + return doRead(path.toAbsolutePath(), emptyMap()); + } + + /** + * Reads a YAML manifest file (defined by the CLI) from a {@link Path} and converts it into a collection of {@link + * ApplicationManifest}s. Note that all resolution (both inheritance and common) is performed during read. + * + * @param path the path to read from + * @param variablesPath use variable substitution (described in Add Variables to a Manifest) + * @return the resolved manifests + */ + public static List read(Path path, Path variablesPath) { + Map variables = deserialize(variablesPath.toAbsolutePath()) + .entrySet() + .stream() + .collect(toMap(e -> String.format("\\(\\(%s\\)\\)", e.getKey()), e -> String.valueOf(e.getValue()))); + + return doRead(path.toAbsolutePath(), variables); } /** @@ -125,21 +145,30 @@ public static void write(OutputStream out, List application } } - private static void as(Map payload, String key, Function mapper, Consumer consumer) { + private static void as(Map payload, String key, Map variables, Function mapper, Consumer consumer) { Optional.ofNullable(payload.get(key)) + .map(o -> { + if(o instanceof String) { + String result = ((String) o); + for(Map.Entry e : variables.entrySet()) { + result = result.replaceAll(e.getKey(), e.getValue()); + } + return result; + } + return o; + }) .map(mapper) .ifPresent(consumer); } - private static void asBoolean(Map payload, String key, Consumer consumer) { - as(payload, key, Boolean.class::cast, consumer); + private static void asBoolean(Map payload, String key, Map variables, Consumer consumer) { + as(payload, key, variables, Boolean.class::cast, consumer); } @SuppressWarnings("unchecked") - private static void asDocker(Map payload, String key, Consumer consumer) { - as(payload, key, value -> { + private static void asDocker(Map payload, String key, Map variables, Consumer consumer) { + as(payload, key, variables, value -> { Map docker = ((Map) value); - return Docker.builder() .image(docker.get("image")) .password(docker.get("password")) @@ -148,35 +177,40 @@ private static void asDocker(Map payload, String key, Consumer payload, String key, Consumer consumer) { - as(payload, key, Integer.class::cast, consumer); + private static void asInteger(Map payload, String key, Map variables, Consumer consumer) { + as(payload, key, variables, (e) -> { + if(e instanceof String) { + return Integer.parseInt((String)e); + } + return (Integer) e; + }, consumer); } @SuppressWarnings("unchecked") - private static void asList(Map payload, String key, Function mapper, Consumer consumer) { - as(payload, key, value -> ((List) value).stream(), + private static void asList(Map payload, String key, Map variables, Function mapper, Consumer consumer) { + as(payload, key, variables, value -> ((List) value).stream(), values -> values .map(mapper) .forEach(consumer)); } - private static void asListOfString(Map payload, String key, Consumer consumer) { - asList(payload, key, String.class::cast, consumer); + private static void asListOfString(Map payload, String key, Map variables, Consumer consumer) { + asList(payload, key, variables, String.class::cast, consumer); } @SuppressWarnings("unchecked") - private static void asMap(Map payload, String key, Function valueMapper, Consumer2 consumer) { - as(payload, key, value -> ((Map) value), + private static void asMap(Map payload, String key, Map variables, Function valueMapper, Consumer2 consumer) { + as(payload, key, variables, value -> ((Map) value), values -> values.forEach((k, v) -> consumer.accept(k, valueMapper.apply(v)))); } - private static void asMapOfStringString(Map payload, String key, Consumer2 consumer) { - asMap(payload, key, String::valueOf, consumer); + private static void asMapOfStringString(Map payload, String key, Map variables, Consumer2 consumer) { + asMap(payload, key, variables, String::valueOf, consumer); } @SuppressWarnings("unchecked") - private static void asMemoryInteger(Map payload, String key, Consumer consumer) { - as(payload, key, raw -> { + private static void asMemoryInteger(Map payload, String key, Map variables, Consumer consumer) { + as(payload, key, variables, raw -> { if (raw instanceof Integer) { return (Integer) raw; } else if (raw instanceof String) { @@ -199,8 +233,8 @@ private static void asMemoryInteger(Map payload, String key, Con }, consumer); } - private static void asString(Map payload, String key, Consumer consumer) { - as(payload, key, String.class::cast, consumer); + private static void asString(Map payload, String key, Map variables, Consumer consumer) { + as(payload, key, variables, String.class::cast, consumer); } @SuppressWarnings("unchecked") @@ -213,7 +247,7 @@ private static Map deserialize(Path path) { throw Exceptions.propagate(e); } - asString(root.get(), "inherit", inherit -> { + asString(root.get(), "inherit", emptyMap(), inherit -> { Map inherited = deserialize(path.getParent().resolve(inherit)); merge(inherited, root.get()); root.set(inherited); @@ -223,17 +257,17 @@ private static Map deserialize(Path path) { } @SuppressWarnings("unchecked") - private static List doRead(Path path) { + private static List doRead(Path path, Map variables) { Map root = deserialize(path); - ApplicationManifest template = getTemplate(path, root); + ApplicationManifest template = getTemplate(path, root, variables); return Optional.ofNullable(root.get("applications")) .map(value -> ((List>) value).stream()) .orElseGet(Stream::empty) .map(application -> { String name = getName(application); - return toApplicationManifest(application, ApplicationManifest.builder().from(template), path) + return toApplicationManifest(application, variables, ApplicationManifest.builder().from(template), path) .name(name) .build(); }) @@ -265,8 +299,8 @@ private static Route getRoute(Map raw) { return Route.builder().route(route).build(); } - private static ApplicationManifest getTemplate(Path path, Map root) { - return toApplicationManifest(root, ApplicationManifest.builder(), path) + private static ApplicationManifest getTemplate(Path path, Map root, Map variables) { + return toApplicationManifest(root, variables, ApplicationManifest.builder(), path) .name("template") .build(); } @@ -322,30 +356,30 @@ private static void putIfPresent(Map yaml, String key, T val } @SuppressWarnings("unchecked") - private static ApplicationManifest.Builder toApplicationManifest(Map application, ApplicationManifest.Builder builder, Path root) { - asListOfString(application, "buildpacks", builder::buildpacks); - asString(application, "buildpack", builder::buildpacks); - asString(application, "command", builder::command); - asMemoryInteger(application, "disk_quota", builder::disk); - asDocker(application, "docker", builder::docker); - asString(application, "domain", builder::domain); - asListOfString(application, "domains", builder::domain); - asMapOfStringString(application, "env", builder::environmentVariable); - asString(application, "health-check-http-endpoint", builder::healthCheckHttpEndpoint); - asString(application, "health-check-type", healthCheckType -> builder.healthCheckType(ApplicationHealthCheck.from(healthCheckType))); - asString(application, "host", builder::host); - asListOfString(application, "hosts", builder::host); - asInteger(application, "instances", builder::instances); - asMemoryInteger(application, "memory", builder::memory); - asString(application, "name", builder::name); - asBoolean(application, "no-hostname", builder::noHostname); - asBoolean(application, "no-route", builder::noRoute); - asString(application, "path", path -> builder.path(root.getParent().resolve(path))); - asBoolean(application, "random-route", builder::randomRoute); - asList(application, "routes", raw -> getRoute((Map) raw), builder::route); - asListOfString(application, "services", builder::service); - asString(application, "stack", builder::stack); - asInteger(application, "timeout", builder::timeout); + private static ApplicationManifest.Builder toApplicationManifest(Map application, Map variables, ApplicationManifest.Builder builder, Path root) { + asListOfString(application, "buildpacks", variables, builder::buildpacks); + asString(application, "buildpack", variables, builder::buildpacks); + asString(application, "command", variables, builder::command); + asMemoryInteger(application, "disk_quota", variables, builder::disk); + asDocker(application, "docker", variables, builder::docker); + asString(application, "domain", variables, builder::domain); + asListOfString(application, "domains", variables, builder::domain); + asMapOfStringString(application, "env", variables, builder::environmentVariable); + asString(application, "health-check-http-endpoint", variables, builder::healthCheckHttpEndpoint); + asString(application, "health-check-type", variables, healthCheckType -> builder.healthCheckType(ApplicationHealthCheck.from(healthCheckType))); + asString(application, "host", variables, builder::host); + asListOfString(application, "hosts", variables, builder::host); + asInteger(application, "instances", variables, builder::instances); + asMemoryInteger(application, "memory", variables, builder::memory); + asString(application, "name", variables, builder::name); + asBoolean(application, "no-hostname", variables, builder::noHostname); + asBoolean(application, "no-route", variables, builder::noRoute); + asString(application, "path", variables, path -> builder.path(root.getParent().resolve(path))); + asBoolean(application, "random-route", variables, builder::randomRoute); + asList(application, "routes", variables, raw -> getRoute((Map) raw), builder::route); + asListOfString(application, "services", variables, builder::service); + asString(application, "stack", variables, builder::stack); + asInteger(application, "timeout", variables, builder::timeout); return builder; } diff --git a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java index b6047f0a87..eeef825c3c 100644 --- a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java +++ b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java @@ -455,6 +455,22 @@ public void readSingleBuildpack() throws IOException { assertThat(actual).isEqualTo(expected); } + @Test + public void readWithVariableSubstitution() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-application") + .buildpack("papa-buildpack") + .instances(2) + .memory(1024) + .build()); + + List actual = ApplicationManifestUtils.read(new ClassPathResource("fixtures/manifest-papa.yml").getFile().toPath(), new ClassPathResource("fixtures/vars-papa.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } + + @Test public void unixRead() throws IOException { assumeTrue(SystemUtils.IS_OS_UNIX); diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml new file mode 100644 index 0000000000..6f85d3ee0d --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml @@ -0,0 +1,6 @@ +--- +applications: +- name: papa-application + buildpack: papa-buildpack + instances: ((instances)) + memory: ((memory)) \ No newline at end of file diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml new file mode 100644 index 0000000000..69eb22a8fc --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml @@ -0,0 +1,2 @@ +instances: 2 +memory: 1G From 5ed5eea1f1fe3bf17b0b1345c80f987fce8bdbca Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Wed, 25 May 2022 10:30:41 +0200 Subject: [PATCH 2/6] Sanitize regex input --- .../operations/applications/ApplicationManifestUtils.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java index 24fc3879cc..5487ecb604 100644 --- a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java +++ b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java @@ -43,6 +43,7 @@ import java.util.stream.Stream; import static java.util.Collections.emptyMap; +import static java.util.regex.Pattern.quote; import static java.util.stream.Collectors.toMap; /** @@ -88,7 +89,7 @@ public static List read(Path path, Path variablesPath) { Map variables = deserialize(variablesPath.toAbsolutePath()) .entrySet() .stream() - .collect(toMap(e -> String.format("\\(\\(%s\\)\\)", e.getKey()), e -> String.valueOf(e.getValue()))); + .collect(toMap(e -> String.format("\\(\\(%s\\)\\)", quote(e.getKey())), e -> String.valueOf(e.getValue()))); return doRead(path.toAbsolutePath(), variables); } From e5b2dd4ffb9009659ef7c4c350b6d655ad460a61 Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Thu, 26 May 2022 08:56:09 +0200 Subject: [PATCH 3/6] Add test for variable substitution in manifest --- .../ApplicationManifestUtilsTest.java | 53 ++++++++++++++++++- ...{manifest-papa.yml => manifest-papa-1.yml} | 2 +- .../resources/fixtures/manifest-papa-2.yml | 4 ++ .../resources/fixtures/manifest-papa-3.yml | 4 ++ .../resources/fixtures/manifest-papa-4.yml | 4 ++ .../{vars-papa.yml => vars-papa-1.yml} | 0 .../test/resources/fixtures/vars-papa-2.yml | 2 + .../test/resources/fixtures/vars-papa-3.yml | 2 + .../test/resources/fixtures/vars-papa-4.yml | 2 + 9 files changed, 70 insertions(+), 3 deletions(-) rename cloudfoundry-operations/src/test/resources/fixtures/{manifest-papa.yml => manifest-papa-1.yml} (77%) create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-2.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-3.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-4.yml rename cloudfoundry-operations/src/test/resources/fixtures/{vars-papa.yml => vars-papa-1.yml} (100%) create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-2.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-3.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-4.yml diff --git a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java index eeef825c3c..e6d5679b9c 100644 --- a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java +++ b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java @@ -31,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.cloudfoundry.operations.applications.ApplicationHealthCheck.NONE; import static org.cloudfoundry.operations.applications.ApplicationHealthCheck.PORT; +import static org.junit.Assert.assertEquals; import static org.junit.Assume.assumeTrue; public final class ApplicationManifestUtilsTest { @@ -459,17 +460,65 @@ public void readSingleBuildpack() throws IOException { public void readWithVariableSubstitution() throws IOException { List expected = Collections.singletonList( ApplicationManifest.builder() - .name("papa-application") + .name("papa-1-application") .buildpack("papa-buildpack") .instances(2) .memory(1024) .build()); - List actual = ApplicationManifestUtils.read(new ClassPathResource("fixtures/manifest-papa.yml").getFile().toPath(), new ClassPathResource("fixtures/vars-papa.yml").getFile().toPath()); + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-1.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-1.yml").getFile().toPath()); assertThat(actual).isEqualTo(expected); } + @Test + public void readWithVariableSubstitution_regexInVariableKey() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-2-application") + .buildpack("((abcdef))") + .build()); + + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-2.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-2.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void readWithVariableSubstitution_endlessSubstitution() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-3-application") + .buildpack("((endless_2))") + .build()); + + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-3.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-3.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void readWithVariableSubstitution_injectionTest() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-4-application") + .buildpack("injected") + .build()); + + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-4.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-4.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } + + @Test public void unixRead() throws IOException { diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-1.yml similarity index 77% rename from cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml rename to cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-1.yml index 6f85d3ee0d..b49efa0485 100644 --- a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa.yml +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-1.yml @@ -1,6 +1,6 @@ --- applications: -- name: papa-application +- name: papa-1-application buildpack: papa-buildpack instances: ((instances)) memory: ((memory)) \ No newline at end of file diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-2.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-2.yml new file mode 100644 index 0000000000..5daa75e2d4 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-2.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-2-application + buildpack: ((abcdef)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-3.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-3.yml new file mode 100644 index 0000000000..4cfc1a703c --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-3.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-3-application + buildpack: ((endless_1)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-4.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-4.yml new file mode 100644 index 0000000000..04a660142b --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-4.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-4-application + buildpack: ((((sub)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-1.yml similarity index 100% rename from cloudfoundry-operations/src/test/resources/fixtures/vars-papa.yml rename to cloudfoundry-operations/src/test/resources/fixtures/vars-papa-1.yml diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-2.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-2.yml new file mode 100644 index 0000000000..e034648103 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-2.yml @@ -0,0 +1,2 @@ +abc.+: test + diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-3.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-3.yml new file mode 100644 index 0000000000..0bc4e50462 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-3.yml @@ -0,0 +1,2 @@ +endless_1: ((endless_2)) +endless_2: ((endless_1)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-4.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-4.yml new file mode 100644 index 0000000000..9bdb5b86e4 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-4.yml @@ -0,0 +1,2 @@ +sub: test)) +test: injected From 3f3db8005491279fd2f02002ee5d891f2af8b801 Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Mon, 30 May 2022 11:23:49 +0200 Subject: [PATCH 4/6] Fix approach with variable substitution --- .../ApplicationManifestUtils.java | 16 +++++--- .../ApplicationManifestUtilsTest.java | 37 +++++++++++++++++-- .../resources/fixtures/manifest-papa-5.yml | 4 ++ .../resources/fixtures/manifest-papa-6.yml | 4 ++ .../test/resources/fixtures/vars-papa-5.yml | 3 ++ .../test/resources/fixtures/vars-papa-6.yml | 3 ++ 6 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-5.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-6.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-5.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-6.yml diff --git a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java index 5487ecb604..2cef92bf5d 100644 --- a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java +++ b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java @@ -39,6 +39,8 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Function; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -63,6 +65,8 @@ public final class ApplicationManifestUtils { YAML = new Yaml(dumperOptions); } + private static final Pattern VARIABLE = Pattern.compile("\\(\\(([a-zA-Z]\\w+)\\)\\)"); + private ApplicationManifestUtils() { } @@ -89,7 +93,7 @@ public static List read(Path path, Path variablesPath) { Map variables = deserialize(variablesPath.toAbsolutePath()) .entrySet() .stream() - .collect(toMap(e -> String.format("\\(\\(%s\\)\\)", quote(e.getKey())), e -> String.valueOf(e.getValue()))); + .collect(toMap(Map.Entry::getKey,e -> String.valueOf(e.getValue()))); return doRead(path.toAbsolutePath(), variables); } @@ -150,11 +154,13 @@ private static void as(Map payload, String key, Map { if(o instanceof String) { - String result = ((String) o); - for(Map.Entry e : variables.entrySet()) { - result = result.replaceAll(e.getKey(), e.getValue()); + Matcher m = VARIABLE.matcher(((String) o)); + StringBuffer stringBuffer = new StringBuffer(); + while(m.find()){ + m.appendReplacement(stringBuffer, variables.getOrDefault(m.group(1), m.group(0))); } - return result; + m.appendTail(stringBuffer); + return stringBuffer.toString(); } return o; }) diff --git a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java index e6d5679b9c..8650acdce2 100644 --- a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java +++ b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java @@ -474,7 +474,7 @@ public void readWithVariableSubstitution() throws IOException { } @Test - public void readWithVariableSubstitution_regexInVariableKey() throws IOException { + public void readWithVariableSubstitution_dontEvaluateRegexInVariableKey() throws IOException { List expected = Collections.singletonList( ApplicationManifest.builder() .name("papa-2-application") @@ -489,7 +489,7 @@ public void readWithVariableSubstitution_regexInVariableKey() throws IOException } @Test - public void readWithVariableSubstitution_endlessSubstitution() throws IOException { + public void readWithVariableSubstitution_avoidEndlessSubstitution() throws IOException { List expected = Collections.singletonList( ApplicationManifest.builder() .name("papa-3-application") @@ -504,11 +504,11 @@ public void readWithVariableSubstitution_endlessSubstitution() throws IOExceptio } @Test - public void readWithVariableSubstitution_injectionTest() throws IOException { + public void readWithVariableSubstitution_dontAllowInjectionTest() throws IOException { List expected = Collections.singletonList( ApplicationManifest.builder() .name("papa-4-application") - .buildpack("injected") + .buildpack("((test))") .build()); List actual = ApplicationManifestUtils.read( @@ -518,6 +518,35 @@ public void readWithVariableSubstitution_injectionTest() throws IOException { assertThat(actual).isEqualTo(expected); } + @Test + public void readWithVariableSubstitution_addMultipleVariablesInOneField() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-5-application") + .buildpack("one and two is a very nice buildpack name for three") + .build()); + + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-5.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-5.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } + + @Test + public void readWithVariableSubstitution_noSubstitutionAtAll() throws IOException { + List expected = Collections.singletonList( + ApplicationManifest.builder() + .name("papa-6-application") + .buildpack("buildpack_papa_6") + .build()); + + List actual = ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-6.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-6.yml").getFile().toPath()); + + assertThat(actual).isEqualTo(expected); + } @Test diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-5.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-5.yml new file mode 100644 index 0000000000..f979de5982 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-5.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-5-application + buildpack: ((replace_1)) and ((replace_2)) is a very nice buildpack name for ((replace_3)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-6.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-6.yml new file mode 100644 index 0000000000..2cf62b2e96 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-6.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-6-application + buildpack: buildpack_papa_6 diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-5.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-5.yml new file mode 100644 index 0000000000..75397d176d --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-5.yml @@ -0,0 +1,3 @@ +replace_1: one +replace_2: two +replace_3: three diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-6.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-6.yml new file mode 100644 index 0000000000..75397d176d --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-6.yml @@ -0,0 +1,3 @@ +replace_1: one +replace_2: two +replace_3: three From 7fcbd9c599633a7ba09d0e911d84bad8a09a9cd6 Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Mon, 30 May 2022 11:26:11 +0200 Subject: [PATCH 5/6] Rename variables --- .../operations/applications/ApplicationManifestUtils.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java index 2cef92bf5d..33411041e9 100644 --- a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java +++ b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java @@ -45,7 +45,6 @@ import java.util.stream.Stream; import static java.util.Collections.emptyMap; -import static java.util.regex.Pattern.quote; import static java.util.stream.Collectors.toMap; /** @@ -65,7 +64,7 @@ public final class ApplicationManifestUtils { YAML = new Yaml(dumperOptions); } - private static final Pattern VARIABLE = Pattern.compile("\\(\\(([a-zA-Z]\\w+)\\)\\)"); + private static final Pattern FIND_VARIABLE_REGEX = Pattern.compile("\\(\\(([a-zA-Z]\\w+)\\)\\)"); private ApplicationManifestUtils() { } @@ -154,7 +153,7 @@ private static void as(Map payload, String key, Map { if(o instanceof String) { - Matcher m = VARIABLE.matcher(((String) o)); + Matcher m = FIND_VARIABLE_REGEX.matcher(((String) o)); StringBuffer stringBuffer = new StringBuffer(); while(m.find()){ m.appendReplacement(stringBuffer, variables.getOrDefault(m.group(1), m.group(0))); From 512160cf146fa8bf0651ca9be4edd03a7f990cd8 Mon Sep 17 00:00:00 2001 From: luschmar <90399580+luschmar@users.noreply.github.com> Date: Wed, 1 Jun 2022 11:42:35 +0200 Subject: [PATCH 6/6] Throw exception on missing variable substitution --- .../ApplicationManifestUtils.java | 6 +++-- .../ApplicationManifestUtilsTest.java | 23 ++++++++++++++----- .../resources/fixtures/manifest-papa-7.yml | 4 ++++ .../test/resources/fixtures/vars-papa-7.yml | 3 +++ 4 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-7.yml create mode 100644 cloudfoundry-operations/src/test/resources/fixtures/vars-papa-7.yml diff --git a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java index 33411041e9..c5d6afae8c 100644 --- a/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java +++ b/cloudfoundry-operations/src/main/java/org/cloudfoundry/operations/applications/ApplicationManifestUtils.java @@ -34,6 +34,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicReference; @@ -153,10 +154,11 @@ private static void as(Map payload, String key, Map { if(o instanceof String) { - Matcher m = FIND_VARIABLE_REGEX.matcher(((String) o)); + Matcher m = FIND_VARIABLE_REGEX.matcher((String) o); StringBuffer stringBuffer = new StringBuffer(); while(m.find()){ - m.appendReplacement(stringBuffer, variables.getOrDefault(m.group(1), m.group(0))); + m.appendReplacement(stringBuffer, Optional.ofNullable(variables.get(m.group(1))) + .orElseThrow(() -> new NoSuchElementException("Expected to find variable: "+m.group(1)))); } m.appendTail(stringBuffer); return stringBuffer.toString(); diff --git a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java index 8650acdce2..8a3cab6ade 100644 --- a/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java +++ b/cloudfoundry-operations/src/test/java/org/cloudfoundry/operations/applications/ApplicationManifestUtilsTest.java @@ -27,11 +27,14 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.NoSuchElementException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType; import static org.cloudfoundry.operations.applications.ApplicationHealthCheck.NONE; import static org.cloudfoundry.operations.applications.ApplicationHealthCheck.PORT; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.junit.Assume.assumeTrue; public final class ApplicationManifestUtilsTest { @@ -474,20 +477,28 @@ public void readWithVariableSubstitution() throws IOException { } @Test - public void readWithVariableSubstitution_dontEvaluateRegexInVariableKey() throws IOException { + public void readWithVariableSubstitution_throwExceptionOnMissing() throws IOException { + assertThatExceptionOfType(NoSuchElementException.class) + .isThrownBy(() -> { + ApplicationManifestUtils.read( + new ClassPathResource("fixtures/manifest-papa-2.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-2.yml").getFile().toPath()); + }).withMessageMatching("Expected to find variable: abcdef"); + } + @Test + public void readWithVariableSubstitution_dontEvaluateRegex() throws IOException { List expected = Collections.singletonList( ApplicationManifest.builder() - .name("papa-2-application") - .buildpack("((abcdef))") + .name("papa-7-application") + .buildpack("((regex*))") .build()); List actual = ApplicationManifestUtils.read( - new ClassPathResource("fixtures/manifest-papa-2.yml").getFile().toPath(), - new ClassPathResource("fixtures/vars-papa-2.yml").getFile().toPath()); + new ClassPathResource("fixtures/manifest-papa-7.yml").getFile().toPath(), + new ClassPathResource("fixtures/vars-papa-7.yml").getFile().toPath()); assertThat(actual).isEqualTo(expected); } - @Test public void readWithVariableSubstitution_avoidEndlessSubstitution() throws IOException { List expected = Collections.singletonList( diff --git a/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-7.yml b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-7.yml new file mode 100644 index 0000000000..4b34f3e072 --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/manifest-papa-7.yml @@ -0,0 +1,4 @@ +--- +applications: +- name: papa-7-application + buildpack: ((regex*)) diff --git a/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-7.yml b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-7.yml new file mode 100644 index 0000000000..75397d176d --- /dev/null +++ b/cloudfoundry-operations/src/test/resources/fixtures/vars-papa-7.yml @@ -0,0 +1,3 @@ +replace_1: one +replace_2: two +replace_3: three