Skip to content

Commit cc15770

Browse files
committed
Add strict export mode to propagate export failures as exceptions
1 parent fb98ed9 commit cc15770

6 files changed

Lines changed: 107 additions & 11 deletions

File tree

plugin/src/main/java/io/jenkins/plugins/casc/Attribute.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,16 @@ public Type getValue(Owner target) throws Exception {
241241
public CNode describe(Owner instance, ConfigurationContext context) throws ConfiguratorException {
242242
final Configurator c = context.lookup(type);
243243
if (c == null) {
244-
return new Scalar("FAILED TO EXPORT\n" + instance.getClass().getName() + "#" + name
245-
+ ": No configurator found for type " + type);
244+
String errorMessage = "FAILED TO EXPORT\n" + instance.getClass().getName() + "#" + name
245+
+ ": No configurator found for type " + type;
246+
247+
// --- NEW STRICT MODE CHECK FOR MISSING CONFIGURATOR ---
248+
if (context.isStrictExport()) {
249+
throw new ConfiguratorException(errorMessage);
250+
}
251+
// ------------------------------------------------------
252+
253+
return new Scalar(errorMessage);
246254
}
247255
try {
248256
Object o = getValue(instance);
@@ -267,7 +275,14 @@ public CNode describe(Owner instance, ConfigurationContext context) throws Confi
267275
return seq;
268276
}
269277
return _describe(c, context, o, shouldBeMasked);
270-
} catch (Exception | /* Jenkins.getDescriptorOrDie */ AssertionError e) {
278+
} catch (Exception | AssertionError e) {
279+
if (context.isStrictExport()) {
280+
if (e instanceof ConfiguratorException) {
281+
throw (ConfiguratorException) e;
282+
}
283+
throw new ConfiguratorException(
284+
"Failed to export " + instance.getClass().getName() + "#" + name, e);
285+
}
271286
// Don't fail the whole export, prefer logging this error
272287
LOGGER.log(Level.WARNING, "Failed to export", e);
273288
return new Scalar(

plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationAsCode.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,10 +595,15 @@ public void doReference(StaplerRequest2 req, StaplerResponse2 res) throws Except
595595

596596
@Restricted(NoExternalUse.class)
597597
public void export(OutputStream out) throws Exception {
598+
export(out, false);
599+
}
598600

601+
@Restricted(NoExternalUse.class)
602+
public void export(OutputStream out, boolean strict) throws Exception {
599603
final List<NodeTuple> tuples = new ArrayList<>();
600604

601605
final ConfigurationContext context = new ConfigurationContext(registry);
606+
context.setStrictExport(strict);
602607
for (RootElementConfigurator root : RootElementConfigurator.all()) {
603608
final CNode config = root.describe(root.getTargetComponent(context), context);
604609
final Node valueNode = toYaml(config);

plugin/src/main/java/io/jenkins/plugins/casc/ConfigurationContext.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public class ConfigurationContext implements ConfiguratorRegistry {
4444

4545
private transient SecretSourceResolver secretSourceResolver;
4646

47+
private boolean strictExport = false;
48+
4749
public ConfigurationContext(ConfiguratorRegistry registry) {
4850
this(registry, null);
4951
}
@@ -126,6 +128,14 @@ public int getYamlCodePointLimit() {
126128
return yamlCodePointLimit;
127129
}
128130

131+
public boolean isStrictExport() {
132+
return strictExport;
133+
}
134+
135+
public void setStrictExport(boolean strictExport) {
136+
this.strictExport = strictExport;
137+
}
138+
129139
// --- delegate methods for ConfigurationContext
130140

131141
@Override

plugin/src/test/java/io/jenkins/plugins/casc/AttributeTest.java

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
package io.jenkins.plugins.casc;
22

33
import static org.junit.jupiter.api.Assertions.assertFalse;
4+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
5+
import static org.junit.jupiter.api.Assertions.assertThrows;
46
import static org.junit.jupiter.api.Assertions.assertTrue;
57

8+
import edu.umd.cs.findbugs.annotations.NonNull;
69
import hudson.util.Secret;
10+
import io.jenkins.plugins.casc.model.CNode;
11+
import io.jenkins.plugins.casc.model.Scalar;
12+
import java.lang.reflect.Type;
713
import java.util.logging.Level;
814
import java.util.logging.Logger;
915
import org.junit.jupiter.api.AfterEach;
@@ -230,4 +236,51 @@ void checkCalculationIsIdempotent() {
230236
assertFalse(firstUnknown);
231237
assertFalse(secondUnknown, "Subsequent calls should return the same fallback FALSE result");
232238
}
239+
240+
@Test
241+
@SuppressWarnings("ExtractMethodRecommender")
242+
void describeHandlesMissingConfiguratorCorrectly() throws Exception {
243+
ConfiguratorRegistry dummyRegistry = new ConfiguratorRegistry() {
244+
@Override
245+
public RootElementConfigurator<?> lookupRootElement(String name) {
246+
return null;
247+
}
248+
249+
@Override
250+
@NonNull
251+
public <T> Configurator<T> lookupOrFail(Type type) throws ConfiguratorException {
252+
throw new ConfiguratorException("Not found");
253+
}
254+
255+
@Override
256+
public <T> Configurator<T> lookup(Type type) {
257+
return null;
258+
}
259+
};
260+
261+
ConfigurationContext context = new ConfigurationContext(dummyRegistry);
262+
263+
Attribute<NonSecretField, String> attr = new Attribute<>("passwordPath", String.class);
264+
NonSecretField dummyInstance = new NonSecretField("my-dummy-path");
265+
266+
context.setStrictExport(false);
267+
CNode node = attr.describe(dummyInstance, context);
268+
269+
assertInstanceOf(Scalar.class, node, "Should return a Scalar node on failure in non-strict mode");
270+
assertTrue(
271+
((Scalar) node).getValue().contains("FAILED TO EXPORT"),
272+
"Scalar should contain the fallback failure message");
273+
274+
context.setStrictExport(true);
275+
ConfiguratorException exception = assertThrows(
276+
ConfiguratorException.class,
277+
() -> {
278+
attr.describe(dummyInstance, context);
279+
},
280+
"Should completely abort and throw ConfiguratorException in strict mode");
281+
282+
assertTrue(
283+
exception.getMessage().contains("No configurator found"),
284+
"Exception message should accurately reflect the missing configurator");
285+
}
233286
}

plugin/src/test/java/io/jenkins/plugins/casc/ConfigurationAsCodeApiTest.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
import static org.hamcrest.MatcherAssert.assertThat;
44
import static org.hamcrest.Matchers.containsString;
55
import static org.hamcrest.Matchers.is;
6+
import static org.junit.Assert.assertFalse;
67

8+
import java.io.ByteArrayOutputStream;
79
import java.net.URL;
10+
import java.nio.charset.StandardCharsets;
811
import java.nio.file.Files;
912
import java.nio.file.Path;
1013
import java.util.List;
@@ -185,4 +188,18 @@ public void testDoReplace_ValidSource() throws Exception {
185188
assertThat(j.jenkins.getSystemMessage(), is("Hello Replace"));
186189
}
187190
}
191+
192+
@Test
193+
public void testExportStrictMode_SuccessOnCleanJenkins() throws Exception {
194+
configureAdminSecurity();
195+
196+
ByteArrayOutputStream out = new ByteArrayOutputStream();
197+
198+
ConfigurationAsCode.get().export(out, true);
199+
200+
String exportedYaml = out.toString(StandardCharsets.UTF_8);
201+
202+
assertThat(exportedYaml, containsString("jenkins:"));
203+
assertFalse(exportedYaml.contains("FAILED TO EXPORT"));
204+
}
188205
}

test-harness/src/main/java/io/jenkins/plugins/casc/misc/JenkinsConfiguredRule.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,18 @@
77

88
public class JenkinsConfiguredRule extends JenkinsRule {
99

10-
// TODO: Looks like API defect, exception should be thrown
1110
/**
1211
* Exports the Jenkins configuration to a string.
1312
* @return YAML as string
1413
* @param strict Fail if any export operation returns error
1514
* @throws Exception Export error
16-
* @throws AssertionError Failed to export the configuration
1715
* @since 1.25
1816
*/
1917
public String exportToString(boolean strict) throws Exception {
2018
final ByteArrayOutputStream out = new ByteArrayOutputStream();
21-
ConfigurationAsCode.get().export(out);
22-
final String s = out.toString(StandardCharsets.UTF_8.name());
23-
if (strict && s.contains("Failed to export")) {
24-
throw new AssertionError("Failed to export the configuration: " + s);
25-
}
26-
return s;
19+
20+
ConfigurationAsCode.get().export(out, strict);
21+
22+
return out.toString(StandardCharsets.UTF_8);
2723
}
2824
}

0 commit comments

Comments
 (0)