[JENKINS-75729] hookSignatureCredentialsId in a JCasC configuration sometimes is removed (#1053)

Add a new constructor to save hook signature settings. Deserialization uses reflection and the order in which setters are called is not guaranteed, this means that for example in JCasC the hookSignatureCredentialsId may not be set if enableHookSignature is not called as first.
Add test case for hook signature in a JCasC configuration

Author: nfalco79

.github/ISSUE_TEMPLATE/config.yml

@@ -34,7 +34,7 @@
 import jenkins.authentication.tokens.api.AuthenticationTokens;
 import jenkins.model.Jenkins;
 import org.apache.commons.lang3.StringUtils;
-import org.jenkinsci.plugins.displayurlapi.ClassicDisplayURLProvider;
+import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider;
 import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.kohsuke.stapler.DataBoundSetter;
 
@@ -85,9 +85,12 @@ public abstract class AbstractBitbucketEndpoint extends AbstractDescribableImpl<
      * @param credentialsId The {@link StandardCredentials#getId()} of the credentials to use for
      *                      auto-management of hooks.
      */
-    AbstractBitbucketEndpoint(boolean manageHooks, @CheckForNull String credentialsId) {
+    AbstractBitbucketEndpoint(boolean manageHooks, @CheckForNull String credentialsId,
+                              boolean enableHookSignature, @CheckForNull String hookSignatureCredentialsId) {
         this.manageHooks = manageHooks && StringUtils.isNotBlank(credentialsId);
         this.credentialsId = manageHooks ? fixEmptyAndTrim(credentialsId) : null;
+        this.enableHookSignature = enableHookSignature && StringUtils.isNotBlank(hookSignatureCredentialsId);
+        this.hookSignatureCredentialsId = enableHookSignature ? fixEmptyAndTrim(hookSignatureCredentialsId) : null;
     }
 
     /**
@@ -149,24 +152,10 @@ public String getHookSignatureCredentialsId() {
         return hookSignatureCredentialsId;
     }
 
-    @DataBoundSetter
-    public void setHookSignatureCredentialsId(String hookSignatureCredentialsId) {
-        if (enableHookSignature) {
-            this.hookSignatureCredentialsId = fixEmptyAndTrim(hookSignatureCredentialsId);
-        } else {
-            this.hookSignatureCredentialsId = null;
-        }
-    }
-
     public boolean isEnableHookSignature() {
         return enableHookSignature;
     }
 
-    @DataBoundSetter
-    public void setEnableHookSignature(boolean enableHookSignature) {
-        this.enableHookSignature = enableHookSignature;
-    }
-
     /**
      * Jenkins Server Root URL to be used by this Bitbucket endpoint.
      * The global setting from Jenkins.get().getRootUrl()
@@ -179,7 +168,7 @@ public void setEnableHookSignature(boolean enableHookSignature) {
     @NonNull
     public String getEndpointJenkinsRootUrl() {
         if (StringUtils.isBlank(bitbucketJenkinsRootUrl)) {
-            return ClassicDisplayURLProvider.get().getRoot();
+            return DisplayURLProvider.get().getRoot();
         } else {
             return bitbucketJenkinsRootUrl;
         }
@@ -213,7 +202,7 @@ public static String getEndpointJenkinsRootUrl(String serverUrl) {
         if (endpoint != null) {
             return endpoint.getEndpointJenkinsRootUrl();
         }
-        return ClassicDisplayURLProvider.get().getRoot();
+        return DisplayURLProvider.get().getRoot();
     }
 
     /**

.github/issue_template.md

@@ -32,8 +32,7 @@
 import hudson.util.FormValidation;
 import java.util.List;
 import jenkins.model.Jenkins;
-import org.kohsuke.accmod.Restricted;
-import org.kohsuke.accmod.restrictions.NoExternalUse;
+import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.verb.POST;
 
@@ -68,30 +67,42 @@ public class BitbucketCloudEndpoint extends AbstractBitbucketEndpoint {
      */
     private final int repositoriesCacheDuration;
 
-    public BitbucketCloudEndpoint(boolean manageHooks, @CheckForNull String credentialsId) {
-        this(false, 0, 0, manageHooks, credentialsId);
+    /**
+     * Default constructor.
+     */
+    public BitbucketCloudEndpoint() {
+        this(false, 0, 0, false, null, false, null);
     }
 
-    @Restricted(NoExternalUse.class) // Used for testing
-    public BitbucketCloudEndpoint(boolean manageHooks, @CheckForNull String credentialsId, String endPointURL) {
-        this(manageHooks, credentialsId);
-        setBitbucketJenkinsRootUrl(endPointURL);
+    @Deprecated(since = "936.3.1")
+    public BitbucketCloudEndpoint(boolean enableCache, int teamCacheDuration, int repositoriesCacheDuration,
+                                  boolean manageHooks, @CheckForNull String credentialsId) {
+        this(enableCache, teamCacheDuration, repositoriesCacheDuration, manageHooks, credentialsId, false, null);
     }
 
     /**
      * Constructor.
      *
-     * @param enableCache   {@code true} if caching should be used to reduce requests to Bitbucket.
-     * @param teamCacheDuration How long, in minutes, to cache the team response.
-     * @param repositoriesCacheDuration How long, in minutes, to cache the repositories response.
-     * @param manageHooks   {@code true} if and only if Jenkins is supposed to auto-manage hooks for this end-point.
-     * @param credentialsId The {@link StandardCredentials#getId()} of the credentials to use for
-     *                      auto-management of hooks.
+     * @param enableCache {@code true} if caching should be used to reduce
+     *        requests to Bitbucket.
+     * @param teamCacheDuration How long, in minutes, to cache the team
+     *        response.
+     * @param repositoriesCacheDuration How long, in minutes, to cache the
+     *        repositories response.
+     * @param manageHooks {@code true} if and only if Jenkins is supposed to
+     *        auto-manage hooks for this end-point.
+     * @param credentialsId The {@link StandardCredentials#getId()} of the
+     *        credentials to use for auto-management of hooks.
+     * @param enableHookSignature {@code true} hooks that comes Bitbucket Data
+     *        Center are signed.
+     * @param credentialsId The {@link StringCredentials#getId()} of the
+     *        credentials to use for verify the signature of payload.
      */
     @DataBoundConstructor
-    public BitbucketCloudEndpoint(boolean enableCache, int teamCacheDuration,
-        int repositoriesCacheDuration, boolean manageHooks, @CheckForNull String credentialsId) {
-        super(manageHooks, credentialsId);
+    public BitbucketCloudEndpoint(boolean enableCache, int teamCacheDuration, int repositoriesCacheDuration,
+                                  boolean manageHooks, @CheckForNull String credentialsId,
+                                  boolean enableHookSignature, @CheckForNull String hookSignatureCredentialsId) {
+        super(manageHooks, credentialsId, enableHookSignature, hookSignatureCredentialsId);
         this.enableCache = enableCache;
         this.teamCacheDuration = teamCacheDuration;
         this.repositoriesCacheDuration = repositoriesCacheDuration;

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/AbstractBitbucketEndpoint.java

@@ -97,19 +97,19 @@ public Permission getRequiredGlobalConfigPagePermission() {
     @Restricted(NoExternalUse.class) // only for plugin internal use.
     @NonNull
     public String readResolveServerUrl(@CheckForNull String bitbucketServerUrl) {
-        String serverUrl = normalizeServerURL(bitbucketServerUrl);
-        serverUrl = StringUtils.defaultIfBlank(serverUrl, BitbucketCloudEndpoint.SERVER_URL);
-        AbstractBitbucketEndpoint endpoint = findEndpoint(serverUrl).orElse(null);
+        String serverURL = normalizeServerURL(bitbucketServerUrl);
+        serverURL = StringUtils.defaultIfBlank(serverURL, BitbucketCloudEndpoint.SERVER_URL);
+        AbstractBitbucketEndpoint endpoint = findEndpoint(serverURL).orElse(null);
         if (endpoint == null && ACL.SYSTEM2.equals(Jenkins.getAuthentication2())) {
-            if (BitbucketCloudEndpoint.SERVER_URL.equals(serverUrl)
-                    || BitbucketCloudEndpoint.BAD_SERVER_URL.equals(serverUrl)) {
+            if (BitbucketCloudEndpoint.SERVER_URL.equals(serverURL)
+                    || BitbucketCloudEndpoint.BAD_SERVER_URL.equals(serverURL)) {
                 // exception case
-                addEndpoint(new BitbucketCloudEndpoint(false, null));
+                addEndpoint(new BitbucketCloudEndpoint());
             } else {
-                addEndpoint(new BitbucketServerEndpoint(null, serverUrl, false, null));
+                addEndpoint(new BitbucketServerEndpoint(serverURL));
             }
         }
-        return endpoint == null ?  serverUrl : endpoint.getServerUrl();
+        return endpoint == null ?  serverURL : endpoint.getServerUrl();
     }
 
     /**
@@ -153,7 +153,7 @@ public boolean configure(StaplerRequest2 req, JSONObject json) throws FormExcept
     @NonNull
     public synchronized List<AbstractBitbucketEndpoint> getEndpoints() {
         return endpoints == null || endpoints.isEmpty()
-                ? Collections.<AbstractBitbucketEndpoint>singletonList(new BitbucketCloudEndpoint(false, null))
+                ? List.of(new BitbucketCloudEndpoint())
                 : Collections.unmodifiableList(endpoints);
     }
 
@@ -176,12 +176,16 @@ public synchronized void setEndpoints(@CheckForNull List<? extends AbstractBitbu
             } else if (!(endpoint instanceof BitbucketCloudEndpoint)
                     && BitbucketCloudEndpoint.SERVER_URL.equals(serverUrl)) {
                 // fix type for the special case
-                iterator.set(new BitbucketCloudEndpoint(endpoint.isManageHooks(), endpoint.getCredentialsId(), endpoint.getBitbucketJenkinsRootUrl()));
+                BitbucketCloudEndpoint cloudEndpoint = new BitbucketCloudEndpoint(false, 0, 0,
+                        endpoint.isManageHooks(), endpoint.getCredentialsId(),
+                        endpoint.isEnableHookSignature(), endpoint.getHookSignatureCredentialsId());
+                cloudEndpoint.setBitbucketJenkinsRootUrl(endpoint.getBitbucketJenkinsRootUrl());
+                iterator.set(cloudEndpoint);
             }
             serverUrls.add(serverUrl);
         }
         if (eps.isEmpty()) {
-            eps.add(new BitbucketCloudEndpoint(false, null));
+            eps.add(new BitbucketCloudEndpoint());
         }
         this.endpoints = eps;
         save();

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketCloudEndpoint.java

@@ -39,6 +39,7 @@
 import java.util.Objects;
 import jenkins.scm.api.SCMName;
 import org.apache.commons.lang3.StringUtils;
+import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -99,32 +100,45 @@ public class BitbucketServerEndpoint extends AbstractBitbucketEndpoint {
     private boolean callChanges = true;
 
     /**
-     * @param displayName   Optional name to use to describe the end-point.
-     * @param serverUrl     The URL of this Bitbucket Server
-     * @param manageHooks   {@code true} if and only if Jenkins is supposed to auto-manage hooks for this end-point.
-     * @param credentialsId The {@link StandardCredentials#getId()} of the credentials to use for
-     *                      auto-management of hooks.
+     * Default constructor.
+     * @param serverURL
+     */
+    BitbucketServerEndpoint(@NonNull String serverURL) {
+        this(null, serverURL, false, null, false, null);
+    }
+
+    @Deprecated(since = "936.3.1")
+    public BitbucketServerEndpoint(@CheckForNull String displayName, @NonNull String serverUrl,
+                                   boolean manageHooks, @CheckForNull String credentialsId) {
+        this(displayName, serverUrl, manageHooks, credentialsId, false, null);
+    }
+
+    /**
+     * Constructor.
+     *
+     * @param displayName Optional name to use to describe the end-point.
+     * @param serverUrl The URL of this Bitbucket Server
+     * @param manageHooks {@code true} if and only if Jenkins is supposed to
+     *        auto-manage hooks for this end-point.
+     * @param credentialsId The {@link StandardCredentials#getId()} of the
+     *        credentials to use for auto-management of hooks.
+     * @param enableHookSignature {@code true} hooks that comes Bitbucket Data
+     *        Center are signed.
+     * @param credentialsId The {@link StringCredentials#getId()} of the
+     *        credentials to use for verify the signature of payload.
      */
     @DataBoundConstructor
-    public BitbucketServerEndpoint(@CheckForNull String displayName,
-                                   @NonNull String serverUrl,
-                                   boolean manageHooks,
-                                   @CheckForNull String credentialsId) {
-        super(manageHooks, credentialsId);
+    public BitbucketServerEndpoint(@CheckForNull String displayName, @NonNull String serverUrl,
+                                   boolean manageHooks, @CheckForNull String credentialsId,
+                                   boolean enableHookSignature, @CheckForNull String hookSignatureCredentialsId) {
+        super(manageHooks, credentialsId, enableHookSignature, hookSignatureCredentialsId);
         // use fixNull to silent nullability check
         this.serverUrl = Util.fixNull(BitbucketEndpointConfiguration.normalizeServerURL(serverUrl));
         this.displayName = StringUtils.isBlank(displayName)
                 ? SCMName.fromUrl(this.serverUrl, COMMON_PREFIX_HOSTNAMES)
                 : displayName.trim();
     }
 
-    @Restricted(NoExternalUse.class) // Used for testing
-    public BitbucketServerEndpoint(@CheckForNull String displayName, @NonNull String serverUrl,
-        boolean manageHooks, @CheckForNull String credentialsId, String endpointUrl) {
-        this(displayName, serverUrl, manageHooks, credentialsId);
-        setBitbucketJenkinsRootUrl(endpointUrl);
-    }
-
     @NonNull
     public static BitbucketServerWebhookImplementation findWebhookImplementation(String serverUrl) {
         final BitbucketServerEndpoint endpoint = BitbucketEndpointConfiguration.get()

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketEndpointConfiguration.java

@@ -26,11 +26,11 @@
 import hudson.model.ModelObject;
 
 public enum BitbucketServerVersion implements ModelObject {
-    VERSION_7("Bitbucket v7.x (and later)"),
-    VERSION_6_5("Bitbucket v6.5 to v6.10"),
-    VERSION_6("Bitbucket v6.0 to v6.4"),
-    VERSION_5_10("Bitbucket v5.10 to v5.16"),
-    VERSION_5("Bitbucket v5.9 (and earlier)");
+    VERSION_7("Bitbucket Data Center v8.x (and later)"),
+    VERSION_6_5("Bitbucket Server v6.5 to v6.10 - EOL reached, any support DISMISSED"),
+    VERSION_6("Bitbucket Server v6.0 to v6.4 - EOL reached, any support DISMISSED"),
+    VERSION_5_10("Bitbucket Server v5.10 to v5.16 - EOL reached, any support DISMISSED"),
+    VERSION_5("Bitbucket Server v5.9 (and earlier) - EOL reached, any support DISMISSED");
 
     private final String displayName;
 

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketServerEndpoint.java

@@ -110,7 +110,7 @@ public SCMFile answer(InvocationOnMock invocation) throws Throwable {
 
         // Auto-registering hooks
         if (includeWebHooks) {
-            when(client.getWebHooks()).thenReturn(Collections.EMPTY_LIST)
+            when(client.getWebHooks()).thenReturn(Collections.emptyList())
                 // Second call
                 .thenReturn(getWebHooks());
         }

src/main/java/com/cloudbees/jenkins/plugins/bitbucket/server/BitbucketServerVersion.java

@@ -35,7 +35,7 @@
 import java.util.Map;
 import java.util.Objects;
 
-@Extension(ordinal = 1000)
+@Extension
 public class BitbucketMockApiFactory extends BitbucketApiFactory {
     private static final String NULL = "\u0000\u0000\u0000\u0000";
     private final Map<String, BitbucketApi> mocks = new HashMap<>();

src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketClientMockUtils.java

@@ -36,7 +36,6 @@
 import com.cloudbees.plugins.credentials.common.StandardCredentials;
 import com.cloudbees.plugins.credentials.domains.Domain;
 import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;
-import hudson.Extension;
 import hudson.model.FreeStyleProject;
 import hudson.model.JobProperty;
 import hudson.model.JobPropertyDescriptor;
@@ -49,6 +48,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
@@ -62,7 +62,7 @@ public class BranchScanningIntegrationTest {
     @Test
     public void indexingTest() throws Exception {
         BitbucketEndpointConfiguration.get()
-                .addEndpoint(new BitbucketServerEndpoint("test", "http://bitbucket.test", false, null));
+                .addEndpoint(new BitbucketServerEndpoint("test", "http://bitbucket.test", false, null, false, null));
         BitbucketMockApiFactory.add("http://bitbucket.test", BitbucketClientMockUtils.getAPIClientMock(false, false));
 
         MockMultiBranchProjectImpl p = j.jenkins.createProject(MockMultiBranchProjectImpl.class, "test");
@@ -142,7 +142,7 @@ public JobPropertyDescriptor getDescriptor() {
             return new DescriptorImpl();
         }
 
-        @Extension
+        @TestExtension
         public static class DescriptorImpl extends JobPropertyDescriptor {
 
             @Override

src/test/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketMockApiFactory.java

@@ -40,13 +40,14 @@
 import jenkins.branch.MultiBranchProjectDescriptor;
 import jenkins.scm.api.SCMSource;
 import jenkins.scm.api.SCMSourceCriteria;
-import org.acegisecurity.Authentication;
 
 public class MockMultiBranchProjectImpl extends MultiBranchProject<FreeStyleProject, FreeStyleBuild> {
 
+    @SuppressWarnings("serial")
     public static final SCMSourceCriteria CRITERIA = new SCMSourceCriteria() {
-        @Override public boolean isHead(SCMSourceCriteria.Probe probe, TaskListener listener) throws IOException {
-            return probe.exists("markerfile.txt");
+        @Override
+        public boolean isHead(SCMSourceCriteria.Probe probe, TaskListener listener) throws IOException {
+            return probe.stat("markerfile.txt").exists();
         }
 
         @Override
@@ -60,15 +61,10 @@ public boolean equals(Object obj) {
         }
     };
 
-    public MockMultiBranchProjectImpl(ItemGroup parent, String name) {
+    public MockMultiBranchProjectImpl(ItemGroup<?> parent, String name) {
         super(parent, name);
     }
 
-    @Override
-    public Authentication getDefaultAuthentication(hudson.model.Queue.Item item) {
-        return getDefaultAuthentication();
-    }
-
     @Override
     protected BranchProjectFactory<FreeStyleProject, FreeStyleBuild> newProjectFactory() {
         return new BranchProjectFactoryImpl();
@@ -130,7 +126,7 @@ public String getDisplayName() {
         }
 
         @Override
-        public TopLevelItem newInstance(ItemGroup parent, String name) {
+        public TopLevelItem newInstance(@SuppressWarnings("rawtypes") ItemGroup parent, String name) {
             return new MockMultiBranchProjectImpl(parent, name);
         }