Skip to content

Conversation

@ton31337
Copy link
Contributor

@ton31337 ton31337 commented May 7, 2025

No description provided.

@ton31337 ton31337 force-pushed the fix/crash_kopia_policy_show branch from 015013b to fc3c5d8 Compare May 7, 2025 10:21
@jkowalski jkowalski changed the title cli: Fix crash when doing kopia policy show <path> fix(cli): fix crash when doing kopia policy show <path> May 12, 2025
@jkowalski jkowalski enabled auto-merge (squash) May 12, 2025 05:28
@jkowalski jkowalski disabled auto-merge May 12, 2025 05:32
@jkowalski
Copy link
Contributor

@ton31337 looks like there are linter failures, should be easy to fix.

@codecov
Copy link

codecov bot commented May 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.34%. Comparing base (cb455c6) to head (7436e64).
Report is 510 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4560      +/-   ##
==========================================
+ Coverage   75.86%   76.34%   +0.48%     
==========================================
  Files         470      527      +57     
  Lines       37301    40057    +2756     
==========================================
+ Hits        28299    30583    +2284     
- Misses       7071     7454     +383     
- Partials     1931     2020      +89     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

```
~# ./kopia policy show /home/donatas/tests
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x18b7b84]

goroutine 1 [running]:
github.com/kopia/kopia/cli.appendOSSnapshotPolicyRows({0xc005759008, 0x26, 0x55}, 0xc005754600?, 0xc005764008)
	/home/donatas/projects/kopia/cli/command_policy_show.go:490 +0x44
github.com/kopia/kopia/cli.printPolicy(0xc0000f5d48, 0xc005754600, 0xc005764008)
	/home/donatas/projects/kopia/cli/command_policy_show.go:135 +0x5e5
github.com/kopia/kopia/cli.(*commandPolicyShow).run(0xc0000f5d10, {0x2311ef8, 0xc0002e5320}, {0x7fb83829f3d0, 0xc0052eeb00})
	/home/donatas/projects/kopia/cli/command_policy_show.go:46 +0x18d
github.com/kopia/kopia/cli.(*App).repositoryReaderAction.func1({0x2311ef8?, 0xc0002e5320?}, {0x7fb83829f3d0?, 0xc0052eeb00?})
	/home/donatas/projects/kopia/cli/app.go:477 +0x2b
github.com/kopia/kopia/cli.(*App).repositoryReaderAction.(*App).maybeRepositoryAction.func2({0x2311ef8, 0xc0002e5320})
	/home/donatas/projects/kopia/cli/app.go:571 +0xbd
github.com/kopia/kopia/cli.(*App).repositoryReaderAction.(*App).maybeRepositoryAction.(*App).baseActionWithContext.func3.1.1()
	/home/donatas/projects/kopia/cli/app.go:554 +0x6e
github.com/kopia/kopia/cli.(*profileFlags).withProfiling(0x0?, 0xc0004a09c0?)
	/home/donatas/projects/kopia/cli/profile.go:45 +0x2b9
github.com/kopia/kopia/cli.(*App).repositoryReaderAction.(*App).maybeRepositoryAction.(*App).baseActionWithContext.func3.1({0x2311ef8?, 0xc0002e5320?})
	/home/donatas/projects/kopia/cli/app.go:549 +0x50
github.com/kopia/kopia/cli.(*App).runAppWithContext.func1(0xc0000f48d0?, 0xc0000f4808, 0xc0000efd30, {0x2311ef8, 0xc0002e52f0})
	/home/donatas/projects/kopia/cli/app.go:521 +0x186
github.com/kopia/kopia/cli.(*App).runAppWithContext(0xc0000f4808, 0xc0002b88f0, 0xc0000efd30)
	/home/donatas/projects/kopia/cli/app.go:522 +0x1a9
github.com/kopia/kopia/cli.(*App).repositoryReaderAction.(*App).maybeRepositoryAction.(*App).baseActionWithContext.func3(0x22e2801?)
	/home/donatas/projects/kopia/cli/app.go:548 +0x3c
github.com/alecthomas/kingpin/v2.(*actionMixin).applyActions(...)
	/home/donatas/go/pkg/mod/github.com/alecthomas/kingpin/v2@v2.4.0/actions.go:28
github.com/alecthomas/kingpin/v2.(*Application).applyActions(0xc0001b8500?, 0xc000453050)
	/home/donatas/go/pkg/mod/github.com/alecthomas/kingpin/v2@v2.4.0/app.go:568 +0xd0
github.com/alecthomas/kingpin/v2.(*Application).execute(0xc0001b8500, 0xc000453050, {0xc0004b6d80, 0x2, 0x2})
	/home/donatas/go/pkg/mod/github.com/alecthomas/kingpin/v2@v2.4.0/app.go:398 +0x65
github.com/alecthomas/kingpin/v2.(*Application).Parse(0xc0001b8500, {0xc00014e090?, 0xc0001b8500?, 0x22e8c80?})
	/home/donatas/go/pkg/mod/github.com/alecthomas/kingpin/v2@v2.4.0/app.go:230 +0x14a
main.main()
	/home/donatas/projects/kopia/main.go:77 +0x1cc
```

This is happening if we apply this before:

```
~# ./kopia policy set --inherit=false --clear-ignore /home/donatas/tests
```

Default value for `p.OSSnapshotPolicy.VolumeShadowCopy.Enable` is nil, so it's not set
which is inherited from a global one and is 0 (never).

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
@ton31337 ton31337 force-pushed the fix/crash_kopia_policy_show branch from fc3c5d8 to 7436e64 Compare May 12, 2025 05:50
@ton31337
Copy link
Contributor Author

Dzieki, should be fixed now :)

@jkowalski jkowalski enabled auto-merge (squash) May 12, 2025 23:18
@jkowalski jkowalski merged commit 67d8399 into kopia:master May 12, 2025
21 checks passed
@julio-lopez julio-lopez requested a review from Copilot May 13, 2025 01:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a crash when executing the command "kopia policy show " by ensuring that Volume Shadow Copy properly falls back to a default state. The change replaces a direct call to String() with one that first applies a default value, thus preventing nil-related crashes.

  • Changed Volume Shadow Copy row value retrieval to use OrDefault with policy.OSSnapshotNever
  • Updated the related table row formatting in the CLI command
Comments suppressed due to low confidence (1)

cli/command_policy_show.go:487

  • Consider adding unit tests to cover cases when p.OSSnapshotPolicy.VolumeShadowCopy.Enable is not explicitly set, ensuring the default policy.OSSnapshotNever is applied correctly.
p.OSSnapshotPolicy.VolumeShadowCopy.Enable.OrDefault(policy.OSSnapshotNever).String(),


func appendOSSnapshotPolicyRows(rows []policyTableRow, p *policy.Policy, def *policy.Definition) []policyTableRow {
rows = append(rows,
policyTableRow{"OS-level snapshot support:", "", ""},
Copy link

Copilot AI May 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding an inline comment to explain why policy.OSSnapshotNever is chosen as the default value to help future maintainers understand the decision.

Suggested change
policyTableRow{"OS-level snapshot support:", "", ""},
policyTableRow{"OS-level snapshot support:", "", ""},
// Defaulting to OSnapshotNever ensures that OS-level snapshots are disabled unless explicitly enabled,
// as they may not be supported or desired in all environments.

Copilot uses AI. Check for mistakes.
@ton31337 ton31337 deleted the fix/crash_kopia_policy_show branch May 13, 2025 06:24
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.

2 participants