Sputnik 2.3.0 fails after migrating to Gerrit 3.3.0/3.3.1

[patbaumgartner]

After migrating, Jenkins fails with the following message. The Gerrit release documentation provided a hint - Breaking Changes in the REST API -> https://www.gerritcodereview.com/3.3.html#rest-api-changes

5:17:45.309 [main] INFO  pl.touk.sputnik.engine.ReviewRunner - Review finished for processor ktlint. Took 0 s
15:17:45.309 [main] INFO  pl.touk.sputnik.engine.ReviewRunner - Review for processor ktlint returned 0 violations
15:17:45.318 [main] INFO  p.touk.sputnik.engine.VisitorBuilder - Using score strategy ScoreAlwaysPass
15:17:45.323 [main] INFO  p.t.s.e.v.SummaryMessageVisitor - Adding summary message to review: ### Perfect!
15:17:45.323 [main] INFO  p.t.s.e.v.score.ScoreAlwaysPass - Adding static passing score {Code-Review=1} to review
Exception in thread "main" pl.touk.sputnik.connector.gerrit.GerritException: Error when setting review
            at pl.touk.sputnik.connector.gerrit.GerritFacade.publish(GerritFacade.java:71)
            at pl.touk.sputnik.engine.Engine.run(Engine.java:43)
            at pl.touk.sputnik.Main.main(Main.java:41)
Caused by: com.urswolfer.gerrit.client.rest.http.HttpStatusException: Request not successful. Message: Bad Request. Status-Code: 400. Content: Invalid application/json in request.
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.throwHttpStatusException(GerritRestClient.java:460)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.checkStatusCodeError(GerritRestClient.java:447)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.checkStatusCodeClientError(GerritRestClient.java:433)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.checkStatusCode(GerritRestClient.java:425)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.request(GerritRestClient.java:227)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.requestRest(GerritRestClient.java:163)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.requestRest(GerritRestClient.java:155)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.requestJson(GerritRestClient.java:132)
            at com.urswolfer.gerrit.client.rest.http.GerritRestClient.postRequest(GerritRestClient.java:111)
            at com.urswolfer.gerrit.client.rest.http.changes.RevisionApiRestClient.review(RevisionApiRestClient.java:75)
            at pl.touk.sputnik.connector.gerrit.GerritFacade.publish(GerritFacade.java:69)
            ... 2 more

with the following properties. Even after commenting the score key it did not work.

connector.type=gerrit
connector.host=gerrit:443
connector.path=/gerrit

# for gerrit, configure password over <gerrit-server>/#/settings/http-password
connector.username=sputnik
connector.password=password-removed
connector.useHttps=true
connector.verifySsl=false
connector.tag=sputnik

# if you wish to have dedicated Gerrit label such as Static-Analysis refer to this manual
# https://gerrit-review.googlesource.com/Documentation/config-labels.html
#score.strategy=ScorePassIfNoErrors
#score.passingKey=Code-Review
#score.passingValue=1
#score.failingKey=Code-Review
#score.failingValue=-2

checkstyle.enabled=false
checkstyle.configurationFile=/opt/sputnik/google_checks.xml
checkstyle.propertiesFile=

# you can export rules as xml file from SonarQube (Quality Profiles -> <rule name> -> Permalinks
# but mind that not all are supported by attached pmd-XXX.jar file
pmd.enabled=false
pmd.ruleSets=rulesets/java/android.xml,rulesets/java/basic.xml,rulesets/java/braces.xml,rulesets/java/clone.xml,rulesets/java/codesize.xml,rulesets/java/comments.xml,rulesets/java/controversial.xml,rulesets/java/coupling.xml,rulesets/java/design.xml,rulesets/java/empty.xml,rulesets/java/finalizers.xml,rulesets/java/imports.xml,rulesets/java/j2ee.xml,rulesets/java/javabeans.xml,rulesets/java/junit.xml,rulesets/java/logging-jakarta-commons.xml,rulesets/java/logging-java.xml,rulesets/java/migrating.xml,rulesets/java/naming.xml,rulesets/java/optimizations.xml,rulesets/java/strictexception.xml,rulesets/java/strings.xml,rulesets/java/sunsecure.xml,rulesets/java/unnecessary.xml,rulesets/java/unusedcode.xml
pmd.showViolationDetails=true

spotbugs.enabled=true
spotbugs.includeFilter=
spotbugs.excludeFilter=

codenarc.enabled=true
codenarc.ruleSets=
codenarc.excludes=**/*.java

jslint.enabled=false
jshint.enabled=true
jshint.configurationFile=/opt/sputnik/jshint.json

tslint.enabled=false
tslint.script=/usr/bin/tslint
tslint.configurationFile=/opt/sputnik/tslint.json

ktlint.enabled=true
ktlint.exclude=no-semi,indent

detekt.enabled=false
detekt.config.file=/opt/sputnik/detekt-config.yml

Any idea how to fix this? I would love to help but have no clue how to start.

Best,
Patrick

[SpOOnman]

Hi, thanks for reporting. This is strange and I don't know why it happens.

We should check exactly HTTP request and response as urswolfer's library is a wrapper. You can capture HTTP traffic or you can check if urswolfer's library contains a logger that can log HTTP traffic.

We can also ask @uwolfer himself :D @uwolfer can you share your thought if you can read this? 😎

[uwolfer]

@SpOOnman: I don't think that we do any HTTP login by default in ithe gerrit-rest-java-client, but you could hook in such a logger in a quite easy way like this.

[SpOOnman]

@patbaumgartner does this help with debugging with your setup?

[patbaumgartner]

Hi @uwolfer and @SpOOnman - thanks for your support so far.

currently, I upgraded to Gerrit 3.3.1 which did not solve the issue yet.

Adding an Interceptor like @uwolfer pointed out does not work since the request body does not get printed. I also used reflection to get access to the HttpEntity.

So I increased the log level like this where I see plenty of logging.

<logger name="pl.touk.sputnik.connector.gerrit" level="DEBUG" /> <logger name="org.apache.http" level="DEBUG" />

Continue on this, keeping you posted.

[patbaumgartner]

Trying to fetch the original request against Gerrit 3.3.1 looks like this

curl -i \
-H "Authorization: Basic aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa=" \
-H "Accept: application/json" \
-H "Content-Length: 177" \
-H "Content-Type: application/json; charset=UTF-8" \
-H "Connection: Keep-Alive" \
-H "User-Agent: gerrit-rest-java-client/0.8.8 using Apache-HttpClient/4.5.3 (Java/11.0.9.1)" \
-d '{"message":"Perfect!","tag":"sputnik","labels":{"Code-Review":1},"comments":{"pom.xml":[]},"strict_labels":true,"drafts":"DELETE","notify":"ALL","omit_duplicate_comments":false}' \
-X POST https://my-gerrit-server.local/gerrit/a/changes/Icdade5567dcf4a806a047b48a331078d24a7ff7e/revisions/ee87ec13019ff147b01ded8bb45f0ddfdc667bb0/review

HTTP/1.1 400 Bad Request
Server: nginx/1.18.0
Date: Thu, 07 Jan 2021 15:39:06 GMT
Content-Type: text/plain;charset=utf-8
Content-Length: 36
Connection: keep-alive
X-Frame-Options: DENY
Content-Disposition: attachment
X-Content-Type-Options: nosniff
Cache-Control: no-cache, no-store, max-age=0, must-revalidate
Pragma: no-cache
Expires: Mon, 01 Jan 1990 00:00:00 GMT
 
Invalid application/json in request

It looks like they changed the ReviewInput API in 2.15 - no longer offers the strict_labels option.

[patbaumgartner]

hi, @uwolfer @SpOOnman - do you have any idea what has changed? Do we need an update on the gerrit-rest-java-client with the new HTTP entities from Gerrit?

[SpOOnman]

Hey @patbaumgartner, I've just released 2.5.0 with gerrit-rest-java-client updated to 0.9.3 (#227). Can you try if this helps?

[patbaumgartner]

Thanks @SpOOnman with this upgrade, it works again!

[SpOOnman]

Thanks for info! All credit goes to @uwolfer and @sp-atu 👍