api: Support to get http client ip #4495

CabinfeverB · 2021-12-23T09:25:24Z

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

What problem does this PR solve?

What is changed and how it works?

According to related blogs, it gets client IP from header X-Forwarded-For to header X-Real-Ip to request.RemoteAddr

Check List

Tests

Unit test

Release note

None.

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

ti-chi-bot · 2021-12-23T09:25:25Z

[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.

server/server_test.go

pkg/apiutil/apiutil.go

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

CabinfeverB · 2021-12-29T08:33:11Z

/run-all-tests

nolouch · 2021-12-29T08:42:06Z

server/server_test.go

-	bodyBytes, err := io.ReadAll(resp.Body)
-	c.Assert(err, IsNil)
-	bodyString := string(bodyBytes)
-	c.Assert(bodyString, Equals, "Hello World\n")


why delete these lines?

seems it should be reserved. Because it should not break the original test

pkg/apiutil/apiutil.go

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

CabinfeverB · 2022-01-04T08:07:33Z

/run-all-tests

codecov · 2022-01-04T09:21:58Z

Codecov Report

Merging #4495 (6dcd750) into master (20c08d2) will decrease coverage by 0.02%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #4495      +/-   ##
==========================================
- Coverage   74.95%   74.92%   -0.03%     
==========================================
  Files         265      265              
  Lines       27851    27861      +10     
==========================================
  Hits        20875    20875              
- Misses       5122     5129       +7     
- Partials     1854     1857       +3

Flag	Coverage Δ
unittests	`74.92% <80.00%> (-0.03%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/apiutil/apiutil.go	`67.74% <80.00%> (+2.35%)`	⬆️
pkg/errs/errs.go	`75.00% <0.00%> (-25.00%)`	⬇️
pkg/tempurl/tempurl.go	`45.00% <0.00%> (-15.00%)`	⬇️
server/schedulers/shuffle_hot_region.go	`55.55% <0.00%> (-10.11%)`	⬇️
server/id/id.go	`76.19% <0.00%> (-9.53%)`	⬇️
server/region_syncer/server.go	`81.41% <0.00%> (-5.13%)`	⬇️
server/config/persist_options.go	`91.66% <0.00%> (-1.90%)`	⬇️
server/schedule/checker/rule_checker.go	`78.97% <0.00%> (-1.03%)`	⬇️
server/server.go	`71.94% <0.00%> (-0.75%)`	⬇️
server/schedule/operator/builder.go	`89.31% <0.00%> (-0.41%)`	⬇️
... and 11 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 20c08d2...6dcd750. Read the comment docs.

rleungx · 2022-01-04T10:14:13Z

server/server_test.go

@@ -204,6 +205,9 @@ func (s *testServerHandlerSuite) TestRegisterServerHandler(c *C) {
 		mux := http.NewServeMux()
 		mux.HandleFunc("/pd/apis/mok/v1/hello", func(w http.ResponseWriter, r *http.Request) {
 			fmt.Fprintln(w, "Hello World")
+			// test getting ip
+			clientIP := apiutil.GetIPAddrFromHTTPRequest(r)


It's better to cover all cases in our test.

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

nolouch

lgtm

nolouch · 2022-01-05T05:02:05Z

/merge

ti-chi-bot · 2022-01-05T05:02:06Z

@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-01-05T05:02:07Z

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

Commit hash: 7efb6ff

CabinfeverB · 2022-01-05T05:50:00Z

/run-all-tests

nolouch · 2022-01-05T06:42:50Z

/run-all-tests

close tikv#4494

3a45237

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

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Dec 23, 2021

ti-chi-bot requested review from nolouch and rleungx December 23, 2021 09:25

rleungx reviewed Dec 23, 2021

View reviewed changes

server/server_test.go Outdated Show resolved Hide resolved

rleungx reviewed Dec 23, 2021

View reviewed changes

pkg/apiutil/apiutil.go Show resolved Hide resolved

bufferflies reviewed Dec 24, 2021

View reviewed changes

pkg/apiutil/apiutil.go Outdated Show resolved Hide resolved

close tikv#4494: add priority comment

725345f

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

Merge branch 'master' into http_source_ip

f30bb7b

nolouch reviewed Dec 29, 2021

View reviewed changes

bufferflies reviewed Dec 29, 2021

View reviewed changes

pkg/apiutil/apiutil.go Show resolved Hide resolved

bufferflies reviewed Dec 29, 2021

View reviewed changes

pkg/apiutil/apiutil.go Show resolved Hide resolved

CabinfeverB added 2 commits December 30, 2021 17:19

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

Merge remote-tracking branch 'upstream/master' into http_source_ip

42cdae0

CabinfeverB mentioned this pull request Jan 4, 2022

api: Add Request Info Middleware #4526

Merged

rleungx reviewed Jan 4, 2022

View reviewed changes

fix test to cover all cases

7efb6ff

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

rleungx approved these changes Jan 5, 2022

View reviewed changes

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

nolouch approved these changes Jan 5, 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 Jan 5, 2022

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

Merge branch 'master' into http_source_ip

6dcd750

ti-chi-bot merged commit a901999 into tikv:master Jan 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: Support to get http client ip #4495

api: Support to get http client ip #4495

CabinfeverB commented Dec 23, 2021

ti-chi-bot commented Dec 23, 2021 •

edited

Loading

CabinfeverB commented Dec 29, 2021

nolouch Dec 29, 2021

CabinfeverB Dec 30, 2021 •

edited

Loading

CabinfeverB commented Jan 4, 2022

codecov bot commented Jan 4, 2022 •

edited

Loading

rleungx Jan 4, 2022

nolouch left a comment

nolouch commented Jan 5, 2022

ti-chi-bot commented Jan 5, 2022

ti-chi-bot commented Jan 5, 2022

CabinfeverB commented Jan 5, 2022

nolouch commented Jan 5, 2022

api: Support to get http client ip #4495

api: Support to get http client ip #4495

Conversation

CabinfeverB commented Dec 23, 2021

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Dec 23, 2021 • edited Loading

CabinfeverB commented Dec 29, 2021

nolouch Dec 29, 2021

Choose a reason for hiding this comment

CabinfeverB Dec 30, 2021 • edited Loading

Choose a reason for hiding this comment

CabinfeverB commented Jan 4, 2022

codecov bot commented Jan 4, 2022 • edited Loading

Codecov Report

rleungx Jan 4, 2022

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Jan 5, 2022

ti-chi-bot commented Jan 5, 2022

ti-chi-bot commented Jan 5, 2022

CabinfeverB commented Jan 5, 2022

nolouch commented Jan 5, 2022

ti-chi-bot commented Dec 23, 2021 •

edited

Loading

CabinfeverB Dec 30, 2021 •

edited

Loading

codecov bot commented Jan 4, 2022 •

edited

Loading