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.

Sign up for GitHub

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

Jump to bottom

Proxysettings inside project configuration #95

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Proxysettings inside project configuration #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5aaae5d
Added custom proxy connection for each project
adripo May 16, 2019
7da8e57
Added proxy inheritance from jenkins
adripo May 17, 2019
085b415
Fixed default switch case
adripo May 17, 2019
d2e9425
Fixed mispelled switch default case
adripo May 17, 2019
03fda63
Changed logger level for proxy connection
adripo May 22, 2019
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
48 changes: 48 additions & 0 deletions src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashBuildTrigger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import hudson.ProxyConfiguration;
import hudson.model.AbstractProject;
import hudson.model.Cause;
import hudson.model.Executor;
Expand Down Expand Up @@ -54,6 +55,7 @@ public class StashBuildTrigger extends Trigger<AbstractProject<?, ?>> {
private final String projectPath;
private final String cron;
private final String stashHost;
private transient JSONObject proxySettings;
private final String credentialsId;
private final String projectCode;
private final String repositoryName;
Expand Down Expand Up @@ -81,6 +83,7 @@ public StashBuildTrigger(
String projectPath,
String cron,
String stashHost,
JSONObject proxySettings,
Copy link

Choose a reason for hiding this comment

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

Is the JSONObject type required here?
It is basically a Map<String,Object>
I would prefer to use proper, strongly typed DTOs here

String credentialsId,
String projectCode,
String repositoryName,
Expand All @@ -100,6 +103,7 @@ public StashBuildTrigger(
this.projectPath = projectPath;
this.cron = cron;
this.stashHost = stashHost;
this.proxySettings = proxySettings;
this.credentialsId = credentialsId;
this.projectCode = projectCode;
this.repositoryName = repositoryName;
Expand All @@ -125,6 +129,50 @@ public String getStashHost() {
return stashHost;
}

public JSONObject getProxySettings() {
Copy link

Choose a reason for hiding this comment

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

why are those getters needed? I think I only see getProxy() being actually called

return proxySettings;
}

public JSONObject getProxyDetails() {
return this.getProxySettings().getJSONObject("proxyChoice");
}

private String getProxyType() {
return this.getProxyDetails().getString("value");
}

public ProxyConfiguration getProxy() {
ProxyConfiguration proxy = null;

switch (this.getProxyType()) {
default:
case "jenkins":
proxy = Jenkins.getInstance().proxy;
break;
case "custom":
JSONObject proxyDetails = this.getProxyDetails();

proxy =
new ProxyConfiguration(
proxyDetails.getString("proxyHost"),
proxyDetails.getInt("proxyPort"),
proxyDetails.getString("proxyUser"),
proxyDetails.getString("proxyPass"));
}

return proxy;
}

/**
* Helper method for the jelly view to determine proxy type.
*
* @param type
* @return
*/
public boolean isProxyType(String type) {
return (getProxyType().equalsIgnoreCase(type)) ? true : false;
}

public String getProjectPath() {
return this.projectPath;
}
Expand Down
1 change: 1 addition & 0 deletions src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/StashRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public void init() {
client =
new StashApiClient(
trigger.getStashHost(),
trigger.getProxy(),
trigger.getUsername(),
trigger.getPassword(),
trigger.getProjectCode(),
Expand Down
38 changes: 23 additions & 15 deletions src/main/java/stashpullrequestbuilder/stashpullrequestbuilder/stash/StashApiClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package stashpullrequestbuilder.stashpullrequestbuilder.stash;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.ProxyConfiguration;
import java.io.IOException;
import java.io.InputStream;
import java.io.StringWriter;
Expand All @@ -24,6 +25,7 @@
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import jenkins.model.Jenkins;
import org.apache.commons.httpclient.ConnectTimeoutException;
import org.apache.commons.httpclient.Credentials;
import org.apache.commons.httpclient.HttpClient;
Expand Down Expand Up @@ -60,12 +62,15 @@ public class StashApiClient {

private String apiBaseUrl;

private final ProxyConfiguration proxy;

private String project;
private String repositoryName;
private Credentials credentials;

public StashApiClient(
String stashHost,
ProxyConfiguration proxy,
String username,
String password,
String project,
Expand All @@ -75,6 +80,7 @@ public StashApiClient(
this.project = project;
this.repositoryName = repositoryName;
this.apiBaseUrl = stashHost.replaceAll("/$", "") + "/rest/api/1.0/projects/";
this.proxy = proxy;
if (ignoreSsl) {
Protocol easyhttps =
new Protocol("https", (ProtocolSocketFactory) new EasySSLProtocolSocketFactory(), 443);
Expand Down Expand Up @@ -197,21 +203,23 @@ private HttpClient getHttpClient() {
httpParams.setParameter(
CoreConnectionPNames.SO_TIMEOUT, StashApiClient.HTTP_SOCKET_TIMEOUT_SECONDS * 1000);

// if (Jenkins.getInstance() != null) {
// ProxyConfiguration proxy = Jenkins.getInstance().proxy;
// if (proxy != null) {
// logger.info("Jenkins proxy: " + proxy.name + ":" + proxy.port);
// client.getHostConfiguration().setProxy(proxy.name, proxy.port);
// String username = proxy.getUserName();
// String password = proxy.getPassword();
// // Consider it to be passed if username specified. Sufficient?
// if (username != null && !"".equals(username.trim())) {
// logger.info("Using proxy authentication (user=" + username + ")");
// client.getState().setProxyCredentials(AuthScope.ANY,
// new UsernamePasswordCredentials(username, password));
// }
// }
// }
if (Jenkins.getInstanceOrNull() != null && proxy != null) {
Copy link

Choose a reason for hiding this comment

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

What's the point of checking Jenkins.getInstanceOrNull() now that the code uses the proxy field?

logger.fine("Using proxy: " + proxy.name + ":" + proxy.port);
client.getHostConfiguration().setProxy(proxy.name, proxy.port);

String username = proxy.getUserName();
String password = proxy.getPassword();

// Consider it to be passed if username specified. Sufficient?
if (username != null && !"".equals(username.trim())) {
logger.fine("With proxy authentication (user=" + username + ")");
client
.getState()
.setProxyCredentials(
AuthScope.ANY, new UsernamePasswordCredentials(username, password));
}
}

return client;
}

Expand Down
26 changes: 26 additions & 0 deletions .../resources/stashpullrequestbuilder/stashpullrequestbuilder/StashBuildTrigger/config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,32 @@
<f:entry title="Stash URL" field="stashHost">
<f:textbox />
</f:entry>
<f:nested>
<table>
<f:optionalBlock title="Enable proxy" name="proxySettings" checked="${instance.getProxySettings() != null}">
<f:nested>
<table>
<f:radioBlock name="proxyChoice" value="jenkins" title="Inherit from Jenkins" checked="${(instance.getProxySettings() != null) ? instance.isProxyType('jenkins') : true}">
</f:radioBlock>
<f:radioBlock name="proxyChoice" value="custom" title="Custom" checked="${(instance.getProxySettings() != null) ? instance.isProxyType('custom') : false}">
<f:entry title="Proxy Host" field="proxyHost">
<f:textbox value="${(instance.getProxySettings() != null &amp;&amp; instance.isProxyType('custom')) ? instance.getProxyDetails().getString('proxyHost') : ''}" />
Copy link

Choose a reason for hiding this comment

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

there is significant repetition here
I would expect jelly has some kind of a block/panel element that would enable you to hide irrelevant fields with one toggle

</f:entry>
<f:entry title="Proxy Port" field="proxyPort">
<f:textbox value="${(instance.getProxySettings() != null &amp;&amp; instance.isProxyType('custom')) ? instance.getProxyDetails().getInt('proxyPort') : ''}" />
</f:entry>
<f:entry title="Proxy Username" field="proxyUser">
<f:textbox value="${(instance.getProxySettings() != null &amp;&amp; instance.isProxyType('custom')) ? instance.getProxyDetails().getString('proxyUser') : ''}" />
</f:entry>
<f:entry title="Password" field="proxyPass">
<f:password value="${(instance.getProxySettings() != null &amp;&amp; instance.isProxyType('custom')) ? instance.getProxyDetails().getString('proxyPass') : ''}" />
</f:entry>
</f:radioBlock>
</table>
</f:nested>
</f:optionalBlock>
</table>
</f:nested>
<f:entry title="Stash credentials (username/password)" field="credentialsId">
<c:select />
</f:entry>
Expand Down