Add first pact verification for WDS integration. #1511

jladieu · 2023-10-25T19:42:43Z

This pairs with DataBiosphere/terra-workspace-data-service#372 to establish a contract between WDS and WSM, initially focused on ReferencedGcpResourceController.

I tested this running a local pact-broker by following the steps in this doc and using the displayed IntelliJ run configuration:

service/src/test/java/bio/terra/workspace/pact/WdsContractVerificationTest.java

service/gradle/dependencies.gradle

jladieu · 2023-10-30T23:38:41Z

service/src/test/java/bio/terra/workspace/pact/TpsApiTest.java

@@ -35,6 +34,8 @@
 @Tag("pact-test")
 @ExtendWith(PactConsumerTestExt.class)
 public class TpsApiTest {
+  private static final String UUID_REGEX =


As part of upgrading the pactjvm dep, we lost access to the UUID_REGEX which was buried in an inaccessible kotlin companion object. This is just an inline of the String previously referenced.

jladieu · 2023-10-30T23:39:37Z

scripts/write-config.sh

@@ -322,6 +322,8 @@ landingzone:
      - [email protected]
      - [email protected]
      - [email protected]
+pactbroker:


This doesn't actually work to provide a local override -- the pactbroker.url defined in application-test.yml ends up taking precedence, which is why to actually run locally I provided a screenshot of the runconfig needed to set the value via environment variable.

if this doesn't work, then can you delete it? If it needs to be present for some reason, add a code comment explaining it doesn't have any effect.

nawatts · 2023-10-31T13:29:45Z

service/gradle/testing.gradle

+    // TODO: do the correct/idiomatic thing here with config values, help needed for WSM specifics, desired behavior:
+    // default: use the dsp-eng-tools version
+    // allow optional local-override for local pactbroker


For what it's worth, DataBiosphere/terra-workspace-data-service#364 does this in the config file: https://github.com/DataBiosphere/terra-workspace-data-service/pull/364/files#diff-cc6240f713f8c21e8355ffde96d288c175673fec877cc6b1b0f0ebd0edda5e57

jladieu · 2023-10-31T14:01:01Z

.github/workflows/verify_consumer_pacts.yaml

This file is copied from DataBiosphere/jade-data-repo#1528, I'll annotate the lines which are WSM-specific.

jladieu · 2023-10-31T14:02:03Z

.github/workflows/verify_consumer_pacts.yaml

@@ -0,0 +1,203 @@
+name: Verify consumer pacts
+# The purpose of this workflow is to verify ANY consumer contract(s) dependent on workspacemanager provider using Pact


Comment adjusted to include workspacemanager pacticipant name, which must match the helmchart name.

jladieu · 2023-10-31T14:02:18Z

.github/workflows/verify_consumer_pacts.yaml

+# - PACT_BROKER_USERNAME - the Pact Broker username
+# - PACT_BROKER_PASSWORD - the Pact Broker password
+# They are managed by Atlantis and were added to Terraform here:
+# https://github.com/broadinstitute/terraform-ap-deployments/pull/1086


Included link to PR where wsm's creds were introduced.

jladieu · 2023-10-31T14:02:38Z

.github/workflows/verify_consumer_pacts.yaml

+# https://github.com/broadinstitute/terraform-ap-deployments/pull/1086
+env:
+  PACT_BROKER_URL: pact-broker.dsp-eng-tools.broadinstitute.org
+  spring_profiles_active: human-readable-logging


Added spring_profiles_active to get human friendly logs, which are important to interpreting pact verification failures.

jladieu · 2023-10-31T14:04:25Z

.github/workflows/verify_consumer_pacts.yaml

+          repo: broadinstitute/terra-github-workflows
+          ref: refs/heads/main
+          token: ${{ secrets.BROADBOT_TOKEN }} # github token for access to kick off a job in the private repo
+          inputs: '{ "pacticipant": "workspacemanager", "version": "${{ needs.verify-consumer-pact.outputs.provider-sha }}" }'


Adjusted pacticipant name to workspacemanager

sonarcloud · 2023-10-31T14:21:22Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

This pairs with DataBiosphere/terra-workspace-data-service#372 to establish a contract between WDS and WSM, initially focused on `ReferencedGcpResourceController`.

REVERT: Attempt to upload results of pact Debug level logging to troubleshoot pact

Prefer helmchart name as the pacticipant identifier: * `bpm-provider` -> `bpm` * `bpm-consumer` -> `bpm` * `sam-provider` -> `sam` * See [thread](https://broadinstitute.slack.com/archives/C043YJ40719/p1698068076950019) discussing the `-consumer` and `-provider` suffix anti-pattern. * See [thread](https://broadinstitute.slack.com/archives/C043YJ40719/p1698241095600099) discussing the recommendation to use the helm chart name. Unblocks: * broadinstitute/rawls#2610 * DataBiosphere/terra-workspace-manager#1511 * broadinstitute/sam#1234

jladieu · 2023-11-03T17:49:53Z

Discussed offline with @blakery, a couple things to incorporate into this PR for next round:

Don't need to worry about the cautionary advice about @MockBeans in the README.md for this test, which is its own test runtime and doesn't need to be subject to those considerations. 🎉
Try to mock one layer lower (DAO level instead of service level) -- this will allow the @State to better track stateful behavior, and hopefully prevent future collision/disagreement between stateful setups that will happen when mocking at the service level (as currently implemented.

This will allow the `@State` to better track stateful behavior, and hopefully prevent future collision/disagreement between stateful setups that might happen when mocking at the service level.

blakery · 2023-11-06T20:19:22Z

service/src/test/java/bio/terra/workspace/pact/WdsContractVerificationTest.java

+@Tag("pact-verification")
+@Provider("workspacemanager") // should match the terra chart name
+@PactBroker()
+public class WdsContractVerificationTest extends BaseUnitTestMocks {


I'm really surprised by how well pushing the mocks lower worked!

Agreed...I was pleasantly surprised that extending from BaseUnitTestMocks seemed to strike the right balance of dependencies that I didn't have to @MockBean out everything into oblivion.

davidangb

A few comments inline, nothing big. Thanks for doing this!

I did not test any of this myself, but I see the checks passing.

.github/workflows/verify_consumer_pacts.yaml

davidangb · 2023-11-07T20:31:48Z

scripts/write-config.sh

@@ -322,6 +322,8 @@ landingzone:
      - [email protected]
      - [email protected]
      - [email protected]
+pactbroker:


if this doesn't work, then can you delete it? If it needs to be present for some reason, add a code comment explaining it doesn't have any effect.

davidangb · 2023-11-07T20:33:49Z

.github/workflows/verify_consumer_pacts.yaml

+# This workflow is triggered when
+#
+# 1. Consumer makes a change that results in a new pact published to Pact Broker (will verify ONLY the changed pact and publish the verification results back to the broker)
+# 2. Provider makes a change (runs verification tests against ALL DEPLOYED consumer pact versions and publishes corresponding verification results)


ALL DEPLOYED consumer pact versions

doesn't this require some configuration/integration with beehive or sherlock? Is this already set up?

Honestly, I'm not sure...this file is the result of a blatant copypaste. I did check in https://beehive.dsp-devops.broadinstitute.org/charts/workspacemanager/contract-testing and confirmed that for workspacemanager contract testing is enabled:

I'll get to exercise some of the provider side again after this merges by integrating another couple pact verifications...hopefully I'll have more of a clue after that process.

sonarcloud · 2023-11-07T21:42:05Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
4 Code Smells

No Coverage information
0.0% Duplication

This is followup after: #1511 That PR (and associated changes) introduced the first WDS <-> WSM pact verification to WSM, but inadvertently broke the `verify_consumer_pacts` GitHub action in the process. This PR fixes that, while also adopting semantic versioning for reporting verification to Pact Broker. Various relevant threads: * https://broadinstitute.slack.com/archives/C043YJ40719/p1699888771297559 * https://broadinstitute.slack.com/archives/C043YJ40719/p1700155493619189 * https://broadinstitute.slack.com/archives/C043YJ40719/p1700154543175189 GitHub Actions logic copied from: DataBiosphere/terra-billing-profile-manager#324

* Fix `verify_consumer_pacts` GitHub action. This is followup after: #1511 That PR (and associated changes) introduced the first WDS <-> WSM pact verification to WSM, but inadvertently broke the `verify_consumer_pacts` GitHub action in the process. This PR fixes that, while also adopting semantic versioning for reporting verification to Pact Broker. Various relevant threads: * https://broadinstitute.slack.com/archives/C043YJ40719/p1699888771297559 * https://broadinstitute.slack.com/archives/C043YJ40719/p1700155493619189 * https://broadinstitute.slack.com/archives/C043YJ40719/p1700154543175189 GitHub Actions logic copied from: DataBiosphere/terra-billing-profile-manager#324 Also enable pending pacts. See: https://docs.pact.io/pact_broker/advanced_topics/pending_pacts --------- Co-authored-by: Ivan <[email protected]>

jladieu mentioned this pull request Oct 25, 2023

AJ-1234: Add WsmPactTest DataBiosphere/terra-workspace-data-service#372

Merged

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from a53dd38 to 85c0a99 Compare October 25, 2023 19:49

jladieu commented Oct 25, 2023

View reviewed changes

service/src/test/java/bio/terra/workspace/pact/WdsContractVerificationTest.java Outdated Show resolved Hide resolved

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from 85c0a99 to 4ad26d4 Compare October 25, 2023 19:51

jladieu requested a review from a team October 30, 2023 14:39

calypsomatic reviewed Oct 30, 2023

View reviewed changes

service/gradle/dependencies.gradle Outdated Show resolved Hide resolved

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 3 times, most recently from ef3a1fb to 23c1d25 Compare October 30, 2023 23:23

jladieu commented Oct 30, 2023

View reviewed changes

nawatts reviewed Oct 31, 2023

View reviewed changes

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from 23c1d25 to 4871397 Compare October 31, 2023 13:57

jladieu changed the title ~~Add WdsContractVerificationTest~~ Add first pact verification for WDS integration. Oct 31, 2023

jladieu commented Oct 31, 2023

View reviewed changes

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from 4871397 to 742f618 Compare October 31, 2023 14:01

jladieu commented Oct 31, 2023

View reviewed changes

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 2 times, most recently from 958eba3 to d1dbe5d Compare October 31, 2023 14:04

jladieu commented Oct 31, 2023

View reviewed changes

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 2 times, most recently from 2975f37 to b3e06b4 Compare October 31, 2023 18:24

This was referenced Nov 1, 2023

AJ-1235: Bump dius.pact from 4.1.7 -> 4.3.19 #1517

Merged

AJ-1235: Use helmchart names for pact identifiers. #1518

Merged

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 2 times, most recently from d1dbe5d to 6fa0e93 Compare November 1, 2023 15:36

jladieu changed the base branch from main to rename-wsm-pacticipants November 1, 2023 15:38

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 3 times, most recently from a4d3630 to 83e7061 Compare November 1, 2023 21:15

jladieu force-pushed the rename-wsm-pacticipants branch 2 times, most recently from 0fea0b2 to 0e115d9 Compare November 2, 2023 21:14

Base automatically changed from rename-wsm-pacticipants to main November 2, 2023 22:02

jladieu added 2 commits November 3, 2023 08:38

Add first pact verification for WDS integration.

946edb3

This pairs with DataBiosphere/terra-workspace-data-service#372 to establish a contract between WDS and WSM, initially focused on `ReferencedGcpResourceController`.

TEMPORARY: Enable workflow for this branch.

c743aba

REVERT: Attempt to upload results of pact Debug level logging to troubleshoot pact

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from e417090 to c743aba Compare November 3, 2023 12:39

jladieu mentioned this pull request Nov 3, 2023

Use helmchart name for pact identifier. DataBiosphere/terra-billing-profile-manager#310

Merged

Comment @MockBeans

1fffefb

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch 2 times, most recently from 03effa5 to 49705f6 Compare November 6, 2023 15:49

Mock one layer lower.

00b1917

This will allow the `@State` to better track stateful behavior, and hopefully prevent future collision/disagreement between stateful setups that might happen when mocking at the service level.

jladieu force-pushed the AJ-1235-provider-pacts-for-wds branch from 49705f6 to 00b1917 Compare November 6, 2023 16:49

jladieu requested a review from blakery November 6, 2023 16:50

Fiddle with GHA config to get it to pass.

69ad659

blakery approved these changes Nov 7, 2023

View reviewed changes

jladieu requested a review from a team November 7, 2023 18:40

davidangb approved these changes Nov 7, 2023

View reviewed changes

Final fixes from PR review.

355dd55

jladieu merged commit 1a29c32 into main Nov 7, 2023
14 checks passed

jladieu deleted the AJ-1235-provider-pacts-for-wds branch November 7, 2023 21:45

jladieu mentioned this pull request Nov 17, 2023

Fix verify_consumer_pacts GitHub action. #1531

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add first pact verification for WDS integration. #1511

Add first pact verification for WDS integration. #1511

jladieu commented Oct 25, 2023 •

edited

Loading

jladieu Oct 30, 2023

jladieu Oct 30, 2023 •

edited

Loading

davidangb Nov 7, 2023

nawatts Oct 31, 2023 •

edited

Loading

jladieu Oct 31, 2023

jladieu Oct 31, 2023

jladieu Oct 31, 2023

jladieu Oct 31, 2023

jladieu Oct 31, 2023

sonarcloud bot commented Oct 31, 2023

jladieu commented Nov 3, 2023 •

edited

Loading

blakery Nov 6, 2023

jladieu Nov 7, 2023

davidangb left a comment

davidangb Nov 7, 2023

davidangb Nov 7, 2023

jladieu Nov 7, 2023

jladieu Nov 7, 2023

sonarcloud bot commented Nov 7, 2023

		@@ -0,0 +1,203 @@
		name: Verify consumer pacts
		# The purpose of this workflow is to verify ANY consumer contract(s) dependent on workspacemanager provider using Pact

Add first pact verification for WDS integration. #1511

Add first pact verification for WDS integration. #1511

Conversation

jladieu commented Oct 25, 2023 • edited Loading

Choose a reason for hiding this comment

jladieu Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nawatts Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 31, 2023

jladieu commented Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidangb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 7, 2023

jladieu commented Oct 25, 2023 •

edited

Loading

jladieu Oct 30, 2023 •

edited

Loading

nawatts Oct 31, 2023 •

edited

Loading

jladieu commented Nov 3, 2023 •

edited

Loading