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

storage/disk: make symlinks work with relative paths #4870

Conversation

srenatus
Copy link
Contributor

@srenatus srenatus commented Jul 11, 2022

So for now, we'll properly create the symlinks without the wrong indirection.

Fixes #4869.

I'm still looking into remediation logic: if the symlink is wrong, fix it automatically.

Update: I think we should keep this as-is. The first is simple enough to guide the few people who could have run into it at this point to the proper steps.

@srenatus srenatus force-pushed the sr/storage/disk/fix-relative-links-and-restarts branch from cb50d20 to bb0a5ba Compare July 11, 2022 11:25
@srenatus srenatus marked this pull request as ready for review July 11, 2022 11:25
@srenatus srenatus force-pushed the sr/storage/disk/fix-relative-links-and-restarts branch from bb0a5ba to d355455 Compare July 11, 2022 12:28

txn := storage.NewTransactionOrDie(ctx, s, storage.WriteParams)
func TestTruncateRelativeStoragePath(t *testing.T) {
dir := "foobar"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit messy, but I think it's better to test it this way -- potentially polluting some work dir -- than not at all.

t.Fatalf("Expected get policy to return %v but got: %v (err: %v)", expectedBytes, bs, err)
}

// Close and re-open store
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new thing -- it would catch the previous failure, which would yield to not being able to deref the symlink foobar/foobar/backupXXXX.

So for now, we'll properly create the symlinks without the wrong indirection.

Fixes open-policy-agent#4869.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@srenatus srenatus force-pushed the sr/storage/disk/fix-relative-links-and-restarts branch from d355455 to 0dee8cf Compare July 11, 2022 12:44
@srenatus srenatus merged commit ff19839 into open-policy-agent:main Jul 11, 2022
@srenatus srenatus deleted the sr/storage/disk/fix-relative-links-and-restarts branch July 11, 2022 16:26
srenatus added a commit to srenatus/opa that referenced this pull request Jul 13, 2022
…nt#4870)

So for now, we'll properly create the symlinks without the wrong indirection.

Fixes open-policy-agent#4869.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus added a commit that referenced this pull request Jul 13, 2022
So for now, we'll properly create the symlinks without the wrong indirection.

Fixes #4869.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
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.

disk storage: symlink logic fails if using relative directories
2 participants