chore: add dbauthz to unhanger tests #14394
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @spikecurtis and the rest of your teammates on |
196399c to
3a2c955
Compare
| @@ -37,7 +43,7 @@ func TestDetectorNoJobs(t *testing.T) { | |||
| statsCh = make(chan unhanger.Stats) | |||
| ) | |||
|
|
|||
| detector := unhanger.New(ctx, db, pubsub, log, tickCh).WithStatsChannel(statsCh) | |||
| detector := unhanger.New(ctx, wrapDBAuthz(db, log), pubsub, log, tickCh).WithStatsChannel(statsCh) | |||
There was a problem hiding this comment.
Feels odd to have a raw db laying around outside this function call scope. I'd probably do this to not have a db laying around. Up to you 🤷♂️
var (
ctx = testutil.Context(t, testutil.WaitLong)
rdb, pubsub = dbtestutil.NewDB(t)
db = wrapDBAuthz(rdb, slogtest.Make(t, nil))
log = slogtest.Make(t, nil)
tickCh = make(chan time.Time)
statsCh = make(chan unhanger.Stats)
)There was a problem hiding this comment.
The annoying thing about that is that I'd have to set up a dbauthz principal for the calls that write the job, user, template, etc. Authz for those operations is irrelevant to this test.
There was a problem hiding this comment.
Ah, yea it is annoying. dbgen gets around that with a genCtx:
coder/coderd/database/dbgen/dbgen.go
Line 35 in 40bf367
If you use dbgen to handle all the initial state, you can ignore that requirement.
There was a problem hiding this comment.
Almost, but not quite all state is dbgen. It make sense to me to keep this way, since it really is just the unhanger under test here, so only it need the dbauthz.
3a2c955 to
b4ba388
Compare
b4ba388 to
4ce4ada
Compare
Merge activity
|
relates to #14393
customer is having issues with the unhanger failing to kill a hung build. I noticed that our unhanger unit tests don't actually use the dbauthz. This PR adds authz to the unhanger tests.
Many tests failed on template import and template dry-run jobs, because the test code inserts build jobs without a corresponding template version, and our authz layer uses permission on the template version as a proxy for permission on the job.
I'm pretty sure that our actual API calls ensure that there is always a template version for the the job, but for some future work, it would be nice to have the database enforce this.