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

tuf: fix on-disk cache when writing targets in subfolders #729

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

asraa
Copy link
Contributor

@asraa asraa commented Oct 6, 2022

Signed-off-by: Asra Ali asraa@google.com

Added a test, patched cosign locally, and can confirm this now works correctly with the v5 root changes.

Note: no old clients will break from this change. It's forward compatible.

It's in anticipation of a root update that we'll push to a separate repository as to not break clients who are still using this version. Clients won't break until the current repository loses lifetime: in late January next year.

Summary

Release Note

Documentation

Signed-off-by: Asra Ali <asraa@google.com>
@haydentherapper
Copy link
Contributor

To confirm, did you test the following after patching:

  • Initialize to current (v4) metadata with no state
  • Initialize to new (v5) metadata with no state
  • Initialize to new metadata from state with current metadata

if err := os.MkdirAll(d.base, 0o700); err != nil {

fp := filepath.FromSlash(filepath.Join(d.base, p))
if err := os.MkdirAll(filepath.Dir(fp), 0o700); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Silly question, what if the file exists but with different permissions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a valid cause of "your cache is corrupt" then, if you were manipulating it in a weird way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(or if hte question is: what practically happens? os.mkdirall won't do anything if it currently exists. if there were no write perms on it then later, then you'd fail on not being able to write to it: which I think is a corrupt cache)

@asraa
Copy link
Contributor Author

asraa commented Oct 6, 2022

Yes!

To confirm, did you test the following after patching:

  • Initialize to current (v4) metadata with no state: yep, that's ./cosign initialize
  • Initialize to new (v5) metadata with no state, yep, that's ./cosign initialize --mirror http://localhost:8001 immediately following the first
  • Initialize to new metadata from state with current metadata rm the dir and retry the previous

@haydentherapper haydentherapper merged commit a862909 into sigstore:main Oct 6, 2022
@haydentherapper
Copy link
Contributor

Possibly related prober failure, investigating now - #729

@haydentherapper
Copy link
Contributor

Unrelated! Fixing issue

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.

None yet

2 participants