Skip to content

fix: prevent panics in KubeConflict#146

Closed
miparnisari wants to merge 4 commits intomainfrom
prevent-panic
Closed

fix: prevent panics in KubeConflict#146
miparnisari wants to merge 4 commits intomainfrom
prevent-panic

Conversation

@miparnisari
Copy link
Copy Markdown
Contributor

Prevents this

E0724 15:48:55.386736   91448 authz.go:103] "failed to perform update" err=<
        dual write failed: workflow had a panic: panic in workflow: runtime error: invalid memory address or nil pointer dereference
        goroutine 15400 [running]:
        runtime/debug.Stack()
                /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/runtime/debug/stack.go:26 +0x64
        github.com/cschleiden/go-workflows/workflow/executor.(*workflow).Execute.func1.1()
                /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@v1.0.1/workflow/executor/workflow.go:49 +0x40
        panic({0x1066da7c0?, 0x109c0de70?})
                /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/runtime/panic.go:792 +0x124
        github.com/authzed/spicedb-kubeapi-proxy/pkg/authz/distributedtx.KubeConflict({0x106fc7ca0, 0x14001afdc20}, 0x140018df680)
                /Users/miparnisari/Documents/GitHub/spicedb-kubeapi-proxy/pkg/authz/distributedtx/workflow.go:427 +0xbc
        github.com/authzed/spicedb-kubeapi-proxy/pkg/authz/distributedtx.PessimisticWriteToSpiceDBAndKube({0x107024e00, 0x1400459f5f0}, 0x140018df680)
                /Users/miparnisari/Documents/GitHub/spicedb-kubeapi-proxy/pkg/authz/distributedtx/workflow.go:201 +0x1cd8
        reflect.Value.call({0x1066a31e0?, 0x106fadc68?, 0x1066a31e0?}, {0x105a63be4, 0x4}, {0x1400459f650, 0x2, 0x1400459f650?})
                /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/reflect/value.go:584 +0x978
        reflect.Value.Call({0x1066a31e0?, 0x106fadc68?, 0x1066a31e0?}, {0x1400459f650?, 0x14003d83a40?, 0x14003a8da58?})
                /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/reflect/value.go:368 +0x94
        github.com/cschleiden/go-workflows/workflow/executor.(*workflow).Execute.func1({0x107024e00, 0x1400459f5f0})
                /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@v1.0.1/workflow/executor/workflow.go:56 +0x22c
        github.com/cschleiden/go-workflows/internal/sync.NewCoroutine.func1()
                /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@vUnauthorized
1.0.1/internal/sync/coroutine.go:85 +0xa0
        created by github.com/cschleiden/go-workflows/internal/sync.NewCoroutine in goroutine 15398
                /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@v1.0.1/internal/sync/coroutine.go:69 +0x160
        
        stack: /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/runtime/panic.go:262 (0x1028faea0)
                sigpanic: panic(memoryError)
        /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/runtime/signal_unix.go:925 (0x1028fae6d)
                sigpanic: panicmem()
        /Users/miparnisari/Documents/GitHub/spicedb-kubeapi-proxy/pkg/authz/distributedtx/workflow.go:427 (0x1045c30bd)
                KubeConflict: }, input.ObjectMeta.Name, err)
        /Users/miparnisari/Documents/GitHub/spicedb-kubeapi-proxy/pkg/authz/distributedtx/workflow.go:201 (0x1045c0fe8)
                PessimisticWriteToSpiceDBAndKube: return KubeConflict(err, input), nil
        /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/reflect/value.go:584 (0x102933f08)
                Value.call: call(frametype, fn, stackArgs, uint32(frametype.Size()), uint32(abid.retOffset), uint32(frameSize), &regArgs)
        /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/reflect/value.go:368 (0x1029333c4)
                Value.Call: return v.call("Call", in)
        /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@v1.0.1/workflow/executor/workflow.go:56 (0x1043c0eac)
                (*workflow).Execute.func1: r := w.fn.Call(args)
        /Users/miparnisari/go/pkg/mod/github.com/cschleiden/go-workflows@v1.0.1/internal/sync/coroutine.go:85 (0x10439d990)
                NewCoroutine.func1: s.err = fn(ctx)
        /Users/miparnisari/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.4.darwin-arm64/src/runtime/asm_arm64.s:1223 (0x1029015d4)
                goexit: MOVD    R0, R0  // NOP

@github-actions github-actions Bot added area/tooling Affects the dev or user toolchain area/core labels Jul 24, 2025
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.79%. Comparing base (674b2a4) to head (6b04cf9).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
- Coverage   67.19%   66.79%   -0.41%     
==========================================
  Files          23       23              
  Lines        3170     3186      +16     
==========================================
- Hits         2130     2128       -2     
- Misses        819      833      +14     
- Partials      221      225       +4     

☔ 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.

Copy link
Copy Markdown
Contributor

@tstirrat15 tstirrat15 left a comment

Choose a reason for hiding this comment

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

Makes sense

@miparnisari
Copy link
Copy Markdown
Contributor Author

Tests are now consistently failing :(

 [FAILED] err isn't `unauthorized`:%!(EXTRA string=the server reported a conflict (delete pods paul-pod-wdd8k))

Comment thread e2e/proxy_test.go
} else {
fmt.Println(err)
Expect(k8serrors.IsUnauthorized(err)).To(BeTrue())
Expect(k8serrors.IsConflict(err) || k8serrors.IsUnauthorized(err)).To(BeTrue())
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ecordell or @vroldanbet can you confirm whether this is OK?

Copy link
Copy Markdown
Contributor

@vroldanbet vroldanbet left a comment

Choose a reason for hiding this comment

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

found the potential issue, requesting changes so this does not merge yet

Comment thread magefiles/test.go Outdated
"-r",
"-vv",
"--fail-fast",
//"--until-it-fails",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps add a new mage command like E2EUntilItFails()?

@vroldanbet
Copy link
Copy Markdown
Contributor

See #148

@vroldanbet
Copy link
Copy Markdown
Contributor

merged via #148

@vroldanbet vroldanbet closed this Jul 31, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area/core area/tooling Affects the dev or user toolchain

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants