-
Notifications
You must be signed in to change notification settings - Fork 461
feat(envd): add optional EntryInfo to watch FilesystemEvent #2930
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a8ba8ce
feat(envd): add optional EntryInfo to watch FilesystemEvent
mishushakov 051e031
refactor(envd): rename watch flag include_entryinfo to include_entry
mishushakov 0f62c3d
fix(envd): make watch entry info best-effort, log on failure
mishushakov 9449fdc
refactor(envd): simplify entry-info NotFound check with connect.CodeOf
mishushakov 04f5e4f
Update version.go
mishushakov e427f06
Merge remote-tracking branch 'origin/main' into mishushakov/watchdir-…
mishushakov 08fc4b7
Merge remote-tracking branch 'origin/mishushakov/watchdir-entryinfo-o…
mishushakov fb2851c
fix(envd): never attach entry info to watch remove/rename events
mishushakov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
166 changes: 166 additions & 0 deletions
166
packages/envd/internal/services/filesystem/watch_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| package filesystem | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
| "os/user" | ||
| "path/filepath" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "connectrpc.com/authn" | ||
| "connectrpc.com/connect" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/e2b-dev/infra/packages/envd/internal/services/spec/filesystem" | ||
| ) | ||
|
|
||
| // collectEvents polls GetWatcherEvents until at least one event is returned or the | ||
| // deadline is reached. fsnotify delivers events asynchronously, so we can't assume | ||
| // they are available immediately after the filesystem operation. | ||
| func collectEvents(t *testing.T, ctx context.Context, svc Service, watcherID string) []*filesystem.FilesystemEvent { | ||
| t.Helper() | ||
|
|
||
| deadline := time.Now().Add(2 * time.Second) | ||
| for time.Now().Before(deadline) { | ||
| resp, err := svc.GetWatcherEvents(ctx, connect.NewRequest(&filesystem.GetWatcherEventsRequest{ | ||
| WatcherId: watcherID, | ||
| })) | ||
| require.NoError(t, err) | ||
|
|
||
| if len(resp.Msg.GetEvents()) > 0 { | ||
| return resp.Msg.GetEvents() | ||
| } | ||
|
|
||
| time.Sleep(20 * time.Millisecond) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func TestWatcherIncludeEntryInfo(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| u, err := user.Current() | ||
| require.NoError(t, err) | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| includeEntryInfo bool | ||
| wantEntry bool | ||
| }{ | ||
| {name: "entry info included when requested", includeEntryInfo: true, wantEntry: true}, | ||
| {name: "entry info omitted when not requested", includeEntryInfo: false, wantEntry: false}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| root := t.TempDir() | ||
| svc := mockService() | ||
| ctx := authn.SetInfo(t.Context(), u) | ||
|
|
||
| created, err := svc.CreateWatcher(ctx, connect.NewRequest(&filesystem.CreateWatcherRequest{ | ||
| Path: root, | ||
| IncludeEntry: tt.includeEntryInfo, | ||
| })) | ||
| require.NoError(t, err) | ||
| watcherID := created.Msg.GetWatcherId() | ||
| t.Cleanup(func() { | ||
| _, _ = svc.RemoveWatcher(ctx, connect.NewRequest(&filesystem.RemoveWatcherRequest{ | ||
| WatcherId: watcherID, | ||
| })) | ||
| }) | ||
|
|
||
| // Trigger an event that leaves a stat-able entry behind. | ||
| filePath := filepath.Join(root, "file.txt") | ||
| require.NoError(t, os.WriteFile(filePath, []byte("hello"), 0o644)) | ||
|
|
||
| events := collectEvents(t, ctx, svc, watcherID) | ||
| require.NotEmpty(t, events, "expected at least one filesystem event") | ||
|
|
||
| for _, e := range events { | ||
| assert.Equal(t, "file.txt", e.GetName()) | ||
|
|
||
| if tt.wantEntry { | ||
| require.NotNil(t, e.GetEntry(), "expected entry info on event %s", e.GetType()) | ||
| assert.Equal(t, "file.txt", e.GetEntry().GetName()) | ||
| assert.Equal(t, filePath, e.GetEntry().GetPath()) | ||
| assert.Equal(t, filesystem.FileType_FILE_TYPE_FILE, e.GetEntry().GetType()) | ||
| } else { | ||
| assert.Nil(t, e.GetEntry(), "expected no entry info on event %s", e.GetType()) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // TestWatcherIncludeEntryInfo_RemoveDoesNotCarryReplacement guards against a TOCTOU race: | ||
| // if an entry is removed and a different entry is created at the same path before the | ||
| // watcher handles the remove event, stat-ing the path would succeed and could attach the | ||
| // replacement's info to the remove event. Remove/rename events must never carry entry info. | ||
| func TestWatcherIncludeEntryInfo_RemoveDoesNotCarryReplacement(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| u, err := user.Current() | ||
| require.NoError(t, err) | ||
|
|
||
| root := t.TempDir() | ||
|
|
||
| // File exists before we start watching, so its removal is what we observe. | ||
| filePath := filepath.Join(root, "file.txt") | ||
| require.NoError(t, os.WriteFile(filePath, []byte("hello"), 0o644)) | ||
|
|
||
| svc := mockService() | ||
| ctx := authn.SetInfo(t.Context(), u) | ||
|
|
||
| created, err := svc.CreateWatcher(ctx, connect.NewRequest(&filesystem.CreateWatcherRequest{ | ||
| Path: root, | ||
| IncludeEntry: true, | ||
| })) | ||
| require.NoError(t, err) | ||
| watcherID := created.Msg.GetWatcherId() | ||
| t.Cleanup(func() { | ||
| _, _ = svc.RemoveWatcher(ctx, connect.NewRequest(&filesystem.RemoveWatcherRequest{ | ||
| WatcherId: watcherID, | ||
| })) | ||
| }) | ||
|
|
||
| require.NoError(t, os.Remove(filePath)) | ||
| // Recreate a different entry at the same path before the watcher handles the remove | ||
| // event, so the path is occupied (and stat-able) by the time the event is processed. | ||
| require.NoError(t, os.WriteFile(filePath, []byte("replacement"), 0o644)) | ||
|
|
||
| // Accumulate events until we have observed both the removal and the replacement, or time out. | ||
| var removeEvent, replacementEvent *filesystem.FilesystemEvent | ||
| deadline := time.Now().Add(2 * time.Second) | ||
| for time.Now().Before(deadline) && (removeEvent == nil || replacementEvent == nil) { | ||
| resp, eventsErr := svc.GetWatcherEvents(ctx, connect.NewRequest(&filesystem.GetWatcherEventsRequest{ | ||
| WatcherId: watcherID, | ||
| })) | ||
| require.NoError(t, eventsErr) | ||
|
|
||
| for _, e := range resp.Msg.GetEvents() { | ||
| switch e.GetType() { | ||
| case filesystem.EventType_EVENT_TYPE_REMOVE: | ||
| removeEvent = e | ||
| case filesystem.EventType_EVENT_TYPE_CREATE, filesystem.EventType_EVENT_TYPE_WRITE: | ||
| if replacementEvent == nil { | ||
| replacementEvent = e | ||
| } | ||
| } | ||
| } | ||
|
|
||
| time.Sleep(20 * time.Millisecond) | ||
| } | ||
|
|
||
| require.NotNil(t, removeEvent, "expected a remove event") | ||
| assert.Nil(t, removeEvent.GetEntry(), "remove event must not carry entry info even when a new entry occupies the path") | ||
|
|
||
| // Sanity check: the replacement is stat-able, so the nil above is by design — not just | ||
| // because the path happens to be empty. | ||
| require.NotNil(t, replacementEvent, "expected a create/write event for the replacement") | ||
| assert.NotNil(t, replacementEvent.GetEntry(), "event for the existing replacement should carry entry info") | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.