Extract pkg/webhook package from rpc/auth package
#1133
Conversation
WalkthroughThis pull request consolidates the separate TTL settings for authorized and unauthorized webhook responses into a unified TTL variable across the Yorkie codebase. Changes include updates to configuration files, backend structures, and tests. Additionally, a new generic Changes
Sequence Diagram(s)sequenceDiagram
participant RPC as Yorkie RPC Auth
participant WClient as Webhook Client
participant WServer as Webhook Server
RPC->>WClient: Send(request)
WClient->>WClient: Check cache for previous response
alt Cache Hit
WClient-->>RPC: Return cached response
else No Cache
WClient->>WServer: HTTP POST request
WServer-->>WClient: HTTP Response
WClient->>WClient: Process response with exponential backoff (if needed)
WClient-->>RPC: Return response & status code
end
alt Status OK (200)
RPC->>RPC: Grant access
else if Forbidden (403) or Unauthorized (401)
RPC->>RPC: Return error based on response
end
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
server/rpc/auth/webhook.go (2)
57-64: Error handling upon Send failure.
Wrapping the original error usingfmt.Errorf("send to webhook: %w", err)preserves the error context. Consider adding structured logs or metrics to help diagnose repeated webhook failures.
82-82: Default fallback mapping.
ReturningErrUnexpectedResponsewhen the status is neither OK, Forbidden, nor Unauthorized helps handle unforeseen edge cases. Consider logging or capturing the response body for troubleshooting.pkg/webhook/client.go (3)
62-73: Constructor for the webhook client.
Initialization ofClientis straightforward. As a minor improvement, you might document any assumptions about thread-safety or usage patterns (e.g., whether a single client can handle concurrent requests).
75-124: Send: JSON marshaling, caching, and HTTP POST request.
- Storing successful responses in the cache (except for unauthorized) efficiently prevents repeated calls.
- Consider adding optional request/response logging for visibility.
- Verify that repeated large requests do not flood the cache.
163-176: ShouldRetry decision.
The function properly checks connection reset and common server error statuses. Consider including configurable status codes or a callback for advanced scenarios (e.g., partial content).cmd/yorkie/server.go (1)
317-321: Enhance the flag description for clarity.The flag description could be more descriptive to help users understand the purpose and implications of this setting.
- "TTL value to set when caching authorization webhook response.", + "Time-to-live duration for caching authorization webhook responses. A longer TTL improves performance but may delay the effect of webhook configuration changes.",test/integration/auth_webhook_test.go (1)
315-315: Add a skip reason.The test case is skipped without any explanation. Please add a comment explaining why this test is being skipped and when it will be re-enabled.
-t.Skip() +t.Skip("TODO: Re-enable after fixing <issue/PR reference>")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
cmd/yorkie/server.go(3 hunks)pkg/types/pair.go(1 hunks)pkg/webhook/client.go(1 hunks)server/backend/backend.go(4 hunks)server/backend/config.go(3 hunks)server/backend/config_test.go(2 hunks)server/config.go(2 hunks)server/config.sample.yml(2 hunks)server/config_test.go(1 hunks)server/rpc/auth/webhook.go(2 hunks)server/rpc/connecthelper/status.go(3 hunks)test/helper/helper.go(2 hunks)test/integration/auth_webhook_test.go(10 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: bench
- GitHub Check: build
🔇 Additional comments (28)
server/rpc/auth/webhook.go (5)
27-27: New import reference.
The added import forwebhookis aligned with the new client-based approach.
47-55: Constructing the webhook client using configurable options.
The approach is cohesive and utilizes typed request/response, TTL caching, and retry parameters effectively. Ensure thatParseAuthWebhookCacheTTLandParseAuthWebhookMaxWaitIntervalyield valid durations to avoid runtime errors.
66-68: Success path for authorized requests.
The condition properly checks if the response indicates permission. This is straightforward and consistent with the 200 OK status.
69-74: Permission denied scenario.
Returning ametaerrors.Newwith details in areasonmap is a good approach to produce a meaningful error. There are no further concerns here.
75-80: Unauthorized scenario.
Similar to the forbidden case above, returning a specialized error helps differentiate between denial and authentication issues.pkg/webhook/client.go (4)
36-45: Meaningful error constants for webhook handling.
Defining granular errors for timeout, unexpected status code, and unexpected response improves debugging. Confirm that the rest of the system interprets each error correctly.
47-60: Options and Client struct definition.
The design’s use of generics and an LRU-expiring cache fosters reusability. Ensure concurrency safety if multiple goroutines share the same client instance and cache.
126-151: Exponential backoff logic.
- Handling status codes and transient network errors in a single function is a neat approach.
- Conduct boundary checks to ensure large retry intervals do not stall the system.
153-161: Exponential backoff interval calculation.
The function implements doubling intervals capped bymaxWaitInterval. This is consistent with typical exponential backoff strategies.pkg/types/pair.go (1)
1-24: Generic pair implementation.
This simple pair type is well-defined, but be mindful of code readability when used extensively. Consider naming or destructuring pairs quickly to maintain clarity.server/backend/config_test.go (3)
32-32: LGTM! Configuration field updated correctly.The
AuthWebhookCacheTTLfield is properly added to thevalidConfstruct with a valid duration value.
46-47: LGTM! Validation test case updated appropriately.The test case correctly validates the error condition for an invalid TTL format.
50-51: Verify the ProjectInfoCacheTTL validation.The test expects an error for "10 minutes" format, but this might be inconsistent with the actual validation logic.
✅ Verification successful
ProjectInfoCacheTTL validation works as expected.
The validation function defined inserver/backend/config.gouses Go’stime.ParseDurationto parse the duration strings. Since"10 minutes"does not match the expected format (for example,"10m"),time.ParseDurationcorrectly returns an error, which is why the test inserver/backend/config_test.goasserts an error.
- Validation logic in
server/backend/config.gocallstime.ParseDuration(c.ProjectInfoCacheTTL)and returns an error on failure.- The test case with
"10 minutes"correctly expects an error due to the format not matchingtime.ParseDurationspecifications.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the validation logic for duration parsing in the codebase # Search for duration parsing patterns rg -A 2 'ParseDuration|time\.'Length of output: 74469
server/config_test.go (1)
73-75: LGTM! Config test updated consistently.The test correctly validates the new unified webhook cache TTL configuration.
server/backend/config.go (2)
70-71: LGTM! Field documentation is clear and accurate.The new
AuthWebhookCacheTTLfield is well-documented with a clear description.
145-154: LGTM! Parse method updated correctly.The
ParseAuthWebhookCacheTTLmethod is properly implemented with appropriate error handling.server/backend/backend.go (2)
49-52: LGTM! Enhanced type safety with Pair type.The
WebhookCachefield now uses a strongly-typedPairstructure, improving type safety and code clarity.
87-89: LGTM! Cache initialization updated correctly.The cache initialization is properly updated to use the new type signature.
server/config.go (2)
66-66: LGTM! Good consolidation of webhook cache TTL settings.The consolidation of separate TTL settings into a single
DefaultAuthWebhookCacheTTLconstant simplifies the configuration while maintaining the same caching duration. This change reduces complexity and makes the codebase more maintainable.
188-190: LGTM! Consistent initialization of the new TTL setting.The initialization of
AuthWebhookCacheTTLfollows the established pattern for duration settings in the configuration.server/rpc/connecthelper/status.go (1)
87-89: LGTM! Clean error mapping refactor.The error mappings have been correctly updated to use the new
webhookpackage errors while maintaining the same error codes, ensuring backward compatibility.test/helper/helper.go (1)
79-79: LGTM! Test configuration updated correctly.The test configuration has been properly updated to use the consolidated webhook cache TTL setting while maintaining the same duration value.
Also applies to: 268-268
test/integration/auth_webhook_test.go (3)
38-38: LGTM!The addition of the webhook package import aligns with the PR objective of extracting webhook functionality into a separate package.
311-311: LGTM!The migration of error codes from
authtowebhookpackage is consistent with the package extraction objective and maintains the same error handling behavior.Also applies to: 353-353, 383-383
439-441: LGTM!The consolidation of TTL variables into a single
authTTLsimplifies the configuration and is consistent with the changes inconfig.sample.yml.Also applies to: 517-519, 581-583
server/config.sample.yml (3)
49-50: LGTM!The addition of
ClientDeactivateThresholdwith a default value of "24h" provides a clear configuration option for the housekeeping process.
66-66: LGTM!The format change for
AuthWebhookMethodsis a minor style improvement.
74-75: LGTM!The consolidation of separate auth/unauth TTL settings into a single
AuthWebhookCacheTTLsimplifies the configuration while maintaining the same caching behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1133 +/- ##
=======================================
Coverage 46.82% 46.82%
=======================================
Files 84 84
Lines 12282 12266 -16
=======================================
- Hits 5751 5744 -7
+ Misses 5954 5946 -8
+ Partials 577 576 -1 ☔ View full report in Codecov by Sentry. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Go Benchmark
| Benchmark suite | Current: 45bf349 | Previous: e0f8765 | Ratio |
|---|---|---|---|
BenchmarkDocument/constructor_test |
1487 ns/op 1337 B/op 24 allocs/op |
1507 ns/op 1337 B/op 24 allocs/op |
0.99 |
BenchmarkDocument/constructor_test - ns/op |
1487 ns/op |
1507 ns/op |
0.99 |
BenchmarkDocument/constructor_test - B/op |
1337 B/op |
1337 B/op |
1 |
BenchmarkDocument/constructor_test - allocs/op |
24 allocs/op |
24 allocs/op |
1 |
BenchmarkDocument/status_test |
965.5 ns/op 1305 B/op 22 allocs/op |
977.7 ns/op 1305 B/op 22 allocs/op |
0.99 |
BenchmarkDocument/status_test - ns/op |
965.5 ns/op |
977.7 ns/op |
0.99 |
BenchmarkDocument/status_test - B/op |
1305 B/op |
1305 B/op |
1 |
BenchmarkDocument/status_test - allocs/op |
22 allocs/op |
22 allocs/op |
1 |
BenchmarkDocument/equals_test |
8058 ns/op 7529 B/op 134 allocs/op |
7861 ns/op 7529 B/op 134 allocs/op |
1.03 |
BenchmarkDocument/equals_test - ns/op |
8058 ns/op |
7861 ns/op |
1.03 |
BenchmarkDocument/equals_test - B/op |
7529 B/op |
7529 B/op |
1 |
BenchmarkDocument/equals_test - allocs/op |
134 allocs/op |
134 allocs/op |
1 |
BenchmarkDocument/nested_update_test |
17229 ns/op 12395 B/op 264 allocs/op |
17307 ns/op 12395 B/op 264 allocs/op |
1.00 |
BenchmarkDocument/nested_update_test - ns/op |
17229 ns/op |
17307 ns/op |
1.00 |
BenchmarkDocument/nested_update_test - B/op |
12395 B/op |
12395 B/op |
1 |
BenchmarkDocument/nested_update_test - allocs/op |
264 allocs/op |
264 allocs/op |
1 |
BenchmarkDocument/delete_test |
23552 ns/op 15923 B/op 347 allocs/op |
27655 ns/op 15924 B/op 347 allocs/op |
0.85 |
BenchmarkDocument/delete_test - ns/op |
23552 ns/op |
27655 ns/op |
0.85 |
BenchmarkDocument/delete_test - B/op |
15923 B/op |
15924 B/op |
1.00 |
BenchmarkDocument/delete_test - allocs/op |
347 allocs/op |
347 allocs/op |
1 |
BenchmarkDocument/object_test |
9154 ns/op 7073 B/op 122 allocs/op |
8911 ns/op 7073 B/op 122 allocs/op |
1.03 |
BenchmarkDocument/object_test - ns/op |
9154 ns/op |
8911 ns/op |
1.03 |
BenchmarkDocument/object_test - B/op |
7073 B/op |
7073 B/op |
1 |
BenchmarkDocument/object_test - allocs/op |
122 allocs/op |
122 allocs/op |
1 |
BenchmarkDocument/array_test |
31180 ns/op 12203 B/op 278 allocs/op |
30710 ns/op 12203 B/op 278 allocs/op |
1.02 |
BenchmarkDocument/array_test - ns/op |
31180 ns/op |
30710 ns/op |
1.02 |
BenchmarkDocument/array_test - B/op |
12203 B/op |
12203 B/op |
1 |
BenchmarkDocument/array_test - allocs/op |
278 allocs/op |
278 allocs/op |
1 |
BenchmarkDocument/text_test |
35403 ns/op 15324 B/op 492 allocs/op |
32391 ns/op 15323 B/op 492 allocs/op |
1.09 |
BenchmarkDocument/text_test - ns/op |
35403 ns/op |
32391 ns/op |
1.09 |
BenchmarkDocument/text_test - B/op |
15324 B/op |
15323 B/op |
1.00 |
BenchmarkDocument/text_test - allocs/op |
492 allocs/op |
492 allocs/op |
1 |
BenchmarkDocument/text_composition_test |
31224 ns/op 18718 B/op 504 allocs/op |
30868 ns/op 18716 B/op 504 allocs/op |
1.01 |
BenchmarkDocument/text_composition_test - ns/op |
31224 ns/op |
30868 ns/op |
1.01 |
BenchmarkDocument/text_composition_test - B/op |
18718 B/op |
18716 B/op |
1.00 |
BenchmarkDocument/text_composition_test - allocs/op |
504 allocs/op |
504 allocs/op |
1 |
BenchmarkDocument/rich_text_test |
86665 ns/op 40181 B/op 1183 allocs/op |
85360 ns/op 40180 B/op 1183 allocs/op |
1.02 |
BenchmarkDocument/rich_text_test - ns/op |
86665 ns/op |
85360 ns/op |
1.02 |
BenchmarkDocument/rich_text_test - B/op |
40181 B/op |
40180 B/op |
1.00 |
BenchmarkDocument/rich_text_test - allocs/op |
1183 allocs/op |
1183 allocs/op |
1 |
BenchmarkDocument/counter_test |
19316 ns/op 11874 B/op 258 allocs/op |
18938 ns/op 11874 B/op 258 allocs/op |
1.02 |
BenchmarkDocument/counter_test - ns/op |
19316 ns/op |
18938 ns/op |
1.02 |
BenchmarkDocument/counter_test - B/op |
11874 B/op |
11874 B/op |
1 |
BenchmarkDocument/counter_test - allocs/op |
258 allocs/op |
258 allocs/op |
1 |
BenchmarkDocument/text_edit_gc_100 |
1372431 ns/op 872537 B/op 17281 allocs/op |
1352178 ns/op 872642 B/op 17282 allocs/op |
1.01 |
BenchmarkDocument/text_edit_gc_100 - ns/op |
1372431 ns/op |
1352178 ns/op |
1.01 |
BenchmarkDocument/text_edit_gc_100 - B/op |
872537 B/op |
872642 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_100 - allocs/op |
17281 allocs/op |
17282 allocs/op |
1.00 |
BenchmarkDocument/text_edit_gc_1000 |
55328857 ns/op 50547054 B/op 186740 allocs/op |
52589001 ns/op 50547922 B/op 186748 allocs/op |
1.05 |
BenchmarkDocument/text_edit_gc_1000 - ns/op |
55328857 ns/op |
52589001 ns/op |
1.05 |
BenchmarkDocument/text_edit_gc_1000 - B/op |
50547054 B/op |
50547922 B/op |
1.00 |
BenchmarkDocument/text_edit_gc_1000 - allocs/op |
186740 allocs/op |
186748 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 |
1986511 ns/op 1589064 B/op 15950 allocs/op |
1989534 ns/op 1589020 B/op 15950 allocs/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - ns/op |
1986511 ns/op |
1989534 ns/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - B/op |
1589064 B/op |
1589020 B/op |
1.00 |
BenchmarkDocument/text_split_gc_100 - allocs/op |
15950 allocs/op |
15950 allocs/op |
1 |
BenchmarkDocument/text_split_gc_1000 |
120840319 ns/op 141481938 B/op 186158 allocs/op |
122494189 ns/op 141480744 B/op 186141 allocs/op |
0.99 |
BenchmarkDocument/text_split_gc_1000 - ns/op |
120840319 ns/op |
122494189 ns/op |
0.99 |
BenchmarkDocument/text_split_gc_1000 - B/op |
141481938 B/op |
141480744 B/op |
1.00 |
BenchmarkDocument/text_split_gc_1000 - allocs/op |
186158 allocs/op |
186141 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 |
17660351 ns/op 10212656 B/op 55683 allocs/op |
17265065 ns/op 10211526 B/op 55680 allocs/op |
1.02 |
BenchmarkDocument/text_delete_all_10000 - ns/op |
17660351 ns/op |
17265065 ns/op |
1.02 |
BenchmarkDocument/text_delete_all_10000 - B/op |
10212656 B/op |
10211526 B/op |
1.00 |
BenchmarkDocument/text_delete_all_10000 - allocs/op |
55683 allocs/op |
55680 allocs/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 |
285569823 ns/op 142945324 B/op 561597 allocs/op |
302008164 ns/op 142958700 B/op 561658 allocs/op |
0.95 |
BenchmarkDocument/text_delete_all_100000 - ns/op |
285569823 ns/op |
302008164 ns/op |
0.95 |
BenchmarkDocument/text_delete_all_100000 - B/op |
142945324 B/op |
142958700 B/op |
1.00 |
BenchmarkDocument/text_delete_all_100000 - allocs/op |
561597 allocs/op |
561658 allocs/op |
1.00 |
BenchmarkDocument/text_100 |
233310 ns/op 120491 B/op 5182 allocs/op |
222262 ns/op 120491 B/op 5182 allocs/op |
1.05 |
BenchmarkDocument/text_100 - ns/op |
233310 ns/op |
222262 ns/op |
1.05 |
BenchmarkDocument/text_100 - B/op |
120491 B/op |
120491 B/op |
1 |
BenchmarkDocument/text_100 - allocs/op |
5182 allocs/op |
5182 allocs/op |
1 |
BenchmarkDocument/text_1000 |
2453159 ns/op 1171279 B/op 51086 allocs/op |
2407446 ns/op 1171277 B/op 51086 allocs/op |
1.02 |
BenchmarkDocument/text_1000 - ns/op |
2453159 ns/op |
2407446 ns/op |
1.02 |
BenchmarkDocument/text_1000 - B/op |
1171279 B/op |
1171277 B/op |
1.00 |
BenchmarkDocument/text_1000 - allocs/op |
51086 allocs/op |
51086 allocs/op |
1 |
BenchmarkDocument/array_1000 |
1266567 ns/op 1091874 B/op 11834 allocs/op |
1243496 ns/op 1091744 B/op 11834 allocs/op |
1.02 |
BenchmarkDocument/array_1000 - ns/op |
1266567 ns/op |
1243496 ns/op |
1.02 |
BenchmarkDocument/array_1000 - B/op |
1091874 B/op |
1091744 B/op |
1.00 |
BenchmarkDocument/array_1000 - allocs/op |
11834 allocs/op |
11834 allocs/op |
1 |
BenchmarkDocument/array_10000 |
13826215 ns/op 9800920 B/op 120301 allocs/op |
13636831 ns/op 9800646 B/op 120301 allocs/op |
1.01 |
BenchmarkDocument/array_10000 - ns/op |
13826215 ns/op |
13636831 ns/op |
1.01 |
BenchmarkDocument/array_10000 - B/op |
9800920 B/op |
9800646 B/op |
1.00 |
BenchmarkDocument/array_10000 - allocs/op |
120301 allocs/op |
120301 allocs/op |
1 |
BenchmarkDocument/array_gc_100 |
152933 ns/op 133293 B/op 1267 allocs/op |
152063 ns/op 133283 B/op 1266 allocs/op |
1.01 |
BenchmarkDocument/array_gc_100 - ns/op |
152933 ns/op |
152063 ns/op |
1.01 |
BenchmarkDocument/array_gc_100 - B/op |
133293 B/op |
133283 B/op |
1.00 |
BenchmarkDocument/array_gc_100 - allocs/op |
1267 allocs/op |
1266 allocs/op |
1.00 |
BenchmarkDocument/array_gc_1000 |
1443934 ns/op 1159697 B/op 12882 allocs/op |
1430364 ns/op 1159690 B/op 12882 allocs/op |
1.01 |
BenchmarkDocument/array_gc_1000 - ns/op |
1443934 ns/op |
1430364 ns/op |
1.01 |
BenchmarkDocument/array_gc_1000 - B/op |
1159697 B/op |
1159690 B/op |
1.00 |
BenchmarkDocument/array_gc_1000 - allocs/op |
12882 allocs/op |
12882 allocs/op |
1 |
BenchmarkDocument/counter_1000 |
201759 ns/op 193336 B/op 5773 allocs/op |
200666 ns/op 193336 B/op 5773 allocs/op |
1.01 |
BenchmarkDocument/counter_1000 - ns/op |
201759 ns/op |
200666 ns/op |
1.01 |
BenchmarkDocument/counter_1000 - B/op |
193336 B/op |
193336 B/op |
1 |
BenchmarkDocument/counter_1000 - allocs/op |
5773 allocs/op |
5773 allocs/op |
1 |
BenchmarkDocument/counter_10000 |
2157609 ns/op 2088267 B/op 59780 allocs/op |
2147996 ns/op 2088254 B/op 59780 allocs/op |
1.00 |
BenchmarkDocument/counter_10000 - ns/op |
2157609 ns/op |
2147996 ns/op |
1.00 |
BenchmarkDocument/counter_10000 - B/op |
2088267 B/op |
2088254 B/op |
1.00 |
BenchmarkDocument/counter_10000 - allocs/op |
59780 allocs/op |
59780 allocs/op |
1 |
BenchmarkDocument/object_1000 |
1429476 ns/op 1428500 B/op 9851 allocs/op |
1397417 ns/op 1428349 B/op 9851 allocs/op |
1.02 |
BenchmarkDocument/object_1000 - ns/op |
1429476 ns/op |
1397417 ns/op |
1.02 |
BenchmarkDocument/object_1000 - B/op |
1428500 B/op |
1428349 B/op |
1.00 |
BenchmarkDocument/object_1000 - allocs/op |
9851 allocs/op |
9851 allocs/op |
1 |
BenchmarkDocument/object_10000 |
15329613 ns/op 12166204 B/op 100565 allocs/op |
15503747 ns/op 12168122 B/op 100569 allocs/op |
0.99 |
BenchmarkDocument/object_10000 - ns/op |
15329613 ns/op |
15503747 ns/op |
0.99 |
BenchmarkDocument/object_10000 - B/op |
12166204 B/op |
12168122 B/op |
1.00 |
BenchmarkDocument/object_10000 - allocs/op |
100565 allocs/op |
100569 allocs/op |
1.00 |
BenchmarkDocument/tree_100 |
1039866 ns/op 943950 B/op 6103 allocs/op |
1051182 ns/op 943958 B/op 6103 allocs/op |
0.99 |
BenchmarkDocument/tree_100 - ns/op |
1039866 ns/op |
1051182 ns/op |
0.99 |
BenchmarkDocument/tree_100 - B/op |
943950 B/op |
943958 B/op |
1.00 |
BenchmarkDocument/tree_100 - allocs/op |
6103 allocs/op |
6103 allocs/op |
1 |
BenchmarkDocument/tree_1000 |
75708778 ns/op 86460004 B/op 60116 allocs/op |
76105606 ns/op 86460600 B/op 60117 allocs/op |
0.99 |
BenchmarkDocument/tree_1000 - ns/op |
75708778 ns/op |
76105606 ns/op |
0.99 |
BenchmarkDocument/tree_1000 - B/op |
86460004 B/op |
86460600 B/op |
1.00 |
BenchmarkDocument/tree_1000 - allocs/op |
60116 allocs/op |
60117 allocs/op |
1.00 |
BenchmarkDocument/tree_10000 |
9844591184 ns/op 8580662656 B/op 600231 allocs/op |
9634440323 ns/op 8580660400 B/op 600217 allocs/op |
1.02 |
BenchmarkDocument/tree_10000 - ns/op |
9844591184 ns/op |
9634440323 ns/op |
1.02 |
BenchmarkDocument/tree_10000 - B/op |
8580662656 B/op |
8580660400 B/op |
1.00 |
BenchmarkDocument/tree_10000 - allocs/op |
600231 allocs/op |
600217 allocs/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 |
78742797 ns/op 87532667 B/op 75269 allocs/op |
75188302 ns/op 87510089 B/op 75272 allocs/op |
1.05 |
BenchmarkDocument/tree_delete_all_1000 - ns/op |
78742797 ns/op |
75188302 ns/op |
1.05 |
BenchmarkDocument/tree_delete_all_1000 - B/op |
87532667 B/op |
87510089 B/op |
1.00 |
BenchmarkDocument/tree_delete_all_1000 - allocs/op |
75269 allocs/op |
75272 allocs/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 |
3935834 ns/op 4147291 B/op 15147 allocs/op |
3812894 ns/op 4147319 B/op 15147 allocs/op |
1.03 |
BenchmarkDocument/tree_edit_gc_100 - ns/op |
3935834 ns/op |
3812894 ns/op |
1.03 |
BenchmarkDocument/tree_edit_gc_100 - B/op |
4147291 B/op |
4147319 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_100 - allocs/op |
15147 allocs/op |
15147 allocs/op |
1 |
BenchmarkDocument/tree_edit_gc_1000 |
320451418 ns/op 383745252 B/op 154861 allocs/op |
303552968 ns/op 383748930 B/op 154868 allocs/op |
1.06 |
BenchmarkDocument/tree_edit_gc_1000 - ns/op |
320451418 ns/op |
303552968 ns/op |
1.06 |
BenchmarkDocument/tree_edit_gc_1000 - B/op |
383745252 B/op |
383748930 B/op |
1.00 |
BenchmarkDocument/tree_edit_gc_1000 - allocs/op |
154861 allocs/op |
154868 allocs/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 |
2669040 ns/op 2413071 B/op 11131 allocs/op |
2566393 ns/op 2413733 B/op 11131 allocs/op |
1.04 |
BenchmarkDocument/tree_split_gc_100 - ns/op |
2669040 ns/op |
2566393 ns/op |
1.04 |
BenchmarkDocument/tree_split_gc_100 - B/op |
2413071 B/op |
2413733 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_100 - allocs/op |
11131 allocs/op |
11131 allocs/op |
1 |
BenchmarkDocument/tree_split_gc_1000 |
196095230 ns/op 222250597 B/op 122002 allocs/op |
185437949 ns/op 222251246 B/op 121999 allocs/op |
1.06 |
BenchmarkDocument/tree_split_gc_1000 - ns/op |
196095230 ns/op |
185437949 ns/op |
1.06 |
BenchmarkDocument/tree_split_gc_1000 - B/op |
222250597 B/op |
222251246 B/op |
1.00 |
BenchmarkDocument/tree_split_gc_1000 - allocs/op |
122002 allocs/op |
121999 allocs/op |
1.00 |
BenchmarkRPC/client_to_server |
430502956 ns/op 19249200 B/op 216114 allocs/op |
428782974 ns/op 19266434 B/op 216407 allocs/op |
1.00 |
BenchmarkRPC/client_to_server - ns/op |
430502956 ns/op |
428782974 ns/op |
1.00 |
BenchmarkRPC/client_to_server - B/op |
19249200 B/op |
19266434 B/op |
1.00 |
BenchmarkRPC/client_to_server - allocs/op |
216114 allocs/op |
216407 allocs/op |
1.00 |
BenchmarkRPC/client_to_client_via_server |
808791506 ns/op 48593832 B/op 462145 allocs/op |
798509302 ns/op 41306976 B/op 459014 allocs/op |
1.01 |
BenchmarkRPC/client_to_client_via_server - ns/op |
808791506 ns/op |
798509302 ns/op |
1.01 |
BenchmarkRPC/client_to_client_via_server - B/op |
48593832 B/op |
41306976 B/op |
1.18 |
BenchmarkRPC/client_to_client_via_server - allocs/op |
462145 allocs/op |
459014 allocs/op |
1.01 |
BenchmarkRPC/attach_large_document |
1107949784 ns/op 1910463064 B/op 11976 allocs/op |
1153817341 ns/op 1898146432 B/op 11958 allocs/op |
0.96 |
BenchmarkRPC/attach_large_document - ns/op |
1107949784 ns/op |
1153817341 ns/op |
0.96 |
BenchmarkRPC/attach_large_document - B/op |
1910463064 B/op |
1898146432 B/op |
1.01 |
BenchmarkRPC/attach_large_document - allocs/op |
11976 allocs/op |
11958 allocs/op |
1.00 |
BenchmarkRPC/adminCli_to_server |
551386992 ns/op 35991448 B/op 289676 allocs/op |
543665266 ns/op 37225008 B/op 289740 allocs/op |
1.01 |
BenchmarkRPC/adminCli_to_server - ns/op |
551386992 ns/op |
543665266 ns/op |
1.01 |
BenchmarkRPC/adminCli_to_server - B/op |
35991448 B/op |
37225008 B/op |
0.97 |
BenchmarkRPC/adminCli_to_server - allocs/op |
289676 allocs/op |
289740 allocs/op |
1.00 |
BenchmarkLocker |
67.36 ns/op 16 B/op 1 allocs/op |
65.61 ns/op 16 B/op 1 allocs/op |
1.03 |
BenchmarkLocker - ns/op |
67.36 ns/op |
65.61 ns/op |
1.03 |
BenchmarkLocker - B/op |
16 B/op |
16 B/op |
1 |
BenchmarkLocker - allocs/op |
1 allocs/op |
1 allocs/op |
1 |
BenchmarkLockerParallel |
39.42 ns/op 0 B/op 0 allocs/op |
39.51 ns/op 0 B/op 0 allocs/op |
1.00 |
BenchmarkLockerParallel - ns/op |
39.42 ns/op |
39.51 ns/op |
1.00 |
BenchmarkLockerParallel - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkLockerParallel - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkLockerMoreKeys |
149.9 ns/op 15 B/op 0 allocs/op |
152.3 ns/op 15 B/op 0 allocs/op |
0.98 |
BenchmarkLockerMoreKeys - ns/op |
149.9 ns/op |
152.3 ns/op |
0.98 |
BenchmarkLockerMoreKeys - B/op |
15 B/op |
15 B/op |
1 |
BenchmarkLockerMoreKeys - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkChange/Push_10_Changes |
4588272 ns/op 149477 B/op 1575 allocs/op |
4563634 ns/op 149202 B/op 1575 allocs/op |
1.01 |
BenchmarkChange/Push_10_Changes - ns/op |
4588272 ns/op |
4563634 ns/op |
1.01 |
BenchmarkChange/Push_10_Changes - B/op |
149477 B/op |
149202 B/op |
1.00 |
BenchmarkChange/Push_10_Changes - allocs/op |
1575 allocs/op |
1575 allocs/op |
1 |
BenchmarkChange/Push_100_Changes |
16369856 ns/op 788804 B/op 8284 allocs/op |
16669048 ns/op 787345 B/op 8283 allocs/op |
0.98 |
BenchmarkChange/Push_100_Changes - ns/op |
16369856 ns/op |
16669048 ns/op |
0.98 |
BenchmarkChange/Push_100_Changes - B/op |
788804 B/op |
787345 B/op |
1.00 |
BenchmarkChange/Push_100_Changes - allocs/op |
8284 allocs/op |
8283 allocs/op |
1.00 |
BenchmarkChange/Push_1000_Changes |
130337721 ns/op 7124844 B/op 77294 allocs/op |
133525309 ns/op 7129956 B/op 77295 allocs/op |
0.98 |
BenchmarkChange/Push_1000_Changes - ns/op |
130337721 ns/op |
133525309 ns/op |
0.98 |
BenchmarkChange/Push_1000_Changes - B/op |
7124844 B/op |
7129956 B/op |
1.00 |
BenchmarkChange/Push_1000_Changes - allocs/op |
77294 allocs/op |
77295 allocs/op |
1.00 |
BenchmarkChange/Pull_10_Changes |
3772188 ns/op 123076 B/op 1406 allocs/op |
3748572 ns/op 122880 B/op 1406 allocs/op |
1.01 |
BenchmarkChange/Pull_10_Changes - ns/op |
3772188 ns/op |
3748572 ns/op |
1.01 |
BenchmarkChange/Pull_10_Changes - B/op |
123076 B/op |
122880 B/op |
1.00 |
BenchmarkChange/Pull_10_Changes - allocs/op |
1406 allocs/op |
1406 allocs/op |
1 |
BenchmarkChange/Pull_100_Changes |
5365228 ns/op 350246 B/op 5040 allocs/op |
5351836 ns/op 350026 B/op 5041 allocs/op |
1.00 |
BenchmarkChange/Pull_100_Changes - ns/op |
5365228 ns/op |
5351836 ns/op |
1.00 |
BenchmarkChange/Pull_100_Changes - B/op |
350246 B/op |
350026 B/op |
1.00 |
BenchmarkChange/Pull_100_Changes - allocs/op |
5040 allocs/op |
5041 allocs/op |
1.00 |
BenchmarkChange/Pull_1000_Changes |
11164953 ns/op 2226944 B/op 43654 allocs/op |
10970463 ns/op 2227084 B/op 43656 allocs/op |
1.02 |
BenchmarkChange/Pull_1000_Changes - ns/op |
11164953 ns/op |
10970463 ns/op |
1.02 |
BenchmarkChange/Pull_1000_Changes - B/op |
2226944 B/op |
2227084 B/op |
1.00 |
BenchmarkChange/Pull_1000_Changes - allocs/op |
43654 allocs/op |
43656 allocs/op |
1.00 |
BenchmarkSnapshot/Push_3KB_snapshot |
19035315 ns/op 921385 B/op 8289 allocs/op |
18901314 ns/op 911322 B/op 8286 allocs/op |
1.01 |
BenchmarkSnapshot/Push_3KB_snapshot - ns/op |
19035315 ns/op |
18901314 ns/op |
1.01 |
BenchmarkSnapshot/Push_3KB_snapshot - B/op |
921385 B/op |
911322 B/op |
1.01 |
BenchmarkSnapshot/Push_3KB_snapshot - allocs/op |
8289 allocs/op |
8286 allocs/op |
1.00 |
BenchmarkSnapshot/Push_30KB_snapshot |
132983081 ns/op 8232644 B/op 87600 allocs/op |
134958572 ns/op 8364078 B/op 88807 allocs/op |
0.99 |
BenchmarkSnapshot/Push_30KB_snapshot - ns/op |
132983081 ns/op |
134958572 ns/op |
0.99 |
BenchmarkSnapshot/Push_30KB_snapshot - B/op |
8232644 B/op |
8364078 B/op |
0.98 |
BenchmarkSnapshot/Push_30KB_snapshot - allocs/op |
87600 allocs/op |
88807 allocs/op |
0.99 |
BenchmarkSnapshot/Pull_3KB_snapshot |
7699398 ns/op 1138892 B/op 19605 allocs/op |
7653775 ns/op 1139143 B/op 19606 allocs/op |
1.01 |
BenchmarkSnapshot/Pull_3KB_snapshot - ns/op |
7699398 ns/op |
7653775 ns/op |
1.01 |
BenchmarkSnapshot/Pull_3KB_snapshot - B/op |
1138892 B/op |
1139143 B/op |
1.00 |
BenchmarkSnapshot/Pull_3KB_snapshot - allocs/op |
19605 allocs/op |
19606 allocs/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot |
20553499 ns/op 9340622 B/op 189554 allocs/op |
19637079 ns/op 9338452 B/op 189554 allocs/op |
1.05 |
BenchmarkSnapshot/Pull_30KB_snapshot - ns/op |
20553499 ns/op |
19637079 ns/op |
1.05 |
BenchmarkSnapshot/Pull_30KB_snapshot - B/op |
9340622 B/op |
9338452 B/op |
1.00 |
BenchmarkSnapshot/Pull_30KB_snapshot - allocs/op |
189554 allocs/op |
189554 allocs/op |
1 |
BenchmarkSplayTree/stress_test_100000 |
0.202 ns/op 0 B/op 0 allocs/op |
0.1945 ns/op 0 B/op 0 allocs/op |
1.04 |
BenchmarkSplayTree/stress_test_100000 - ns/op |
0.202 ns/op |
0.1945 ns/op |
1.04 |
BenchmarkSplayTree/stress_test_100000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_100000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/stress_test_200000 |
0.3979 ns/op 0 B/op 0 allocs/op |
0.3895 ns/op 0 B/op 0 allocs/op |
1.02 |
BenchmarkSplayTree/stress_test_200000 - ns/op |
0.3979 ns/op |
0.3895 ns/op |
1.02 |
BenchmarkSplayTree/stress_test_200000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_200000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/stress_test_300000 |
0.5912 ns/op 0 B/op 0 allocs/op |
0.5915 ns/op 0 B/op 0 allocs/op |
1.00 |
BenchmarkSplayTree/stress_test_300000 - ns/op |
0.5912 ns/op |
0.5915 ns/op |
1.00 |
BenchmarkSplayTree/stress_test_300000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/stress_test_300000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_100000 |
0.01414 ns/op 0 B/op 0 allocs/op |
0.01262 ns/op 0 B/op 0 allocs/op |
1.12 |
BenchmarkSplayTree/random_access_100000 - ns/op |
0.01414 ns/op |
0.01262 ns/op |
1.12 |
BenchmarkSplayTree/random_access_100000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_100000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_200000 |
0.03139 ns/op 0 B/op 0 allocs/op |
0.03325 ns/op 0 B/op 0 allocs/op |
0.94 |
BenchmarkSplayTree/random_access_200000 - ns/op |
0.03139 ns/op |
0.03325 ns/op |
0.94 |
BenchmarkSplayTree/random_access_200000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_200000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/random_access_300000 |
0.04856 ns/op 0 B/op 0 allocs/op |
0.0432 ns/op 0 B/op 0 allocs/op |
1.12 |
BenchmarkSplayTree/random_access_300000 - ns/op |
0.04856 ns/op |
0.0432 ns/op |
1.12 |
BenchmarkSplayTree/random_access_300000 - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/random_access_300000 - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSplayTree/editing_trace_bench |
0.002302 ns/op 0 B/op 0 allocs/op |
0.002056 ns/op 0 B/op 0 allocs/op |
1.12 |
BenchmarkSplayTree/editing_trace_bench - ns/op |
0.002302 ns/op |
0.002056 ns/op |
1.12 |
BenchmarkSplayTree/editing_trace_bench - B/op |
0 B/op |
0 B/op |
1 |
BenchmarkSplayTree/editing_trace_bench - allocs/op |
0 allocs/op |
0 allocs/op |
1 |
BenchmarkSync/memory_sync_10_test |
8872 ns/op 3765 B/op 69 allocs/op |
8470 ns/op 3765 B/op 69 allocs/op |
1.05 |
BenchmarkSync/memory_sync_10_test - ns/op |
8872 ns/op |
8470 ns/op |
1.05 |
BenchmarkSync/memory_sync_10_test - B/op |
3765 B/op |
3765 B/op |
1 |
BenchmarkSync/memory_sync_10_test - allocs/op |
69 allocs/op |
69 allocs/op |
1 |
BenchmarkSync/memory_sync_100_test |
56385 ns/op 11108 B/op 303 allocs/op |
54952 ns/op 11112 B/op 303 allocs/op |
1.03 |
BenchmarkSync/memory_sync_100_test - ns/op |
56385 ns/op |
54952 ns/op |
1.03 |
BenchmarkSync/memory_sync_100_test - B/op |
11108 B/op |
11112 B/op |
1.00 |
BenchmarkSync/memory_sync_100_test - allocs/op |
303 allocs/op |
303 allocs/op |
1 |
BenchmarkSync/memory_sync_1000_test |
603328 ns/op 76861 B/op 2144 allocs/op |
575993 ns/op 77962 B/op 2211 allocs/op |
1.05 |
BenchmarkSync/memory_sync_1000_test - ns/op |
603328 ns/op |
575993 ns/op |
1.05 |
BenchmarkSync/memory_sync_1000_test - B/op |
76861 B/op |
77962 B/op |
0.99 |
BenchmarkSync/memory_sync_1000_test - allocs/op |
2144 allocs/op |
2211 allocs/op |
0.97 |
BenchmarkSync/memory_sync_10000_test |
7775584 ns/op 761929 B/op 20499 allocs/op |
7333061 ns/op 760930 B/op 20561 allocs/op |
1.06 |
BenchmarkSync/memory_sync_10000_test - ns/op |
7775584 ns/op |
7333061 ns/op |
1.06 |
BenchmarkSync/memory_sync_10000_test - B/op |
761929 B/op |
760930 B/op |
1.00 |
BenchmarkSync/memory_sync_10000_test - allocs/op |
20499 allocs/op |
20561 allocs/op |
1.00 |
BenchmarkTextEditing |
5273107002 ns/op 3982602864 B/op 20647705 allocs/op |
5170487545 ns/op 3982619104 B/op 20647716 allocs/op |
1.02 |
BenchmarkTextEditing - ns/op |
5273107002 ns/op |
5170487545 ns/op |
1.02 |
BenchmarkTextEditing - B/op |
3982602864 B/op |
3982619104 B/op |
1.00 |
BenchmarkTextEditing - allocs/op |
20647705 allocs/op |
20647716 allocs/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf |
4218315 ns/op 6265164 B/op 70025 allocs/op |
4273767 ns/op 6263015 B/op 70025 allocs/op |
0.99 |
BenchmarkTree/10000_vertices_to_protobuf - ns/op |
4218315 ns/op |
4273767 ns/op |
0.99 |
BenchmarkTree/10000_vertices_to_protobuf - B/op |
6265164 B/op |
6263015 B/op |
1.00 |
BenchmarkTree/10000_vertices_to_protobuf - allocs/op |
70025 allocs/op |
70025 allocs/op |
1 |
BenchmarkTree/10000_vertices_from_protobuf |
224848748 ns/op 442173491 B/op 290039 allocs/op |
226310565 ns/op 442171937 B/op 290039 allocs/op |
0.99 |
BenchmarkTree/10000_vertices_from_protobuf - ns/op |
224848748 ns/op |
226310565 ns/op |
0.99 |
BenchmarkTree/10000_vertices_from_protobuf - B/op |
442173491 B/op |
442171937 B/op |
1.00 |
BenchmarkTree/10000_vertices_from_protobuf - allocs/op |
290039 allocs/op |
290039 allocs/op |
1 |
BenchmarkTree/20000_vertices_to_protobuf |
9307999 ns/op 12716937 B/op 140028 allocs/op |
9361589 ns/op 12717017 B/op 140028 allocs/op |
0.99 |
BenchmarkTree/20000_vertices_to_protobuf - ns/op |
9307999 ns/op |
9361589 ns/op |
0.99 |
BenchmarkTree/20000_vertices_to_protobuf - B/op |
12716937 B/op |
12717017 B/op |
1.00 |
BenchmarkTree/20000_vertices_to_protobuf - allocs/op |
140028 allocs/op |
140028 allocs/op |
1 |
BenchmarkTree/20000_vertices_from_protobuf |
881462784 ns/op 1697267832 B/op 580043 allocs/op |
870655004 ns/op 1697263936 B/op 580043 allocs/op |
1.01 |
BenchmarkTree/20000_vertices_from_protobuf - ns/op |
881462784 ns/op |
870655004 ns/op |
1.01 |
BenchmarkTree/20000_vertices_from_protobuf - B/op |
1697267832 B/op |
1697263936 B/op |
1.00 |
BenchmarkTree/20000_vertices_from_protobuf - allocs/op |
580043 allocs/op |
580043 allocs/op |
1 |
BenchmarkTree/30000_vertices_to_protobuf |
14979185 ns/op 19318466 B/op 210031 allocs/op |
14340752 ns/op 19318353 B/op 210030 allocs/op |
1.04 |
BenchmarkTree/30000_vertices_to_protobuf - ns/op |
14979185 ns/op |
14340752 ns/op |
1.04 |
BenchmarkTree/30000_vertices_to_protobuf - B/op |
19318466 B/op |
19318353 B/op |
1.00 |
BenchmarkTree/30000_vertices_to_protobuf - allocs/op |
210031 allocs/op |
210030 allocs/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf |
1954882957 ns/op 3752043832 B/op 870046 allocs/op |
1965511887 ns/op 3752062472 B/op 870149 allocs/op |
0.99 |
BenchmarkTree/30000_vertices_from_protobuf - ns/op |
1954882957 ns/op |
1965511887 ns/op |
0.99 |
BenchmarkTree/30000_vertices_from_protobuf - B/op |
3752043832 B/op |
3752062472 B/op |
1.00 |
BenchmarkTree/30000_vertices_from_protobuf - allocs/op |
870046 allocs/op |
870149 allocs/op |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
server/rpc/auth/webhook.go (2)
58-58: Log partial request details
If the webhook response is malformed or partial, additional logging may aid debugging.
83-83: Fallback for unexpected statuses
ReturningErrUnexpectedResponseis appropriate. If you need more context, consider appendingres.Reasonas well.test/integration/auth_webhook_test.go (3)
487-487: Possible test flakiness
Usingtime.Sleep(authTTL)can introduce timing issues under heavy load. Consider a more deterministic test approach.
555-555: Avoid indefinite sleeps
Time-based sleeps in tests can be brittle. Explore mocking or waiting on explicit signals.
619-619: Potential for flaky tests
Relying ontime.Sleepcan cause nondeterministic results. Using alternative synchronization is more robust.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pkg/webhook/client.go(1 hunks)server/rpc/auth/auth.go(1 hunks)server/rpc/auth/webhook.go(2 hunks)test/integration/auth_webhook_test.go(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- server/rpc/auth/auth.go
- pkg/webhook/client.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: bench
🔇 Additional comments (15)
server/rpc/auth/webhook.go (8)
27-27: New import looks good
No particular issues. Thewebhookpackage import helps centralize webhook logic.
43-43: Validateprjreference
Ifprjis nil, accessing its fields could cause a panic. Add a nil-check or ensure the function is never called with a nil project.
47-56: Consider validation for the webhook URL
Ifprj.AuthWebhookURLis empty or malformed, it may cause issues at runtime. Ensure it’s valid before creating the client.
64-64: Good error wrapping
Using%wpreserves original error context for future checks and logs.
67-67: Clear distinction for 200 vs. 403
HandlingAllowedvs.Forbiddenensures method-level security is properly enforced.Also applies to: 70-70
71-73: Helpful detail in meta error
Attachingres.Reasonmakes it easier to diagnose rejected requests.
76-76: 401 handling
Consistent approach for unauthorized status codes.
77-80: Extra debug context for 401
Includingres.Reasonis beneficial when diagnosing authentication issues.test/integration/auth_webhook_test.go (7)
38-38: Addedwebhookimport
Pulling in “webhook” clarifies error references and centralizes logic.
311-311: Refined error assertion
Usingwebhook.ErrUnexpectedStatusCodealigns with the consolidated webhook error definitions.
352-352: Matching unexpected response
Asserting againstwebhook.ErrUnexpectedResponseensures accurate handling of mismatched success flags.
382-382: Timeout error handling
Checkingwebhook.ErrWebhookTimeoutconforms to the new robust retry logic.
438-440: Unified TTL
Switching to a singleauthTTLvariable mirrors the PR’s intent to merge TTLs.
516-519: Consistent TTL approach
Applying oneauthTTLacross different tests helps unify the caching logic.
580-583: Maintaining a single TTL
Consolidating TTL usage improves clarity and aligns with the rest of the code.
What this PR does / why we need it:
This commit extracts webhook functionality from
rpc/authintopkg/webhookpackage to enable reuse in
DocEventhandling. This change also simplifiesTTL management by unifying
unauthTTLandauthTTL, while preventing cachekey collisions through project-based key partitioning.
Which issue(s) this PR fixes:
Addresses #1002
Related to #1131
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Tests
Chores