Skip to content

Commit

Permalink
Merge pull request #221 from KostyaSha/pr210
Browse files Browse the repository at this point in the history
End Pr210
  • Loading branch information
KostyaSha authored Oct 7, 2019
2 parents 73e2513 + b2b7d15 commit e9866b0
Show file tree
Hide file tree
Showing 10 changed files with 290 additions and 21 deletions.
16 changes: 16 additions & 0 deletions pom.xml
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<concurrency>1</concurrency>
<java.level>8</java.level>
<workflow.version>1.14.2</workflow.version>
<configuration-as-code.version>1.32</configuration-as-code.version>
</properties>

<repositories>
Expand Down Expand Up @@ -190,6 +191,21 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,11 @@ public void clearCredentials() {
}

/**
* @deprecated use {@link GitHubPluginConfig#isOverrideHookURL()}
* @deprecated use {@link GitHubPluginConfig#isOverrideHookUrl()}
*/
@Deprecated
public boolean hasOverrideURL() {
return GitHubPlugin.configuration().isOverrideHookURL();
return GitHubPlugin.configuration().isOverrideHookUrl();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.github.GitHub;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.interceptor.RequirePOST;
Expand All @@ -40,6 +41,7 @@

import static com.google.common.base.Charsets.UTF_8;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.isEmpty;
import static org.apache.commons.lang3.StringUtils.isNotEmpty;
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.allowedToManageHooks;
import static org.jenkinsci.plugins.github.config.GitHubServerConfig.loginToGithub;
Expand All @@ -62,14 +64,12 @@ public class GitHubPluginConfig extends GlobalConfiguration {
* Helps to avoid null in {@link GitHubPlugin#configuration()}
*/
public static final GitHubPluginConfig EMPTY_CONFIG =
new GitHubPluginConfig(Collections.<GitHubServerConfig>emptyList());
new GitHubPluginConfig(Collections.emptyList());

private List<GitHubServerConfig> configs = new ArrayList<GitHubServerConfig>();
private List<GitHubServerConfig> configs = new ArrayList<>();
private URL hookUrl;
private HookSecretConfig hookSecretConfig = new HookSecretConfig(null);

private transient boolean overrideHookUrl;

/**
* Used to get current instance identity.
* It compared with same value when testing hook url availability in {@link #doCheckHookUrl(String)}
Expand All @@ -87,6 +87,7 @@ public GitHubPluginConfig(List<GitHubServerConfig> configs) {
}

@SuppressWarnings("unused")
@DataBoundSetter
public void setConfigs(List<GitHubServerConfig> configs) {
this.configs = configs;
}
Expand All @@ -99,16 +100,18 @@ public boolean isManageHooks() {
return from(getConfigs()).filter(allowedToManageHooks()).first().isPresent();
}

public void setHookUrl(URL hookUrl) {
if (overrideHookUrl) {
this.hookUrl = hookUrl;
} else {
@DataBoundSetter
public void setHookUrl(String hookUrl) {
if (isEmpty(hookUrl)) {
this.hookUrl = null;
} else {
this.hookUrl = parseHookUrl(hookUrl);
}
}

@DataBoundSetter
@Deprecated
public void setOverrideHookUrl(boolean overrideHookUrl) {
this.overrideHookUrl = overrideHookUrl;
}

/**
Expand All @@ -123,10 +126,16 @@ public URL getHookUrl() throws GHPluginConfigException {
}
}

public boolean isOverrideHookURL() {
@SuppressWarnings("unused")
public boolean isOverrideHookUrl() {
return hookUrl != null;
}

@Deprecated
public boolean isOverrideHookURL() {
return isOverrideHookUrl();
}

/**
* Filters all stored configs against given predicate then
* logs in as the given user and returns the non null connection objects
Expand Down Expand Up @@ -265,7 +274,16 @@ public HookSecretConfig getHookSecretConfig() {
return hookSecretConfig;
}

@DataBoundSetter
public void setHookSecretConfig(HookSecretConfig hookSecretConfig) {
this.hookSecretConfig = hookSecretConfig;
}

private URL parseHookUrl(String hookUrl) {
try {
return new URL(hookUrl);
} catch (MalformedURLException e) {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void migrate() throws IOException {
if (descriptor.getDeprecatedHookUrl() != null) {
LOGGER.warn("Migration for old GitHub Plugin hook url started");
GitHubPlugin.configuration().setOverrideHookUrl(true);
GitHubPlugin.configuration().setHookUrl(descriptor.getDeprecatedHookUrl());
GitHubPlugin.configuration().setHookUrl(descriptor.getDeprecatedHookUrl().toString());
descriptor.clearDeprecatedHookUrl();
descriptor.save();
GitHubPlugin.configuration().save();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package org.jenkinsci.plugins.github.config.GitHubPluginConfig

import com.cloudbees.jenkins.GitHubPushTrigger
import lib.FormTagLib

def f = namespace(lib.FormTagLib);
def f = namespace(FormTagLib);

f.section(title: descriptor.displayName) {
f.entry(title: _("GitHub Servers"),
Expand All @@ -26,8 +27,7 @@ f.section(title: descriptor.displayName) {
table(width: "100%", style: "margin-left: 7px;") {
f.optionalBlock(title: _("Specify another hook URL for GitHub configuration"),
inline: true,
field: "overrideHookUrl",
checked: instance.overrideHookURL) {
checked: instance.isOverrideHookUrl()) {
f.entry(field: "hookUrl") {
f.textbox(checkMethod: "post")
}
Expand Down
31 changes: 28 additions & 3 deletions src/test/java/com/cloudbees/jenkins/GlobalConfigSubmitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
@Ignore("Have troubles with memory consumption")
public class GlobalConfigSubmitTest {

public static final String OVERRIDE_HOOK_URL_CHECKBOX = "_.overrideHookUrl";
public static final String OVERRIDE_HOOK_URL_CHECKBOX = "_.isOverrideHookUrl";
public static final String HOOK_URL_INPUT = "_.hookUrl";

private static final String WEBHOOK_URL = "http://jenkinsci.example.com/jenkins/github-webhook/";
Expand All @@ -33,14 +33,39 @@ public class GlobalConfigSubmitTest {
public JenkinsRule jenkins = new JenkinsRule();

@Test
public void shouldTurnOnOverridingWhenThereIsCredentials() throws Exception {
public void shouldSetHookUrl() throws Exception {
HtmlForm form = globalConfig();

form.getInputByName(OVERRIDE_HOOK_URL_CHECKBOX).setChecked(true);
form.getInputByName(HOOK_URL_INPUT).setValueAttribute(WEBHOOK_URL);
jenkins.submit(form);

assertThat(GitHubPlugin.configuration().isOverrideHookURL(), is(true));
assertThat(GitHubPlugin.configuration().getHookUrl(), equalTo(new URL(WEBHOOK_URL)));
}

@Test
public void shouldNotSetHookUrl() throws Exception {
GitHubPlugin.configuration().setHookUrl(WEBHOOK_URL);

HtmlForm form = globalConfig();

form.getInputByName(OVERRIDE_HOOK_URL_CHECKBOX).setChecked(false);
form.getInputByName(HOOK_URL_INPUT).setValueAttribute("http://foo");
jenkins.submit(form);

assertThat(GitHubPlugin.configuration().getHookUrl(), equalTo(new URL(WEBHOOK_URL)));
}

@Test
public void shouldNotOverrideAPreviousHookUrlIfNotChecked() throws Exception {
GitHubPlugin.configuration().setHookUrl(WEBHOOK_URL);

HtmlForm form = globalConfig();

form.getInputByName(OVERRIDE_HOOK_URL_CHECKBOX).setChecked(false);
form.getInputByName(HOOK_URL_INPUT).setValueAttribute("");
jenkins.submit(form);

assertThat(GitHubPlugin.configuration().getHookUrl(), equalTo(new URL(WEBHOOK_URL)));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package org.jenkinsci.plugins.github.config;

import io.jenkins.plugins.casc.ConfigurationContext;
import io.jenkins.plugins.casc.Configurator;
import io.jenkins.plugins.casc.ConfiguratorRegistry;
import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
import io.jenkins.plugins.casc.model.CNode;
import io.jenkins.plugins.casc.model.Mapping;
import org.junit.Rule;
import org.junit.Test;

import java.util.List;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;
import static org.jenkinsci.plugins.github.test.GitHubServerConfigMatcher.*;

public class ConfigAsCodeTest {

@Rule
public JenkinsConfiguredWithCodeRule r = new JenkinsConfiguredWithCodeRule();

@Test
@ConfiguredWithCode("configuration-as-code.yml")
public void shouldSupportConfigurationAsCode() throws Exception {

GitHubPluginConfig gitHubPluginConfig = GitHubPluginConfig.all().get(GitHubPluginConfig.class);

/** Test Global Config Properties */

assertThat(
"getHookUrl() is configured",
gitHubPluginConfig.getHookUrl().toString(),
is("http://some.com/github-webhook/secret-path")
);

assertThat(
"getHookSecretConfig().getCredentialsId() is configured",
gitHubPluginConfig.getHookSecretConfig().getCredentialsId(),
is("hook_secret_cred_id")
);

/** Test GitHub Server Configs */

assertThat("configs are loaded", gitHubPluginConfig.getConfigs(), hasSize(2));

assertThat("configs are set", gitHubPluginConfig.getConfigs(), hasItems(
both(withName(is("Public GitHub")))
.and(withApiUrl(is("https://api.github.com")))
.and(withCredsId(is("public_cred_id")))
.and(withClientCacheSize(is(20)))
.and(withIsManageHooks(is(true))),
both(withName(is("Private GitHub")))
.and(withApiUrl(is("https://api.some.com")))
.and(withCredsId(is("private_cred_id")))
.and(withClientCacheSize(is(40)))
.and(withIsManageHooks(is(false)))
));
}

@Test
@ConfiguredWithCode("configuration-as-code.yml")
public void exportConfiguration() throws Exception {
GitHubPluginConfig globalConfiguration = GitHubPluginConfig.all().get(GitHubPluginConfig.class);

ConfiguratorRegistry registry = ConfiguratorRegistry.get();
ConfigurationContext context = new ConfigurationContext(registry);
final Configurator c = context.lookupOrFail(GitHubPluginConfig.class);

@SuppressWarnings("unchecked")
CNode node = c.describe(globalConfiguration, context);
assertThat(node, notNullValue());
final Mapping mapping = node.asMapping();

assertThat(mapping.getScalarValue("hookUrl"), is("http://some.com/github-webhook/secret-path"));

CNode configsNode = mapping.get("configs");
assertThat(configsNode, notNullValue());

List<Mapping> configsMapping = (List) configsNode.asSequence();
assertThat(configsMapping, hasSize(2));

assertThat("configs are set", configsMapping,
hasItems(
both(withCredsIdS(is("public_cred_id")))
.and(withNameS(is("Public GitHub"))),
both(withNameS(is("Private GitHub")))
.and(withApiUrlS(is("https://api.some.com")))
.and(withCredsIdS(is("private_cred_id")))
.and(withClientCacheSizeS(is(40)))
.and(withIsManageHooksS(is(false)))
)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ public void shouldNotThrowExcMalformedHookUrlInOldConfig() throws IOException {
assertThat("self hook url", trigger.getDescriptor().getDeprecatedHookUrl(), nullValue());
assertThat("imported hook url", valueOf(trigger.getDescriptor().getHookUrl()),
containsString(Jenkins.getInstance().getRootUrl() + GitHubWebHook.URLNAME));
assertThat("in plugin - override", GitHubPlugin.configuration().isOverrideHookURL(), is(false));
assertThat("in plugin - override", GitHubPlugin.configuration().isOverrideHookUrl(), is(false));
}

@Test
@LocalData
public void shouldMigrateHookUrl() {
assertThat("in plugin - override", GitHubPlugin.configuration().isOverrideHookURL(), is(true));
assertThat("in plugin - override", GitHubPlugin.configuration().isOverrideHookUrl(), is(true));
assertThat("in plugin", valueOf(GitHubPlugin.configuration().getHookUrl()), is(HOOK_FROM_LOCAL_DATA));

assertThat("should nullify hook url after migration",
Expand Down
Loading

0 comments on commit e9866b0

Please sign in to comment.