Skip to content

Commit e3a6e23

Browse files
somiljain2006somiljain2006
authored
Return configuration reload errors as JSON (#2864)
1 parent d627440 commit e3a6e23

2 files changed

Lines changed: 83 additions & 7 deletions

File tree

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
package io.jenkins.plugins.casc;
22

3+
import static java.util.logging.Level.SEVERE;
4+
35
import edu.umd.cs.findbugs.annotations.CheckForNull;
46
import hudson.Extension;
57
import hudson.model.UnprotectedRootAction;
68
import hudson.security.ACL;
79
import hudson.security.ACLContext;
10+
import hudson.util.HttpResponses;
11+
import jakarta.servlet.ServletException;
812
import jakarta.servlet.http.HttpServletRequest;
913
import java.io.IOException;
1014
import java.nio.charset.StandardCharsets;
@@ -42,7 +46,7 @@ public String getUrlName() {
4246
}
4347

4448
@RequirePOST
45-
public void doIndex(StaplerRequest2 request, StaplerResponse2 response) throws IOException {
49+
public void doIndex(StaplerRequest2 request, StaplerResponse2 response) throws IOException, ServletException {
4650
String token = getReloadToken();
4751

4852
if (token == null || token.isEmpty()) {
@@ -58,6 +62,13 @@ public void doIndex(StaplerRequest2 request, StaplerResponse2 response) throws I
5862

5963
try (ACLContext ignored = ACL.as2(ACL.SYSTEM2)) {
6064
ConfigurationAsCode.get().configure();
65+
} catch (ConfiguratorException e) {
66+
LOGGER.log(SEVERE, "Failed to reload Jenkins Configuration as Code via token", e);
67+
68+
String message = e.getMessage() != null ? e.getMessage() : "Unknown configuration error";
69+
70+
HttpResponses.errorJSON("Failed to reload configuration: " + message)
71+
.generateResponse(request, response, this);
6172
}
6273
} else {
6374
response.sendError(401);

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

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package io.jenkins.plugins.casc;
22

3+
import static java.nio.file.Files.createTempFile;
4+
import static java.nio.file.Files.write;
5+
import static java.util.Arrays.asList;
36
import static org.junit.Assert.assertEquals;
47
import static org.junit.Assert.assertFalse;
58
import static org.junit.Assert.assertTrue;
@@ -8,20 +11,28 @@
811
import io.jenkins.plugins.casc.misc.EnvVarsRule;
912
import io.jenkins.plugins.casc.misc.Envs;
1013
import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
14+
import jakarta.servlet.ServletException;
1115
import jakarta.servlet.http.HttpServletResponse;
1216
import java.io.IOException;
17+
import java.net.URL;
18+
import java.nio.file.Files;
19+
import java.nio.file.Path;
1320
import java.util.Collections;
1421
import java.util.Date;
1522
import java.util.HashMap;
1623
import java.util.List;
1724
import java.util.Map;
1825
import java.util.logging.Level;
1926
import java.util.logging.LogRecord;
27+
import org.htmlunit.HttpMethod;
28+
import org.htmlunit.WebRequest;
29+
import org.htmlunit.WebResponse;
2030
import org.junit.Before;
2131
import org.junit.Rule;
2232
import org.junit.Test;
2333
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
2434
import org.junit.rules.RuleChain;
35+
import org.jvnet.hudson.test.JenkinsRule;
2536
import org.jvnet.hudson.test.LoggerRule;
2637
import org.kohsuke.stapler.RequestImpl;
2738
import org.kohsuke.stapler.ResponseImpl;
@@ -91,7 +102,7 @@ public void setUp() {
91102
}
92103

93104
@Test
94-
public void reloadIsDisabledByDefault() throws IOException {
105+
public void reloadIsDisabledByDefault() throws IOException, ServletException {
95106
System.clearProperty("casc.reload.token");
96107

97108
RequestImpl request = newRequest(null);
@@ -108,7 +119,7 @@ public void reloadIsDisabledByDefault() throws IOException {
108119
}
109120

110121
@Test
111-
public void reloadReturnsUnauthorizedIfTokenDoesNotMatch() throws IOException {
122+
public void reloadReturnsUnauthorizedIfTokenDoesNotMatch() throws IOException, ServletException {
112123
System.setProperty("casc.reload.token", "someSecretValue");
113124

114125
RequestImpl request = newRequest(null);
@@ -118,7 +129,7 @@ public void reloadReturnsUnauthorizedIfTokenDoesNotMatch() throws IOException {
118129
}
119130

120131
@Test
121-
public void reloadReturnsOkWhenCalledWithValidToken() throws IOException {
132+
public void reloadReturnsOkWhenCalledWithValidToken() throws IOException, ServletException {
122133
System.setProperty("casc.reload.token", "someSecretValue");
123134

124135
tokenReloadAction.doIndex(newRequest("someSecretValue"), new ResponseImpl(null, response));
@@ -128,15 +139,15 @@ public void reloadReturnsOkWhenCalledWithValidToken() throws IOException {
128139

129140
@Test
130141
@Envs({@Env(name = "CASC_RELOAD_TOKEN", value = "someSecretValue")})
131-
public void reloadReturnsOkWhenCalledWithValidTokenSetByEnvVar() throws IOException {
142+
public void reloadReturnsOkWhenCalledWithValidTokenSetByEnvVar() throws IOException, ServletException {
132143
tokenReloadAction.doIndex(newRequest("someSecretValue"), new ResponseImpl(null, response));
133144

134145
assertConfigReloaded();
135146
}
136147

137148
@Test
138149
@Envs({@Env(name = "CASC_RELOAD_TOKEN", value = "someSecretValue")})
139-
public void reloadShouldNotUseTokenFromPropertyIfEnvVarIsSet() throws IOException {
150+
public void reloadShouldNotUseTokenFromPropertyIfEnvVarIsSet() throws IOException, ServletException {
140151
System.setProperty("casc.reload.token", "otherSecretValue");
141152

142153
tokenReloadAction.doIndex(newRequest("otherSecretValue"), new ResponseImpl(null, response));
@@ -146,7 +157,7 @@ public void reloadShouldNotUseTokenFromPropertyIfEnvVarIsSet() throws IOExceptio
146157

147158
@Test
148159
@Envs({@Env(name = "CASC_RELOAD_TOKEN", value = "")})
149-
public void reloadShouldUsePropertyAsTokenIfEnvVarIsEmpty() throws IOException {
160+
public void reloadShouldUsePropertyAsTokenIfEnvVarIsEmpty() throws IOException, ServletException {
150161
System.setProperty("casc.reload.token", "someSecretValue");
151162

152163
tokenReloadAction.doIndex(newRequest("someSecretValue"), new ResponseImpl(null, response));
@@ -158,4 +169,58 @@ public void reloadShouldUsePropertyAsTokenIfEnvVarIsEmpty() throws IOException {
158169
public void displayName() {
159170
assertEquals("Reload Configuration as Code", tokenReloadAction.getDisplayName());
160171
}
172+
173+
@Test
174+
public void reloadReturnsInternalServerErrorAndJsonOnFailure() throws Exception {
175+
String oldToken = System.getProperty("casc.reload.token");
176+
String oldConfig = System.getProperty("casc.jenkins.config");
177+
178+
System.setProperty("casc.reload.token", "someSecretValue");
179+
180+
Path tempFile = createTempFile("invalid-casc-config", ".yaml");
181+
write(tempFile, asList("unclassified:", " this_fake_property_forces_a_configurator_exception: true"));
182+
System.setProperty("casc.jenkins.config", tempFile.toAbsolutePath().toString());
183+
184+
try {
185+
JenkinsRule.WebClient wc = j.createWebClient();
186+
wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
187+
188+
URL url = new URL(j.getURL(), "reload-configuration-as-code/?casc-reload-token=someSecretValue");
189+
WebRequest request = new WebRequest(url, HttpMethod.POST);
190+
WebResponse response = wc.getPage(request).getWebResponse();
191+
192+
assertEquals(HttpServletResponse.SC_OK, response.getStatusCode());
193+
assertTrue("Content-Type should be JSON", response.getContentType().contains("application/json"));
194+
195+
String body = response.getContentAsString();
196+
assertTrue(
197+
"Response body should contain the failure contextual message",
198+
body.contains("Failed to reload configuration"));
199+
assertTrue(
200+
"Response body should expose the specific invalid YAML property",
201+
body.contains("this_fake_property_forces_a_configurator_exception"));
202+
203+
List<LogRecord> messages = loggerRule.getRecords();
204+
boolean hasSevereLog = messages.stream()
205+
.anyMatch(record -> record.getLevel() == Level.SEVERE
206+
&& record.getMessage()
207+
.contains("Failed to reload Jenkins Configuration as Code via token"));
208+
209+
assertTrue("Expected a SEVERE log message regarding reload failure", hasSevereLog);
210+
211+
} finally {
212+
Files.deleteIfExists(tempFile);
213+
if (oldToken != null) {
214+
System.setProperty("casc.reload.token", oldToken);
215+
} else {
216+
System.clearProperty("casc.reload.token");
217+
}
218+
219+
if (oldConfig != null) {
220+
System.setProperty("casc.jenkins.config", oldConfig);
221+
} else {
222+
System.clearProperty("casc.jenkins.config");
223+
}
224+
}
225+
}
161226
}

0 commit comments

Comments
 (0)