Fix TestServerControlSocketActivated flake.
- Clear `runner.ExtraFiles` before calling running other
commands. This appears to be the primary source of
the spurious test failures.
Refactor `TestServerControlSocketActivated` to:
- start server from the test (go)routine instead of a background one;
- process server's output synchrounously in the test goroutine;
- fix race: close the listener file descriptor synchronously in the
test goroutine instead of async, which ensures the descriptor is
closed before sending the first request to the (status) server;
otherwise, shutdown hangs (when the listener close is delayed);
- check and assert server's exit status;
- improve test cleanup: ensure child server process terminates
and is reaped on assertion failure.
Also, address potential race in `TestServerControlSocketActivatedTooManyFDs`
check with explanation comment. There is still a posible race where
the stderr pipe is closed before the background async callback gets
the error message. This will be addressed separately.
- use struct field initialization idiom
- clarify error message
- directly use s.rootctx in server request processing
- refactor: use require in e2e ACL test
- inline constant definition
- modernize with `strings.Cut`
- modernize with slices.Backward
- simplify timeFormat initialization
- refactor testing: check `Scanner.Err()` in testenv
- remove spurious parenthesis
- trueStr and falseStr consts
- use inheritPolicyString
- use const in cache info
- "bytes" const in ui task counters
- use common consts in gettool
Fixes for new linter warnings:
- fix: add //nolint annotations where it makes sense
- fix: remove no-longer necessary linter annotations
- fix linter: add or remove new lines
- fix linter: use Fprintf instead of WriteString
- fix: check for overflow in conversion to uint8 / byte
Increase error tolerance for resulting weights in
TestPickWeighted to reduce spurious failures.
The test is non-deterministic by nature.
Refactor:
- run test cases in sub-tests
- make constant more legible
- use require
* feat(cli): restrict insecure unauthenticated server to loopback binds
Reject starting the server with --insecure and --without-password when
--address would bind outside loopback (including empty host / all
interfaces). Validate the actual listener after Listen so systemd socket
activation cannot bypass the check.
Add hidden --allow-extremely-dangerous-unauthenticated-server-on-the-network
to opt into the previous behavior for isolated environments.
Extract validation into internal/insecureserverbind with table-driven tests.
Add end-to-end smoke tests for rejection and escape hatch.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Update internal/insecureserverbind/insecureserverbind.go
Co-authored-by: Julio López <1953782+julio-lopez@users.noreply.github.com>
* pr feedback
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Julio López <1953782+julio-lopez@users.noreply.github.com>
- make benchmarking params uint
- prevent error in compression benchmarking
- lint on Windows and address linter warnings
- upgrade golang.org/x/exp
- upgrade github.com/cncf/xds/go
- upgrade github.com/dustinkirkland/golang-petname
- direct users to forum
- add warning about _recovery recipes_
Revert "feat(snapshots): localfs support for passing options (#5044)"
commit c8c4615595.
Fix: return `ErrorEntry` for permission denied instead of aborting
When iterating a directory, if `lstat` fails with permission denied on
an entry, return an `ErrorEntry` instead of an error that stops the
entire directory iteration.
Previously, a single inaccessible entry, such as, a FUSE/sshfs mount
owned by another user, would cause the entire containing directory to
fail and be omitted from the snapshot, resulting in data loss.
Now, the inaccessible entry is returned as an ErrorEntry which is
handled according to the configured error handling policy, allowing
iteration to continue and the rest of the directory to be backed up.
- Fixes: #5045
Differentiate entry type when ignoring failed entries
Fix tests for new behavior, including handling timing-dependent
behavior when snapshots --fail-fast
---------
Co-authored-by: Geoffrey D. Bennett <g@netcraft.com.au>
* rename loop var for clarity
* only log on error
* rename var for clarity
* use t.Cleanup
* save environment once for all subtests
* get perms for modified entry once
* move repo setup to testPermissions
* create empty dir<maxDirDepth> so there are 3 entries in dir<maxDirDepth-1>
- leverage require
- remove unused return value
- remove unnecessary intermediate vars
- rename variable to maxDirDepth for clarity
- limit test file sizes
- improve to readability
- improve sub-test naming format
- run each file mode case in a sub-test
- move test case to separate function
- move long loop block to separate function
- run loop iterations in sub-tests
- rename test params for consistency
- add t.Helper()
- coarser-grained parallelism
- restore each snapshot into a separate directory
- update function comment
- Move MaybePrefixLongFilenameOnWindows to ospath package and rename
it to SafeLongFilename, along with corresponding test.
- Elide the function implementation at build time on non-Windows
platforms.
- Update documentation and comments for clarity.
- Rename package-local helper function.
Refactor `--profile-*` flags:
- Multiple profile types can be enabled at once, before only
a single type profiling could be done during a process execution.
- The new `--profiles-store-on-exit` enables all available profile
types, except for CPU profiling which needs to be explicitly enabled.
- Profiling parameters can now be set via new flags. This allows setting
the profile parameters for the pprof endpoint, as well as when saving
profiles to files on exit.
- Group profiling flags with other observability flags
- Adds a `--diagnostics-output-directory` flag that unifies and
supersedes the `--profile-dir` and `--metrics-directory` flags
Enhancements and behavior changes:
- Profile flags now have effect for all kopia commands, including
`server start`. Before these flags did not have any effect
in a few commands.
- Multiple profile types can be enabled at once, before only
a single type profiling could be done during a process execution.
- The new `--profiles-store-on-exit` enables all available profile
types, except for CPU profiling which needs to be explicitly enabled.
- Profiling parameters can now be set via new flags. This allows setting
the profile parameters for the pprof endpoint, as well as when saving
profiles to files on exit.
The following flags have been removed:
- `--profile-dir`: superseded by the `--diagnostics-output-directory` flag
- `--profile-blocking`: the `--profile-store-on-exit` flag enables blocking
profiling. Use `--profile-blocking-rate=0` to explicitly disable it.
- `--profile-memory`: the `--profile-store-on-exit` flag enables memory
profiling. Use `--profile-memory-rate=0` to explicitly disable it.
- `--profile-mutex`: the `--profile-store-on-exit` flag enables mutex
profiling. Use `--profile-mutex-fraction=0` to explicitly disable it.
Add CLI test for profile flags.
Objective: make the tests more robust and reduce random failures.
Preliminary refactoring:
- Accept testing.TB in testenv helpers. This is needed to
use `require.EventuallyWithT` in socket activation tests.
- Rename parameters for clarity
Tests refactoring:
- use t.Cleanup instead of defer where appropriate
- create file handlers in test routine instead of go routines
- remove unnecessary var declaration
- increased wait time to 30 seconds.
- allow running socket activation test on Darwin
Ref:
- #3283
- #3313
- #3318
- upgrade to golangci-lint 2.6.1
- updates for gosec
- updates for govet
- updates for perfsprint
- updates modernize
Leaves out modernize:omitempty due to conflicts with tests
Add return values in long helper function. This Improves clarity
and avoids 'naked' return and removes `//nolint:nakedret` annotation.
Explicit return values for GetShardedPathAndFilePath
Prevent running "auto-maintenance" on snapshot create.
Ref:
- #4851
Context: the test fails because there are concurrent "endurance"
runners and each of these advances the clock, some of them
significantly (action{Small,Medium,Large}ClockJump), which
causes the clock skewness check to fail when
(auto-)maintenance runs.
The test maintenance action does not experience this issue
because it runs exclusively on its own, that is, other
actions have to wait until maintenance completes.
This is a breaking change to users who might be using Kopia as a library.
### Log Format
```json
{"t":"<timestamp-rfc-3389-microseconds>", "span:T1":"V1", "span:T2":"V2", "n":"<source>", "m":"<message>", /*parameters*/}
```
Where each record is associated with one or more spans that describe its scope:
* `"span:client": "<hash-of-username@hostname>"`
* `"span:repo": "<random>"` - random identifier of a repository connection (from `repo.Open`)
* `"span:maintenance": "<random>"` - random identifier of a maintenance session
* `"span:upload": "<hash-of-username@host:/path>"` - uniquely identifies upload session of a given directory
* `"span:checkpoint": "<random>"` - encapsulates each checkpoint operation during Upload
* `"span:server-session": "<random>"` -single client connection to the server
* `"span:flush": "<random>"` - encapsulates each Flush session
* `"span:maintenance": "<random>"` - encapsulates each maintenance operation
* `"span:loadIndex" : "<random>"` - encapsulates index loading operation
* `"span:emr" : "<random>"` - encapsulates epoch manager refresh
* `"span:writePack": "<pack-blob-ID>"` - encapsulates pack blob preparation and writing
(plus additional minor spans for various phases of the maintenance).
Notable points:
- Used internal zero allocation JSON writer for reduced memory usage.
- renamed `--disable-internal-log` to `--disable-repository-log` (controls saving blobs to repository)
- added `--disable-content-log` (controls writing of `content-log` files)
- all storage operations are also logged in a structural way and associated with the corresponding spans.
- all content IDs are logged in a truncated format (since first N bytes that are usually enough to be unique) to improve compressibility of logs (blob IDs are frequently repeated but content IDs usually appear just once).
This format should make it possible to recreate the journey of any single content throughout pack blobs, indexes and compaction events.
Removes the following flags:
--caching: no-op
--list-caching: no-op
--enable-jaeger-collector: errors out when specified.
Also removes no-longer-used `deprecatedFlag` function.
* remove unnecessary type argument
* modernize with max
* unexport getPartial and update comment
* unexport getFull
* verifyNotCached helper
* use require helpers
* leverage verify[Not]Cached
* use windowsOSName const
* fix comment wrapping
* require in stat_test
* use 512 as the write size and log allocated size
* rename const to expectedMinAllocSize
* write a single byte to test file
* add TestGetBlockSizeFromCurrentFS
* require Positive
* log before invariant check
- enable `forcetypeassert` linter in non-test files
- add `//nolint` annotations
- add `testutil.EnsureType` helper for type assertions
- enable `forcetypeassert` linter in test files
Avoid allocating 1GB of RAM to write a test file.
Exclude test from race detector.
- Fixes: #4610
- Ref: #4439
nits:
- use `require.Greater`.
- add types to constants to used them with `require.*`.
- factor out function to write file with random data.
* fix(snapshots): Remove checkpoints after a complete snapshot
... by setting start time of checkpoints one nanosecond earlier than
that of the snapshot.
* test(snapshots): Test for leftover checkpoints
* fix linter issues
* removed stray curly brace
---------
Co-authored-by: Jarek Kowalski <jaak@jkowalski.net>
Adds a `--stats-only` flag to the `diff` command to reduce
output verbosity.
When `kopia diff` is invoked with the `--stats-only` flag it
only outputs the aggregate statistics of changes.
Cleanup robustness tests and `local_fs_test.go`
"Mechanical" changes:
- Use `require` helpers
- Use `testing.T` helpers
Note change in functionality: The use of `require` helpers
stops tests once a check fails. Before, various checks
were using `t.Error`, which fails the test but allows the
test to continue its execution.
* refactor(general): cleanup robustness/snapmeta/kopia_persister_light_test.go
Use `require` helpers
Use `testing.T` helpers
* refactor(general): cleanup local_fs_test.go
* fix import order
* use `os.CreateTemp` in `tempfile.CreateAutoDelete`
* refactor `TestTempFile`: add `VerifyTempfile` test helper
* add test for `createUnixFallback`
* rename function to `tempfile.CreateAutoDelete`
* remove the `dir` parameter to `tempfile.CreateAutoDelete`,
only an empty string was passed, apart from test cases.
* guard against panic in TestShadowCopy
* use uint8 for clarity
* unexport writeContentAsyncAndVerify
* fix typo in test function name
* remove commented interface functions
* use atomic.Int32
* cleanups in socket server activation test
* leverage stdlib's maps and slices packages
replace uses of `golang.org/x/exp/maps`
* nit: leverage `maps.Values`
Julio López