chore(GHA): add action for testing against MPL HEAD #1187

josecorella · 2024-07-10T23:22:26Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

texastony

Nice.

josecorella · 2024-07-12T00:10:52Z

java test vectors will fail with this approach because the pinned version of the MPL 1.4.0 defines the test vector version as 1.0-SNAPSHOT while this pr updates the test vectors to depend on the same version for the mpl and the test vectors. The right thing to do here is to revert the change that changes the dependency definition of the test vectors so that we can merge this pr. The mpl-head-ci action failling daily is "ok" because it tells us that there needs to be project changes in order to merge with latest mpl head. This is why we wrote the action.

.github/workflows/ci_test_vector_java.yml

texastony · 2024-07-12T00:18:35Z

java test vectors will fail with this approach because the pinned version of the MPL 1.4.0 defines the test vector version as 1.0-SNAPSHOT while this pr updates the test vectors to depend on the same version for the mpl and the test vectors. The right thing to do here is to revert the change that changes the dependency definition of the test vectors so that we can merge this pr. The mpl-head-ci action failling daily is "ok" because it tells us that there needs to be project changes in order to merge with latest mpl head. This is why we wrote the action.

When I was working on the KMS Exception issue,
one of the issues I struggled with was getting local changes picked up.

The build system relies on mvn_local_deploy.
This allows a locally build TestVector project to be utilized.
The issue I had was that I had a yee-old TestVector locally built and installed in my Maven Cache.

I believe that tying the Test Vector version to the MPL's version
ensures that all consumers update both at the same time.

Which will be necessary,
as the TestVectors will always need to be updated to consume the latest MPL,
or it will not be achieving it's objective of testing the MPL.

Recognizing that the TestVectors will always need to Version with the MPL,
it seems appropriate that TestVectors should have the same version.

This is not necessary; the TestVectors could follow any versioning number,
so long as that number is always incremented when the MPL's version is incremented.

But it feels unnecessary to have two separate numbers without a justification.

josecorella · 2024-07-12T00:22:39Z

java test vectors will fail with this approach because the pinned version of the MPL 1.4.0 defines the test vector version as 1.0-SNAPSHOT while this pr updates the test vectors to depend on the same version for the mpl and the test vectors. The right thing to do here is to revert the change that changes the dependency definition of the test vectors so that we can merge this pr. The mpl-head-ci action failling daily is "ok" because it tells us that there needs to be project changes in order to merge with latest mpl head. This is why we wrote the action.

When I was working on the KMS Exception issue, one of the issues I struggled with was getting local changes picked up.

The build system relies on mvn_local_deploy. This allows a locally build TestVector project to be utilized. The issue I had was that I had a yee-old TestVector locally built and installed in my Maven Cache.

I believe that tying the Test Vector version to the MPL's version ensures that all consumers update both at the same time.

Which will be necessary, as the TestVectors will always need to be updated to consume the latest MPL, or it will not be achieving it's objective of testing the MPL.

Recognizing that the TestVectors will always need to Version with the MPL, it seems appropriate that TestVectors should have the same version.

This is not necessary; the TestVectors could follow any versioning number, so long as that number is always incremented when the MPL's version is incremented.

But it feels unnecessary to have two separate numbers without a justification.

I might have not been clear in my comment blurb. I am not saying we need to keep track of two separate versions. At this time the DBESDK has not been bumped from 1.4.0 to 1.5.1

I do not want to bump in this PR. What I am saying is that the action mpl-head-ci is built to test against the latest changes of MPL, when it fails it means that there was a change in the MPL that this repo needs to update in order to build with the latest mpl such that we can bump to the latest released MPL. I left comments in the build.gradle.kts file to ensure we update the syntax.

seebees

Looks good

seebees · 2024-07-12T14:52:42Z

.github/workflows/ci_test_java.yml

+      mpl-version:
+        description: "MPL version to use"
+        required: false
+        type: string
+      mpl-head:
+        description: "Running on MPL HEAD"
+        required: false
+        default: false
+        type: boolean


Is there a way to make it so you can only pass one?

seebees · 2024-07-12T14:53:29Z

.github/workflows/ci_test_java.yml

@@ -42,6 +51,20 @@ jobs:
        with:
          dafny-version: ${{ inputs.dafny }}

+      - name: Update MPL submodule if using MPL HEAD
+        if: ${{ inputs.mpl-head == true }}


What if head and version are true? Our ci is getting complicated :)

seebees · 2024-07-12T14:56:07Z

TestVectors/runtimes/java/build.gradle.kts

@@ -28,6 +28,7 @@ var dafnyRuntimeJavaVersion = props.getProperty("dafnyRuntimeJavaVersion")
 var smithyDafnyJavaConversionVersion = props.getProperty("smithyDafnyJavaConversionVersion")

 group = "software.amazon.cryptography"
+// change to ${mplVersion} for next MPL update


Not for this PR but can we put this under our release process?

## [3.5.1](v3.5.0...v3.5.1) (2024-07-12) ### Maintenance * bump dafny verification version to 4.7 ([#1181](#1181)) ([e7801ec](e7801ec)) * Fix nightly build (aside from verification) ([#1029](#1029)) ([862420e](862420e)) * **GHA:** add action for testing against MPL HEAD ([#1187](#1187)) ([b2f70ca](b2f70ca)) * **GHA:** fix daily ci ([#1194](#1194)) ([a1427e0](a1427e0)) * update project.properties to be SNAPSHOT ([#1087](#1087)) ([6f2825e](6f2825e))

## [3.6.0](v3.5.0...v3.6.0) (2024-07-23) ### Features * allow indirect attribute names with MultiKeyStore ([#1208](#1208)) ([4ab97bc](4ab97bc)) ### Maintenance * bump dafny verification version to 4.7 ([#1181](#1181)) ([e7801ec](e7801ec)) * **CI/CD:** use latest conventional-changelog-conventionalcommits ([#1195](#1195)) ([510227e](510227e)) * Fix nightly build (aside from verification) ([#1029](#1029)) ([862420e](862420e)) * **GHA:** add action for testing against MPL HEAD ([#1187](#1187)) ([b2f70ca](b2f70ca)) * **GHA:** fix daily ci ([#1194](#1194)) ([a1427e0](a1427e0)) * **MPL:** Bump MPL to 1.5.1 ([#1201](#1201)) ([808a5b4](808a5b4)) * Sonatype Migration to User Tokens ([#1216](#1216)) ([a3b4ef9](a3b4ef9)) * Try to update existing issues ([31c6b98](31c6b98)) * Try to update existing issues ([4471295](4471295)) * update project.properties to be SNAPSHOT ([#1087](#1087)) ([6f2825e](6f2825e))

* chore(release): 3.6.0 ## [3.6.0](v3.5.0...v3.6.0) (2024-07-23) ### Features * allow indirect attribute names with MultiKeyStore ([#1208](#1208)) ([4ab97bc](4ab97bc)) ### Maintenance * bump dafny verification version to 4.7 ([#1181](#1181)) ([e7801ec](e7801ec)) * **CI/CD:** use latest conventional-changelog-conventionalcommits ([#1195](#1195)) ([510227e](510227e)) * Fix nightly build (aside from verification) ([#1029](#1029)) ([862420e](862420e)) * **GHA:** add action for testing against MPL HEAD ([#1187](#1187)) ([b2f70ca](b2f70ca)) * **GHA:** fix daily ci ([#1194](#1194)) ([a1427e0](a1427e0)) * **MPL:** Bump MPL to 1.5.1 ([#1201](#1201)) ([808a5b4](808a5b4)) * Sonatype Migration to User Tokens ([#1216](#1216)) ([a3b4ef9](a3b4ef9)) * Try to update existing issues ([31c6b98](31c6b98)) * Try to update existing issues ([4471295](4471295)) * update project.properties to be SNAPSHOT ([#1087](#1087)) ([6f2825e](6f2825e))

josecorella and others added 20 commits July 10, 2024 16:19

chore(GHA): add action for testing against MPL HEAD

2cb1bb5

format

30cf455

test

efe69df

pass in dafny version

8a325e8

update

986d735

a

85d0f69

a

989acc2

?

64dfa0e

Merge branch 'main' into jocorell/gha-build-mpl-head

84abbd4

update

3213879

no codegen for mpl head tests

1ed2565

update verification

e6fd8e9

update

23f37be

recurse submodules

04fb953

use mpl head

7030c57

a

e57354f

check out mpl head for java testing

0397353

u

2175591

enable dotnet in mpl head build

3aeef6f

make mpl-head a cron job

7792af9

josecorella marked this pull request as ready for review July 12, 2024 00:01

josecorella requested a review from a team as a code owner July 12, 2024 00:01

texastony previously approved these changes Jul 12, 2024

View reviewed changes

texastony reviewed Jul 12, 2024

View reviewed changes

.github/workflows/ci_test_vector_java.yml Show resolved Hide resolved

update

2f3ece3

josecorella dismissed texastony’s stale review via 2f3ece3 July 12, 2024 00:14

texastony approved these changes Jul 12, 2024

View reviewed changes

josecorella merged commit b2f70ca into main Jul 12, 2024
33 checks passed

josecorella deleted the jocorell/gha-build-mpl-head branch July 12, 2024 01:35

seebees reviewed Jul 12, 2024

View reviewed changes

ajewellamz mentioned this pull request Jul 23, 2024

chore(release): 3.6.0 #1217

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(GHA): add action for testing against MPL HEAD #1187

chore(GHA): add action for testing against MPL HEAD #1187

josecorella commented Jul 10, 2024

texastony left a comment

josecorella commented Jul 12, 2024

texastony commented Jul 12, 2024

josecorella commented Jul 12, 2024

seebees left a comment

seebees Jul 12, 2024

seebees Jul 12, 2024

seebees Jul 12, 2024

chore(GHA): add action for testing against MPL HEAD #1187

chore(GHA): add action for testing against MPL HEAD #1187

Conversation

josecorella commented Jul 10, 2024

texastony left a comment

Choose a reason for hiding this comment

josecorella commented Jul 12, 2024

texastony commented Jul 12, 2024

josecorella commented Jul 12, 2024

seebees left a comment

Choose a reason for hiding this comment

seebees Jul 12, 2024

Choose a reason for hiding this comment

seebees Jul 12, 2024

Choose a reason for hiding this comment

seebees Jul 12, 2024

Choose a reason for hiding this comment