Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use .statement.predicate everywhere instead of just .predicate #772

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

mbestavros
Copy link
Contributor

Resolves #756.

@mbestavros mbestavros force-pushed the gh-756 branch 2 times, most recently from d6f3546 to c1e3c8d Compare October 18, 2023 13:58
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2023

Codecov Report

Merging #772 (769f59c) into main (0ed9fe3) will not change coverage.
Report is 4 commits behind head on main.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #772   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           86        84    -2     
  Lines         3342      3318   -24     
=========================================
- Hits          3342      3318   -24     
Files Coverage Δ
policy/lib/tekton/pipeline.rego 100.00% <100.00%> (ø)
policy/lib/tekton/task.rego 100.00% <100.00%> (ø)
policy/lib/tekton/task_test.rego 100.00% <100.00%> (ø)
policy/release/attestation_type.rego 100.00% <100.00%> (ø)
policy/release/attestation_type_test.rego 100.00% <100.00%> (ø)
policy/release/external_parameters.rego 100.00% <100.00%> (ø)
policy/release/lib/attestations.rego 100.00% <100.00%> (ø)
policy/release/lib/attestations_test.rego 100.00% <100.00%> (ø)
policy/release/provenance_materials.rego 100.00% <100.00%> (ø)
policy/release/sbom_spdx.rego 100.00% <100.00%> (ø)
... and 6 more

Copy link
Member

@zregvart zregvart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't realized this is only in the tests, 👍

@lcarva
Copy link
Member

lcarva commented Oct 18, 2023

More work needs to be done. See policy/lib/statement.rego

@mbestavros mbestavros force-pushed the gh-756 branch 3 times, most recently from f77dd06 to 3daa0ba Compare October 18, 2023 22:04
Copy link
Member

@simonbaird simonbaird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 based on Luiz's comment

Edit: I see there are some revisions since then, disregard.

@mbestavros mbestavros force-pushed the gh-756 branch 7 times, most recently from 46d6e5e to 404dd61 Compare October 30, 2023 20:23
@lcarva
Copy link
Member

lcarva commented Oct 31, 2023

The test that is failing appears to be a bad test which has been exposed!

This makes it pass:

diff --git a/policy/release/step_image_registries_test.rego b/policy/release/step_image_registries_test.rego
index 73b88a0..627ff6f 100644
--- a/policy/release/step_image_registries_test.rego
+++ b/policy/release/step_image_registries_test.rego
@@ -66,7 +66,7 @@ test_unexpected_image_ref if {
 	lib.assert_equal_results(step_image_registries.deny, {{
 		"code": "step_image_registries.task_step_images_permitted",
 		"msg": sprintf("Step 0 in task 'mytask' has disallowed image ref '%s'", [unexpected_image]),
-	}}) with input.attestations as mock_data(unexpected_image)
+	}}) with input.attestations as [mock_data(unexpected_image)]
 }
 
 test_step_image_registry_prefix_list_found if {

@mbestavros mbestavros force-pushed the gh-756 branch 4 times, most recently from bbcfb7b to 6428ba3 Compare October 31, 2023 21:05
@mbestavros
Copy link
Contributor Author

@lcarva I thought that might have been it! I tried that fix in the library function, but that still broke things. Didn't think to do it in the test itself. Thanks!

Tests do indeed pass now! 🎉

policy/release/attestation_type_test.rego Show resolved Hide resolved
policy/release/lib/attestations.rego Outdated Show resolved Hide resolved
policy/release/lib/attestations.rego Outdated Show resolved Hide resolved
@mbestavros mbestavros force-pushed the gh-756 branch 2 times, most recently from 019bc08 to dae3b97 Compare November 1, 2023 22:23
Signed-off-by: Mark Bestavros <mbestavr@redhat.com>
Copy link
Member

@lcarva lcarva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Let's get this in!

@mbestavros mbestavros requested a review from simonbaird November 2, 2023 14:15
@simonbaird simonbaird dismissed their stale review November 2, 2023 15:29

Out of date

@mbestavros mbestavros merged commit 178cfa0 into enterprise-contract:main Nov 2, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use .statement.predicate in all places
5 participants