From 5ec77a426e405a43e5d9523a35288ff37216c958 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Thu, 2 May 2024 13:48:39 -0400 Subject: [PATCH] reduce impact of staging flakiness Signed-off-by: Appu Goundan --- .github/workflows/ci.yaml | 3 +- .../kotlin/build-logic.testing.gradle.kts | 3 + .../test/java/dev/sigstore/KeylessTest.java | 4 ++ .../rekor/client/RekorClientTest.java | 61 ++++++++----------- .../annotations/DisabledIfSkipStaging.kt | 27 ++++++++ 5 files changed, 60 insertions(+), 38 deletions(-) create mode 100644 sigstore-testkit/src/main/kotlin/dev/sigstore/testkit/annotations/DisabledIfSkipStaging.kt diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c4c12eeb..3c11dd76 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/build-logic/jvm/src/main/kotlin/build-logic.testing.gradle.kts b/build-logic/jvm/src/main/kotlin/build-logic.testing.gradle.kts index 22bf860c..cad6882b 100644 --- a/build-logic/jvm/src/main/kotlin/build-logic.testing.gradle.kts +++ b/build-logic/jvm/src/main/kotlin/build-logic.testing.gradle.kts @@ -5,4 +5,7 @@ tasks.withType().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")!!) + } } diff --git a/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java b/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java index 140a326e..87aa3df7 100644 --- a/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java +++ b/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 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()); } } diff --git a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java b/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java index ae6d5b5f..ee8fa5f3 100644 --- a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java +++ b/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,11 +71,9 @@ 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(patrick@chainguard.dev): 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)); @@ -84,20 +81,15 @@ public void searchEntries_nullParams() throws IOException { @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 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 diff --git a/sigstore-testkit/src/main/kotlin/dev/sigstore/testkit/annotations/DisabledIfSkipStaging.kt b/sigstore-testkit/src/main/kotlin/dev/sigstore/testkit/annotations/DisabledIfSkipStaging.kt new file mode 100644 index 00000000..37c025c3 --- /dev/null +++ b/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 {}