Merge pull request #687 from sigstore/staging-flaky

reduce impact of staging flakiness

Author: loosebazooka

.github/workflows/ci.yaml

@@ -54,8 +54,9 @@ jobs:
     - name: Setup Gradle
       uses: gradle/actions/setup-gradle@6cec5d49d4d6d4bb982fbed7047db31ea6d38f11 # v3.3.0
 
+    # tests that hit staging are current disabled due to flakiness (-PskipStaging)
     - name: Test sigstore-java
-      run: ./gradlew build
+      run: ./gradlew build -PskipStaging
 
     - name: Ensure sigstore-java self signing still works
       run: ./gradlew sigstore-java:publishToMavenLocal -Prelease -PskipPgpSigning

build-logic/jvm/src/main/kotlin/build-logic.testing.gradle.kts

@@ -5,4 +5,7 @@ tasks.withType<Test>().configureEach {
     if (project.hasProperty("org.gradle.jvmargs")) {
         systemProperty("sigstore-java.test.org.gradle.jvmargs", project.findProperty("org.gradle.jvmargs")!!)
     }
+    if (project.hasProperty("skipStaging")) {
+        systemProperty("sigstore-java.test.skipStaging", project.findProperty("skipStaging")!!)
+    }
 }

sigstore-java/src/test/java/dev/sigstore/KeylessTest.java

@@ -20,6 +20,7 @@
 import dev.sigstore.encryption.certificates.Certificates;
 import dev.sigstore.rekor.client.RekorTypeException;
 import dev.sigstore.rekor.client.RekorTypes;
+import dev.sigstore.testkit.annotations.DisabledIfSkipStaging;
 import dev.sigstore.testkit.annotations.EnabledIfOidcExists;
 import dev.sigstore.testkit.annotations.OidcProviderType;
 import java.io.IOException;
@@ -77,6 +78,7 @@ public void sign_production() throws Exception {
 
   @Test
   @EnabledIfOidcExists(provider = OidcProviderType.ANY)
+  @DisabledIfSkipStaging
   public void sign_staging() throws Exception {
     var signer = KeylessSigner.builder().sigstoreStagingDefaults().build();
     var results = signer.sign(artifactDigests);
@@ -114,6 +116,8 @@ private void verifySigningResult(List<KeylessSignature> results)
       Assertions.assertArrayEquals(
           Base64.getDecoder().decode(hr.getSignature().getPublicKey().getContent()),
           Certificates.toPemBytes(result.getCertPath().getCertificates().get(0)));
+      // check if required inclusion proof exists
+      Assertions.assertNotNull(result.getEntry().get().getVerification().getInclusionProof());
     }
   }
 

sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java

@@ -24,8 +24,6 @@
 import dev.sigstore.encryption.signers.Signers;
 import dev.sigstore.testing.CertGenerator;
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
 import java.security.InvalidKeyException;
 import java.security.MessageDigest;
@@ -39,30 +37,31 @@
 import org.hamcrest.MatcherAssert;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 public class RekorClientTest {
 
-  private static final String REKOR_URL = "https://rekor.sigstage.dev";
-  private RekorClient client;
+  private static RekorClient client;
+  private static HashedRekordRequest req;
+  private static RekorResponse resp;
 
-  @BeforeEach
-  public void setupClient() throws URISyntaxException {
-    // this tests directly against rekor in staging, it's a bit hard to bring up a rekor instance
-    // without docker compose.
-    client = RekorClient.builder().setUri(URI.create(REKOR_URL)).build();
+  @BeforeAll
+  public static void setupClient() throws Exception {
+    // this tests directly against rekor in prod, it's a bit hard to bring up a rekor instance
+    client = RekorClient.builder().build();
+    req = createdRekorRequest();
+    resp = client.putEntry(req);
   }
 
   @Test
-  public void putEntry_toStaging() throws Exception {
+  public void putEntry() throws Exception {
     HashedRekordRequest req = createdRekorRequest();
     var resp = client.putEntry(req);
-
     // pretty basic testing
     MatcherAssert.assertThat(
         resp.getEntryLocation().toString(),
-        CoreMatchers.startsWith(REKOR_URL + "/api/v1/log/entries/"));
+        CoreMatchers.startsWith(RekorClient.PUBLIC_GOOD_URI + "/api/v1/log/entries/"));
 
     assertNotNull(resp.getUuid());
     assertNotNull(resp.getRaw());
@@ -72,32 +71,25 @@ public void putEntry_toStaging() throws Exception {
     assertNotNull(entry.getLogID());
     Assertions.assertTrue(entry.getLogIndex() > 0);
     assertNotNull(entry.getVerification().getSignedEntryTimestamp());
-    //    Assertions.assertNotNull(entry.getVerification().getInclusionProof());
+    Assertions.assertNotNull(entry.getVerification().getInclusionProof());
   }
 
-  // TODO([email protected]): don't use data from prod, create the data as part of the test
-  // setup in staging.
   @Test
   public void searchEntries_nullParams() throws IOException {
     assertEquals(ImmutableList.of(), client.searchEntry(null, null, null, null));
   }
 
   @Test
   public void searchEntries_oneResult_hash() throws Exception {
-    var newRekordRequest = createdRekorRequest();
-    client.putEntry(newRekordRequest);
     assertEquals(
         1,
         client
-            .searchEntry(
-                null, newRekordRequest.getHashedRekord().getData().getHash().getValue(), null, null)
+            .searchEntry(null, req.getHashedRekord().getData().getHash().getValue(), null, null)
             .size());
   }
 
   @Test
   public void searchEntries_oneResult_publicKey() throws Exception {
-    var newRekordRequest = createdRekorRequest();
-    var resp = client.putEntry(newRekordRequest);
     assertEquals(
         1,
         client
@@ -138,29 +130,24 @@ public void searchEntries_zeroResults() throws IOException {
 
   @Test
   public void getEntry_entryExists() throws Exception {
-    var newRekordRequest = createdRekorRequest();
-    var resp = client.putEntry(newRekordRequest);
     var entry = client.getEntry(resp.getUuid());
-    assertEntry(resp, entry);
+    assertEntry(resp, entry.get());
   }
 
   @Test
   public void getEntry_hashedRekordRequest_byCalculatedUuid() throws Exception {
-    var hashedRekordRequest = createdRekorRequest();
-    var resp = client.putEntry(hashedRekordRequest);
     // getting an entry by hashedrekordrequest should implicitly calculate uuid
     // from the contents of the hashedrekord
-    var entry = client.getEntry(hashedRekordRequest);
-    assertEntry(resp, entry);
+    var entry = client.getEntry(req);
+    assertEntry(resp, entry.get());
   }
 
-  private void assertEntry(RekorResponse resp, Optional<RekorEntry> entry) {
-    assertTrue(entry.isPresent());
-    assertEquals(resp.getEntry().getLogID(), entry.get().getLogID());
-    assertNotNull(entry.get().getVerification().getInclusionProof().getTreeSize());
-    assertNotNull(entry.get().getVerification().getInclusionProof().getRootHash());
-    assertNotNull(entry.get().getVerification().getInclusionProof().getLogIndex());
-    assertTrue(entry.get().getVerification().getInclusionProof().getHashes().size() > 0);
+  private void assertEntry(RekorResponse resp, RekorEntry entry) {
+    assertEquals(resp.getEntry().getLogID(), entry.getLogID());
+    assertNotNull(entry.getVerification().getInclusionProof().getTreeSize());
+    assertNotNull(entry.getVerification().getInclusionProof().getRootHash());
+    assertNotNull(entry.getVerification().getInclusionProof().getLogIndex());
+    assertTrue(entry.getVerification().getInclusionProof().getHashes().size() > 0);
   }
 
   @Test
@@ -172,7 +159,7 @@ public void getEntry_entryDoesntExist() throws Exception {
   }
 
   @NotNull
-  private HashedRekordRequest createdRekorRequest()
+  private static HashedRekordRequest createdRekorRequest()
       throws NoSuchAlgorithmException, InvalidKeyException, SignatureException,
           OperatorCreationException, CertificateException, IOException {
     // the data we want to sign

sigstore-testkit/src/main/kotlin/dev/sigstore/testkit/annotations/DisabledIfSkipStaging.kt

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2022 The Sigstore Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package dev.sigstore.testkit.annotations
+
+import org.junit.jupiter.api.condition.DisabledIfSystemProperty
+
+@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
+@DisabledIfSystemProperty(
+    named = "sigstore-java.test.skipStaging",
+    matches = "^\\s*+(true|y|on|)\\s*+$",
+    disabledReason = "sigstore-java.test.skipStaging system property is present",
+)
+annotation class DisabledIfSkipStaging {}