Add support for base and target trees in git ingest, add .tar.gz bundler #5181

evankanderson · 2024-12-11T08:36:42Z

Summary

Prepare for running external tools via Minder rego. This adds two features (sorry, I got on a roll!):

git ingest during a pull request can access the PR base branch (what you want to merge to) via base_file.* analogues of file.* operations.
Added a file.bundle operation that takes a list of file paths and produces a .tar.gz as a string.

Change Type

Mark the type of change your PR introduces:

Bug fix (resolves an issue without affecting existing features)
Feature (adds new functionality without breaking changes)
Breaking change (may impact existing functionalities or require documentation updates)
Documentation (updates or additions to documentation)
Refactoring or test improvements (no bug fixes or new functionality)

Testing

Unit testing for lib and some light manual testing for the git ingest.

Review Checklist:

Reviewed my own code for quality and clarity.
Added comments to complex or tricky code sections.
Updated any affected documentation.
Included tests that validate the fix or feature.
Checked that related changes are merged.

coveralls · 2024-12-11T14:49:23Z

coverage: 55.103% (+0.09%) from 55.009%
when pulling 9c14b19 on evankanderson:file-machinery
into 9286b31 on mindersec:main.

JAORMX · 2024-12-12T06:55:52Z

internal/engine/eval/rego/lib.go

+func BaseFileArchive(res *interfaces.Result) func(*rego.Rego) {
+	return rego.Function1(
+		&rego.Function{
+			Name: "base_file.archive",


The PR description
says it would be file.bundle. is this naming difference intentional?

Talking with Ed, he suggested archive instead.

JAORMX · 2024-12-12T06:56:44Z

internal/engine/eval/rego/lib.go

+// a tarball.  It takes one argument: a list of file or directory paths to
+// include, and outputs the tarball as a string.
+// It's exposed as 'base_file.archive`.
+func BaseFileArchive(res *interfaces.Result) func(*rego.Rego) {


Do we want to expose this function to non PR/diff contexts too? I think it would be handy in general.

See FileArchive, above. 😁

JAORMX · 2024-12-12T06:58:02Z

internal/engine/ingester/git/git.go

-	// If the branch is not provided in the rule-type
-	// configuration, use the default branch
-	return defaultBranch
+	checkpoint := checkpoints.NewCheckpointV1Now().WithBranch(ent.GetTargetRef()).WithCommitHash(head.Hash().String())


We don't need to fix this in this PR... But... I wonder if we should have a base and target checkpoints checksum checkpoints?

Since "base" is the branch that the PR will be merged into, I think that only changes if the target/head branch changes.

evankanderson · 2024-12-11T14:13:52Z

internal/engine/eval/rego/lib.go

@@ -57,30 +70,47 @@ func FileExists(res *interfaces.Result) func(*rego.Rego) {
 			Name: "file.exists",
 			Decl: types.NewFunction(types.Args(types.S), types.B),
 		},
-		func(_ rego.BuiltinContext, op1 *ast.Term) (*ast.Term, error) {


This diff looks really bad, but I just moved this to the return function from fsExists

evankanderson · 2024-12-12T14:02:14Z

internal/engine/eval/rego/lib.go

+func BaseFileArchive(res *interfaces.Result) func(*rego.Rego) {
+	return rego.Function1(
+		&rego.Function{
+			Name: "base_file.archive",


Talking with Ed, he suggested archive instead.

evankanderson · 2024-12-12T14:02:56Z

internal/engine/eval/rego/lib.go

+// a tarball.  It takes one argument: a list of file or directory paths to
+// include, and outputs the tarball as a string.
+// It's exposed as 'base_file.archive`.
+func BaseFileArchive(res *interfaces.Result) func(*rego.Rego) {


See FileArchive, above. 😁

evankanderson · 2024-12-12T14:08:22Z

internal/engine/ingester/git/git.go

-	// If the branch is not provided in the rule-type
-	// configuration, use the default branch
-	return defaultBranch
+	checkpoint := checkpoints.NewCheckpointV1Now().WithBranch(ent.GetTargetRef()).WithCommitHash(head.Hash().String())


Since "base" is the branch that the PR will be merged into, I think that only changes if the target/head branch changes.

evankanderson · 2024-12-16T17:47:44Z

I'm going to put these behind flags, then it should be ready to merge.

evankanderson · 2024-12-16T19:41:02Z

Okay, I think this is ready to merge, assuming that the tests pass. I've put both the new functions and the new ingest type behind flags, and corrected the flag check on UpdateRuleType to block the pr templates (it was only on create previously).

Add support for base and target trees in git ingest, add .tar.gz bundler

6402c6e

evankanderson requested a review from a team as a code owner December 11, 2024 08:36

Fix line-too-long lint error

335c6b9

Rename 'bundle' to 'archive' per feedback from Ed

889ac19

evankanderson force-pushed the file-machinery branch from 3333aa7 to 889ac19 Compare December 11, 2024 21:38

JAORMX reviewed Dec 12, 2024

View reviewed changes

evankanderson commented Dec 12, 2024

View reviewed changes

JAORMX previously approved these changes Dec 12, 2024

View reviewed changes

Clean up some extra logging, add more details on PR evaluation

ed1cb1d

evankanderson dismissed JAORMX’s stale review via ed1cb1d December 12, 2024 21:42

evankanderson mentioned this pull request Dec 12, 2024

Rule to invoke Bandit from webservice running on Cloud Run mindersec/minder-rules-and-profiles#225

Draft

JAORMX previously approved these changes Dec 13, 2024

View reviewed changes

evankanderson dismissed JAORMX’s stale review via b1e157e December 16, 2024 19:08

Flag-protect new functionality

b50e41e

evankanderson force-pushed the file-machinery branch from b1e157e to b50e41e Compare December 16, 2024 19:40

evankanderson added 2 commits December 16, 2024 12:06

Merge remote-tracking branch 'upstream/main' into file-machinery

3304a6e

Update new tests for changes to Eval method

082416c

evankanderson force-pushed the file-machinery branch from c6abe06 to d645a52 Compare December 16, 2024 20:41

Fix lint /tableflip

9c14b19

evankanderson force-pushed the file-machinery branch from d645a52 to 9c14b19 Compare December 17, 2024 14:18

JAORMX approved these changes Dec 17, 2024

View reviewed changes

evankanderson merged commit e66fa17 into mindersec:main Dec 17, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for base and target trees in git ingest, add .tar.gz bundler #5181

Add support for base and target trees in git ingest, add .tar.gz bundler #5181

evankanderson commented Dec 11, 2024

coveralls commented Dec 11, 2024 •

edited

Loading

JAORMX Dec 12, 2024 •

edited

Loading

evankanderson Dec 12, 2024

JAORMX Dec 12, 2024

evankanderson Dec 12, 2024

JAORMX Dec 12, 2024

evankanderson Dec 12, 2024

evankanderson Dec 11, 2024

evankanderson Dec 12, 2024

evankanderson Dec 12, 2024

evankanderson Dec 12, 2024

evankanderson commented Dec 16, 2024

evankanderson commented Dec 16, 2024

Add support for base and target trees in git ingest, add .tar.gz bundler #5181

Add support for base and target trees in git ingest, add .tar.gz bundler #5181

Conversation

evankanderson commented Dec 11, 2024

Summary

Change Type

Testing

Review Checklist:

coveralls commented Dec 11, 2024 • edited Loading

JAORMX Dec 12, 2024 • 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

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

evankanderson commented Dec 16, 2024

evankanderson commented Dec 16, 2024

coveralls commented Dec 11, 2024 •

edited

Loading

JAORMX Dec 12, 2024 •

edited

Loading