Refactored token filecache #755
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
approved
micahhausler
force-pushed
the
filecache-injection
branch
3 times, most recently
from
975cca8 to
e313702
Compare
The token filecache used to use a private global function for creating a filelock, and overrode it in tests with a hand-crafted mocks for filesystem and environment variable operations. This change adds adds injectability to the filecache's filesystem and file lock using afero. This change also will simplify future changes when updating the AWS SDK with new credential interfaces. Signed-off-by: Micah Hausler <mhausler@amazon.com>
micahhausler
force-pushed
the
filecache-injection
branch
from
e313702 to
8464316
Compare
| // A mockable flock interface | ||
| type filelock interface { | ||
| // FileLocker is a subset of the methods exposed by *flock.Flock | ||
| type FileLocker interface { |
There was a problem hiding this comment.
I made this interface public since flock.Flock is an external type and we're just abstracting it for our needs here
| } | ||
|
|
||
| func (t *testFS) ReadFile(filename string) ([]byte, error) { |
There was a problem hiding this comment.
All these methods are no longer needed since we're just using an afero in-memory filesystem
| if t.err == nil { | ||
| return &t.fileinfo, nil | ||
| } else { | ||
| func (t *testFS) Stat(filename string) (fs.FileInfo, error) { |
There was a problem hiding this comment.
We only override Stat() because the in-memory filesystem returns all fs.FileInfo as tmpfiles.
| @@ -181,21 +145,37 @@ func validateFileCacheProvider(t *testing.T, p FileCacheProvider, err error, c * | |||
| } | |||
| } | |||
|
|
|||
| // testSetEnv sets an env var, and returns a cleanup func | |||
There was a problem hiding this comment.
Rather than mocking os Env methods, I just set/reset them
| validateFileCacheProvider(t, p, err, c) | ||
| if !p.cachedCredential.IsExpired() { | ||
| t.Errorf("missing cache file should result in expired cached credential") | ||
| } | ||
| tf.err = nil |
There was a problem hiding this comment.
Since we're no longer using a global mock, cleanup at the end of every test is no longer necessary
|
/lgtm /approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jyotimahapatra, micahhausler The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
The token filecache used to use a private global function for creating a filelock, and overrode it in tests with a hand-crafted mocks for filesystem and environment variable operations.
This change adds adds injectability to the filecache's filesystem and file lock using afero. This change also will simplify future changes when updating the AWS SDK with new credential interfaces.