Skip to content

Commit 7773049

Browse files
VlatombeVlatombe
authored
Merge pull request #1181 from PereBueno/BEE-16671
2 parents 144bcd1 + 9686343 commit 7773049

File tree

4 files changed

+111
-105
lines changed

4 files changed

+111
-105
lines changed

‎src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty.java‎

Lines changed: 53 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,105 +1,69 @@
11
package org.csanchez.jenkins.plugins.kubernetes;
22

33
import java.util.ArrayList;
4+
import java.util.Collection;
5+
import java.util.Collections;
46
import java.util.Comparator;
57
import java.util.HashSet;
68
import java.util.List;
79
import java.util.Set;
8-
import java.util.logging.Level;
9-
import java.util.logging.Logger;
1010
import java.util.stream.Collectors;
1111

1212
import edu.umd.cs.findbugs.annotations.NonNull;
1313
import hudson.model.Job;
14-
import org.apache.commons.lang.StringUtils;
14+
15+
import org.jenkinsci.Symbol;
1516
import org.kohsuke.accmod.Restricted;
1617
import org.kohsuke.accmod.restrictions.DoNotUse;
1718
import org.kohsuke.stapler.DataBoundConstructor;
19+
import org.kohsuke.stapler.DataBoundSetter;
20+
import org.kohsuke.stapler.Stapler;
1821
import org.kohsuke.stapler.StaplerRequest;
1922

2023
import com.cloudbees.hudson.plugins.folder.AbstractFolder;
2124
import com.cloudbees.hudson.plugins.folder.AbstractFolderProperty;
2225
import com.cloudbees.hudson.plugins.folder.AbstractFolderPropertyDescriptor;
26+
import com.cloudbees.hudson.plugins.folder.Folder;
2327

2428
import hudson.Extension;
2529
import hudson.model.ItemGroup;
2630
import hudson.model.Descriptor.FormException;
2731
import hudson.slaves.Cloud;
2832
import jenkins.model.Jenkins;
33+
2934
import net.sf.json.JSONArray;
30-
import net.sf.json.JSONException;
3135
import net.sf.json.JSONObject;
3236

3337
/**
3438
* Provides folder level Kubernetes configuration.
3539
*/
3640
public class KubernetesFolderProperty extends AbstractFolderProperty<AbstractFolder<?>> {
3741

38-
private static final Logger LOGGER = Logger.getLogger(KubernetesFolderProperty.class.getName());
42+
private static final String PREFIX_USAGE_PERMISSION = "usage-permission-";
3943

40-
private Set<String> permittedClouds = new HashSet<>();
44+
private List<String> permittedClouds = new ArrayList<>();
4145

4246
/**
4347
* Constructor.
4448
*/
4549
@DataBoundConstructor
46-
public KubernetesFolderProperty() {
50+
public KubernetesFolderProperty() {}
51+
52+
@DataBoundSetter
53+
public void setPermittedClouds(Collection<String> permittedClouds){
54+
this.permittedClouds = permittedClouds == null ? Collections.emptyList() : new ArrayList<>(permittedClouds);
4755
}
4856

49-
private Set<String> getPermittedClouds() {
50-
return permittedClouds;
57+
public Collection<String> getPermittedClouds() {
58+
return permittedClouds == null ? Collections.emptyList() : Collections.unmodifiableList(permittedClouds);
5159
}
5260

53-
private Set<String> getInheritedClouds() {
61+
private static Set<String> getInheritedClouds(ItemGroup parent) {
5462
Set<String> inheritedPermissions = new HashSet<>();
55-
collectAllowedClouds(inheritedPermissions, getOwner().getParent());
56-
63+
collectAllowedClouds(inheritedPermissions, parent);
5764
return inheritedPermissions;
5865
}
5966

60-
/**
61-
* Called from Jelly.
62-
*
63-
* @return
64-
*/
65-
@SuppressWarnings("unused") // Used by jelly
66-
@Restricted(DoNotUse.class) // Used by jelly
67-
public List<UsagePermission> getEffectivePermissions() {
68-
List<UsagePermission> ps = buildPermissions();
69-
70-
Set<String> inheritedClouds = getInheritedClouds();
71-
72-
for (UsagePermission p : ps) {
73-
if (inheritedClouds.contains(p.name)) {
74-
p.setGranted(true);
75-
p.setInherited(true);
76-
}
77-
78-
if (getPermittedClouds().contains(p.name)) {
79-
p.setGranted(true);
80-
}
81-
}
82-
83-
// a non-admin User is only allowed to see granted clouds
84-
if (!userHasAdministerPermission()) {
85-
ps = ps.stream().filter(UsagePermission::isGranted).collect(Collectors.toList());
86-
}
87-
88-
return ps;
89-
}
90-
91-
private static List<UsagePermission> buildPermissions() {
92-
List<UsagePermission> ps = new ArrayList<>();
93-
94-
for (KubernetesCloud kubernetesCloud : getUsageRestrictedKubernetesClouds()) {
95-
UsagePermission p = new UsagePermission();
96-
p.setName(kubernetesCloud.name);
97-
ps.add(p);
98-
}
99-
100-
return ps;
101-
}
102-
10367
@SuppressWarnings({"rawtypes"})
10468
public static boolean isAllowed(KubernetesSlave agent, Job job) {
10569
ItemGroup parent = job.getParent();
@@ -113,8 +77,6 @@ public static boolean isAllowed(KubernetesSlave agent, Job job) {
11377
return true;
11478
}
11579

116-
private static String PREFIX_USAGE_PERMISSION = "usage-permission-";
117-
11880
@Override
11981
public AbstractFolderProperty<?> reconfigure(StaplerRequest req, JSONObject form) throws FormException {
12082
if (form == null) {
@@ -126,32 +88,20 @@ public AbstractFolderProperty<?> reconfigure(StaplerRequest req, JSONObject form
12688
if (!userHasAdministerPermission()) {
12789
return this;
12890
}
129-
130-
try {
131-
Set<String> inheritedGrants = new HashSet<>();
132-
collectAllowedClouds(inheritedGrants, getOwner().getParent());
133-
134-
Set<String> permittedClouds = new HashSet<>();
135-
JSONArray names = form.names();
136-
if (names != null) {
137-
for (Object name : names) {
138-
String strName = (String) name;
139-
140-
if (strName.startsWith(PREFIX_USAGE_PERMISSION) && form.getBoolean(strName)) {
141-
String cloud = StringUtils.replaceOnce(strName, PREFIX_USAGE_PERMISSION, "");
142-
143-
if (isUsageRestrictedKubernetesCloud(Jenkins.get().getCloud(cloud))
144-
&& !inheritedGrants.contains(cloud)) {
145-
permittedClouds.add(cloud);
146-
}
147-
}
148-
}
91+
// Backwards compatibility: this method was expecting a set of entries PREFIX_USAGE_PERMISSION+cloudName --> true | false
92+
// Now we're getting a set of permitted cloud names inside permittedClouds entry
93+
Set<String> formCloudNames = new HashSet<>();
94+
if (!form.has("permittedClouds")) {
95+
form.names().stream().filter(x -> form.getBoolean(x.toString())).forEach(x -> formCloudNames.add(x.toString().replace(PREFIX_USAGE_PERMISSION, "")));
96+
} else {
97+
JSONArray clouds = form.optJSONArray("permittedClouds");
98+
if (clouds != null) {
99+
formCloudNames.addAll(clouds.stream().map(x -> x.toString()).collect(Collectors.toSet()));
100+
} else {
101+
formCloudNames.add(form.getString("permittedClouds")); // arrays with 1 element are considered objects
149102
}
150-
this.permittedClouds.clear();
151-
this.permittedClouds.addAll(permittedClouds);
152-
} catch (JSONException e) {
153-
LOGGER.log(Level.SEVERE, e, () -> "reconfigure failed: " + e.getMessage());
154103
}
104+
setPermittedClouds(formCloudNames);
155105
return this;
156106
}
157107

@@ -195,6 +145,12 @@ public static class UsagePermission {
195145

196146
private String name;
197147

148+
public UsagePermission(String name, boolean granted, boolean inherited) {
149+
this.name = name;
150+
this.granted = granted;
151+
this.inherited = inherited;
152+
}
153+
198154
private void setInherited(boolean inherited) {
199155
this.inherited = inherited;
200156
}
@@ -222,12 +178,7 @@ private void setName(String name) {
222178
* @return
223179
*/
224180
public String getName() {
225-
return name + (isInherited() ? " (inherited)" : "");
226-
}
227-
228-
@SuppressWarnings("unused") // by stapler/jelly
229-
public String getFormName() {
230-
return PREFIX_USAGE_PERMISSION + getName();
181+
return name;
231182
}
232183

233184
@SuppressWarnings("unused") // by stapler/jelly
@@ -252,6 +203,7 @@ private static boolean isUsageRestrictedKubernetesCloud(Cloud cloud) {
252203
* Descriptor class.
253204
*/
254205
@Extension
206+
@Symbol("kubernetes")
255207
public static class DescriptorImpl extends AbstractFolderPropertyDescriptor {
256208

257209
@NonNull
@@ -260,6 +212,20 @@ public String getDisplayName() {
260212
return Messages.KubernetesFolderProperty_displayName();
261213
}
262214

215+
@SuppressWarnings("unused") // Used by jelly
216+
@Restricted(DoNotUse.class)
217+
public List<UsagePermission> getEffectivePermissions() {
218+
Set<String> inheritedClouds = getInheritedClouds(Stapler.getCurrentRequest().findAncestorObject(Folder.class).getParent());
219+
List<UsagePermission> ps = getUsageRestrictedKubernetesClouds().stream()
220+
.map(cloud -> new UsagePermission(cloud.name, inheritedClouds.contains(cloud.name), inheritedClouds.contains(cloud.name)))
221+
.collect(Collectors.toList());
222+
// a non-admin User is only allowed to see granted clouds
223+
if (!userHasAdministerPermission()) {
224+
ps = ps.stream().filter(UsagePermission::isGranted).collect(Collectors.toList());
225+
}
226+
227+
return ps;
228+
}
263229
}
264230

265231
}

‎src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderProperty/config.jelly‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,30 +4,20 @@
44
<f:description>
55
${%Allow pipeline support for the following restricted Kubernetes Clouds}
66
</f:description>
7-
<j:choose>
8-
<j:when test="${instance != null}">
9-
<j:set var="effectivePermissions" value="${instance.effectivePermissions}"/>
7+
<f:entry field="permittedClouds">
8+
<j:set var="effectivePermissions" value="${descriptor.effectivePermissions}"/>
109
<j:choose>
1110
<j:when test="${empty effectivePermissions}">
1211
<f:entry title="-- none --" />
1312
</j:when>
1413
<j:otherwise>
1514
<j:forEach var="permission" items="${effectivePermissions}">
16-
<f:entry field="${permission.formName}">
1715
<j:set var="readOnlyMode" value="${permission.readonly}"/>
18-
<f:checkbox title="${permission.name}" checked="${permission.granted}" />
19-
</f:entry>
16+
<f:checkbox title="${permission.name} ${permission.inherited ? '(inherited)' : ''}" json="${permission.name}"
17+
checked="${permission.granted or instance.permittedClouds.contains(permission.name)}" /><br/>
2018
</j:forEach>
2119
</j:otherwise>
2220
</j:choose>
23-
</j:when>
24-
<j:otherwise>
25-
<f:entry title="${%You have to save this folder first before you can manage Kubernetes Clouds.}" />
26-
<f:invisibleEntry>
27-
<!-- Needed to create the property on save -->
28-
<input type="checkbox"/>
29-
</f:invisibleEntry>
30-
</j:otherwise>
31-
</j:choose>
21+
</f:entry>
3222
</f:section>
3323
</j:jelly>

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesFolderPropertyTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package org.csanchez.jenkins.plugins.kubernetes;
2+
3+
import java.util.Collections;
4+
5+
import org.junit.Rule;
6+
import org.junit.Test;
7+
import org.jvnet.hudson.test.JenkinsRule;
8+
9+
import com.cloudbees.hudson.plugins.folder.Folder;
10+
11+
import static org.hamcrest.MatcherAssert.assertThat;
12+
import static org.hamcrest.Matchers.contains;
13+
import static org.hamcrest.Matchers.containsInAnyOrder;
14+
import static org.hamcrest.Matchers.empty;
15+
import static org.hamcrest.Matchers.notNullValue;
16+
17+
public class KubernetesFolderPropertyTest {
18+
19+
@Rule
20+
public JenkinsRule j = new JenkinsRule();
21+
22+
@Test
23+
public void propertySavedOnFirstSaveTest() throws Exception {
24+
KubernetesCloud kube1 = new KubernetesCloud("kube1");
25+
kube1.setUsageRestricted(true);
26+
KubernetesCloud kube2 = new KubernetesCloud("kube2");
27+
kube2.setUsageRestricted(true);
28+
j.jenkins.clouds.add(kube1);
29+
j.jenkins.clouds.add(kube2);
30+
31+
Folder folder = j.jenkins.createProject(Folder.class, "folder001");
32+
KubernetesFolderProperty prop = new KubernetesFolderProperty();
33+
folder.addProperty(prop);
34+
35+
Folder after = j.configRoundtrip(folder);
36+
assertThat("Property exists after saving", after.getProperties().get(KubernetesFolderProperty.class), notNullValue());
37+
assertThat("No selected clouds", after.getProperties().get(KubernetesFolderProperty.class).getPermittedClouds(), empty());
38+
39+
folder.getProperties().get(KubernetesFolderProperty.class).setPermittedClouds(Collections.singletonList("kube1"));
40+
after = j.configRoundtrip(folder);
41+
assertThat("Kube1 cloud is added", after.getProperties().get(KubernetesFolderProperty.class).getPermittedClouds(), contains("kube1"));
42+
43+
Folder subFolder = folder.createProject(Folder.class, "subfolder001");
44+
KubernetesFolderProperty prop2 = new KubernetesFolderProperty();
45+
prop2.setPermittedClouds(Collections.singletonList("kube2"));
46+
subFolder.addProperty(prop2);
47+
48+
after = j.configRoundtrip(subFolder);
49+
assertThat("Contains own and inherited cloud", after.getProperties().get(KubernetesFolderProperty.class).getPermittedClouds(), containsInAnyOrder("kube1", "kube2"));
50+
}
51+
52+
}

‎src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesQueueTaskDispatcherTest.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public void setUpTwoClouds() throws Exception {
7373
slaveB = new KubernetesSlave("B", new PodTemplate(), "testB", "B", "dockerB", new KubernetesLauncher(), RetentionStrategy.INSTANCE);
7474
}
7575

76-
7776
@Test
7877
public void checkRestrictedTwoClouds() throws Exception {
7978
setUpTwoClouds();
@@ -82,8 +81,7 @@ public void checkRestrictedTwoClouds() throws Exception {
8281
FreeStyleProject projectB = folderB.createProject(FreeStyleProject.class, "buildJob");
8382
KubernetesQueueTaskDispatcher dispatcher = new KubernetesQueueTaskDispatcher();
8483

85-
assertNull(dispatcher.canTake(slaveA, new Queue.BuildableItem(new Queue.WaitingItem(Calendar.getInstance(),
86-
projectA, new ArrayList<>()))));
84+
assertNull(dispatcher.canTake(slaveA, new Queue.BuildableItem(new Queue.WaitingItem(Calendar.getInstance(), projectA, new ArrayList<>()))));
8785
assertTrue(canTake(dispatcher, slaveB, projectA) instanceof KubernetesQueueTaskDispatcher.KubernetesCloudNotAllowed);
8886
assertTrue(canTake(dispatcher, slaveA, projectB) instanceof KubernetesQueueTaskDispatcher.KubernetesCloudNotAllowed);
8987
assertNull(canTake(dispatcher, slaveB, projectB));

0 commit comments

Comments
 (0)