support for default_backend #222

d-h1 · 2017-04-26T20:33:19Z

support for setting service as default backend with new serviceDefaultBackend label.
related issue: haproxy.tmpl doesn't set a default_backend #220

vfarcic · 2017-04-27T12:41:49Z

docs/usage.md

@@ -25,6 +25,7 @@ The following query parameters can be used to send a *reconfigure* request to *D
 |reqMode        |The request mode. The proxy should be able to work with any mode supported by HAProxy. However, actively supported and tested modes are `http`, `tcp`, and `sni`. The `sni` mode implies TCP with an SNI-based routing.<br>Example: `tcp`|No|http|
 |reqPathReplace |A regular expression to apply the modification. If specified, `reqPathSearch` needs to be set as well.<br>Example: `/demo/`|No| |
 |reqPathSearch  |A regular expression to search the content to be replaced. If specified, `reqPathReplace` needs to be set as well.<br>Example: `/something/`|No| |
+|ServiceDefaultBackend  | If true, the service will be set to the default_backend rule, meaning it will catch all requests not matching any other rules.<br>Example: `true`|No| |


ServiceDefaultBackend might be confusing. People might expect to put some value other than true. Maybe it would be better to name it isDefaultBackend?

I agree, isDefaultBackend sounds much better, I'll update my PR

vfarcic · 2017-04-27T13:46:02Z

One more thing... Can you add a test to https://github.com/vfarcic/docker-flow-proxy/blob/master/integration_tests/integration_swarm_test.go?

d-h1 · 2017-04-27T16:36:32Z

The integration test turned out to be more complicated than I thought. My excuse is that I'm a complete newbie in golang...

To test isDefaultBackend properly we'd need to:

Start a service with a serviceDomain
Get the service's ip
Add the serviceDomain and a subdomain pointing to the service's ip in the hosts file
request (GET) the subdomain and receive 503.

I'm not sure how to do step 3..
Some help would be appreciated 😄

vfarcic

Apart from the two minor comments, this looks great.

I'll take care of integration tests (hopefully during the weekend).

vfarcic · 2017-04-28T09:08:16Z

server/server_test.go

@@ -759,6 +760,7 @@ func (s *ServerTestSuite) Test_GetServicesFromEnvVars_ReturnsServices() {
 	os.Setenv("DFP_SERVICE_DISTRIBUTE", strconv.FormatBool(service.Distribute))
 	os.Setenv("DFP_SERVICE_HTTPS_ONLY", strconv.FormatBool(service.HttpsOnly))
 	os.Setenv("DFP_SERVICE_HTTPS_PORT", strconv.Itoa(service.HttpsPort))
+	os.Setenv("DFP_SERVICE_DEFAULT_BACKEND", strconv.FormatBool(service.IsDefaultBackend))


Why not call it DFP_SERVICE_IS_DEFAULT_BACKEND? That would make it consistent and easy to remember that env. var. names are the same as request params with the DFP_SERVICE_ prefix.

vfarcic · 2017-04-28T09:08:54Z

docs/usage.md

@@ -21,6 +21,7 @@ The following query parameters can be used to send a *reconfigure* request to *D
 |delReqHeader   |Additional headers that will be deleted in the request before forwarding it to the service. Multiple headers should be separated with comma (`,`). Please consult [Delete a header in the request](https://www.haproxy.com/doc/aloha/7.0/haproxy/http_rewriting.html#delete-a-header-in-the-request) for more info.<br>Example: `X-Forwarded-For,Cookie`|No| |
 |delResHeader   |Additional headers that will be deleted in the response before forwarding it to the client. Multiple headers should be separated with comma (`,`). Please consult [Delete a header in the response](https://www.haproxy.com/doc/aloha/7.0/haproxy/http_rewriting.html#delete-a-header-in-the-response) for more info.<br>Example: `X-Varnish,X-Cache`|No| |
 |httpsPort      |The internal HTTPS port of a service that should be reconfigured. The port is used only in the `swarm` mode. If not specified, the `port` parameter will be used instead.<br>Example: `443`|No| |
+|isDefaultBackend  | If true, the service will be set to the default_backend rule, meaning it will catch all requests not matching any other rules.<br>Example: `true`|No| |


Can you also add an entry in the Environment Variables section below?

vfarcic · 2017-04-28T09:32:59Z

I'd like to run past you a scenario for integration tests.

Configure proxy to serve requests on /xxx (it does not exist) and be the default backend at the same time.
Send a request to /demo/hello.
Receive 200.

If my understanding is correct, default backend should forward a request to an address so the above scenario should work. Am I correct in my assumption. If I am, I'll add the test ASAP.

vfarcic · 2017-04-28T09:46:28Z

Take a look at https://gist.github.com/vfarcic/c26a46defba4ff84d1ac7f918fd80c25 and let me know if you think that would be a valid test. If it is, please add it to the PR.

d-h1 · 2017-04-28T16:08:54Z

You are right! That integration test should be good enough to try isDefaultBackend, thanks for the help..

I have a new issue now, when I change "DFP_SERVICE_DEFAULT_BACKEND" to "DFP_SERVICE_IS_DEFAULT_BACKEND" the test "Test_GetServicesFromEnvVars_ReturnsServices" seems to fail and I'm not sure why?

vfarcic · 2017-04-29T10:46:11Z

I have a new issue now, when I change "DFP_SERVICE_DEFAULT_BACKEND" to "DFP_SERVICE_IS_DEFAULT_BACKEND" the test "Test_GetServicesFromEnvVars_ReturnsServices" seems to fail and I'm not sure why?

The problem is in envconfig:"default_backend" . By specifying it, you told the code to use DEFAULT_BACKEND as env. var. name. That worked before but when you renamed it to IS_DEFAULT_BACKEND, the envconfig still continued looking for DEFAULT_BACKEND.

I fixed it and merged the PR. If all the tests pass, it'll be ready for a release.

vfarcic · 2017-04-29T12:35:34Z

Integration tests were missing isDefaultBackend=true. Now everything works and is merged to master. Release 1.336 was created. Thanks @leostarcevic.

Leo-Starcevic added 2 commits April 26, 2017 20:19

support for setting a service to the default_backend rule

2ec4aad

docs

b15d1a2

vfarcic reviewed Apr 27, 2017

View reviewed changes

Leo-Starcevic added 4 commits April 27, 2017 16:02

merged upstream

f6ab9a2

renamed serviceDefaultBackend to isDefaultBackend

3992069

removed duplicate

6056351

fix

01ff805

vfarcic reviewed Apr 28, 2017

View reviewed changes

added integration test and changed env to IS_DEFAULT_BACKEND

9f3593c

moved test

fd4e4c3

Leo and others added 2 commits April 29, 2017 13:04

Merge branch 'master' into feature/default_backend

3b6b2ae

Update integration_swarm_test.go

6e3e96a

vfarcic merged commit cece848 into vfarcic:master Apr 29, 2017

d-h1 deleted the feature/default_backend branch April 29, 2017 12:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for default_backend #222

support for default_backend #222

d-h1 commented Apr 26, 2017 •

edited

Loading

vfarcic Apr 27, 2017

d-h1 Apr 27, 2017 •

edited

Loading

vfarcic commented Apr 27, 2017

d-h1 commented Apr 27, 2017 •

edited

Loading

vfarcic left a comment

vfarcic Apr 28, 2017

vfarcic Apr 28, 2017

vfarcic commented Apr 28, 2017

vfarcic commented Apr 28, 2017

d-h1 commented Apr 28, 2017 •

edited

Loading

vfarcic commented Apr 29, 2017

vfarcic commented Apr 29, 2017

support for default_backend #222

support for default_backend #222

Conversation

d-h1 commented Apr 26, 2017 • edited Loading

vfarcic Apr 27, 2017

Choose a reason for hiding this comment

d-h1 Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

vfarcic commented Apr 27, 2017

d-h1 commented Apr 27, 2017 • edited Loading

vfarcic left a comment

Choose a reason for hiding this comment

vfarcic Apr 28, 2017

Choose a reason for hiding this comment

vfarcic Apr 28, 2017

Choose a reason for hiding this comment

vfarcic commented Apr 28, 2017

vfarcic commented Apr 28, 2017

d-h1 commented Apr 28, 2017 • edited Loading

vfarcic commented Apr 29, 2017

vfarcic commented Apr 29, 2017

d-h1 commented Apr 26, 2017 •

edited

Loading

d-h1 Apr 27, 2017 •

edited

Loading

d-h1 commented Apr 27, 2017 •

edited

Loading

d-h1 commented Apr 28, 2017 •

edited

Loading