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

Add test for inbound policy index deletion #12143

Merged
merged 3 commits into from Mar 7, 2024
Merged

Conversation

adleong
Copy link
Member

@adleong adleong commented Feb 23, 2024

PR #12088 fixed an issue where removing and then re-adding certain policy resources could leave the policy index in an incorrect state. We add a test for the specific condition that triggered this behavior to prevent against future regressions.

Verified that this test fails prior to #12088 but passes on main.

Signed-off-by: Alex Leong <alex@buoyant.io>
Signed-off-by: Alex Leong <alex@buoyant.io>
@adleong adleong requested a review from a team as a code owner February 23, 2024 23:48
Copy link
Member

@mateiidavid mateiidavid left a comment

Choose a reason for hiding this comment

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

Looks great 🙌🏻

);
test.index.write().apply(server.clone());

let authz = ClientAuthorization {
Copy link
Member

Choose a reason for hiding this comment

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

this is really nitpicky but it was immediately clear to me that this is only used at the end as the expected configuration (i.e. expected InboundServer authz policy). Maybe moving it closer to the assert or naming it more explicitly might help.

Ultimately once you go through this test top-to-bottom once or twice it starts to add up anyway so I don't think it's a major problem and will just leave it up to you.

Signed-off-by: Alex Leong <alex@buoyant.io>
@adleong adleong merged commit 83be0dd into main Mar 7, 2024
35 checks passed
@adleong adleong deleted the alex/index-delete-testing branch March 7, 2024 17:03
GrigoriyMikhalkin pushed a commit to GrigoriyMikhalkin/linkerd2 that referenced this pull request Mar 10, 2024
PR linkerd#12088 fixed an issue where removing and then re-adding certain policy resources could leave the policy index in an incorrect state. We add a test for the specific condition that triggered this behavior to prevent against future regressions.

Verified that this test fails prior to linkerd#12088 but passes on main.

Signed-off-by: Alex Leong <alex@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants