router test: config template: don't match against the whole file #311

alebedev87 · 2021-06-18T17:41:23Z

Improvement to the test introduced in #307.
The check for the presence of config snippets is not made against the whole config file anymore.
It now searches only the backend part of the config corresponding to the route.
This helps to avoid the false positives and negatives.

Note the usage of https://github.com/haproxytech/config-parser

alebedev87 · 2021-06-18T18:13:01Z

/assign @Miciah @candita

alebedev87 · 2021-08-02T07:32:04Z

/retest

frobware · 2021-08-02T08:40:09Z

pkg/router/template/util/haproxy/config_parser.go

+	edgeBePrefix    = "be_edge_http"
+)
+
+// ConfigParser is a primitive HAProxy config parser.


I'm wondering whether you looked at https://github.com/haproxytech/config-parser and if that would do what we need.

No, I didn't, I wasn't expecting any golang libs from HAProxy community. But this one is even from the upstream! Thanks a lot for the advice. This module looks very promising:

$ cat config.cfg | grep -A 15 be_edge_http:test-route:hello-openshift2 backend be_edge_http:test-route:hello-openshift2 mode http option redispatch option forwardfor balance timeout check 5000ms http-request add-header X-Forwarded-Host %[req.hdr(host)] http-request add-header X-Forwarded-Port %[dst_port] http-request add-header X-Forwarded-Proto http if !{ ssl_fc } http-request add-header X-Forwarded-Proto https if { ssl_fc } http-request add-header X-Forwarded-Proto-Version h2 if { ssl_fc_alpn -i h2 } http-request add-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)] http-response set-header Strict-Transport-Security 'max-age=99999;includesubdomain' cookie f7e936ebe97cd64e8c888d1eba08cb29 insert indirect nocache httponly secure attr SameSite=None server pod:hello-openshift-7b8c68587c-mtck6:hello-openshift:8080-tcp:10.217.0.24:8080 10.217.0.24:8080 cookie 97ddca1460eca46376928fd7bcf8c89c weight 256 check inter 5000ms $ cat main.go package main import ( "fmt" "github.com/haproxytech/config-parser/v4" "github.com/haproxytech/config-parser/v4/options" "github.com/haproxytech/config-parser/v4/types" ) func main() { p, err := parser.New(options.Path("config.cfg")) if err != nil { fmt.Println("failed to parse the config:", err) } data, err := p.Get(parser.Backends, "be_edge_http:test-route:hello-openshift2", "http-response", false) if err != nil { fmt.Println("failed to get backend:", err) } responses := data.([]types.HTTPAction) for _, r := range responses { fmt.Println(r) } } $ go run main.go set-header Strict-Transport-Security 'max-age=99999;includesubdomain'

Let me try to integrate it into my PR.

alebedev87 · 2021-08-02T12:17:49Z

/hold

I will try to integrate https://github.com/haproxytech/config-parser module instead of the home made parser.

alebedev87 · 2021-08-04T22:27:09Z

/retest

alebedev87 · 2021-08-05T09:23:27Z

/hold cancel

alebedev87 · 2021-08-05T09:24:19Z

/assign @frobware

alebedev87 · 2021-08-05T13:36:40Z

/retest

frobware

It's great that the upstream config parser slips in so easily.

pkg/router/router_test.go

frobware · 2021-08-05T17:06:08Z

/lgtm

openshift-ci · 2021-08-05T17:06:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alebedev87, frobware

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [alebedev87,frobware]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-08-05T17:43:18Z

/retest-required