Skip to content

Commit

Permalink
fix (jkube-kit/enricher) : YAML multiline annotations incorrectly ser…
Browse files Browse the repository at this point in the history
…ialized on windows

Related to #2841

Currently for enforcing trailing newline in case of multi line strings
in resource annotations, we were relying on presence of `System.lineSeparator()`.

This was causing problems as YAML multiline scalar values are converted
into strings with Line Feed (`\n`) endings when they're deserialized
into objects. Modify logic in MetadataVisitor with respect to this
behavior.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia committed May 3, 2024
1 parent dc0ce29 commit c399390
Showing 3 changed files with 97 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.openshift.api.model.Template;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

@@ -215,4 +217,79 @@ void saveYaml_withConfigMap_savesFile(@TempDir Path targetDir) throws IOExceptio
"data:%n" +
" key: value%n"));
}

@Nested
@DisplayName("Multi-line strings are serialized to YAML using scalar blocks")
class MultiLineStringsSerializedToScalarYamlBlocks {
@Test
@DisplayName("string ends with newline, then add | to use block style in serialized object")
void whenStringEndingWithNewline_thenAddBlockIndicatorInSerializedObject(@TempDir Path targetDir) throws IOException {
// Given
final File targetFile = targetDir.resolve("cm.yaml").toFile();
final ConfigMap source = new ConfigMapBuilder()
.withNewMetadata()
.addToAnnotations("proxy.istio.io/config", String.format("proxyMetadata:%n ISTIO_META_DNS_CAPTURE: \"false\"%nholdApplicationUntilProxyStarts: true\n"))
.endMetadata()
.build();
// When
Serialization.saveYaml(targetFile, source);
// Then
assertThat(targetFile)
.content()
.isEqualTo(String.format("---%n" +
"apiVersion: v1%n" +
"kind: ConfigMap%n" +
"metadata:%n" +
" annotations:%n" +
" proxy.istio.io/config: |%n" +
" proxyMetadata:%n" +
" ISTIO_META_DNS_CAPTURE: \"false\"%n"+
" holdApplicationUntilProxyStarts: true%n"));
}

@Test
@DisplayName("when string contains windows line breaks, then convert then to unix line breaks during deserialization")
void unmarshal_withWindowsLineEndings_shouldDeserializeMultilineStringWithLineFeeds() {
// Given
String input = "apiVersion: v1\r\n" +
"kind: ConfigMap\r\n" +
"metadata:\r\n" +
" annotations:\r\n" +
" proxy.istio.io/config: |\r\n" +
" proxyMetadata:\r\n" +
" ISTIO_META_DNS_CAPTURE: \"false\"\r\n"+
" holdApplicationUntilProxyStarts: true\r\n";

// When
ConfigMap configMap = Serialization.unmarshal(input, ConfigMap.class);

// Then
assertThat(configMap.getMetadata().getAnnotations())
.containsEntry("proxy.istio.io/config", "proxyMetadata:\n" +
" ISTIO_META_DNS_CAPTURE: \"false\"\n" +
"holdApplicationUntilProxyStarts: true\n");
}

@Test
@DisplayName("when string contains unix line breaks, then line breaks remain unchanged during deserialization")
void unmarshal_withUnixLineEndings_shouldDeserializeMultilineStringWithLineFeeds() {
// Given
String input = "apiVersion: v1\n" +
"kind: ConfigMap\n" +
"metadata:\n" +
" annotations:\n" +
" proxy.istio.io/config: |\n" +
" proxyMetadata:\n" +
" ISTIO_META_DNS_CAPTURE: \"false\"\n"+
" holdApplicationUntilProxyStarts: true\n";
// When
ConfigMap configMap = Serialization.unmarshal(input, ConfigMap.class);

// Then
assertThat(configMap.getMetadata().getAnnotations())
.containsEntry("proxy.istio.io/config", "proxyMetadata:\n" +
" ISTIO_META_DNS_CAPTURE: \"false\"\n" +
"holdApplicationUntilProxyStarts: true\n");
}
}
}
Original file line number Diff line number Diff line change
@@ -20,6 +20,8 @@
import java.util.Properties;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.eclipse.jkube.kit.config.resource.MetaDataConfig;
import org.eclipse.jkube.kit.config.resource.ResourceConfig;
@@ -73,6 +75,7 @@ public class MetadataVisitor<T extends VisitableBuilder> extends TypedVisitor<T>
private final Supplier<Properties> labelSupplier;
private final Function<T, ObjectMetaFluent<?>> objectMeta;
private final Function<ObjectMetaFluent<?>, Runnable> endMetadata;
private static final Pattern CONTAINS_LINE_BREAK = Pattern.compile("\r?\n");

public MetadataVisitor(
Class<T> clazz,
@@ -112,8 +115,13 @@ private static MetaDataConfig getLabels(ResourceConfig resourceConfig) {
}

private String appendTrailingNewLineIfMultiline(String value) {
if (value.contains(System.lineSeparator()) && !value.endsWith(System.lineSeparator())) {
return value + System.lineSeparator();
Matcher m = CONTAINS_LINE_BREAK.matcher(value);
String matchedLineBreak = null;
while (m.find()) {
matchedLineBreak = m.group();
}
if (matchedLineBreak != null && !value.endsWith("\n")) {
return value.replaceAll(matchedLineBreak, "\n") + "\n";
}
return value;
}
Original file line number Diff line number Diff line change
@@ -38,6 +38,8 @@
import io.fabric8.openshift.api.model.RouteBuilder;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.data.MapEntry.entry;
@@ -350,12 +352,16 @@ void route() {
assertThat(route.build().getMetadata().getLabels()).containsOnly(entry("route", "Yay"));
}

@Test
void metadataVisit_whenMultilineAnnotationProvided_shouldAddTrailingNewline() {
@ParameterizedTest
@ValueSource(strings = {
"proxyMetadata:\n ISTIO_META_DNS_CAPTURE: \"false\"\nholdUntilProxyStarts: true",
"proxyMetadata:\r\n ISTIO_META_DNS_CAPTURE: \"false\"\r\nholdUntilProxyStarts: true"
})
void metadataVisit_whenMultilineAnnotationProvided_shouldAddTrailingNewline(String multiLineScalar) {
// Given
Properties allProps = new Properties();
final ObjectMetaBuilder db = new ObjectMetaBuilder();
allProps.put("multiline/config", String.format("proxyMetadata:%n ISTIO_META_DNS_CAPTURE: \"false\"%nholdUntilProxyStarts: true"));
allProps.put("multiline/config", String.format(multiLineScalar));
ResourceConfig rc = ResourceConfig.builder()
.annotations(MetaDataConfig.builder()
.all(allProps)
@@ -367,6 +373,6 @@ void metadataVisit_whenMultilineAnnotationProvided_shouldAddTrailingNewline() {

// Then
assertThat(db.build().getAnnotations())
.containsOnly(entry("multiline/config", String.format("proxyMetadata:%n ISTIO_META_DNS_CAPTURE: \"false\"%nholdUntilProxyStarts: true%n")));
.containsOnly(entry("multiline/config", "proxyMetadata:\n ISTIO_META_DNS_CAPTURE: \"false\"\nholdUntilProxyStarts: true\n"));
}
}

0 comments on commit c399390

Please sign in to comment.