-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
rbac: add delay_deny implementation in RBAC network filter #33875
Conversation
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
abfa3e4
to
240f07b
Compare
cc @kyessenov , @lambdai |
If specified, the RBAC network filter will delay the specified duration before actually closing the connection. This implements envoyproxy#33771. Signed-off-by: Yangmin Zhu <[email protected]>
240f07b
to
6f3aabf
Compare
Signed-off-by: Yangmin Zhu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm api
(PS: sorry for the delay, I was on vacation last days.
@yangminzhu could you merge the main and resolve the conflict? Thanks. |
assign this to @yanavlasov for final review and because @yanavlasov is the code owner (and senior maintainer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/wait
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Signed-off-by: Yangmin Zhu <[email protected]>
Signed-off-by: Yangmin Zhu <[email protected]>
/wait Seems like an RBAC test relevant to this change is timing out in CI @yangminzhu |
@KBaichoo yeah, but I think per @yanavlasov 's comment (#33875 (comment)), it is working on local workstataion but not in CI. |
Signed-off-by: Yangmin Zhu <[email protected]>
@yanavlasov oh I'm a bit surprised it worked on your workstation (the test just always timeout in my local test). Also do you think if we can get this PR in without using the simulated timer (or we can get back to this once we found the root cause in #35128 as this PR has been out for very long time)? the new partialWrite test should still provide pretty good coverage in the test. thanks. |
It worked both on workstation and CI. See #35128 Can you copy the test from my PR into yours and see if it will work? I would also remove the test with /wait-any |
Signed-off-by: Yangmin Zhu <[email protected]>
Signed-off-by: Yangmin Zhu <[email protected]>
@yanavlasov ah thanks, updated the test and it works now, also removed the |
Signed-off-by: Yangmin Zhu <[email protected]>
@yangminzhu it looks like main merge is needed to resolve the coverage error. |
/retest |
Signed-off-by: Yangmin Zhu <[email protected]>
…y#33875) If specified, the RBAC network filter will delay the specified duration before actually closing the connection. This implements envoyproxy#33771. --------- Signed-off-by: Yangmin Zhu <[email protected]> Signed-off-by: Martin Duke <[email protected]>
#35693) …" (#35655) This reverts commit 7ae5ca5. The flakiness in #35653 is not caused by this PR. If you look at [the failure](https://btx.cloud.google.com/invocations/08f19e64-5934-4ed9-9f4c-e0d14e113360/targets/%2F%2Ftest%2Fextensions%2Ffilters%2Fnetwork%2Frbac:integration_test;config=a48d3689347bfb6aaa2bb9b4c87edaab1bb2d2c4710bec9b6690537c48f15140/log) (you can also find this by going to the issue and clicking the link), the failing test is `RoleBasedAccessControlNetworkFilterIntegrationTest.DelayDenied`. That test was added in #33875 and is still flaky at head. Signed-off-by: antoniovleonti <[email protected]>
…y#33875) If specified, the RBAC network filter will delay the specified duration before actually closing the connection. This implements envoyproxy#33771. --------- Signed-off-by: Yangmin Zhu <[email protected]> Signed-off-by: asingh-g <[email protected]>
…nvoyproxy#33875)" This reverts commit bf65ad3. Signed-off-by: Ryan Northey <[email protected]>
Commit Message: If specified, the RBAC network filter will delay the specified duration before actually closing the connection.
Additional Description: This implements #33771.
Risk Level: Low
Testing: Added Unit Test and Integration Test
Manual Local Test Log:
Docs Changes: N/A
Release Notes: Updated version history
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]