Skip to content

Commit 52c7975

Browse files
KostyaShaKostyaSha
committed
Merge pull request jenkinsci#68 from lanwen/global_conf
use GlobalConfiguration extension point to store all global config
2 parents 81fbf3f + 2108560 commit 52c7975

File tree

8 files changed

+183
-112
lines changed

8 files changed

+183
-112
lines changed

‎src/main/java/org/jenkinsci/plugins/github/GitHubPlugin.java

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,34 @@
11
package org.jenkinsci.plugins.github;
22

33
import hudson.Plugin;
4-
import hudson.model.Descriptor.FormException;
5-
import jenkins.model.Jenkins;
6-
import net.sf.json.JSONObject;
74
import org.jenkinsci.plugins.github.config.GitHubPluginConfig;
85
import org.jenkinsci.plugins.github.migration.Migrator;
9-
import org.kohsuke.stapler.StaplerRequest;
106

11-
import javax.servlet.ServletException;
12-
import java.io.IOException;
7+
import javax.annotation.Nonnull;
138

14-
import static java.lang.String.format;
15-
import static org.apache.commons.lang3.Validate.notNull;
9+
import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
1610

1711
/**
1812
* Main entry point for this plugin
19-
* Stores global configuration
13+
*
14+
* Launches migration from old config versions
15+
* Contains helper method to get global plugin configuration - {@link #configuration()}
2016
*
2117
* @author lanwen (Merkushev Kirill)
2218
*/
2319
public class GitHubPlugin extends Plugin {
24-
private GitHubPluginConfig configuration = new GitHubPluginConfig();
25-
26-
public GitHubPluginConfig getConfiguration() {
27-
return configuration;
28-
}
29-
3020
/**
3121
* Launched before plugin starts
3222
* Adds alias for {@link GitHubPlugin} to simplify resulting xml
3323
*/
3424
public static void init() {
35-
Jenkins.XSTREAM2.alias("github-plugin", GitHubPlugin.class);
3625
Migrator.enableCompatibilityAliases();
26+
Migrator.enableAliases();
3727
}
3828

3929
@Override
4030
public void start() throws Exception {
4131
init();
42-
load();
4332
}
4433

4534
/**
@@ -50,40 +39,16 @@ public void postInitialize() throws Exception {
5039
new Migrator().migrate();
5140
}
5241

53-
@Override
54-
public void configure(StaplerRequest req, JSONObject formData) throws IOException, ServletException, FormException {
55-
try {
56-
configuration = req.bindJSON(GitHubPluginConfig.class, formData);
57-
} catch (Exception e) {
58-
throw new FormException(
59-
format("Mailformed GitHub Plugin configuration (%s)", e.getMessage()), e, "github-configuration");
60-
}
61-
save();
62-
}
63-
64-
@Override
65-
protected void load() throws IOException {
66-
super.load();
67-
if (configuration == null) {
68-
configuration = new GitHubPluginConfig();
69-
save();
70-
}
71-
}
72-
73-
/**
74-
* @return instance of this plugin
75-
*/
76-
public static GitHubPlugin get() {
77-
return notNull(Jenkins.getInstance(), "Jenkins is not ready to return instance")
78-
.getPlugin(GitHubPlugin.class);
79-
}
80-
8142
/**
82-
* Shortcut method for {@link GitHubPlugin#get()#getConfiguration()}.
43+
* Shortcut method for getting instance of {@link GitHubPluginConfig}.
8344
*
8445
* @return configuration of plugin
8546
*/
47+
@Nonnull
8648
public static GitHubPluginConfig configuration() {
87-
return get().getConfiguration();
49+
return defaultIfNull(
50+
GitHubPluginConfig.all().get(GitHubPluginConfig.class),
51+
GitHubPluginConfig.EMPTY_CONFIG
52+
);
8853
}
8954
}

‎src/main/java/org/jenkinsci/plugins/github/config/GitHubPluginConfig.java

Lines changed: 95 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,21 @@
44
import com.google.common.base.Predicate;
55
import com.google.common.base.Predicates;
66
import hudson.Extension;
7-
import hudson.model.AbstractDescribableImpl;
7+
import hudson.XmlFile;
88
import hudson.model.AbstractProject;
99
import hudson.model.Descriptor;
1010
import hudson.util.FormValidation;
11+
import jenkins.model.GlobalConfiguration;
1112
import jenkins.model.Jenkins;
13+
import net.sf.json.JSONObject;
1214
import org.apache.commons.codec.binary.Base64;
1315
import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
1416
import org.jenkinsci.plugins.github.GitHubPlugin;
1517
import org.jenkinsci.plugins.github.internal.GHPluginConfigException;
18+
import org.jenkinsci.plugins.github.migration.Migrator;
1619
import org.kohsuke.github.GitHub;
17-
import org.kohsuke.stapler.DataBoundConstructor;
18-
import org.kohsuke.stapler.DataBoundSetter;
1920
import org.kohsuke.stapler.QueryParameter;
21+
import org.kohsuke.stapler.StaplerRequest;
2022
import org.slf4j.Logger;
2123
import org.slf4j.LoggerFactory;
2224

@@ -30,6 +32,7 @@
3032
import java.util.Collections;
3133
import java.util.List;
3234

35+
import static java.lang.String.format;
3336
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.allowedToManageHooks;
3437
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.loginToGithub;
3538
import static org.jenkinsci.plugins.github.util.FluentIterableWrapper.from;
@@ -41,31 +44,50 @@
4144
* @author lanwen (Merkushev Kirill)
4245
* @since TODO
4346
*/
44-
public class GitHubPluginConfig extends AbstractDescribableImpl<GitHubPluginConfig> {
47+
@Extension
48+
public class GitHubPluginConfig extends GlobalConfiguration {
4549
private static final Logger LOGGER = LoggerFactory.getLogger(GitHubPluginConfig.class);
50+
public static final String GITHUB_PLUGIN_CONFIGURATION_ID = "github-plugin-configuration";
51+
52+
/**
53+
* Helps to avoid null in {@link GitHubPlugin#configuration()}
54+
*/
55+
public static final GitHubPluginConfig EMPTY_CONFIG =
56+
new GitHubPluginConfig(Collections.<GitHubServerConfig>emptyList());
4657

4758
private List<GitHubServerConfig> configs = new ArrayList<GitHubServerConfig>();
4859
private URL hookUrl;
4960
private transient boolean overrideHookUrl;
5061

51-
@DataBoundConstructor
62+
/**
63+
* Used to get current instance identity.
64+
* It compared with same value when testing hook url availability in {@link #doCheckHookUrl(String)}
65+
*/
66+
@Inject
67+
@SuppressWarnings("unused")
68+
private transient InstanceIdentity identity;
69+
5270
public GitHubPluginConfig() {
71+
load();
5372
}
5473

55-
public List<GitHubServerConfig> getConfigs() {
56-
return configs;
74+
public GitHubPluginConfig(List<GitHubServerConfig> configs) {
75+
this.configs = configs;
5776
}
5877

59-
@DataBoundSetter
78+
@SuppressWarnings("unused")
6079
public void setConfigs(List<GitHubServerConfig> configs) {
6180
this.configs = configs;
6281
}
6382

83+
public List<GitHubServerConfig> getConfigs() {
84+
return configs;
85+
}
86+
6487
public boolean isManageHooks() {
6588
return from(getConfigs()).filter(allowedToManageHooks()).first().isPresent();
6689
}
6790

68-
@DataBoundSetter
6991
public void setHookUrl(URL hookUrl) {
7092
if (overrideHookUrl) {
7193
this.hookUrl = hookUrl;
@@ -74,7 +96,6 @@ public void setHookUrl(URL hookUrl) {
7496
}
7597
}
7698

77-
@DataBoundSetter
7899
public void setOverrideHookUrl(boolean overrideHookUrl) {
79100
this.overrideHookUrl = overrideHookUrl;
80101
}
@@ -111,60 +132,78 @@ public List<Descriptor> actions() {
111132
return Collections.singletonList(Jenkins.getInstance().getDescriptor(GitHubTokenCredentialsCreator.class));
112133
}
113134

114-
@Extension
115-
public static class GitHubPluginConfigDescriptor extends Descriptor<GitHubPluginConfig> {
135+
/**
136+
* To avoid long class name as id in xml tag name and config file
137+
*/
138+
@Override
139+
public String getId() {
140+
return GITHUB_PLUGIN_CONFIGURATION_ID;
141+
}
116142

117-
/**
118-
* Used to get current instance identity. It compared with same value when testing hook url availability
119-
*/
120-
@Inject
121-
@SuppressWarnings("unused")
122-
private transient InstanceIdentity identity;
143+
/**
144+
* @return config file with global {@link com.thoughtworks.xstream.XStream} instance
145+
* with enabled aliases in {@link Migrator#enableAliases()}
146+
*/
147+
@Override
148+
protected XmlFile getConfigFile() {
149+
return new XmlFile(Jenkins.XSTREAM2, super.getConfigFile().getFile());
150+
}
123151

124-
@Override
125-
public String getDisplayName() {
126-
return "GitHub Plugin Configuration";
152+
@Override
153+
public boolean configure(StaplerRequest req, JSONObject json) throws FormException {
154+
try {
155+
req.bindJSON(this, json);
156+
} catch (Exception e) {
157+
throw new FormException(
158+
format("Mailformed GitHub Plugin configuration (%s)", e.getMessage()), e, "github-configuration");
127159
}
160+
save();
161+
return true;
162+
}
128163

129-
@SuppressWarnings("unused")
130-
public FormValidation doReRegister() {
131-
if (!GitHubPlugin.configuration().isManageHooks()) {
132-
return FormValidation.warning("Works only when Jenkins manages hooks (one ore more creds specified)");
133-
}
134-
135-
List<AbstractProject> registered = GitHubWebHook.get().reRegisterAllHooks();
164+
@Override
165+
public String getDisplayName() {
166+
return "GitHub Plugin Configuration";
167+
}
136168

137-
LOGGER.info("Called registerHooks() for {} jobs", registered.size());
138-
return FormValidation.ok("Called re-register hooks for %s jobs", registered.size());
169+
@SuppressWarnings("unused")
170+
public FormValidation doReRegister() {
171+
if (!GitHubPlugin.configuration().isManageHooks()) {
172+
return FormValidation.warning("Works only when Jenkins manages hooks (one ore more creds specified)");
139173
}
140174

141-
@SuppressWarnings("unused")
142-
public FormValidation doCheckHookUrl(@QueryParameter String value) {
143-
try {
144-
HttpURLConnection con = (HttpURLConnection) new URL(value).openConnection();
145-
con.setRequestMethod("POST");
146-
con.setRequestProperty(GitHubWebHook.URL_VALIDATION_HEADER, "true");
147-
con.connect();
148-
if (con.getResponseCode() != 200) {
149-
return FormValidation.error("Got %d from %s", con.getResponseCode(), value);
150-
}
151-
String v = con.getHeaderField(GitHubWebHook.X_INSTANCE_IDENTITY);
152-
if (v == null) {
153-
// people might be running clever apps that's not Jenkins, and that's OK
154-
return FormValidation.warning("It doesn't look like %s is talking to any Jenkins. " +
155-
"Are you running your own app?", value);
156-
}
157-
RSAPublicKey key = identity.getPublic();
158-
String expected = new String(Base64.encodeBase64(key.getEncoded()));
159-
if (!expected.equals(v)) {
160-
// if it responds but with a different ID, that's more likely wrong than correct
161-
return FormValidation.error("%s is connecting to different Jenkins instances", value);
162-
}
163-
164-
return FormValidation.ok();
165-
} catch (IOException e) {
166-
return FormValidation.error(e, "Failed to test a connection to %s", value);
175+
List<AbstractProject> registered = GitHubWebHook.get().reRegisterAllHooks();
176+
177+
LOGGER.info("Called registerHooks() for {} jobs", registered.size());
178+
return FormValidation.ok("Called re-register hooks for %s jobs", registered.size());
179+
}
180+
181+
@SuppressWarnings("unused")
182+
public FormValidation doCheckHookUrl(@QueryParameter String value) {
183+
try {
184+
HttpURLConnection con = (HttpURLConnection) new URL(value).openConnection();
185+
con.setRequestMethod("POST");
186+
con.setRequestProperty(GitHubWebHook.URL_VALIDATION_HEADER, "true");
187+
con.connect();
188+
if (con.getResponseCode() != 200) {
189+
return FormValidation.error("Got %d from %s", con.getResponseCode(), value);
167190
}
191+
String v = con.getHeaderField(GitHubWebHook.X_INSTANCE_IDENTITY);
192+
if (v == null) {
193+
// people might be running clever apps that's not Jenkins, and that's OK
194+
return FormValidation.warning("It doesn't look like %s is talking to any Jenkins. "
195+
+ "Are you running your own app?", value);
196+
}
197+
RSAPublicKey key = identity.getPublic();
198+
String expected = new String(Base64.encodeBase64(key.getEncoded()));
199+
if (!expected.equals(v)) {
200+
// if it responds but with a different ID, that's more likely wrong than correct
201+
return FormValidation.error("%s is connecting to different Jenkins instances", value);
202+
}
203+
204+
return FormValidation.ok();
205+
} catch (IOException e) {
206+
return FormValidation.error(e, "Failed to test a connection to %s", value);
168207
}
169208
}
170209
}

‎src/main/java/org/jenkinsci/plugins/github/migration/Migrator.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.google.common.base.Function;
77
import jenkins.model.Jenkins;
88
import org.jenkinsci.plugins.github.GitHubPlugin;
9+
import org.jenkinsci.plugins.github.config.GitHubPluginConfig;
910
import org.jenkinsci.plugins.github.config.GitHubServerConfig;
1011
import org.jenkinsci.plugins.github.config.GitHubTokenCredentialsCreator;
1112
import org.jenkinsci.plugins.github.deprecated.Credential;
@@ -48,7 +49,7 @@ public void migrate() throws IOException {
4849

4950
descriptor.clearCredentials();
5051
descriptor.save();
51-
GitHubPlugin.get().save();
52+
GitHubPlugin.configuration().save();
5253
}
5354

5455
if (descriptor.getDeprecatedHookUrl() != null) {
@@ -57,7 +58,7 @@ public void migrate() throws IOException {
5758
GitHubPlugin.configuration().setHookUrl(descriptor.getDeprecatedHookUrl());
5859
descriptor.clearDeprecatedHookUrl();
5960
descriptor.save();
60-
GitHubPlugin.get().save();
61+
GitHubPlugin.configuration().save();
6162
}
6263
}
6364

@@ -96,4 +97,11 @@ public GitHubServerConfig apply(Credential input) {
9697
public static void enableCompatibilityAliases() {
9798
Jenkins.XSTREAM2.addCompatibilityAlias("com.cloudbees.jenkins.Credential", Credential.class);
9899
}
100+
101+
/**
102+
* Simplifies long node names in config files
103+
*/
104+
public static void enableAliases() {
105+
Jenkins.XSTREAM2.alias(GitHubPluginConfig.GITHUB_PLUGIN_CONFIGURATION_ID, GitHubPluginConfig.class);
106+
}
99107
}

‎src/main/resources/org/jenkinsci/plugins/github/GitHubPlugin/config.groovy

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎src/test/java/org/jenkinsci/plugins/github/migration/MigratorTest.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.hamcrest.MatcherAssert.assertThat;
1919
import static org.hamcrest.Matchers.both;
2020
import static org.hamcrest.Matchers.containsString;
21+
import static org.hamcrest.Matchers.equalTo;
2122
import static org.hamcrest.Matchers.hasItems;
2223
import static org.hamcrest.Matchers.hasSize;
2324
import static org.hamcrest.Matchers.is;
@@ -78,6 +79,18 @@ public void shouldMigrateCredentials() throws Exception {
7879
));
7980
}
8081

82+
@Test
83+
@LocalData
84+
public void shouldLoadDataAfterStart() throws Exception {
85+
assertThat("should load 3 configs", GitHubPlugin.configuration().getConfigs(), hasSize(2));
86+
assertThat("migrate custom url", GitHubPlugin.configuration().getConfigs(), hasItems(
87+
withApiUrl(is(CUSTOM_GH_URL)),
88+
withApiUrl(is(GITHUB_URL))
89+
));
90+
assertThat("should load hook url",
91+
GitHubPlugin.configuration().getHookUrl().toString(), equalTo(HOOK_FROM_LOCAL_DATA));
92+
}
93+
8194
@Test
8295
public void shouldConvertCredsToServerConfig() throws Exception {
8396
GitHubServerConfig conf = new Migrator().toGHServerConfig()

‎src/test/resources/org/jenkinsci/plugins/github/migration/MigratorTest/shouldLoadDataAfterStart/com.cloudbees.jenkins.GitHubPushTrigger.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
<?xml version='1.0' encoding='UTF-8'?>
2+
<com.cloudbees.jenkins.GitHubPushTrigger_-DescriptorImpl plugin="github@1.12.0"/>

0 commit comments

Comments
 (0)