feat: support HTTPS for Manager API #1824

nic-chen · 2021-04-25T00:25:35Z

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

Bugfix
New feature provided
Improve performance
Backport patches

What changes will this PR take into?

Support for HTTPS by configuring SSL certificate.

Related issues

resolve #1706

Checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

netlify · 2021-04-25T00:25:41Z

Deploy preview for apisix-dashboard ready!

Built with commit 356ca4c

https://deploy-preview-1824--apisix-dashboard.netlify.app

codecov-commenter · 2021-04-25T00:27:18Z

Codecov Report

Merging #1824 (9f7ebee) into master (2f3717f) will decrease coverage by 0.48%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1824      +/-   ##
==========================================
- Coverage   72.07%   71.59%   -0.49%     
==========================================
  Files         125      172      +47     
  Lines        2944     6119    +3175     
  Branches      711      711              
==========================================
+ Hits         2122     4381    +2259     
- Misses        822     1490     +668     
- Partials        0      248     +248

Flag	Coverage Δ
backend-e2e-test	`45.98% <0.00%> (?)`
backend-e2e-test-ginkgo	`49.00% <0.00%> (?)`
backend-unit-test	`52.44% <ø> (?)`
frontend-e2e-test	`72.14% <ø> (+0.06%)`	⬆️

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

Impacted Files	Coverage Δ
api/cmd/managerapi.go	`48.35% <0.00%> (ø)`
api/internal/conf/conf.go	`61.03% <0.00%> (ø)`
api/internal/route.go	`85.29% <0.00%> (ø)`
api/internal/handler/data_loader/route_export.go	`73.18% <0.00%> (ø)`
api/internal/log/zap.go	`60.86% <0.00%> (ø)`
api/internal/handler/tool/tool.go	`94.44% <0.00%> (ø)`
api/internal/handler/global_rule/global_rule.go	`83.87% <0.00%> (ø)`
api/internal/filter/recover.go	`6.66% <0.00%> (ø)`
api/internal/handler/healthz/healthz.go	`100.00% <0.00%> (ø)`
api/internal/handler/consumer/consumer.go	`91.07% <0.00%> (ø)`
... and 40 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 5d6d7fc...9f7ebee. Read the comment docs.

starsz · 2021-04-25T02:28:44Z

api/cmd/managerapi.go

+					}
+				}()
+			}
+
 			printInfo()


Need to update the printInfo.

bisakhmondal · 2021-04-25T14:42:45Z

api/cmd/managerapi.go

+				addrSSL := fmt.Sprintf("%s:%d", conf.ServerHost, conf.SSLPort)
+				serverSSL := &http.Server{
+					Addr:         addrSSL,
+					Handler:      r,
+					ReadTimeout:  time.Duration(1000) * time.Millisecond,
+					WriteTimeout: time.Duration(5000) * time.Millisecond,
+				}


Hey, Nice work 👍 .
I think we should go with adding a custom TLS configuration to make it more secure. Also, a go server automatically upgrades requests to HTTP/2 if a tlsconfig is set with HTTPS.

Referring to an awesome blog from cloudflare.
Thanks :)

updated. thanks.

tokers · 2021-04-26T15:15:45Z

api/cmd/managerapi.go

@@ -124,6 +128,29 @@ func NewManagerAPICommand() *cobra.Command {
 				}
 			}()

+			// HTTPS
+			if conf.SSLCert != "" && conf.SSLKey != "" {
+				addrSSL := fmt.Sprintf("%s:%d", conf.ServerHost, conf.SSLPort)


Use net.JoinHostPort

api/conf/conf.yaml

imjoey · 2021-04-27T14:51:08Z

@nic-chen FYI, the CI failure in FE e2e has been fixed in #1826. Please rebase the code to get through it.

nic-chen · 2021-04-28T01:00:49Z

@nic-chen FYI, the CI failure in FE e2e has been fixed in #1826. Please rebase the code to get through it.

done, thanks.

tokers · 2021-04-28T01:43:28Z

api/cmd/managerapi.go

+					err := serverSSL.ListenAndServeTLS(conf.SSLCert, conf.SSLKey)
+					if err != nil && err != http.ErrServerClosed {
+						utils.CloseAll()
+						log.Fatalf("listen and serv fail: %s", err)


Suggested change

log.Fatalf("listen and serv fail: %s", err)

log.Fatalf("listen and serve fail: %s", err)

tokers · 2021-04-28T01:44:07Z

api/conf/conf.yaml

+
+  # ssl:
+  #   host: 127.0.0.1     # the address on which the `Manager API` should listen for HTTPS.
+                          # The default value is 0.0.0.0, if want to specify, please enable it.


We may also support the mTLS.

sure. we may support mTLS in the future.

bisakhmondal

Perfect 💯

imjoey

/lgtm

feat: support HTPS for Manager API

f44863d

starsz reviewed Apr 25, 2021

View reviewed changes

juzhiyuan changed the title ~~feat: support HTPS for Manager API~~ feat: support HTTPS for Manager API Apr 25, 2021

bisakhmondal reviewed Apr 25, 2021

View reviewed changes

johzchen added 4 commits April 26, 2021 09:50

fix: error

5c6ccb4

fix: cli test failed

1e7bef8

fix: cli test failed

216f0aa

fix: review

ba1808f

nic-chen requested review from starsz, gxthrj, imjoey, Jaycean and tokers April 26, 2021 08:42

tokers reviewed Apr 26, 2021

View reviewed changes

gxthrj reviewed Apr 26, 2021

View reviewed changes

api/conf/conf.yaml Show resolved Hide resolved

fix: code code optimization

dbdef86

Merge branch 'master' into https

9f7ebee

starsz approved these changes Apr 28, 2021

View reviewed changes

tokers reviewed Apr 28, 2021

View reviewed changes

fix review

651cfe3

bisakhmondal approved these changes Apr 29, 2021

View reviewed changes

tokers approved these changes Apr 29, 2021

View reviewed changes

imjoey approved these changes Apr 29, 2021

View reviewed changes

fix CI

356ca4c

nic-chen merged commit f146898 into apache:master Apr 29, 2021

nic-chen deleted the https branch April 29, 2021 16:57

hanqingwu mentioned this pull request Jan 4, 2024

feat: Configure apisix mTLS authentication on dashboard #2880

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support HTTPS for Manager API #1824

feat: support HTTPS for Manager API #1824

nic-chen commented Apr 25, 2021

netlify bot commented Apr 25, 2021 •

edited

Loading

codecov-commenter commented Apr 25, 2021 •

edited

Loading

starsz Apr 25, 2021

nic-chen Apr 26, 2021

bisakhmondal Apr 25, 2021

nic-chen Apr 26, 2021

tokers Apr 26, 2021

nic-chen Apr 27, 2021

imjoey commented Apr 27, 2021

nic-chen commented Apr 28, 2021

tokers Apr 28, 2021

nic-chen Apr 28, 2021

tokers Apr 28, 2021

nic-chen Apr 28, 2021

bisakhmondal left a comment

imjoey left a comment

	log.Fatalf("listen and serv fail: %s", err)
	log.Fatalf("listen and serve fail: %s", err)

feat: support HTTPS for Manager API #1824

feat: support HTTPS for Manager API #1824

Conversation

nic-chen commented Apr 25, 2021

netlify bot commented Apr 25, 2021 • edited Loading

codecov-commenter commented Apr 25, 2021 • 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

imjoey commented Apr 27, 2021

nic-chen commented Apr 28, 2021

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

bisakhmondal left a comment

Choose a reason for hiding this comment

imjoey left a comment

Choose a reason for hiding this comment

netlify bot commented Apr 25, 2021 •

edited

Loading

codecov-commenter commented Apr 25, 2021 •

edited

Loading