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.

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

[JENKINS-60874] Configuring ForkPullRequestDiscoveryTrait using Job DSL #752

Merged
merged 1 commit into from Dec 8, 2023

Conversation

mtughan
Copy link
Contributor

@mtughan mtughan commented Dec 4, 2023

Description

JENKINS-26535 describes an issue where wildcards in generics aren't handled well and result in the inability to configure classes that use wildcards in Job DSL, such as the SCMHeadAuthority class used for the forked PR trust policy. To work around this issue, create a new abstract class named GitHubForkTrustPolicy which extends the necessary SCMHeadAuthority superclass with its proper generics and use it in the constructor instead. Update all the trust policies to extend from this abstract class so they can be used.

Example use:

branchSources {
    branchSource {
        source {
            github {
                // snipped standard repository fields

                traits {
                    gitHubBranchDiscovery {
                        strategyId 1
                    }
                    gitHubPullRequestDiscovery {
                        strategyId 1
                    }
                    gitHubForkDiscovery {
                        strategyId 1
                        trust {
                            gitHubTrustPermissions()
                        }
                    }
                }
            }
        }
    }
}

Fixes JENKINS-60874.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Automated tests have been added to exercise the changes
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verify that the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

Documentation changes

  • Link to jenkins.io PR, or an explanation for why no doc changes are needed
    This will be discovered and exposed through the Job DSL API viewer.

Users/aliases to notify

@mtughan mtughan requested a review from a team as a code owner December 4, 2023 20:56
JENKINS-26535 describes an issue where wildcards in generics aren't
handled well and result in the inability to configure classes that use
wildcards in Job DSL, such as the SCMHeadAuthority class used for the
forked PR trust policy. To work around this issue, create a new abstract
class named GitHubForkTrustPolicy which extends the necessary
SCMHeadAuthority superclass with its proper generics and use it in the
constructor instead. Update all the trust policies to extend from this
abstract class so they can be used.

Fixes JENKINS-60874.
@lprimak
Copy link

lprimak commented Dec 5, 2023

yes!

@mtughan
Copy link
Contributor Author

mtughan commented Dec 8, 2023

@jenkinsci/github-branch-source-plugin-developers, is there anything else being waited for here? Or can this be merged?

@jglick jglick added the bug label Dec 8, 2023
@jglick jglick merged commit cdbd136 into jenkinsci:master Dec 8, 2023
14 checks passed
@mtughan mtughan deleted the JENKINS-60874 branch December 11, 2023 13:52
@lprimak
Copy link

lprimak commented Dec 12, 2023

Time to release? Thank you!

@car-roll
Copy link
Contributor

unfortunately there is a failure in the master branch, that is preventing an automatic release from happening

@jglick
Copy link
Member

jglick commented Dec 12, 2023

I triggered https://ci.jenkins.io/job/Plugins/job/github-branch-source-plugin/job/master/449/ (and @car-roll you should have been able to do the same).

@car-roll
Copy link
Contributor

@jglick that's what I thought too, but I didn't see any rebuild options on CI

@jglick
Copy link
Member

jglick commented Dec 12, 2023

@car-roll not in CI, in GitHub. Navigate to the trunk commit with a build failure, select the failed check, and Re-run from there.

@basil
Copy link
Member

basil commented Dec 13, 2023

Breaks PCT (Plugin Compatibility Tester) when gitlab-branch-source is on the test classpath:

java.lang.AssertionError: 

Expected: is "@github(credentialsId=foo,id=test,repoOwner=repo-owner,repository=repo,traits=[@gitHubBranchDiscovery$org.jenkinsci.plugins.github_branch_source.BranchDiscoveryTrait(strategyId=1), @gitHubPullRequestDiscovery$org.jenkinsci.plugins.github_branch_source.OriginPullRequestDiscoveryTrait(strategyId=1), @gitHubForkDiscovery$org.jenkinsci.plugins.github_branch_source.ForkPullRequestDiscoveryTrait(strategyId=2,trust=@gitHubTrustPermissions$org.jenkinsci.plugins.github_branch_source.ForkPullRequestDiscoveryTrait$TrustPermission()), @headWildcardFilter$WildcardSCMHeadFilterTrait(excludes=production,includes=i*)])"
     but: was "@github(credentialsId=foo,id=test,repoOwner=repo-owner,repository=repo,traits=[@gitHubBranchDiscovery$org.jenkinsci.plugins.github_branch_source.BranchDiscoveryTrait(strategyId=1), @gitHubPullRequestDiscovery$org.jenkinsci.plugins.github_branch_source.OriginPullRequestDiscoveryTrait(strategyId=1), @gitHubForkDiscovery$org.jenkinsci.plugins.github_branch_source.ForkPullRequestDiscoveryTrait(strategyId=2,trust=@gitHubTrustPermissions$TrustPermission()), @headWildcardFilter$WildcardSCMHeadFilterTrait(excludes=production,includes=i*)])"
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.junit.Assert.assertThat(Assert.java:964)
	at org.junit.Assert.assertThat(Assert.java:930)
	at org.jenkinsci.plugins.github_branch_source.GitHubSCMSourceTraitsTest.given__configuredInstance__when__uninstantiating__then__deprecatedFieldsIgnored(GitHubSCMSourceTraitsTest.java:87)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:655)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.lang.Thread.run(Thread.java:1583)

The problem can be reproduced with

diff --git a/pom.xml b/pom.xml
index d51d675..57f6a34 100644
--- a/pom.xml
+++ b/pom.xml
@@ -95,6 +95,11 @@
       <artifactId>test-harness</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>io.jenkins.plugins</groupId>
+      <artifactId>gitlab-branch-source</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>io.jenkins.plugins</groupId>
       <artifactId>pipeline-groovy-lib</artifactId>

Suggest the following patch:

diff --git a/src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSourceTraitsTest.java b/src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSourceTraitsTest.java
index 9454ddc..2d3e24d 100644
--- a/src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSourceTraitsTest.java
+++ b/src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSourceTraitsTest.java
@@ -69,12 +69,6 @@ public class GitHubSCMSourceTraitsTest {
         recreated.setIncludes("i*");
         recreated.setExcludes("production");
         recreated.setScanCredentialsId("foo");
-        String trust;
-        if (r.jenkins.getPlugin("gitlab-branch-source") != null) {
-            trust = "org.jenkinsci.plugins.github_branch_source.ForkPullRequestDiscoveryTrait$TrustPermission";
-        } else {
-            trust = "TrustPermission";
-        }
         String originTrait;
         String forkTrait;
         if (r.jenkins.getPlugin("cloudbees-bitbucket-branch-source") != null) {
@@ -96,9 +90,7 @@ public class GitHubSCMSourceTraitsTest {
                         + "@gitHubPullRequestDiscovery$" + originTrait + "(strategyId=1), "
                         + "@gitHubForkDiscovery$" + forkTrait + "("
                         + "strategyId=2,"
-                        + "trust=@gitHubTrustPermissions$"
-                        + trust
-                        + "()), "
+                        + "trust=@gitHubTrustPermissions$TrustPermission()), "
                         + "@headWildcardFilter$WildcardSCMHeadFilterTrait(excludes=production,includes=i*)])"));
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants