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

Adding functionality for dirhash in cli #436

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

matglas
Copy link
Contributor

@matglas matglas commented Apr 29, 2024

What this PR does / why we need it

Adding the dirhash-glob arguments

Which issue(s) this PR fixes (optional)

Depends on:

Fixes: #216 and in-toto/go-witness#65

Acceptance Criteria Met

  • Docs changes if needed
  • Testing changes if needed
  • All workflow checks passing (automatically enforced)
  • All review conversations resolved (automatically enforced)
  • DCO Sign-off

Special notes for your reviewer:

Todo's

  • PR merged for checks to pass: Adding functionality for dirhash in library go-witness#223
  • discuss what documentation is needed.
    • idea to add subject types to the attestor documentation for easier understanding of the different subjects.
    • examples or reasoning for using dirhash. also mentioning the downsides for certain use cases.

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for witness-project ready!

Name Link
🔨 Latest commit 091a031
🔍 Latest deploy log https://app.netlify.com/sites/witness-project/deploys/6750bc388104110008a3bbaa
😎 Deploy Preview https://deploy-preview-436--witness-project.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@matglas matglas changed the title chore: Adding dirhas-glob argument. chore: Adding dirhash-glob argument. Apr 29, 2024
cmd/run.go Outdated Show resolved Hide resolved
@matglas matglas force-pushed the dirhash-glob branch 3 times, most recently from d3fcdb6 to 78188b8 Compare May 1, 2024 16:03
@matglas matglas changed the title chore: Adding dirhash-glob argument. Adding functionality for dirhash in cli May 1, 2024
@matglas matglas force-pushed the dirhash-glob branch 2 times, most recently from 16026a4 to 95d1c7e Compare May 21, 2024 19:01
@matglas
Copy link
Contributor Author

matglas commented May 21, 2024

@jkjell @ChaosInTheCRD I updated both PRs with the latest updates and added changes that we discussed during the Community call. Let me know if you see anything that still needs to be addressed.

@matglas
Copy link
Contributor Author

matglas commented Aug 22, 2024

I need some help to figure out the cause of this test failure which is hard to pinpoint. @mikhailswift or @jkjell maybe you can help a bit here.

Note this is the result when you run the code with the update from the go-witness library PR that belongs to it.

...
level=info msg="Completed product attestors stage..."
level=info msg="Starting postproduct attestors stage..."
level=info msg="Completed postproduct attestors stage..."
level=info msg="Starting verify attestors stage..."
level=info msg="Starting policyverify attestor..."
level=info msg="policy signature verified"
level=info msg="Finished policyverify attestor... (0.000167042s)"
level=info msg="Completed verify attestors stage..."
level=error msg="Verification failed"
level=error msg="Evidence:"
level=error msg="Step: step02"
level=error msg="verification failure: Reason: failed to verify artifacts for step step02: \nfailed to verify artifacts: []"
level=error msg="Step: step01"
    verify_test.go:293: 
                Error Trace:    /Users/matthias.glastra/Projects/github/in-toto/witness/cmd/verify_test.go:293
                Error:          Received unexpected error:
                                failed to verify policy: policy verification failed
                Test:           TestRunVerifyKeyPair
--- FAIL: TestRunVerifyKeyPair (0.03s)
FAIL
coverage: 30.1% of statements
FAIL    github.com/in-toto/witness/cmd  1.223s
=== RUN   TestUTPolicySuite
=== RUN   TestUTPolicySuite/TestLoadPolicyArchivista
=== RUN   TestUTPolicySuite/TestLoadPolicyArchivistaNotFound
=== RUN   TestUTPolicySuite/TestLoadPolicyFile
=== RUN   TestUTPolicySuite/TestLoadPolicyFileNotFound
--- PASS: TestUTPolicySuite (0.00s)
    --- PASS: TestUTPolicySuite/TestLoadPolicyArchivista (0.00s)
    --- PASS: TestUTPolicySuite/TestLoadPolicyArchivistaNotFound (0.00s)
    --- PASS: TestUTPolicySuite/TestLoadPolicyFile (0.00s)
    --- PASS: TestUTPolicySuite/TestLoadPolicyFileNotFound (0.00s)
PASS
coverage: 68.2% of statements
ok      github.com/in-toto/witness/internal/policy      1.049s  coverage: 68.2% of statements
FAIL
make: *** [test] Error 1

@matglas
Copy link
Contributor Author

matglas commented Aug 29, 2024

@jkjell @mikhailswift if the go-witness lib is updated in go.mod then this PR can be merged too and functionality can be completed.
Would you be willing to give it review?

@kairoaraujo kairoaraujo added the waiting-go-witness-release Requires a release from go-witness to be merged label Oct 1, 2024
@kairoaraujo
Copy link
Collaborator

Waiting for release > 0.6.0

@jkjell jkjell removed waiting-go-witness-release Requires a release from go-witness to be merged DON'T MERGE labels Dec 4, 2024
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.

Add flag to exclude files from subjector
3 participants