Skip to content

Add test for inbound policy index deletion#12143

Merged
adleong merged 3 commits intomainfrom
alex/index-delete-testing
Mar 7, 2024
Merged

Add test for inbound policy index deletion#12143
adleong merged 3 commits intomainfrom
alex/index-delete-testing

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 <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
@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

@mateiidavid mateiidavid Mar 1, 2024

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 <[email protected]>
@adleong adleong merged commit 83be0dd into main Mar 7, 2024
@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 <[email protected]>
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.

3 participants