Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-70729] Rework clouds management into multiple pages #7658

Merged
merged 56 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
6da6aa1
Rework clouds management into multiple pages
Vlatombe Jan 26, 2023
2551c94
Fix broken test + spotbugs
Vlatombe Feb 17, 2023
a09ec32
Use one column layout
timja Mar 4, 2023
082f650
Progress
timja Mar 4, 2023
ed9117c
Fix azure plugin
timja Mar 4, 2023
fe0315e
Remove redundant permission check
timja Mar 4, 2023
6dc25b1
Improve icon accessibility
timja Mar 4, 2023
72fe6a8
Clouds compatibility
Vlatombe Mar 4, 2023
8254ed9
Merge branch 'master' into manage-clouds
timja Mar 4, 2023
a9be4d0
Consistent casing and titles across cloud pages
timja Mar 4, 2023
b66695b
Refactor empty state handling
timja Mar 5, 2023
6d9bc85
Address review comments
timja Mar 5, 2023
ffac4a7
Fix Nodes configure link not being marked as active
timja Mar 5, 2023
c873c17
Fixups
timja Mar 5, 2023
db8227b
Updating links to "configureClouds" to go to "cloudSet"
Vlatombe Mar 6, 2023
057bab5
Deprecating GlobalCloudConfiguration
Vlatombe Mar 6, 2023
730d593
Removing missing sidepanel, and layout is one-column anyway
Vlatombe Mar 6, 2023
c6b26d0
Remove dead code
timja Mar 6, 2023
03c2672
Add trailing / to prevent unneeded redirect
timja Mar 6, 2023
82aaed3
Restore action box
Vlatombe Mar 6, 2023
e9d5d62
Update core/src/main/resources/jenkins/agents/CloudSet/index.jelly
timja Mar 6, 2023
ff3b1c6
Form apply support
Vlatombe Mar 6, 2023
c72b4f5
Revert back to isAdmin check
Vlatombe Mar 6, 2023
71638cc
Use /cloud instead of /cloudSet and keep navigation consistent within…
Vlatombe Mar 8, 2023
788ce99
Fix tests
Vlatombe Mar 8, 2023
0f6e0ad
Update missed reference
timja Mar 9, 2023
6d54cac
Update core/src/main/resources/hudson/model/ComputerSet/sidepanel.jelly
timja Mar 10, 2023
498b469
Merge branch 'master' into manage-clouds
timja Mar 10, 2023
4dbd2fd
Adjust content after management links shortened
timja Mar 10, 2023
5908c1d
Add clouds link
timja Mar 10, 2023
125ce33
Merge branch 'master' into manage-clouds
Vlatombe Mar 30, 2023
e0944c7
Cloud#reconfigure doesn't need to be public
Vlatombe Mar 30, 2023
3dc86ed
Add license header, remove StaplerFallback
Vlatombe Mar 30, 2023
4d4f2a7
Remove Item from method name since Cloud are not Items
Vlatombe Mar 30, 2023
b7acbcf
Implement StaplerProxy permission check trick
Vlatombe Mar 30, 2023
c6b3f4a
Use Functions#getAncestorUrl to compute current url
Vlatombe Mar 30, 2023
8dce53c
Remove Jenkins#getCloud()
Vlatombe Mar 30, 2023
8d754f6
Remove unused import
Vlatombe Mar 30, 2023
b41373b
Revert the previous change as we have a problem with views under Clou…
Vlatombe Mar 30, 2023
282a500
Permission check for descriptor as well through StaplerProxy
Vlatombe Mar 30, 2023
b4bc5cd
Seeing clouds only requires SYSTEM_READ
Vlatombe Mar 30, 2023
d67ad70
Remove unused import
Vlatombe Mar 31, 2023
1f266a0
l18n
Vlatombe Apr 4, 2023
94d4a9c
Update core/src/main/resources/jenkins/agents/CloudSet/index.jelly
Vlatombe Apr 4, 2023
305443a
No reason to expose getIcon to Rest API
Vlatombe Apr 5, 2023
c8ea200
Fix reviews
Vlatombe Apr 6, 2023
3c05c11
There is a shorter alternative
Vlatombe Apr 6, 2023
a7e46e0
Avoid descriptor.newInstance because of implementations doing field v…
Vlatombe Apr 7, 2023
95f42bd
Provide a default inline help for the name attribute of Cloud
Vlatombe Apr 7, 2023
de8ddc8
Ensure we don't end up with a Cloud without a name
Vlatombe Apr 7, 2023
3304d8f
Merge branch 'master' into manage-clouds
Vlatombe Apr 26, 2023
3aa80fb
When hitting a cloud with duplicate name, allow browsing it by index.
Vlatombe Apr 26, 2023
fcb6cd4
Avoid the transient field and compute on the fly
Vlatombe Apr 26, 2023
a5b312e
Add missing license header
Vlatombe Apr 26, 2023
ac990a9
Update core/src/main/java/jenkins/agents/CloudSet.java
timja Apr 26, 2023
45e656c
Make the config.jelly optional
Vlatombe Apr 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 66 additions & 2 deletions core/src/main/java/hudson/slaves/Cloud.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,24 @@
import hudson.security.PermissionScope;
import hudson.slaves.NodeProvisioner.PlannedNode;
import hudson.util.DescriptorList;
import hudson.util.FormApply;
import java.io.IOException;
import java.util.Collection;
import java.util.Objects;
import java.util.concurrent.Future;
import javax.servlet.ServletException;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.apache.commons.lang.Validate;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.HttpRedirect;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.interceptor.RequirePOST;
import org.kohsuke.stapler.verb.POST;

/**
* Creates {@link Node}s to dynamically expand/shrink the agents attached to Hudson.
Expand Down Expand Up @@ -104,9 +115,10 @@ public abstract class Cloud extends Actionable implements ExtensionPoint, Descri
* This is expected to be short ID-like string that does not contain any character unsafe as variable name or
* URL path token.
*/
public final String name;
public String name;

protected Cloud(String name) {
Validate.notEmpty(name, Messages.Cloud_RequiredName());
this.name = name;
}

Expand All @@ -122,7 +134,7 @@ public String getDisplayName() {
* @return Jenkins relative URL.
*/
public @NonNull String getUrl() {
return "cloud/" + Util.rawEncode(name);
return "cloud/" + Util.rawEncode(name) + "/";
}

@Override
Expand Down Expand Up @@ -275,6 +287,58 @@ public static void registerPermissions() {
Objects.hash(PERMISSION_SCOPE, PROVISION);
}

public String getIcon() {
return "symbol-cloud";
}

public String getIconClassName() {
return "symbol-cloud";
}

@SuppressWarnings("unused") // stapler
public String getIconAltText() {
return getClass().getSimpleName().replace("Cloud", "");
}

/**
* Deletes the cloud.
*/
@RequirePOST
public HttpResponse doDoDelete() throws IOException {
checkPermission(Jenkins.ADMINISTER);
Jenkins.get().clouds.remove(this);
return new HttpRedirect("..");
}

/**
* Accepts the update to the node configuration.
*/
@POST
public HttpResponse doConfigSubmit(StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
checkPermission(Jenkins.ADMINISTER);

Jenkins j = Jenkins.get();
Cloud cloud = j.getCloud(this.name);
if (cloud == null) {
throw new ServletException("No such cloud " + this.name);
}
Cloud result = cloud.reconfigure(req, req.getSubmittedForm());
String proposedName = result.name;
if (!proposedName.equals(this.name)
&& j.getCloud(proposedName) != null) {
throw new Descriptor.FormException(jenkins.agents.Messages.CloudSet_CloudAlreadyExists(proposedName), "name");
}
j.clouds.replace(this, result);
j.save();
// take the user back to the cloud top page.
return FormApply.success(".");
}

private Cloud reconfigure(@NonNull final StaplerRequest req, JSONObject form) throws Descriptor.FormException {
if (form == null) return null;
return getDescriptor().newInstance(req, form);
}

/**
* Parameter object for {@link hudson.slaves.Cloud}.
* @since 2.259
Expand Down
272 changes: 272 additions & 0 deletions core/src/main/java/jenkins/agents/CloudSet.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,272 @@
/*
* The MIT License
*
* Copyright (c) 2023, CloudBees Inc, and other contributors
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package jenkins.agents;
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved

import hudson.Extension;
import hudson.Functions;
import hudson.Util;
import hudson.model.AbstractModelObject;
import hudson.model.AutoCompletionCandidates;
import hudson.model.Describable;
import hudson.model.Descriptor;
import hudson.model.Failure;
import hudson.model.RootAction;
import hudson.model.UpdateCenter;
import hudson.slaves.Cloud;
import hudson.util.FormValidation;
import java.io.IOException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.servlet.ServletException;
import jenkins.model.Jenkins;
import jenkins.model.ModelObjectWithChildren;
import jenkins.model.ModelObjectWithContextMenu;
import net.sf.json.JSONObject;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.interceptor.RequirePOST;
import org.kohsuke.stapler.verb.POST;

@Restricted(NoExternalUse.class)
public class CloudSet extends AbstractModelObject implements Describable<CloudSet>, ModelObjectWithChildren, RootAction, StaplerProxy {
private static final Logger LOGGER = Logger.getLogger(CloudSet.class.getName());

@Override
public Descriptor<CloudSet> getDescriptor() {
return Jenkins.get().getDescriptorOrDie(CloudSet.class);
}

public Cloud getDynamic(String token) {
return Jenkins.get().getCloud(token);
}

@Override
@Restricted(NoExternalUse.class)
public Object getTarget() {
Jenkins.get().checkPermission(Jenkins.SYSTEM_READ);
return this;
}

@Override
public String getIconFileName() {
return null;
}

@Override
public String getDisplayName() {
return Messages.CloudSet_DisplayName();
}

@Override
public String getUrlName() {
return "cloud";
}

@Override
public String getSearchUrl() {
return "/cloud/";
}

@SuppressWarnings("unused") // stapler
@Restricted(DoNotUse.class) // stapler
public String getCloudUrl(StaplerRequest request, Jenkins jenkins, Cloud cloud) {
String context = Functions.getNearestAncestorUrl(request, jenkins);
if (Jenkins.get().getCloud(cloud.name) != cloud) { // this cloud is not the first occurrence with this name
return context + "/cloud/cloudByIndex/" + getClouds().indexOf(cloud) + "/";
} else {
return context + "/" + cloud.getUrl();
}
}

@SuppressWarnings("unused") // stapler
@Restricted(DoNotUse.class) // stapler
public Cloud getCloudByIndex(int index) {
return Jenkins.get().clouds.get(index);
}

@SuppressWarnings("unused") // stapler
public boolean isCloudAvailable() {
return !Cloud.all().isEmpty();
}

@SuppressWarnings("unused") // stapler
public String getCloudUpdateCenterCategoryLabel() {
return URLEncoder.encode(UpdateCenter.getCategoryDisplayName("cloud"), StandardCharsets.UTF_8);
}

@Override
public ModelObjectWithContextMenu.ContextMenu doChildrenContextMenu(StaplerRequest request, StaplerResponse response) throws Exception {
ModelObjectWithContextMenu.ContextMenu m = new ModelObjectWithContextMenu.ContextMenu();
Jenkins.get().clouds.stream().forEach(c -> m.add(c));
return m;
}

public Cloud getDynamic(String name, StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException {
return Jenkins.get().clouds.getByName(name);
}

@SuppressWarnings("unused") // stapler
@Restricted(DoNotUse.class) // stapler
public Jenkins.CloudList getClouds() {
return Jenkins.get().clouds;
}

@SuppressWarnings("unused") // stapler
@Restricted(DoNotUse.class) // stapler
public boolean hasClouds() {
return !Jenkins.get().clouds.isEmpty();
}

/**
* Makes sure that the given name is good as an agent name.
* @return trimmed name if valid; throws ParseException if not
*/
public String checkName(String name) throws Failure {
if (name == null)
throw new Failure("Query parameter 'name' is required");

name = name.trim();
Jenkins.checkGoodName(name);

if (Jenkins.get().getCloud(name) != null)
throw new Failure(Messages.CloudSet_CloudAlreadyExists(name));

// looks good
return name;
}

@SuppressWarnings("unused") // stapler
public FormValidation doCheckName(@QueryParameter String value) {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
if (Util.fixEmpty(value) == null) {
return FormValidation.ok();
}
try {
checkName(value);
return FormValidation.ok();
} catch (Failure e) {
return FormValidation.error(e.getMessage());
}
}

/**
* First check point in creating a new cloud.
*/
@RequirePOST
public synchronized void doCreate(StaplerRequest req, StaplerResponse rsp,
@QueryParameter String name, @QueryParameter String mode,
@QueryParameter String from) throws IOException, ServletException, Descriptor.FormException {
final Jenkins jenkins = Jenkins.get();
jenkins.checkPermission(Jenkins.ADMINISTER);

if (mode != null && mode.equals("copy")) {
name = checkName(name);

Cloud src = jenkins.getCloud(from);
if (src == null) {
if (Util.fixEmpty(from) == null) {
throw new Failure(Messages.CloudSet_SpecifyCloudToCopy());
} else {
throw new Failure(Messages.CloudSet_NoSuchCloud(from));
}
}

// copy through XStream
String xml = Jenkins.XSTREAM.toXML(src);
// Not great, but cloud name is final
xml = xml.replace("<name>" + src.name + "</name>", "<name>" + name + "</name>");
Cloud result = (Cloud) Jenkins.XSTREAM.fromXML(xml);
jenkins.clouds.add(result);
// send the browser to the config page
rsp.sendRedirect2(Functions.getNearestAncestorUrl(req, jenkins) + "/" + result.getUrl() + "configure");
} else {
// proceed to step 2
if (mode == null) {
throw new Failure("No mode given");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a user-friendly error message. Should be localizable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue in ItemGroupMixIn and ComputerSet. Should be addressed globally.

}

Descriptor<Cloud> d = Cloud.all().findByName(mode);
if (d == null) {
throw new Failure("No node type ‘" + mode + "’ is known");
Copy link
Member

@daniel-beck daniel-beck Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refers to nodes when we're configuring clouds.

Should be localizable.

}
handleNewCloudPage(d, name, req, rsp);
}
}

private void handleNewCloudPage(Descriptor<Cloud> descriptor, String name, StaplerRequest req, StaplerResponse rsp) throws IOException, ServletException, Descriptor.FormException {
checkName(name);
JSONObject formData = req.getSubmittedForm();
formData.put("name", name);
formData.put("cloudName", name); // ec2 uses that field name
formData.remove("mode"); // Cloud descriptors won't have this field.
req.setAttribute("instance", formData);
req.setAttribute("descriptor", descriptor);
req.getView(this, "_new.jelly").forward(req, rsp);
}

/**
* Really creates a new agent.
*/
@POST
public synchronized void doDoCreate(StaplerRequest req, StaplerResponse rsp,
@QueryParameter String type) throws IOException, ServletException, Descriptor.FormException {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
Cloud cloud = Cloud.all().find(type).newInstance(req, req.getSubmittedForm());
if (!Jenkins.get().clouds.add(cloud)) {
LOGGER.log(Level.WARNING, () -> "Creating duplicate cloud name " + cloud.name + ". Plugin " + Jenkins.get().getPluginManager().whichPlugin(cloud.getClass()) + " should be updated to support user provided name.");
}
// take the user back to the cloud list top page
rsp.sendRedirect2(".");
}

@Extension
public static class DescriptorImpl extends Descriptor<CloudSet> implements StaplerProxy {

/**
* Auto-completion for the "copy from" field in the new cloud page.
*/
@SuppressWarnings("unused") // stapler
public AutoCompletionCandidates doAutoCompleteCopyNewItemFrom(@QueryParameter final String value) {
daniel-beck marked this conversation as resolved.
Show resolved Hide resolved
Vlatombe marked this conversation as resolved.
Show resolved Hide resolved
final AutoCompletionCandidates r = new AutoCompletionCandidates();
Jenkins.get().clouds.stream()
.filter(c -> c.name.startsWith(value))
.forEach(c -> r.add(c.name));
return r;
}

@Override
public Object getTarget() {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
return this;
}
}
}
Loading