Skip to content

Commit 7d33fe2

Browse files
AhmedFatthy1040AhmedFatthy1040
authoredMar 19, 2025
Improve YAML validation when using check endpoint (#2648)
* fix: improve error handling in /configuration-as-code/check endpoint to return JSON responses instead of HTML * fix: Improve error handling and response structure in doCheck * Add integration test for YAML validation error handling (#2628) * Fix code formatting style in YamlValidationTest for issue #2628
1 parent 287d063 commit 7d33fe2

File tree

2 files changed

+163
-7
lines changed

2 files changed

+163
-7
lines changed
 

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

+41-7
Original file line numberDiff line numberDiff line change
@@ -453,19 +453,53 @@ public List<String> getBundledCasCURIs() {
453453
@RequirePOST
454454
@Restricted(NoExternalUse.class)
455455
public void doCheck(StaplerRequest2 req, StaplerResponse2 res) throws Exception {
456-
457456
if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
458457
res.sendError(HttpServletResponse.SC_FORBIDDEN);
459458
return;
460459
}
461460

462-
final Map<Source, String> issues = checkWith(YamlSource.of(req));
463461
res.setContentType("application/json");
464-
final JSONArray warnings = new JSONArray();
465-
issues.entrySet().stream()
466-
.map(e -> new JSONObject().accumulate("line", e.getKey().line).accumulate("warning", e.getValue()))
467-
.forEach(warnings::add);
468-
warnings.write(res.getWriter());
462+
463+
try {
464+
final Map<Source, String> validationIssues = checkWith(YamlSource.of(req));
465+
466+
// Create JSON array for response
467+
JSONArray response = new JSONArray();
468+
469+
// If there are validation issues, add them to the response array
470+
validationIssues.entrySet().stream()
471+
.map(e -> new JSONObject()
472+
.accumulate("line", e.getKey() != null ? e.getKey().line : -1)
473+
.accumulate("message", e.getValue()))
474+
.forEach(response::add);
475+
476+
// Write the response - will be empty array for valid YAML
477+
response.write(res.getWriter());
478+
479+
} catch (ConfiguratorException e) {
480+
// Return 400 Bad Request for configuration errors
481+
res.setStatus(HttpServletResponse.SC_BAD_REQUEST);
482+
483+
JSONArray errors = new JSONArray();
484+
errors.add(new JSONObject().accumulate("line", -1).accumulate("message", e.getMessage()));
485+
486+
errors.write(res.getWriter());
487+
488+
} catch (Exception e) {
489+
// Return 400 Bad Request for all other errors
490+
res.setStatus(HttpServletResponse.SC_BAD_REQUEST);
491+
492+
JSONArray errors = new JSONArray();
493+
errors.add(new JSONObject()
494+
.accumulate("line", -1)
495+
.accumulate(
496+
"message",
497+
e.getMessage() != null
498+
? e.getMessage()
499+
: e.getClass().getName()));
500+
501+
errors.write(res.getWriter());
502+
}
469503
}
470504

471505
@RequirePOST

‎test-harness/src/test/java/io/jenkins/plugins/casc/YamlValidationTest.java

+122
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package io.jenkins.plugins.casc;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.containsString;
5+
import static org.hamcrest.Matchers.is;
6+
7+
import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
8+
import io.jenkins.plugins.casc.misc.junit.jupiter.WithJenkinsConfiguredWithCode;
9+
import java.net.URL;
10+
import java.nio.charset.StandardCharsets;
11+
import java.text.MessageFormat;
12+
import net.sf.json.JSONArray;
13+
import org.htmlunit.HttpMethod;
14+
import org.htmlunit.WebClient;
15+
import org.htmlunit.WebRequest;
16+
import org.htmlunit.WebResponse;
17+
import org.junit.jupiter.api.Test;
18+
import org.jvnet.hudson.test.Issue;
19+
20+
/**
21+
* Tests for YAML validation in the Configuration-as-Code endpoint.
22+
*/
23+
@WithJenkinsConfiguredWithCode
24+
public class YamlValidationTest {
25+
26+
/**
27+
* Tests that valid YAML returns an empty array response.
28+
*/
29+
@Test
30+
void validYaml(JenkinsConfiguredWithCodeRule j) throws Exception {
31+
// Disable CSRF protection to simplify testing
32+
j.jenkins.setCrumbIssuer(null);
33+
34+
WebResponse response = performYamlValidation(j, "jenkins:\n systemMessage: \"Hello from test\"");
35+
36+
assertThat(response.getStatusCode(), is(200));
37+
assertThat(response.getContentType(), is("application/json"));
38+
39+
JSONArray jsonResponse = JSONArray.fromObject(response.getContentAsString());
40+
assertThat(jsonResponse.size(), is(0));
41+
}
42+
43+
/**
44+
* Tests that YAML with incorrect indentation returns proper JSON response.
45+
*/
46+
@Test
47+
@Issue("2628")
48+
void invalidIndentation(JenkinsConfiguredWithCodeRule j) throws Exception {
49+
j.jenkins.setCrumbIssuer(null);
50+
51+
WebResponse response = performYamlValidation(j, "jenkins:\nsystemMessage: \"Bad indentation\"");
52+
53+
assertThat(response.getContentType(), is("application/json"));
54+
55+
// Verify the response contains valid JSON regardless of status code
56+
String responseString = response.getContentAsString();
57+
JSONArray jsonResponse = JSONArray.fromObject(responseString);
58+
59+
// The response should be a non-empty JSON array
60+
assertThat("Should return error details", jsonResponse.size() > 0, is(true));
61+
}
62+
63+
/**
64+
* Tests that YAML with invalid structure returns proper JSON response.
65+
*/
66+
@Test
67+
@Issue("2628")
68+
void invalidStructure(JenkinsConfiguredWithCodeRule j) throws Exception {
69+
j.jenkins.setCrumbIssuer(null);
70+
71+
WebResponse response = performYamlValidation(j, "jenkins:\n numExecutors: {");
72+
73+
assertThat(response.getContentType(), is("application/json"));
74+
75+
// Verify the response contains valid JSON regardless of status code
76+
String responseString = response.getContentAsString();
77+
JSONArray jsonResponse = JSONArray.fromObject(responseString);
78+
79+
// The response should be a non-empty JSON array
80+
assertThat("Should return error details", jsonResponse.size() > 0, is(true));
81+
}
82+
83+
/**
84+
* Tests that YAML with unknown property returns proper JSON response.
85+
*/
86+
@Test
87+
@Issue("2628")
88+
void unknownProperty(JenkinsConfiguredWithCodeRule j) throws Exception {
89+
j.jenkins.setCrumbIssuer(null);
90+
91+
WebResponse response = performYamlValidation(
92+
j,
93+
"jenkins:\n nonExistentProperty: \"This property doesn't exist\"\n systemMessage: \"Valid property\"");
94+
95+
assertThat(response.getContentType(), is("application/json"));
96+
97+
// Verify the response contains valid JSON regardless of status code
98+
String responseString = response.getContentAsString();
99+
JSONArray jsonResponse = JSONArray.fromObject(responseString);
100+
101+
// The response should be a non-empty JSON array with details about the unknown property
102+
assertThat("Should return error details", jsonResponse.size() > 0, is(true));
103+
assertThat(responseString, containsString("nonExistentProperty"));
104+
}
105+
106+
/**
107+
* Helper method to perform YAML validation.
108+
*/
109+
private WebResponse performYamlValidation(JenkinsConfiguredWithCodeRule j, String yamlContent) throws Exception {
110+
URL apiURL = new URL(MessageFormat.format(
111+
"{0}configuration-as-code/check", j.getURL().toString()));
112+
WebRequest request = new WebRequest(apiURL, HttpMethod.POST);
113+
request.setCharset(StandardCharsets.UTF_8);
114+
request.setRequestBody(yamlContent);
115+
116+
WebClient client = j.createWebClient();
117+
// Don't throw exceptions for error status codes
118+
client.getOptions().setThrowExceptionOnFailingStatusCode(false);
119+
120+
return client.getPage(request).getWebResponse();
121+
}
122+
}

0 commit comments

Comments
 (0)