Skip to content

Commit

Permalink
[JENKINS-75073] Missing serverUrl in fillCredentialsIdItems cause som…
Browse files Browse the repository at this point in the history 
…e credentials missing in the initial configuration (#953)

If the number of server instances is only one, when a new Jenkins project is created the server list combobox is not available. This causes, in the UI component that loads credentials, to pass the stapler parameter serverUrl as an empty string. In this scenario, serverUrl must be taken from the first of configured endpoints so that it can properly filter the list of credentials.
Move some unit test to JUnit5
  • Loading branch information
@nfalco79
nfalco79 authored Jan 10, 2025
1 parent 859f608 commit 6a0d987
Show file tree
Hide file tree
Showing 16 changed files with 636 additions and 600 deletions.
7 changes: 3 additions & 4 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketGitSCMBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,9 @@ public BitbucketGitSCMBuilder(@NonNull BitbucketSCMSource scmSource, @NonNull SC
this.scmSource = scmSource;

String serverURL = scmSource.getServerUrl();
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(serverURL);
if (endpoint == null) {
endpoint = new BitbucketServerEndpoint(null, serverURL, false, null);
}
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(serverURL)
.orElse(new BitbucketServerEndpoint(null, serverURL, false, null));

String repositoryURL = endpoint.getRepositoryUrl(scmSource.getRepoOwner(), scmSource.getRepository());
if (BitbucketApiUtils.isCloud(endpoint.getServerUrl())) {
Expand Down
45 changes: 27 additions & 18 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;

import static com.cloudbees.jenkins.plugins.bitbucket.impl.util.BitbucketApiUtils.getFromBitbucket;

Expand Down Expand Up @@ -288,9 +289,9 @@ public String getServerUrl() {
}

@DataBoundSetter
public void setServerUrl(String serverUrl) {
public void setServerUrl(@CheckForNull String serverUrl) {
serverUrl = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
if (!StringUtils.equals(this.serverUrl, serverUrl)) {
if (serverUrl != null && !StringUtils.equals(this.serverUrl, serverUrl)) {

Check warning on line 294 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 294 is only partially covered, one branch is missing
this.serverUrl = serverUrl;
resetId();
}
Expand Down Expand Up @@ -384,14 +385,14 @@ public String getPattern() {
@DataBoundSetter
public void setBitbucketServerUrl(String url) {
url = BitbucketEndpointConfiguration.normalizeServerUrl(url);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(url);
if (endpoint != null) {
// we have a match
setServerUrl(url);
return;
url = StringUtils.defaultIfBlank(url, BitbucketCloudEndpoint.SERVER_URL); // when BitbucketServerUrl was null means cloud was configured
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(url)
.orElse(null);
if (endpoint == null) {
LOGGER.log(Level.WARNING, "Call to legacy setBitbucketServerUrl({0}) method is configuring a url missing "
+ "from the global configuration.", url);
}
LOGGER.log(Level.WARNING, "Call to legacy setBitbucketServerUrl({0}) method is configuring a url missing "
+ "from the global configuration.", url);
setServerUrl(url);
}

Expand All @@ -400,7 +401,10 @@ public void setBitbucketServerUrl(String url) {
@RestrictedSince("2.2.0")
@CheckForNull
public String getBitbucketServerUrl() {
if (BitbucketEndpointConfiguration.get().findEndpoint(serverUrl) instanceof BitbucketCloudEndpoint) {
if (BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.filter(BitbucketCloudEndpoint.class::isInstance)
.isPresent()) {
return null;
}
return serverUrl;
Expand Down Expand Up @@ -629,7 +633,7 @@ public String getIconClassName() {
@Override
public SCMNavigator newInstance(String name) {
BitbucketSCMNavigator instance = new BitbucketSCMNavigator(StringUtils.defaultString(name));
instance.setTraits((List) getTraitsDefaults());
instance.setTraits(getTraitsDefaults());
return instance;
}

Expand All @@ -647,16 +651,21 @@ public ListBoxModel doFillServerUrlItems(@AncestorInPath SCMSourceOwner context)
return BitbucketEndpointConfiguration.get().getEndpointItems();
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public FormValidation doCheckCredentialsId(@AncestorInPath SCMSourceOwner context, @QueryParameter String serverUrl, @QueryParameter String value) {
return BitbucketCredentials.checkCredentialsId(context, value, serverUrl);
public static FormValidation doCheckCredentialsId(@AncestorInPath SCMSourceOwner context,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL,
@QueryParameter String value) {
return BitbucketCredentials.checkCredentialsId(context, value, serverURL);
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public static FormValidation doCheckMirrorId(@QueryParameter String value, @QueryParameter String serverUrl) {
public static FormValidation doCheckMirrorId(@QueryParameter String value,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
if (!value.isEmpty()) {
BitbucketServerWebhookImplementation webhookImplementation =
BitbucketServerEndpoint.findWebhookImplementation(serverUrl);
BitbucketServerEndpoint.findWebhookImplementation(serverURL);

Check warning on line 668 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 636-668 are not covered by tests
if (webhookImplementation == BitbucketServerWebhookImplementation.PLUGIN) {
return FormValidation.error("Mirror can only be used with native webhooks");
}
Expand All @@ -666,13 +675,13 @@ public static FormValidation doCheckMirrorId(@QueryParameter String value, @Quer

@SuppressWarnings("unused") // used By stapler
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath SCMSourceOwner context,
@QueryParameter String serverUrl) {
return BitbucketCredentials.fillCredentialsIdItems(context, serverUrl);
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
return BitbucketCredentials.fillCredentialsIdItems(context, serverURL);
}

@SuppressWarnings("unused") // used By stapler
public ListBoxModel doFillMirrorIdItems(@AncestorInPath SCMSourceOwner context,
@QueryParameter String serverUrl,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverUrl,
@QueryParameter String credentialsId,
@QueryParameter String repoOwner)
throws FormFillFailure {
Expand Down
26 changes: 19 additions & 7 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,11 @@ public String getServerUrl() {

@DataBoundSetter
public void setServerUrl(@CheckForNull String serverUrl) {
this.serverUrl = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
String url = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
if (url == null) {

Check warning on line 379 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 379 is only partially covered, one branch is missing
url = BitbucketEndpointConfiguration.get().getDefaultEndpoint().getServerUrl();

Check warning on line 380 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 380 is not covered by tests
}
this.serverUrl = url;
}

@NonNull
Expand All @@ -401,7 +405,10 @@ public void setTraits(@CheckForNull List<SCMSourceTrait> traits) {
@DataBoundSetter
public void setBitbucketServerUrl(String url) {
url = BitbucketEndpointConfiguration.normalizeServerUrl(url);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(url);
url = StringUtils.defaultIfBlank(url, BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(url)
.orElse(null);
if (endpoint != null) {
// we have a match
setServerUrl(endpoint.getServerUrl());
Expand All @@ -418,7 +425,10 @@ public void setBitbucketServerUrl(String url) {
@CheckForNull
public String getBitbucketServerUrl() {
String serverUrl = getServerUrl();
if (BitbucketEndpointConfiguration.get().findEndpoint(serverUrl) instanceof BitbucketCloudEndpoint) {
if (BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.filter(BitbucketCloudEndpoint.class::isInstance)
.isPresent()) {
return null;
}
return serverUrl;
Expand Down Expand Up @@ -1336,8 +1346,8 @@ public String getDisplayName() {
@SuppressWarnings("unused") // used By stapler
public FormValidation doCheckCredentialsId(@CheckForNull @AncestorInPath SCMSourceOwner context,
@QueryParameter String value,
@QueryParameter String serverUrl) {
return BitbucketCredentials.checkCredentialsId(context, value, serverUrl);
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
return BitbucketCredentials.checkCredentialsId(context, value, serverURL);
}

@SuppressWarnings("unused") // used By stapler
Expand All @@ -1353,11 +1363,13 @@ public static FormValidation doCheckServerUrl(@AncestorInPath SCMSourceOwner con
return FormValidation.ok();
}

@RequirePOST
@SuppressWarnings("unused") // used By stapler
public static FormValidation doCheckMirrorId(@QueryParameter String value, @QueryParameter String serverUrl) {
public static FormValidation doCheckMirrorId(@QueryParameter String value,
@QueryParameter(fixEmpty = true, value = "serverUrl") String serverURL) {
if (!value.isEmpty()) {
BitbucketServerWebhookImplementation webhookImplementation =
BitbucketServerEndpoint.findWebhookImplementation(serverUrl);
BitbucketServerEndpoint.findWebhookImplementation(serverURL);

Check warning on line 1372 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 1350-1372 are not covered by tests
if (webhookImplementation == BitbucketServerWebhookImplementation.PLUGIN) {
return FormValidation.error("Mirror can only be used with native webhooks");
}
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSourceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*/
package com.cloudbees.jenkins.plugins.bitbucket;

import com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketEndpointConfiguration;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import jenkins.scm.api.trait.SCMSourceBuilder;
Expand Down Expand Up @@ -74,7 +73,7 @@ public BitbucketSCMSourceBuilder(@CheckForNull String id, @NonNull String server
@NonNull String repoName, @CheckForNull String mirrorId) {
super(BitbucketSCMSource.class, repoName);
this.id = id;
this.serverUrl = BitbucketEndpointConfiguration.normalizeServerUrl(serverUrl);
this.serverUrl = serverUrl;
this.credentialsId = credentialsId;
this.mirrorId = mirrorId;
this.repoOwner = repoOwner;
Expand Down
12 changes: 7 additions & 5 deletions src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ protected boolean isMatch(@Nullable String serverUrl) {
@Override
protected BitbucketApi create(@Nullable String serverUrl, @Nullable BitbucketAuthenticator authenticator,
@NonNull String owner, @CheckForNull String projectKey, @CheckForNull String repository) {
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(BitbucketCloudEndpoint.SERVER_URL);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(BitbucketCloudEndpoint.SERVER_URL)
.orElse(null);
boolean enableCache = false;
int teamCacheDuration = 0;
int repositoriesCacheDuration = 0;
if (endpoint != null && endpoint instanceof BitbucketCloudEndpoint) {
enableCache = ((BitbucketCloudEndpoint) endpoint).isEnableCache();
teamCacheDuration = ((BitbucketCloudEndpoint) endpoint).getTeamCacheDuration();
repositoriesCacheDuration = ((BitbucketCloudEndpoint) endpoint).getRepositoriesCacheDuration();
if (endpoint instanceof BitbucketCloudEndpoint cloudEndpoint) {
enableCache = cloudEndpoint.isEnableCache();
teamCacheDuration = cloudEndpoint.getTeamCacheDuration();
repositoriesCacheDuration = cloudEndpoint.getRepositoriesCacheDuration();

Check warning on line 34 in src/main/java/com/cloudbees/jenkins/plugins/bitbucket/client/BitbucketCloudApiFactory.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 25-34 are not covered by tests
}
return new BitbucketCloudApiClient(
enableCache, teamCacheDuration, repositoriesCacheDuration,
Expand Down
9 changes: 6 additions & 3 deletions ...ain/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/AbstractBitbucketEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,11 @@ public void setBitbucketJenkinsRootUrl(String bitbucketJenkinsRootUrl) {
*/
@NonNull
public String getEndpointJenkinsRootUrl() {
if (StringUtils.isBlank(bitbucketJenkinsRootUrl))
if (StringUtils.isBlank(bitbucketJenkinsRootUrl)) {
return ClassicDisplayURLProvider.get().getRoot();
else
} else {
return bitbucketJenkinsRootUrl;
}
}

/**
Expand All @@ -172,7 +173,9 @@ public static String getEndpointJenkinsRootUrl(String serverUrl) {
// Note: do not pre-initialize to the global value, so it can be
// reconfigured on the fly.

AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get().findEndpoint(serverUrl);
AbstractBitbucketEndpoint endpoint = BitbucketEndpointConfiguration.get()
.findEndpoint(serverUrl)
.orElse(null);
if (endpoint != null) {
return endpoint.getEndpointJenkinsRootUrl();
}
Expand Down
Loading

0 comments on commit 6a0d987

Please sign in to comment.