diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java b/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java index d92b180358..eec44b8e06 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java @@ -241,8 +241,14 @@ public Type getValue(Owner target) throws Exception { public CNode describe(Owner instance, ConfigurationContext context) throws ConfiguratorException { final Configurator c = context.lookup(type); if (c == null) { - return new Scalar("FAILED TO EXPORT\n" + instance.getClass().getName() + "#" + name - + ": No configurator found for type " + type); + String errorMessage = "FAILED TO EXPORT\n" + instance.getClass().getName() + "#" + name + + ": No configurator found for type " + type; + + if (context.isStrictExport()) { + throw new ConfiguratorException(errorMessage); + } + + return new Scalar(errorMessage); } try { Object o = getValue(instance); @@ -268,6 +274,13 @@ public CNode describe(Owner instance, ConfigurationContext context) throws Confi } return _describe(c, context, o, shouldBeMasked); } catch (Exception | /* Jenkins.getDescriptorOrDie */ AssertionError e) { + if (context.isStrictExport()) { + if (e instanceof ConfiguratorException) { + throw (ConfiguratorException) e; + } + throw new ConfiguratorException( + "Failed to export " + instance.getClass().getName() + "#" + name, e); + } // Don't fail the whole export, prefer logging this error LOGGER.log(Level.WARNING, "Failed to export", e); return new Scalar( diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/CasCGlobalConfig.java b/plugin/src/main/java/io/jenkins/plugins/casc/CasCGlobalConfig.java index 61a20c4f89..a1f81f06f8 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/CasCGlobalConfig.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/CasCGlobalConfig.java @@ -15,6 +15,7 @@ public class CasCGlobalConfig extends GlobalConfiguration { private String configurationPath; + private boolean strictExport = false; @DataBoundConstructor public CasCGlobalConfig(String configurationPath) { @@ -40,6 +41,15 @@ public void setConfigurationPath(String configurationPath) { this.configurationPath = configurationPath; } + public boolean isStrictExport() { + return strictExport; + } + + @DataBoundSetter + public void setStrictExport(boolean strictExport) { + this.strictExport = strictExport; + } + @Override public boolean configure(StaplerRequest2 req, JSONObject json) throws FormException { req.bindJSON(this, json); diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java index d3bebfd4a3..fd7c7530ba 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java @@ -595,12 +595,13 @@ public void doReference(StaplerRequest2 req, StaplerResponse2 res) throws Except @Restricted(NoExternalUse.class) public void export(OutputStream out) throws Exception { - final List tuples = new ArrayList<>(); final ConfigurationContext context = new ConfigurationContext(registry); - for (RootElementConfigurator root : RootElementConfigurator.all()) { - final CNode config = root.describe(root.getTargetComponent(context), context); + Optional.ofNullable(GlobalConfiguration.all().get(CasCGlobalConfig.class)) + .ifPresent(c -> context.setStrictExport(c.isStrictExport())); + for (RootElementConfigurator root : RootElementConfigurator.all()) { + final CNode config = describeRoot(root, context); final Node valueNode = toYaml(config); if (valueNode == null) { continue; @@ -616,6 +617,11 @@ public void export(OutputStream out) throws Exception { } } + private CNode describeRoot(RootElementConfigurator root, ConfigurationContext context) throws Exception { + T component = root.getTargetComponent(context); + return root.describe(component, context); + } + @Restricted(NoExternalUse.class) // for testing only public static void serializeYamlNode(Node root, Writer writer) throws IOException { DumperOptions options = new DumperOptions(); diff --git a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java index 77c3d7487a..fa12205f0e 100644 --- a/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java +++ b/plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java @@ -44,6 +44,8 @@ public class ConfigurationContext implements ConfiguratorRegistry { private transient SecretSourceResolver secretSourceResolver; + private boolean strictExport = false; + public ConfigurationContext(ConfiguratorRegistry registry) { this(registry, null); } @@ -126,6 +128,14 @@ public int getYamlCodePointLimit() { return yamlCodePointLimit; } + public boolean isStrictExport() { + return strictExport; + } + + public void setStrictExport(boolean strictExport) { + this.strictExport = strictExport; + } + // --- delegate methods for ConfigurationContext @Override diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java index e8ac478dbf..347140cd9c 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java @@ -1,9 +1,18 @@ package io.jenkins.plugins.casc; +import static java.lang.reflect.Proxy.newProxyInstance; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.util.Secret; +import io.jenkins.plugins.casc.model.CNode; +import io.jenkins.plugins.casc.model.Scalar; +import java.lang.reflect.Type; import java.util.logging.Level; import java.util.logging.Logger; import org.junit.jupiter.api.AfterEach; @@ -230,4 +239,151 @@ void checkCalculationIsIdempotent() { assertFalse(firstUnknown); assertFalse(secondUnknown, "Subsequent calls should return the same fallback FALSE result"); } + + @Test + @SuppressWarnings("ExtractMethodRecommender") + void describeHandlesMissingConfiguratorCorrectly() throws Exception { + ConfiguratorRegistry dummyRegistry = new ConfiguratorRegistry() { + @Override + public RootElementConfigurator lookupRootElement(String name) { + return null; + } + + @Override + @NonNull + public Configurator lookupOrFail(Type type) throws ConfiguratorException { + throw new ConfiguratorException("Not found"); + } + + @Override + public Configurator lookup(Type type) { + return null; + } + }; + + ConfigurationContext context = new ConfigurationContext(dummyRegistry); + + Attribute attr = new Attribute<>("passwordPath", String.class); + NonSecretField dummyInstance = new NonSecretField("my-dummy-path"); + + context.setStrictExport(false); + CNode node = attr.describe(dummyInstance, context); + + assertInstanceOf(Scalar.class, node, "Should return a Scalar node on failure in non-strict mode"); + assertThat( + "Scalar should contain the fallback failure message", + ((Scalar) node).getValue(), + containsString("FAILED TO EXPORT")); + + context.setStrictExport(true); + ConfiguratorException exception = assertThrows( + ConfiguratorException.class, + () -> { + attr.describe(dummyInstance, context); + }, + "Should completely abort and throw ConfiguratorException in strict mode"); + + assertThat(exception.getMessage(), containsString("No configurator found")); + } + + @Test + @SuppressWarnings({"ExtractMethodRecommender", "unchecked"}) + void describeWrapsGenericExceptionsInStrictMode() throws Exception { + + Configurator dummyConfigurator = (Configurator) newProxyInstance( + Configurator.class.getClassLoader(), new Class[] {Configurator.class}, (proxy, method, args) -> { + String methodName = method.getName(); + return switch (methodName) { + case "equals" -> args != null && args.length == 1 && proxy == args[0]; + case "hashCode" -> System.identityHashCode(proxy); + case "toString" -> "DummyConfiguratorProxy"; + case "describe" -> + throw new IllegalStateException("Intentional generic failure from dummy configurator"); + default -> null; + }; + }); + + ConfiguratorRegistry dummyRegistry = new ConfiguratorRegistry() { + @Override + public RootElementConfigurator lookupRootElement(String name) { + return null; + } + + @Override + @NonNull + public Configurator lookupOrFail(Type type) { + return (Configurator) dummyConfigurator; + } + + @Override + public Configurator lookup(Type type) { + return (Configurator) dummyConfigurator; + } + }; + + ConfigurationContext context = new ConfigurationContext(dummyRegistry); + context.setStrictExport(true); + + Attribute attr = new Attribute<>("passwordPath", String.class); + NonSecretField dummyInstance = new NonSecretField("my-dummy-path"); + + ConfiguratorException exception = assertThrows( + ConfiguratorException.class, + () -> { + attr.describe(dummyInstance, context); + }, + "Should wrap generic exception into a ConfiguratorException in strict mode"); + + assertThat( + exception.getMessage(), + containsString("Failed to export io.jenkins.plugins.casc.AttributeTest$NonSecretField#passwordPath")); + assertThat( + exception.getCause().getMessage(), + containsString("Intentional generic failure from dummy configurator")); + } + + @Test + @SuppressWarnings({"ExtractMethodRecommender", "unchecked"}) + void describeRethrowsConfiguratorExceptionInStrictMode() throws Exception { + + Configurator dummyConfigurator = (Configurator) newProxyInstance( + Configurator.class.getClassLoader(), + new Class[] {Configurator.class}, + (proxy, method, args) -> switch (method.getName()) { + case "equals" -> args != null && args.length == 1 && proxy == args[0]; + case "hashCode" -> System.identityHashCode(proxy); + case "toString" -> "DummyConfiguratorProxy"; + case "describe" -> throw new ConfiguratorException("Direct configurator failure"); + default -> null; + }); + + ConfiguratorRegistry dummyRegistry = new ConfiguratorRegistry() { + @Override + public RootElementConfigurator lookupRootElement(String name) { + return null; + } + + @Override + @NonNull + public Configurator lookupOrFail(Type type) { + return (Configurator) dummyConfigurator; + } + + @Override + public Configurator lookup(Type type) { + return (Configurator) dummyConfigurator; + } + }; + + ConfigurationContext context = new ConfigurationContext(dummyRegistry); + context.setStrictExport(true); + + Attribute attr = new Attribute<>("passwordPath", String.class); + + NonSecretField instance = new NonSecretField("dummy"); + + ConfiguratorException ex = assertThrows(ConfiguratorException.class, () -> attr.describe(instance, context)); + + assertThat(ex.getMessage(), containsString("Direct configurator failure")); + } } diff --git a/plugin/src/test/java/io/jenkins/plugins/casc/ConfigurationAsCodeApiTest.java b/plugin/src/test/java/io/jenkins/plugins/casc/ConfigurationAsCodeApiTest.java index 34f0ec7a3e..0b2cf2661e 100644 --- a/plugin/src/test/java/io/jenkins/plugins/casc/ConfigurationAsCodeApiTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/casc/ConfigurationAsCodeApiTest.java @@ -3,11 +3,15 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertFalse; +import java.io.ByteArrayOutputStream; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import jenkins.model.GlobalConfiguration; import jenkins.model.Jenkins; import org.htmlunit.HttpMethod; import org.htmlunit.WebRequest; @@ -185,4 +189,23 @@ public void testDoReplace_ValidSource() throws Exception { assertThat(j.jenkins.getSystemMessage(), is("Hello Replace")); } } + + @Test + public void testExportStrictMode_SuccessOnCleanJenkins() throws Exception { + configureAdminSecurity(); + + CasCGlobalConfig config = GlobalConfiguration.all().get(CasCGlobalConfig.class); + if (config != null) { + config.setStrictExport(true); + } + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + + ConfigurationAsCode.get().export(out); + + String exportedYaml = out.toString(StandardCharsets.UTF_8); + + assertThat(exportedYaml, containsString("jenkins:")); + assertFalse(exportedYaml.contains("FAILED TO EXPORT")); + } } diff --git a/test-harness/src/main/java/io/jenkins/plugins/casc/misc/JenkinsConfiguredRule.java b/test-harness/src/main/java/io/jenkins/plugins/casc/misc/JenkinsConfiguredRule.java index bd7359ceca..364fe7c142 100644 --- a/test-harness/src/main/java/io/jenkins/plugins/casc/misc/JenkinsConfiguredRule.java +++ b/test-harness/src/main/java/io/jenkins/plugins/casc/misc/JenkinsConfiguredRule.java @@ -1,28 +1,39 @@ package io.jenkins.plugins.casc.misc; +import io.jenkins.plugins.casc.CasCGlobalConfig; import io.jenkins.plugins.casc.ConfigurationAsCode; import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; +import jenkins.model.GlobalConfiguration; import org.jvnet.hudson.test.JenkinsRule; public class JenkinsConfiguredRule extends JenkinsRule { - // TODO: Looks like API defect, exception should be thrown /** * Exports the Jenkins configuration to a string. * @return YAML as string * @param strict Fail if any export operation returns error * @throws Exception Export error - * @throws AssertionError Failed to export the configuration * @since 1.25 */ public String exportToString(boolean strict) throws Exception { final ByteArrayOutputStream out = new ByteArrayOutputStream(); - ConfigurationAsCode.get().export(out); - final String s = out.toString(StandardCharsets.UTF_8.name()); - if (strict && s.contains("Failed to export")) { - throw new AssertionError("Failed to export the configuration: " + s); + + CasCGlobalConfig config = GlobalConfiguration.all().get(CasCGlobalConfig.class); + boolean originalStrict = config != null && config.isStrictExport(); + + if (config != null) { + config.setStrictExport(strict); } - return s; + + try { + ConfigurationAsCode.get().export(out); + } finally { + if (config != null) { + config.setStrictExport(originalStrict); + } + } + + return out.toString(StandardCharsets.UTF_8); } } diff --git a/test-harness/src/test/java/io/jenkins/plugins/casc/JenkinsConfiguredRuleTest.java b/test-harness/src/test/java/io/jenkins/plugins/casc/JenkinsConfiguredRuleTest.java new file mode 100644 index 0000000000..a18e565635 --- /dev/null +++ b/test-harness/src/test/java/io/jenkins/plugins/casc/JenkinsConfiguredRuleTest.java @@ -0,0 +1,26 @@ +package io.jenkins.plugins.casc; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +import io.jenkins.plugins.casc.misc.JenkinsConfiguredRule; +import java.util.Objects; +import jenkins.model.GlobalConfiguration; +import org.junit.Rule; +import org.junit.Test; + +public class JenkinsConfiguredRuleTest { + + @Rule + public JenkinsConfiguredRule j = new JenkinsConfiguredRule(); + + @Test + public void exportToString_restoresOriginalState() throws Exception { + CasCGlobalConfig config = GlobalConfiguration.all().get(CasCGlobalConfig.class); + Objects.requireNonNull(config).setStrictExport(false); + + j.exportToString(true); + + assertThat(config.isStrictExport(), is(false)); + } +}