api: add HTTP audit middleware #4537

CabinfeverB · 2022-01-04T07:54:41Z

Signed-off-by: Cabinfever_B cabinfeveroier@gmail.com

What problem does this PR solve?

close #4601
This PR is used to support the audit
This PR should be merged after #4526

What is changed and how it works?

Add audit middleware for HTTP API.
Define audit backend interface.

Check List

Tests

Unit test
Integration test

Code changes

Has HTTP API interfaces change (Don't forget to add the declarative for API)

Release note

Add HTTP Audit middleware

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot · 2022-01-04T07:54:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch
rleungx

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2022-01-04T08:14:09Z

Codecov Report

Merging #4537 (436892f) into master (ea69785) will increase coverage by 0.08%.
The diff coverage is 78.68%.

@@            Coverage Diff             @@
##           master    #4537      +/-   ##
==========================================
+ Coverage   74.68%   74.76%   +0.08%     
==========================================
  Files         281      282       +1     
  Lines       27714    27760      +46     
==========================================
+ Hits        20697    20756      +59     
+ Misses       5154     5147       -7     
+ Partials     1863     1857       -6

Flag	Coverage Δ
unittests	`74.76% <78.68%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/config/config.go	`77.84% <ø> (ø)`
server/api/middleware.go	`74.50% <62.96%> (-13.50%)`	⬇️
pkg/audit/audit.go	`75.00% <75.00%> (ø)`
server/server.go	`74.31% <81.81%> (+0.05%)`	⬆️
pkg/requestutil/context.go	`100.00% <100.00%> (ø)`
pkg/requestutil/request_info.go	`78.94% <100.00%> (ø)`
server/api/admin.go	`51.56% <100.00%> (ø)`
server/api/router.go	`99.56% <100.00%> (+2.23%)`	⬆️
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/tempurl/tempurl.go	`45.00% <0.00%> (-15.00%)`	⬇️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea69785...436892f. Read the comment docs.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-01-21T08:56:09Z

pkg/audit/audit.go

@@ -38,10 +38,21 @@ func (m *LabelMatcher) Match(labels *BackendLabels) bool {
 	return false
 }

+// Sequence is used to help backend implement audit.Backend


Where does the Sequence init?

creates and inits Sequence when creating Backend

disksing · 2022-01-24T02:12:03Z

server/api/middleware.go

+	beforeNextBackends := make([]audit.Backend, 0)
+	afterNextBackends := make([]audit.Backend, 0)
+	for _, backend := range s.svr.GetAuditBackend() {
+		if backend.Match(labels) {
+			if backend.ProcessBeforeHandler() {
+				beforeNextBackends = append(beforeNextBackends, backend)
+			} else {
+				afterNextBackends = append(afterNextBackends, backend)
+			}
+		}
+	}
+	for _, backend := range beforeNextBackends {
+		backend.ProcessHTTPRequest(r)
+	}


Suggested change

beforeNextBackends := make([]audit.Backend, 0)

afterNextBackends := make([]audit.Backend, 0)

for _, backend := range s.svr.GetAuditBackend() {

if backend.Match(labels) {

if backend.ProcessBeforeHandler() {

beforeNextBackends = append(beforeNextBackends, backend)

} else {

afterNextBackends = append(afterNextBackends, backend)

}

}

}

for _, backend := range beforeNextBackends {

backend.ProcessHTTPRequest(r)

}

for _, backend := range s.svr.GetAuditBackend() {

if backend.Match(labels) && backend.ProcessBeforeHandler() {

backend.ProcessHTTPRequest(r)

}

}

The backend can be divided into two types: before the service Handle and after the service Handle

rleungx · 2022-01-24T03:39:48Z

pkg/requestutil/request_info.go

+
+// ExecutionInfo holds request execution info
+type ExecutionInfo struct {
+	EndTimeStamp int64


How about making it simple as we only have one field here?

has replaced struct to int64

rleungx · 2022-01-24T03:43:57Z

server/api/middleware.go

+
+	requestInfo, ok := requestutil.RequestInfoFrom(r.Context())
+	if !ok {
+		next(w, r)


Do we need to return?

I'm not sure. If return here, what response do we write?

Add error log if can't get requestInfo

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-01-25T02:52:50Z

pkg/requestutil/context_test.go

+	c.Assert(result.StartTimeStamp, Equals, timeNow)
+}
+
+func (s *testRequestContextSuite) TestExcutionInfo(c *C) {


The name of the case should also be changed.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx · 2022-01-26T08:28:58Z

server/api/router.go

@@ -124,6 +125,11 @@ func (s *serviceMiddlewareBuilder) middlewareFunc(next func(http.ResponseWriter,
 // @BasePath /pd/api/v1
 func createRouter(prefix string, svr *server.Server) *mux.Router {
 	rd := createIndentRender()
+	setAudit := func(labels ...string) createRouteOption {


RegistServiceForHTTP is not only for audit I think.

I will change RegistServiceForHTTP name to "SetServiceAuditBackendForHTTP". Other actions such as rate limit may be processed by other createRouteOption functions.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx

Mostly LGTM

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

nolouch

lgtm

nolouch · 2022-02-11T04:30:22Z

/merge

ti-chi-bot · 2022-02-11T04:30:23Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-02-11T04:30:24Z

This pull request has been accepted and is ready to merge.

Commit hash: 1b170fe

ti-chi-bot · 2022-02-11T04:30:37Z

@CabinfeverB: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ref #4537, close #4602 Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

ref #4537, close #4608 Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

CabinfeverB added 14 commits December 23, 2021 17:19

close tikv#4494

3a45237

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

close tikv#4494: add priority comment

725345f

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge branch 'master' into http_source_ip

f30bb7b

add request info middleware

198fb0b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

close tikv#4494

a80af26

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge remote-tracking branch 'origin/http_source_ip' into http_source_ip

d9ed82f

change service label getter and setter

5af1d1c

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge branch 'master' into service_middleware

3041f1f

add lock

232f798

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add audit middleware

3af3e83

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add audit middleware

bd9553b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

add audit middleware

01f88c7

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge service middleware

a4bc982

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix

e25cc68

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 4, 2022

ti-chi-bot requested review from disksing and nolouch January 4, 2022 07:54

Merge branch 'http_source_ip' into service_middleware

a313450

CabinfeverB and others added 9 commits January 4, 2022 16:24

tikv#4538

5ba83fe

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix return

40ef486

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix

e3ddcd9

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix check

edd63ae

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix check

271e58f

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

change service label placement

2540d56

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge master

ea08f30

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

merge master

4800b61

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

Merge branch 'master' into service_middleware

9a2eaec

fix router

cd8f7b7

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Jan 21, 2022

View reviewed changes

disksing reviewed Jan 24, 2022

View reviewed changes

rleungx reviewed Jan 24, 2022

View reviewed changes

CabinfeverB added 2 commits January 24, 2022 17:22

remove service middleware switch

8b25afe

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix

186fa9e

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB mentioned this pull request Jan 24, 2022

api: HTTP audit using prometheus histogram metrics #4609

Merged

fix test

577480a

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Jan 25, 2022

View reviewed changes

CabinfeverB added 2 commits January 25, 2022 11:09

log error when audit error

6217a8e

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

log error when audit error

e958793

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Jan 26, 2022

View reviewed changes

change function name

36be023

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

rleungx reviewed Jan 27, 2022

View reviewed changes

add comment

1b170fe

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

nolouch approved these changes Feb 8, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Feb 8, 2022

rleungx approved these changes Feb 10, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Feb 10, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 11, 2022

Merge branch 'master' into audit_middleware

436892f

ti-chi-bot merged commit 62c38e9 into tikv:master Feb 11, 2022

ti-chi-bot added a commit that referenced this pull request Feb 22, 2022

api: local log audit (#4536)

8328528

ref #4537, close #4602 Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: ShuNing <nolouch@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io>

CabinfeverB mentioned this pull request Apr 25, 2022

api: Refactor Service Middleware #4840

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: add HTTP audit middleware #4537

api: add HTTP audit middleware #4537

CabinfeverB commented Jan 4, 2022 •

edited

Loading

ti-chi-bot commented Jan 4, 2022 •

edited

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading

rleungx Jan 21, 2022

CabinfeverB Jan 21, 2022

disksing Jan 24, 2022

CabinfeverB Jan 24, 2022

rleungx Jan 24, 2022

CabinfeverB Jan 24, 2022

rleungx Jan 24, 2022

CabinfeverB Jan 24, 2022

CabinfeverB Jan 24, 2022

rleungx Jan 25, 2022

CabinfeverB Jan 25, 2022

rleungx Jan 26, 2022

CabinfeverB Jan 27, 2022

rleungx left a comment

nolouch left a comment

nolouch commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

api: add HTTP audit middleware #4537

api: add HTTP audit middleware #4537

Conversation

CabinfeverB commented Jan 4, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Jan 4, 2022 • edited Loading

codecov bot commented Jan 4, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleungx left a comment

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

ti-chi-bot commented Feb 11, 2022

CabinfeverB commented Jan 4, 2022 •

edited

Loading

ti-chi-bot commented Jan 4, 2022 •

edited

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading