ChangeJson: Ignore PUSH_CERTIFICATES if GPG API is disabled

GpgApiAdapter throws an exception if the GPG API is disabled. The
client respects whether the server advertises signed push is enabled
when deciding whether to send the PUSH_CERTIFICATES option, but we
can't assume all clients will be so well behaved.

Recently in I04ddc340 we started passing this option when generating
events, causing breakage when processing a patch set with a push
certificate but the GPG API is disabled.

Change-Id: I53981d521f67906def7e8d3018f87816d5e43ab9
This commit is contained in:
Dave Borowitz
2016-08-12 17:24:44 -04:00
committed by Jonathan Nieder
parent 2dd29b7452
commit d373a09673
5 changed files with 19 additions and 1 deletions

View File

@@ -34,6 +34,8 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope; import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GerritConfigs;
import com.google.gerrit.acceptance.GitUtil; import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
@@ -1342,6 +1344,10 @@ public class ChangeIT extends AbstractDaemonTest {
} }
@Test @Test
@GerritConfigs({
@GerritConfig(name = "gerrit.editGpgKeys", value = "true"),
@GerritConfig(name = "receive.enableSignedPush", value = "true"),
})
public void pushCertificates() throws Exception { public void pushCertificates() throws Exception {
PushOneCommit.Result r1 = createChange(); PushOneCommit.Result r1 = createChange();
PushOneCommit.Result r2 = amendChange(r1.getChangeId()); PushOneCommit.Result r2 = amendChange(r1.getChangeId());

View File

@@ -56,6 +56,11 @@ public class GpgApiAdapterImpl implements GpgApiAdapter {
this.pushCertCheckerFactory = pushCertCheckerFactory; this.pushCertCheckerFactory = pushCertCheckerFactory;
} }
@Override
public boolean isEnabled() {
return true;
}
@Override @Override
public Map<String, GpgKeyInfo> listGpgKeys(AccountResource account) public Map<String, GpgKeyInfo> listGpgKeys(AccountResource account)
throws RestApiException, GpgException { throws RestApiException, GpgException {

View File

@@ -61,6 +61,11 @@ public class GpgApiModule extends RestApiModule {
private static class NoGpgApi implements GpgApiAdapter { private static class NoGpgApi implements GpgApiAdapter {
private static final String MSG = "GPG key APIs disabled"; private static final String MSG = "GPG key APIs disabled";
@Override
public boolean isEnabled() {
return false;
}
@Override @Override
public Map<String, GpgKeyInfo> listGpgKeys(AccountResource account) { public Map<String, GpgKeyInfo> listGpgKeys(AccountResource account) {
throw new NotImplementedException(MSG); throw new NotImplementedException(MSG);

View File

@@ -27,6 +27,8 @@ import java.util.List;
import java.util.Map; import java.util.Map;
public interface GpgApiAdapter { public interface GpgApiAdapter {
boolean isEnabled();
Map<String, GpgKeyInfo> listGpgKeys(AccountResource account) Map<String, GpgKeyInfo> listGpgKeys(AccountResource account)
throws RestApiException, GpgException; throws RestApiException, GpgException;

View File

@@ -1032,7 +1032,7 @@ public class ChangeJson {
new RevisionResource(changeResourceFactory.create(ctl), in)); new RevisionResource(changeResourceFactory.create(ctl), in));
} }
if (has(PUSH_CERTIFICATES)) { if (gpgApi.isEnabled() && has(PUSH_CERTIFICATES)) {
if (in.getPushCertificate() != null) { if (in.getPushCertificate() != null) {
out.pushCertificate = gpgApi.checkPushCertificate( out.pushCertificate = gpgApi.checkPushCertificate(
in.getPushCertificate(), in.getPushCertificate(),