Skip to content

improve test cov for auth caches#33939

Merged
fspmarshall merged 2 commits intomasterfrom
fspmarshall/improve-auth-cache-tests-subset
Nov 16, 2023
Merged

improve test cov for auth caches#33939
fspmarshall merged 2 commits intomasterfrom
fspmarshall/improve-auth-cache-tests-subset

Conversation

@fspmarshall
Copy link
Copy Markdown
Contributor

Improves some cache-related auth server tests. While looking into why a recent bug snuck past our test coverage I discovered that most of our test coverage in lib/auth doesn't have the auth cache enabled. In fact, almost all tests in lib/auth fail if the auth cache is enabled (not because of undiscovered bugs, tests just need to be written differently if there is a slight delay between resources being written and becoming readable).

This PR starts the work of making caching enabled for lib/auth tests, but only actually enables them for the unified resource cache tests. A decent amount of additional work is needed to ensure that the rest of the existing tests are updated and, importantly, that doing so does not create too onerous a burden on test authors going forward.

The flushCache helper in this PR is an experiment. I'm working on a more 'correct' way to implement a cache flush operation so that the rest of tests can be updated without needing to be reworked and/or needing to wrap everything in require.Eventually calls. For the time being, this PR will close the aforementioned hole in our coverage by ensuring that incompatibilities between the primary auth cache and the unified resource cache are caught by our test coverage.

@github-actions
Copy link
Copy Markdown
Contributor

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@fspmarshall fspmarshall added the no-changelog Indicates that a PR does not require a changelog entry label Oct 26, 2023
Comment thread lib/auth/auth_with_roles_test.go Outdated
Comment thread lib/auth/accesspoint/accesspoint.go Outdated
Comment thread lib/auth/accesspoint/accesspoint.go Outdated
Comment thread lib/auth/helpers.go Outdated
Comment on lines 1267 to 1298
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be in a _test file?

Copy link
Copy Markdown
Contributor Author

@fspmarshall fspmarshall Nov 16, 2023

Choose a reason for hiding this comment

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

I could move it now, but it will need to be moved back once we start having the cache enabled across more auth tests as some of the other elements of helpers.go can't function in the context of the cache without this helper. I think the "real" answer is that helpers.go probably needs to be a separate package only imported by _test.go files, but that seems out of scope for this PR.

@fspmarshall fspmarshall force-pushed the fspmarshall/improve-auth-cache-tests-subset branch from 1f4f0f1 to b5b28cb Compare November 16, 2023 01:45
@fspmarshall fspmarshall added this pull request to the merge queue Nov 16, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 16, 2023
@fspmarshall fspmarshall added this pull request to the merge queue Nov 16, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 16, 2023
@fspmarshall fspmarshall added this pull request to the merge queue Nov 16, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 16, 2023
@fspmarshall fspmarshall added this pull request to the merge queue Nov 16, 2023
Merged via the queue into master with commit 047b6cc Nov 16, 2023
@fspmarshall fspmarshall deleted the fspmarshall/improve-auth-cache-tests-subset branch November 16, 2023 19:01
@public-teleport-github-review-bot
Copy link
Copy Markdown

@fspmarshall See the table below for backport results.

Branch Result
branch/v14 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants