cc @apelisse @mbohlool @pwittrock from original PR

Thanks @liggitt for working on that, and sorry for breaking your use-case. All good points,

Can you clarify what the difference is between:

uses a disk-based cache that is not safe between processes (does not use atomic fs operations)

and

is vulnerable to partially written cache responses being used as responses to future requests

Or maybe the question is, would a fix for the first point also fix that second point? If not, can you clarify what you meant for the latter.

uses a disk-based cache that is not safe between processes (does not use atomic fs operations)

this means that two successful writes by separate processes would stomp on each other (process 1 starts writing, process 2 starts writing, they both write to the file in an interleaved way)

is vulnerable to partially written cache responses being used as responses to future requests

this means that if a single process fails halfway through writing the file, the half-written file is left in place. a future invocation of the cache will scan existing files to determine what is in cache, and the half-written file gets put in the index of available cached responses.

would a fix for the first point also fix that second point?

possibly, depending on the fix... if the disk cache changed to always write to a tmp file and only on success do a move to a file that is used to serve future cached responses, I think that would likely address both issues.

if the disk cache changed to always write to a tmp file and only on success do a move to a file that is used to serve future cached responses, I think that would likely address both issues.

I think we're on the same page then, that's exactly what I had in mind. Thanks

/lgtm

Given that this impacts concurrent kubectl clients in unpredictable ways, and also caches secret data on disk.

rebased to fix godep conflict

fixed conflict in staging godep

/retest

/lgtm

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, smarterclayton, thockin

Associated issue: 47353

The full list of commands accepted by this bot can be found here.

unit flaked on #50022
/retest

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

/test all [submit-queue is verifying that this PR is safe to merge]

@liggitt: The following test failed, say /retest to rerun them all:

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Automatic merge from submit-queue (batch tested with PRs 50254, 50174, 50179)