lfit / github2gerrit

Github PR to Gerrit Change request
Other
2 stars 3 forks source link

Implement logic necessary to inject Issue-Id into commit message #25

Open ModeSevenIndustrialSolutions opened 1 month ago

ModeSevenIndustrialSolutions commented 1 month ago

As per the dependabot change here: https://github.com/onap/portal-ng-bff/pull/3 The Github2Gerrit workflow run failed on this PR: https://github.com/onap/portal-ng-bff/actions/runs/11481797408/job/32152835438?pr=3 This is due to the project Gerrit servers requiring all commit messages to contain an Issue-Id linked to a valid ticket in JIRA. Having discussed with Andrew, bypassing this check is NOT an option. We will therefore need to implement the logic and code necessary to enumerate the source of the PR, and populate the appropriate JIRA Issue-Id in some way. Perhaps a lookup table of some kind, stashed as JSON or similar in a GitHub variable for each project? The system also needs to be able to lookup more than one source, as Dependabot will not be the only external PRs generated in Github.

git review --yes -t GH-portal-ng-bff-3 --reviewers releng+onap-gh2gerrit@linuxfoundation.org
git review .... inprogress
remote: 
remote: Processing changes: refs: 1        
remote: Processing changes: refs: 1        
remote: Processing changes: refs: 1, done            
remote: commit ec96f6f: warning: subject >50 characters; use shorter first paragraph        
remote: commit ec96f6f: Missing issue-id in commit message        
remote: Commit ec96f6f33397dee73769a54c647e7c7ce7a1ba3f not associated to any issue        
remote: 
remote: Hint: insert one or more issue-id anywhere in the commit message.        
remote:       Issue-ids are strings matching Issue-ID: ([A-Z][A-Z0-9]{1,9}-\d+)        
remote:       and are pointing to existing tickets on its-jira Issue-Tracker        
To ssh://gerrit.onap.org:29418/portal-ng/bff
 ! [remote rejected] HEAD -> refs/for/master%topic=GH-portal-ng-bff-3,r=releng+onap-gh2gerrit@linuxfoundation.org (commit ec96f6f: Missing issue-id in commit message)
error: failed to push some refs to 'ssh://gerrit.onap.org:29418/portal-ng/bff'
Error: Process completed with exit code 1.
##[debug]Finishing: Submit the change to Gerrit repository
askb commented 4 weeks ago

This issue request for a fix is outside the intended scope of the Github2Gerrit (G2G) workflow. G2G functions as a simple pass-through, forwarding developer (or Dependabot) changes directly to the configured Gerrit repository without modifications.

To address the Issue-Id: requirement, consider these options:

  1. Conditional Check: Implement a check at the source of the change to skip the Issue-Id requirement for Dependabot-initiated submissions.
  2. Prolog Filter Rule: Create a rule to ignore the Issue-Id check for specific scenarios, such as Dependabot changes. While custom modifications are welcome, using a static Issue-Id for all Dependabot changes isn't an optimal solution. Ideally, the Issue-Id should be integrated into the workflow or repository that triggers the Dependabot change.
ModeSevenIndustrialSolutions commented 4 weeks ago

@askb I understand your point about this being outside the scope of your intended work. I'm not sure either of the suggestions above will be acceptable to solve the current problem. @tykeal Given the discussion here, is the conversation heading in the direction that we potentially implement this as a second action/workflow that is triggered before G2G in the calling workflow. This would perform an Issue-ID lookup for different GitHub change scenarios prior to G2G running against a new change? The calling workflow would simply have an additional pre-step amending the commit message to contain the required Issue-Id, then invoke G2G. This has the benefit of keeping the individual components separate and modular, and I won't have to try and grok the existing logic Anil has implemented and try and integrate with it. If an Issue-Id string is already there, the workflow simply aborts early and steps out of the way.

tykeal commented 3 weeks ago

This issue request for a fix is outside the intended scope of the Github2Gerrit (G2G) workflow. G2G functions as a simple pass-through, forwarding developer (or Dependabot) changes directly to the configured Gerrit repository without modifications.

To address the Issue-Id: requirement, consider these options:

  1. Conditional Check: Implement a check at the source of the change to skip the Issue-Id requirement for Dependabot-initiated submissions.

Issue ID checking in Gerrit is done during the receive phase of the change going to Gerrit it is not bypassable if the Gerrit is configured to require one.

  1. Prolog Filter Rule: Create a rule to ignore the Issue-Id check for specific scenarios, such as Dependabot changes. While custom modifications are welcome, using a static Issue-Id for all Dependabot changes isn't an optimal solution. Ideally, the Issue-Id should be integrated into the workflow or repository that triggers the Dependabot change.

Prolog rules are no longer a thing in the version of Gerrit that we're using. If a rule exists, it's a grandfathered thing, but at this point we don't have any Gerrit system that has prolog rules. Even if we did, see my comment on your option 1, it's not something that is review time, it's on change instantiation time.

So, no, this is not actually outside of the scope of G2G, it's a use case that G2G needs to handle in some way if we are to ever allow our projects to use Dependabot or pre-commit.ci to raise changes automatically against our Gerrit based projects.

@tykeal Given the discussion here, is the conversation heading in the direction that we potentially implement this as a second action/workflow that is triggered before G2G in the calling workflow. This would perform an Issue-ID lookup for different GitHub change scenarios prior to G2G running against a new change? The calling workflow would simply have an additional pre-step amending the commit message to contain the required Issue-Id, then invoke G2G. This has the benefit of keeping the individual components separate and modular, and I won't have to try and grok the existing logic Anil has implemented and try and integrate with it. If an Issue-Id string is already there, the workflow simply aborts early and steps out of the way.

This can't be a secondary workflow given the mechanisms in witch G2G is striving to operate. It would fall down on multi-commit PRs along with any other PRs that are effectively being re-opened.

Here's what I envision we would have: