Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow configuring max aliases for a collection #1375

Merged
merged 8 commits into from
May 24, 2020
Merged

Allow configuring max aliases for a collection #1375

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
80b822b
increase max aliases for collection
jetersen May 8, 2020
57aebdd
Move ConfigurationContext one level up to enable configurable max ali…
jetersen May 12, 2020
3b61d7e
Merge branch 'master' into fix/1374
jetersen May 24, 2020
d2a3d3f
add test for max aliases in collections
jetersen May 24, 2020
e05453a
add test for invalid input
jetersen May 24, 2020
6a5a841
provide a more helpful error message
jetersen May 24, 2020
b5bf2a8
fix imports
jetersen May 24, 2020
8455a55
check the cause message
jetersen May 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion plugin/src/main/java/io/jenkins/plugins/casc/yaml/YamlUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.logging.Logger;
import org.yaml.snakeyaml.LoaderOptions;
import org.yaml.snakeyaml.composer.Composer;
import org.yaml.snakeyaml.error.YAMLException;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
import org.yaml.snakeyaml.nodes.NodeId;
Expand Down Expand Up @@ -58,7 +59,17 @@ public static Node read(YamlSource source, ConfigurationContext context) throws
new ParserImpl(new StreamReaderWithSource(source)),
new Resolver(),
loaderOptions);
return composer.getSingleNode();
try {
return composer.getSingleNode();
} catch (YAMLException e) {
if (e.getMessage().startsWith("Number of aliases for non-scalar nodes exceeds the specified max")) {
throw new ConfiguratorException(String.format(
"%s%nYou can increase the maximum by setting an environment variable or property%n ENV: %s=\"100\"%n PROPERTY: -D%s=\"100\"",
e.getMessage(), ConfigurationContext.CASC_YAML_MAX_ALIASES_ENV,
ConfigurationContext.CASC_YAML_MAX_ALIASES_PROPERTY));
}
throw e;
}
}

private static void merge(Node root, Node node, String source) throws ConfiguratorException {
Expand Down
5 changes: 2 additions & 3 deletions test-harness/src/test/java/io/jenkins/plugins/casc/YamlMaxAliasesCollection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public class YamlMaxAliasesCollection {

private EnvVarsRule env;


@Rule
public RuleChain rc = RuleChain.outerRule(env = new EnvVarsRule())
.around(new RestoreSystemProperties())
Expand All @@ -27,14 +26,14 @@ public class YamlMaxAliasesCollection {
@Test
public void testAMaxOfOneEnv() {
env.set(ConfigurationContext.CASC_YAML_MAX_ALIASES_ENV, "1");
assertThrows(YAMLException.class, () ->
assertThrows(ConfiguratorException.class, () ->
Copy link
Member

Choose a reason for hiding this comment

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

Worth checking the message here?

Copy link
Member Author

Choose a reason for hiding this comment

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

assertThrows does not allow for asserting the exception message.

Copy link
Member

Choose a reason for hiding this comment

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

It returns the exception and then you can assert on it

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh it returns the exception so sure 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

The stack trace somewhat hides it but it is still visible.

io.jenkins.plugins.casc.ConfiguratorException: Failed to read YamlSource: file:/C:/git/code/configuration.as.code.plugin/test-harness/target/test-classes/io/jenkins/plugins/casc/maxAliasesLimit.yml
	at io.jenkins.plugins.casc.yaml.YamlUtils.merge(YamlUtils.java:48)
	at io.jenkins.plugins.casc.yaml.YamlUtils.loadFrom(YamlUtils.java:127)
	at io.jenkins.plugins.casc.ConfigurationAsCode.configureWith(ConfigurationAsCode.java:618)
	at io.jenkins.plugins.casc.ConfigurationAsCode.configure(ConfigurationAsCode.java:587)
	at io.jenkins.plugins.casc.ConfigurationAsCode.configure(ConfigurationAsCode.java:576)
	at io.jenkins.plugins.casc.YamlMaxAliasesCollection.lambda$testAMaxOfOneProp$1(YamlMaxAliasesCollection.java:42)
	at org.junit.Assert.assertThrows(Assert.java:1001)
	at org.junit.Assert.assertThrows(Assert.java:981)
	at io.jenkins.plugins.casc.YamlMaxAliasesCollection.testAMaxOfOneProp(YamlMaxAliasesCollection.java:40)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:597)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)
Caused by: io.jenkins.plugins.casc.ConfiguratorException: Number of aliases for non-scalar nodes exceeds the specified max=1
You can increase the maximum by setting an environment variable or property
  ENV: CASC_YAML_MAX_ALIASES="100"
  PROPERTY: -Dcasc.yaml.max.aliases="100"
	at io.jenkins.plugins.casc.yaml.YamlUtils.read(YamlUtils.java:66)
	at io.jenkins.plugins.casc.yaml.YamlUtils.merge(YamlUtils.java:38)
	... 21 more

ConfigurationAsCode.get().configure(getClass().getResource("maxAliasesLimit.yml").toExternalForm()));
}

@Test
public void testAMaxOfOneProp() {
System.setProperty(ConfigurationContext.CASC_YAML_MAX_ALIASES_PROPERTY, "1");
assertThrows(YAMLException.class, () ->
assertThrows(ConfiguratorException.class, () ->
ConfigurationAsCode.get().configure(getClass().getResource("maxAliasesLimit.yml").toExternalForm()));
}

Expand Down