Skip to content

Commit 568a80c

Browse files
prathamalwayscomeslastprathamalwayscomeslast
committed
fix: resolved test failures
Signed-off-by: Pratham Vaghela <prathamcomeslast@gmail.com>
1 parent ebba161 commit 568a80c

File tree

5 files changed

+35
-28
lines changed

5 files changed

+35
-28
lines changed

‎src/main/java/com/sap/prd/jenkins/plugins/agent_maintenance/MaintenanceAction.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ public synchronized HttpResponse doConfigSubmit(StaplerRequest2 req) throws IOEx
447447
public void doEnable(StaplerResponse2 rsp) throws IOException {
448448
Computer c = getAgentComputer();
449449
if (c != null) {
450-
PermissionManager.checkCanModify(target);
450+
PermissionManager.checkHasPermissions(target, false, Computer.CONFIGURE);
451451
MaintenanceHelper.getInstance().injectRetentionStrategy(c);
452452
}
453453
rsp.sendRedirect(".");
@@ -463,7 +463,7 @@ public void doEnable(StaplerResponse2 rsp) throws IOException {
463463
public void doDisable(StaplerResponse2 rsp) throws IOException {
464464
Computer c = getAgentComputer();
465465
if (c != null) {
466-
PermissionManager.checkCanModify(target);
466+
PermissionManager.checkHasPermissions(target, false, Computer.CONFIGURE);
467467
MaintenanceHelper.getInstance().removeRetentionStrategy(c);
468468
}
469469

‎src/main/java/com/sap/prd/jenkins/plugins/agent_maintenance/MaintenanceHelper.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,8 @@ private File getTargetDirectory(MaintenanceTarget.TargetType target) throws IOEx
412412
case CLOUD -> "clouds";
413413
};
414414
File targetDir = new File(Jenkins.get().getRootDir(), type);
415-
if (!targetDir.exists()) {
416-
if (!targetDir.mkdirs() && !targetDir.exists()) {
417-
throw new IOException("Failed to create " + type + " directory: " + targetDir);
418-
}
415+
if (!targetDir.mkdirs() && !targetDir.exists()) {
416+
throw new IOException("Failed to create " + type + " directory: " + targetDir);
419417
} else if (!targetDir.isDirectory()) {
420418
throw new IOException(targetDir + " is not a directory");
421419
}
@@ -433,21 +431,24 @@ public void deleteAgent(String targetKey) {
433431
* @param newName new name of the agent
434432
*/
435433
public void renameAgent(String oldName, String newName) {
436-
MaintenanceDefinitions md = cache.get(oldName);
434+
MaintenanceTarget oldTarget = new MaintenanceTarget(MaintenanceTarget.TargetType.AGENT, oldName);
435+
MaintenanceTarget newTarget = new MaintenanceTarget(MaintenanceTarget.TargetType.AGENT, newName);
436+
MaintenanceDefinitions md = cache.get(oldTarget.toKey());
437437
if (md != null) {
438438
LOGGER.log(Level.FINEST, "Persisting existing maintenance windows after agent rename");
439-
cache.remove(oldName);
440-
cache.put(newName, md);
439+
cache.remove(oldTarget.toKey());
440+
cache.put(newTarget.toKey(), md);
441441
try {
442-
saveMaintenanceWindows(newName, md);
442+
saveMaintenanceWindows(newTarget.toKey(), md);
443443
} catch (IOException e) {
444444
LOGGER.log(Level.WARNING, "Failed to persists agent maintenance windows after agent rename {0}", newName);
445445
}
446446
}
447447
}
448448

449449
public void createAgent(String nodeName) {
450-
cache.put(nodeName, new MaintenanceDefinitions(new TreeSet<>(), new HashSet<>()));
450+
MaintenanceTarget target = new MaintenanceTarget(MaintenanceTarget.TargetType.AGENT, nodeName);
451+
cache.put(target.toKey(), new MaintenanceDefinitions(new TreeSet<>(), new HashSet<>()));
451452
}
452453

453454
@Restricted(NoExternalUse.class)
@@ -491,6 +492,7 @@ public boolean injectRetentionStrategy(Computer c) {
491492
public boolean removeRetentionStrategy(Computer c) {
492493
if (c instanceof SlaveComputer computer) {
493494
String computerName = computer.getName();
495+
MaintenanceTarget target = new MaintenanceTarget(MaintenanceTarget.TargetType.AGENT, computerName);
494496
@SuppressWarnings("unchecked")
495497
RetentionStrategy<SlaveComputer> strategy = computer.getRetentionStrategy();
496498
if (strategy instanceof AgentMaintenanceRetentionStrategy maintenanceStrategy) {
@@ -499,8 +501,8 @@ public boolean removeRetentionStrategy(Computer c) {
499501
node.setRetentionStrategy(maintenanceStrategy.getRegularRetentionStrategy());
500502
try {
501503
node.save();
502-
deleteAgent(computerName);
503-
XmlFile maintenanceFile = getMaintenanceWindowsFile(computerName);
504+
deleteAgent(target.toKey());
505+
XmlFile maintenanceFile = getMaintenanceWindowsFile(target.toKey());
504506
if (maintenanceFile.exists()) {
505507
maintenanceFile.delete();
506508
}

‎src/test/java/com/sap/prd/jenkins/plugins/agent_maintenance/MaintenanceActionTest.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,19 @@ void configurePermissionDoesNotExposeDeleteLinkForCloud() throws Exception {
128128
WebClient w = rule.createWebClient();
129129
w.login(CONFIGURE);
130130
String url = getMaintenanceUrl(MaintenanceTarget.TargetType.CLOUD);
131-
HtmlPage managePage = w.withThrowExceptionOnFailingStatusCode(false).goTo(url);
132-
assertThat(managePage.getWebResponse().getStatusCode(), is(403));
131+
String id = getMaintenanceId(MaintenanceTarget.TargetType.CLOUD);
132+
HtmlPage managePage = w.goTo(url);
133+
assertThat(managePage.querySelector("#" + id + " .am__action-delete"), is(nullValue()));
133134
}
134135

135136
@Test
136137
void disconnectPermissionDoesNotExposeDeleteLinkForCloud() throws Exception {
137138
WebClient w = rule.createWebClient();
138139
w.login(DISCONNECT);
139140
String url = getMaintenanceUrl(MaintenanceTarget.TargetType.CLOUD);
140-
HtmlPage managePage = w.withThrowExceptionOnFailingStatusCode(false).goTo(url);
141-
assertThat(managePage.getWebResponse().getStatusCode(), is(403));
141+
String id = getMaintenanceId(MaintenanceTarget.TargetType.CLOUD);
142+
HtmlPage managePage = w.goTo(url);
143+
assertThat(managePage.querySelector("#" + id + " .am__action-delete"), is(nullValue()));
142144
}
143145

144146
@Test

‎src/test/java/com/sap/prd/jenkins/plugins/agent_maintenance/MaintenanceLinkTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ void systemReadPermissionDoesNotExposeDeleteLink() throws Exception {
5656
HtmlPage managePage = w.goTo("target-maintenances/");
5757
assertThat(managePage.querySelector("#" + maintenanceId + " .am__link-delete"), is(nullValue()));
5858
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(nullValue()));
59-
assertThat(managePage.getElementById(cloudMaintenanceId), is(nullValue()));
59+
assertThat(managePage.querySelector("#" + cloudMaintenanceId + " .am__link-delete"), is(nullValue()));
6060
}
6161

6262
@Test
@@ -66,7 +66,7 @@ void managePermissionDoesNotExposeDeleteLink() throws Exception {
6666
HtmlPage managePage = w.goTo("target-maintenances/");
6767
assertThat(managePage.querySelector("#" + maintenanceId + " .am__link-delete"), is(nullValue()));
6868
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(nullValue()));
69-
assertThat(managePage.getElementById(cloudMaintenanceId), is(nullValue()));
69+
assertThat(managePage.querySelector("#" + cloudMaintenanceId + " .am__link-delete"), is(nullValue()));
7070
}
7171

7272
@ParameterizedTest(name = "deleteMaintenanceWindow[{0}]")
@@ -109,7 +109,7 @@ void configurePermissionDoesExposeDeleteLink() throws Exception {
109109

110110
assertThat(managePage.querySelector("#" + maintenanceId + " .am__link-delete"), is(notNullValue()));
111111
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(nullValue()));
112-
assertThat(managePage.getElementById(cloudMaintenanceId), is(nullValue()));
112+
assertThat(managePage.querySelector("#" + cloudMaintenanceId + " .am__link-delete"), is(nullValue()));
113113
}
114114

115115
@Test
@@ -120,7 +120,7 @@ void disconnectPermissionDoesExposeDeleteLink() throws Exception {
120120

121121
assertThat(managePage.querySelector("#" + maintenanceId + " .am__link-delete"), is(notNullValue()));
122122
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(nullValue()));
123-
assertThat(managePage.getElementById(cloudMaintenanceId), is(nullValue()));
123+
assertThat(managePage.querySelector("#" + cloudMaintenanceId + " .am__link-delete"), is(nullValue()));
124124
}
125125

126126
@Test
@@ -130,7 +130,7 @@ void adminPermissionDoesExposeDeleteLink() throws Exception {
130130
HtmlPage managePage = w.goTo("target-maintenances/");
131131

132132
assertThat(managePage.querySelector("#" + maintenanceId + " .am__link-delete"), is(notNullValue()));
133-
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(nullValue()));
133+
assertThat(managePage.querySelector("#" + maintenanceIdRestricted + " .am__link-delete"), is(notNullValue()));
134134
assertThat(managePage.querySelector("#" + cloudMaintenanceId + " .am__link-delete"), is(notNullValue()));
135135
}
136136
}

‎src/test/java/com/sap/prd/jenkins/plugins/agent_maintenance/MaintenanceNodeListenerTest.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class MaintenanceNodeListenerTest extends BaseIntegrationTest {
2929
private File folder;
3030

3131
private Slave agent;
32+
private MaintenanceTarget target;
3233

3334
/**
3435
* Setup from some tests.
@@ -43,11 +44,12 @@ void setup(JenkinsRule rule) throws Exception {
4344
AgentMaintenanceRetentionStrategy strategy =
4445
new AgentMaintenanceRetentionStrategy(new Always());
4546
agent.setRetentionStrategy(strategy);
47+
target = getTarget(MaintenanceTarget.TargetType.AGENT, agent.getNodeName());
4648
}
4749

4850
@AfterEach
4951
void tearDown() throws IOException {
50-
maintenanceHelper.getMaintenanceWindows(agent.getNodeName()).clear();
52+
maintenanceHelper.getMaintenanceWindows(target.toKey()).clear();
5153
}
5254

5355
@Test
@@ -64,10 +66,10 @@ void agentDeleted() throws Exception {
6466
"5",
6567
"test",
6668
null);
67-
maintenanceHelper.addMaintenanceWindow(agent.getNodeName(), mw);
69+
maintenanceHelper.addMaintenanceWindow(target.toKey(), mw);
6870
assertThat(agent.toComputer().isAcceptingTasks(), is(false));
6971
rule.jenkins.removeNode(agent);
70-
assertThat(maintenanceHelper.hasMaintenanceWindows(agent.getNodeName()), is(false));
72+
assertThat(maintenanceHelper.hasMaintenanceWindows(target.toKey()), is(false));
7173
}
7274

7375
@Test
@@ -84,14 +86,15 @@ void agentRenamed() throws Exception {
8486
"5",
8587
"test",
8688
null);
87-
maintenanceHelper.addMaintenanceWindow(agent.getNodeName(), mw);
89+
maintenanceHelper.addMaintenanceWindow(target.toKey(), mw);
8890
assertThat(agent.toComputer().isAcceptingTasks(), is(false));
8991
Slave newAgent =
9092
new DumbSlave(
9193
"newAgent", newFolder(folder, "junit").getAbsolutePath(), rule.createComputerLauncher(null));
94+
MaintenanceTarget newTarget = getTarget(MaintenanceTarget.TargetType.AGENT, newAgent.getNodeName());
9295
rule.jenkins.getNodesObject().replaceNode(agent, newAgent);
93-
assertThat(maintenanceHelper.hasMaintenanceWindows(agent.getNodeName()), is(false));
94-
assertThat(maintenanceHelper.hasMaintenanceWindows(newAgent.getNodeName()), is(true));
96+
assertThat(maintenanceHelper.hasMaintenanceWindows(target.toKey()), is(false));
97+
assertThat(maintenanceHelper.hasMaintenanceWindows(newTarget.toKey()), is(true));
9598
}
9699

97100
@Test

0 commit comments

Comments
 (0)