Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(auth): flaky credentials cache test #5979

Merged
merged 44 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
c622306
isolate test
Hweinstock Nov 12, 2024
fc8a1c6
add exception for eslint
Hweinstock Nov 12, 2024
0f92bfe
implement waitUntil on windows
Hweinstock Nov 12, 2024
c29a589
bake timeout into the fs module
Hweinstock Nov 12, 2024
b2a4734
add errors if waitUntil fails
Hweinstock Nov 12, 2024
f4e2736
undo fs changes
Hweinstock Nov 12, 2024
1860d22
add console.log statements
Hweinstock Nov 12, 2024
176ae5f
track code path
Hweinstock Nov 12, 2024
2be7f2f
add more logging
Hweinstock Nov 12, 2024
f2c17b8
check if cache is being returned
Hweinstock Nov 12, 2024
75b77f5
add more logging
Hweinstock Nov 12, 2024
5e316a1
reduce repeat
Hweinstock Nov 12, 2024
ce85893
add one more logging statement
Hweinstock Nov 12, 2024
f805cb8
rebump to 1000
Hweinstock Nov 12, 2024
2bddbfe
change logging statements
Hweinstock Nov 12, 2024
cb1c3b6
add more debugging
Hweinstock Nov 12, 2024
2cfe87e
another log
Hweinstock Nov 12, 2024
3c43115
add more logging
Hweinstock Nov 12, 2024
fa8c103
add more logging
Hweinstock Nov 12, 2024
e0eae0c
try adding a waitUntil to check if file content updated
Hweinstock Nov 12, 2024
a42f264
implement a retry mechanism
Hweinstock Nov 12, 2024
9b167ac
adjust logging, set timeout manually
Hweinstock Nov 13, 2024
5db69dc
avoid retry as it causes hang
Hweinstock Nov 13, 2024
143fa34
add logging to refresh logic
Hweinstock Nov 13, 2024
e65fe50
adjust test and its logging
Hweinstock Nov 13, 2024
46ac152
remove all logging
Hweinstock Nov 13, 2024
1161e54
add waitUntil for stat changes
Hweinstock Nov 13, 2024
a5225f9
try a long interval
Hweinstock Nov 13, 2024
354164a
wrap entire check in a waitUntil
Hweinstock Nov 13, 2024
10051bc
simplify test code
Hweinstock Nov 14, 2024
0ca8943
add back test isolation
Hweinstock Nov 14, 2024
9291ebb
add check for mtime
Hweinstock Nov 14, 2024
8c31442
add logging statements
Hweinstock Nov 14, 2024
c284254
adjust logging and timeout
Hweinstock Nov 14, 2024
53fc20e
adjust logging
Hweinstock Nov 14, 2024
fbb215d
rerun test a bunch
Hweinstock Nov 14, 2024
9dff77c
add really explicit logging statements
Hweinstock Nov 15, 2024
d7499fd
reformat logs
Hweinstock Nov 15, 2024
ce87909
try sleeping in between regen
Hweinstock Nov 15, 2024
3cbf39a
remove logging
Hweinstock Nov 15, 2024
87fbc01
focus on individual test
Hweinstock Nov 15, 2024
92c8138
adapt other test
Hweinstock Nov 15, 2024
dc8a7de
change test wording
Hweinstock Nov 15, 2024
29b31a2
remove loop on test
Hweinstock Nov 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,23 @@
* SPDX-License-Identifier: Apache-2.0
*/

import fs from '../../shared/fs/fs'
import { getLogger, Logger } from '../../shared/logger'
import { loadSharedCredentialsSections, updateAwsSdkLoadConfigEnvVar } from '../credentials/sharedCredentials'
import { CredentialsProviderType } from './credentials'
import { BaseCredentialsProviderFactory } from './credentialsProviderFactory'
import { SharedCredentialsProvider } from './sharedCredentialsProvider'
import { getCredentialsFilename, getConfigFilename } from '../credentials/sharedCredentialsFile'

export class SharedCredentialsProviderFactory extends BaseCredentialsProviderFactory<SharedCredentialsProvider> {
private readonly logger: Logger = getLogger()

private loadedCredentialsModificationMillis?: number
private loadedConfigModificationMillis?: number

public async refresh(): Promise<void> {
if (await this.needsRefresh()) {
await this.loadSharedCredentialsProviders()
}
await this.loadSharedCredentialsProviders()
}

public override getProviderType(): CredentialsProviderType | undefined {
return SharedCredentialsProvider.getProviderType()
}

protected override resetProviders() {
this.loadedCredentialsModificationMillis = undefined
this.loadedConfigModificationMillis = undefined

super.resetProviders()
}

private async needsRefresh(): Promise<boolean> {
const credentialsLastModMillis = await this.getLastModifiedMillis(getCredentialsFilename())
const configLastModMillis = await this.getLastModifiedMillis(getConfigFilename())

return (
this.loadedCredentialsModificationMillis !== credentialsLastModMillis ||
this.loadedConfigModificationMillis !== configLastModMillis
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't remember if this was discussed already: maybe the delete/modification in the test happened too quickly, so the mtime didn't actually change on the re-created file?

Copy link
Contributor Author

@Hweinstock Hweinstock Nov 15, 2024

Choose a reason for hiding this comment

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

Thats what I suspect as well. When I added a sleep in-between the two, the test appeared to pass.

)
}

private async loadSharedCredentialsProviders(): Promise<void> {
this.resetProviders()

Expand All @@ -52,9 +28,6 @@ export class SharedCredentialsProviderFactory extends BaseCredentialsProviderFac
const errors = result.errors.map((e) => e.message).join('\t\n')
getLogger().warn(`credentials: errors while parsing:\n%s`, errors)
}

this.loadedCredentialsModificationMillis = await this.getLastModifiedMillis(getCredentialsFilename())
this.loadedConfigModificationMillis = await this.getLastModifiedMillis(getConfigFilename())
await updateAwsSdkLoadConfigEnvVar()

getLogger().verbose(
Expand All @@ -79,13 +52,4 @@ export class SharedCredentialsProviderFactory extends BaseCredentialsProviderFac
this.addProvider(provider)
}
}

private async getLastModifiedMillis(filepath: string): Promise<number | undefined> {
try {
const stat = await fs.stat(filepath)
return stat.mtime
} catch (err) {
return undefined
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe('SharedCredentialsProviderFactory', async function () {
)
})

it('refresh does not reload from file if the file has not changed', async function () {
it('refresh always reloads from file', async function () {
const sut = new SharedCredentialsProviderFactory()

// First load
Expand All @@ -129,10 +129,7 @@ describe('SharedCredentialsProviderFactory', async function () {
// Expect: No reload
await sut.refresh()

assert.ok(
loadSharedCredentialsSectionsStub.calledOnce,
'Credentials should have only been loaded from disk once'
)
assert.ok(loadSharedCredentialsSectionsStub.calledTwice, 'Credentials should have loaded from disk twice')
})

it('refresh reloads from file if the file has changed', async function () {
Expand Down
Loading