Skip to content

Commit b3eb747

Browse files
committed
BUG/MEDIUM: healthcheck: keep the type when no parameters are given
SerializeHealthCheckSection removed the "type" line when a healthcheck of type httpchk, smtpchk, mysql-check or pgsql-check came without its parameters, so the section was written without a type. The next write of that section then reached the default branch, which unset a plain "type" attribute no parser registers and failed with "attribute not found". Build the type line before touching the section. httpchk, smtpchk and mysql-check parameters are optional in HAProxy and the bare "type <check>" line is written when they are missing. pgsql-check requires a user and is rejected with a validation error, as is an unknown type. An empty type only removes the existing type line. Serializer errors go through HandleError so implicit transactions are cleaned up. Add the bare "type httpchk" form to the parser tests.
1 parent 9393791 commit b3eb747

8 files changed

Lines changed: 184 additions & 87 deletions

File tree

config-parser/tests/integration/healthcheck_data_test.go

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config-parser/tests/integration/healthcheck_test.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config-parser/tests/type-httpchk_generated_test.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config-parser/types/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,7 @@ type OptionHttpchk struct {
984984
//name:type httpchk
985985
//no:parse
986986
//test:ok:type httpchk OPTIONS * HTTP/1.1\\r\\nHost:\\ www
987+
//test:ok:type httpchk
987988
//test:ok:type httpchk <uri>
988989
//test:ok:type httpchk <method> <uri>
989990
//test:ok:type httpchk <method> <uri> <version>

configuration/healthcheck.go

Lines changed: 49 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
strfmt "github.com/go-openapi/strfmt"
2323
parser "github.com/haproxytech/client-native/v6/config-parser"
24+
"github.com/haproxytech/client-native/v6/config-parser/common"
2425
parsererrors "github.com/haproxytech/client-native/v6/config-parser/errors"
2526
"github.com/haproxytech/client-native/v6/config-parser/types"
2627
"github.com/haproxytech/client-native/v6/misc"
@@ -256,7 +257,7 @@ func (c *client) CreateHealthcheck(data *models.HealthCheck, transactionID strin
256257
}
257258

258259
if err = SerializeHealthCheckSection(p, data); err != nil {
259-
return err
260+
return c.HandleError(data.Name, "", "", t, transactionID == "", err)
260261
}
261262

262263
return c.SaveData(p, t, transactionID == "")
@@ -282,7 +283,7 @@ func (c *client) EditHealthcheck(name string, data *models.HealthCheck, transact
282283
}
283284

284285
if err = SerializeHealthCheckSection(p, data); err != nil {
285-
return err
286+
return c.HandleError(data.Name, "", "", t, transactionID == "", err)
286287
}
287288

288289
return c.SaveData(p, t, transactionID == "")
@@ -329,103 +330,65 @@ func clearAllTypes(p parser.Parser, data *models.HealthCheck) error {
329330
return nil
330331
}
331332

332-
func SerializeHealthCheckSection(p parser.Parser, data *models.HealthCheck) error { //nolint:gocognit
333-
if data == nil {
334-
return errors.New("empty health check")
335-
}
336-
337-
var err error
338-
err = clearAllTypes(p, data)
339-
if err != nil {
340-
return err
341-
}
342-
333+
// serializeHealthCheckType returns the parser attribute and value of the
334+
// "type" line of a health check, or an empty attribute when no type is set.
335+
// The parameters of httpchk, smtpchk and mysql-check are optional in HAProxy
336+
// and the bare "type <check>" line is written when they are not given, while
337+
// pgsql-check requires a user.
338+
func serializeHealthCheckType(data *models.HealthCheck) (string, common.ParserData, error) {
343339
switch data.Type {
340+
case "":
341+
return "", nil, nil
344342
case "smtpchk":
343+
smtpchk := types.TypeSmtpchk{}
345344
if data.SmtpchkParams != nil {
346-
smtpchck := types.TypeSmtpchk{
347-
Domain: data.SmtpchkParams.Domain,
348-
Hello: data.SmtpchkParams.Hello,
349-
NoType: false,
350-
}
351-
if err := p.Set(parser.HealthChecks, data.Name, "type smtpchk", smtpchck); err != nil {
352-
return err
353-
}
354-
} else {
355-
if err = p.Set(parser.HealthChecks, data.Name, "type smtpchk", nil); err != nil {
356-
return err
357-
}
345+
smtpchk.Domain = data.SmtpchkParams.Domain
346+
smtpchk.Hello = data.SmtpchkParams.Hello
358347
}
348+
return "type smtpchk", smtpchk, nil
359349
case "mysql-check":
350+
mysqlchk := types.TypeMysqlCheck{}
360351
if data.MysqlCheckParams != nil {
361-
mysqlchk := types.TypeMysqlCheck{
362-
ClientVersion: data.MysqlCheckParams.ClientVersion,
363-
User: data.MysqlCheckParams.Username,
364-
NoType: false,
365-
}
366-
if err := p.Set(parser.HealthChecks, data.Name, "type mysql-check", mysqlchk); err != nil {
367-
return err
368-
}
369-
} else {
370-
if err = p.Set(parser.HealthChecks, data.Name, "type mysql-check", nil); err != nil {
371-
return err
372-
}
352+
mysqlchk.ClientVersion = data.MysqlCheckParams.ClientVersion
353+
mysqlchk.User = data.MysqlCheckParams.Username
373354
}
355+
return "type mysql-check", mysqlchk, nil
374356
case "pgsql-check":
375-
if data.PgsqlCheckParams != nil {
376-
pgsqlchk := types.TypePgsqlCheck{
377-
User: data.PgsqlCheckParams.Username,
378-
NoType: false,
379-
}
380-
if err := p.Set(parser.HealthChecks, data.Name, "type pgsql-check", pgsqlchk); err != nil {
381-
return err
382-
}
383-
} else {
384-
if err = p.Set(parser.HealthChecks, data.Name, "type pgsql-check", nil); err != nil {
385-
return err
386-
}
357+
if data.PgsqlCheckParams == nil || data.PgsqlCheckParams.Username == "" {
358+
return "", nil, NewConfError(ErrValidationError, "pgsql_check_params.username is mandatory with type pgsql-check")
387359
}
360+
return "type pgsql-check", types.TypePgsqlCheck{User: data.PgsqlCheckParams.Username}, nil
388361
case "httpchk":
362+
httpchk := types.TypeHttpchk{}
389363
if data.HttpchkParams != nil {
390-
httpchk := types.TypeHttpchk{
391-
Method: data.HttpchkParams.Method,
392-
URI: data.HttpchkParams.URI,
393-
Version: data.HttpchkParams.Version,
394-
NoType: false,
395-
}
396-
if err := p.Set(parser.HealthChecks, data.Name, "type httpchk", httpchk); err != nil {
397-
return err
398-
}
399-
} else {
400-
if err = p.Set(parser.HealthChecks, data.Name, "type httpchk", nil); err != nil {
401-
return err
402-
}
403-
}
404-
case "ssl-hello-chk":
405-
if err = p.Set(parser.HealthChecks, data.Name, "type ssl-hello-chk", &types.SimpleType{}); err != nil {
406-
return err
407-
}
408-
case "redis-check":
409-
if err = p.Set(parser.HealthChecks, data.Name, "type redis-check", &types.SimpleType{}); err != nil {
410-
return err
411-
}
412-
case "ldap-check":
413-
if err = p.Set(parser.HealthChecks, data.Name, "type ldap-check", &types.SimpleType{}); err != nil {
414-
return err
415-
}
416-
case "spop-check":
417-
if err = p.Set(parser.HealthChecks, data.Name, "type spop-check", &types.SimpleType{}); err != nil {
418-
return err
419-
}
420-
case "tcp-check":
421-
if err = p.Set(parser.HealthChecks, data.Name, "type tcp-check", &types.SimpleType{}); err != nil {
422-
return err
364+
httpchk.Method = data.HttpchkParams.Method
365+
httpchk.URI = data.HttpchkParams.URI
366+
httpchk.Version = data.HttpchkParams.Version
423367
}
368+
return "type httpchk", httpchk, nil
369+
case "ssl-hello-chk", "redis-check", "ldap-check", "spop-check", "tcp-check":
370+
return "type " + data.Type, &types.SimpleType{}, nil
424371
default:
425-
if err = p.Set(parser.HealthChecks, data.Name, "type", nil); err != nil {
426-
return err
427-
}
372+
return "", nil, NewConfError(ErrValidationError, "unknown health check type "+data.Type)
428373
}
374+
}
429375

430-
return nil
376+
func SerializeHealthCheckSection(p parser.Parser, data *models.HealthCheck) error {
377+
if data == nil {
378+
return errors.New("empty health check")
379+
}
380+
381+
// validate before touching the section so that a rejected type leaves
382+
// the existing configuration intact
383+
attribute, value, err := serializeHealthCheckType(data)
384+
if err != nil {
385+
return err
386+
}
387+
if err = clearAllTypes(p, data); err != nil {
388+
return err
389+
}
390+
if attribute == "" {
391+
return nil
392+
}
393+
return p.Set(parser.HealthChecks, data.Name, attribute, value)
431394
}

configuration/healthcheck_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright 2026 HAProxy Technologies
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
16+
package configuration
17+
18+
import (
19+
"testing"
20+
21+
"github.com/stretchr/testify/require"
22+
23+
parser "github.com/haproxytech/client-native/v6/config-parser"
24+
"github.com/haproxytech/client-native/v6/config-parser/options"
25+
"github.com/haproxytech/client-native/v6/models"
26+
)
27+
28+
func newHealthCheckParser(t *testing.T, config string) parser.Parser {
29+
t.Helper()
30+
p, err := parser.New(options.String(config))
31+
require.NoError(t, err)
32+
return p
33+
}
34+
35+
// The check types whose parameters are optional in HAProxy must render the
36+
// bare "type <check>" line when no parameters are given, and that line must
37+
// be read back with the same type.
38+
func TestSerializeHealthCheckSectionTypeWithoutParams(t *testing.T) {
39+
for _, typ := range []string{"httpchk", "smtpchk", "mysql-check"} {
40+
t.Run(typ, func(t *testing.T) {
41+
p := newHealthCheckParser(t, "healthcheck hc\n")
42+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: "hc", Type: typ}}
43+
require.NoError(t, SerializeHealthCheckSection(p, hc))
44+
45+
out := p.String()
46+
require.Contains(t, out, "\n type "+typ+"\n", out)
47+
48+
got := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: "hc"}}
49+
require.NoError(t, ParseHealthcheckSection(newHealthCheckParser(t, out), got))
50+
require.Equal(t, typ, got.Type)
51+
})
52+
}
53+
}
54+
55+
// An empty type only removes the existing type line, it is not an error.
56+
func TestSerializeHealthCheckSectionEmptyType(t *testing.T) {
57+
p := newHealthCheckParser(t, "healthcheck hc\n type httpchk GET /\n")
58+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: "hc"}}
59+
require.NoError(t, SerializeHealthCheckSection(p, hc))
60+
require.NotContains(t, p.String(), "type")
61+
}
62+
63+
// pgsql-check has a mandatory user in HAProxy: a bare "type pgsql-check"
64+
// is rejected instead of being written to the configuration.
65+
func TestSerializeHealthCheckSectionPgsqlCheckRequiresUser(t *testing.T) {
66+
for name, params := range map[string]*models.PgsqlCheckParams{
67+
"nil params": nil,
68+
"empty username": {},
69+
} {
70+
t.Run(name, func(t *testing.T) {
71+
p := newHealthCheckParser(t, "healthcheck hc\n")
72+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{
73+
Name: "hc", Type: "pgsql-check", PgsqlCheckParams: params,
74+
}}
75+
require.ErrorIs(t, SerializeHealthCheckSection(p, hc), ErrValidationError)
76+
require.NotContains(t, p.String(), "type")
77+
})
78+
}
79+
}
80+
81+
func TestSerializeHealthCheckSectionUnknownType(t *testing.T) {
82+
p := newHealthCheckParser(t, "healthcheck hc\n")
83+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: "hc", Type: "foo-check"}}
84+
require.ErrorIs(t, SerializeHealthCheckSection(p, hc), ErrValidationError)
85+
require.NotContains(t, p.String(), "type")
86+
}

configuration/structured_healthcheck.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func serializeHealthcheckSection(a StructuredToParserArgs, h *models.HealthCheck
187187
return err
188188
}
189189
if err = SerializeHealthCheckSection(p, h); err != nil {
190-
return err
190+
return a.HandleError(h.Name, "", "", a.TID, a.TID == "", err)
191191
}
192192

193193
for i, httpCheck := range h.HTTPCheckList {

test/healthchecks_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,3 +335,44 @@ func TestCreateEditDeleteHealthcheck(t *testing.T) { //nolint:gocognit,gocyclo
335335
})
336336
}
337337
}
338+
339+
// A type without parameters must survive a create followed by an edit: the
340+
// type line used to be dropped on create, leaving a section without a type,
341+
// and the following edit then failed with "attribute not found".
342+
func TestCreateEditHealthcheckTypeWithoutParams(t *testing.T) {
343+
name := fmt.Sprintf("created_bare_type_%d", version)
344+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: name, Type: "httpchk"}}
345+
346+
require.NoError(t, clientTest.CreateHealthcheck(hc, "", version))
347+
version++
348+
349+
_, got, err := clientTest.GetHealthcheck(name, "")
350+
require.NoError(t, err)
351+
require.Equal(t, "httpchk", got.Type)
352+
require.Equal(t, &models.HttpchkParams{}, got.HttpchkParams)
353+
354+
hc.Type = "smtpchk"
355+
require.NoError(t, clientTest.EditHealthcheck(name, hc, "", version))
356+
version++
357+
358+
_, got, err = clientTest.GetHealthcheck(name, "")
359+
require.NoError(t, err)
360+
require.Equal(t, "smtpchk", got.Type)
361+
require.Nil(t, got.HttpchkParams)
362+
363+
require.NoError(t, clientTest.DeleteHealthcheck(name, "", version))
364+
version++
365+
}
366+
367+
func TestCreateHealthcheckPgsqlCheckWithoutUser(t *testing.T) {
368+
name := fmt.Sprintf("created_pgsql_no_user_%d", version)
369+
hc := &models.HealthCheck{HealthCheckBase: models.HealthCheckBase{Name: name, Type: "pgsql-check"}}
370+
371+
require.Error(t, clientTest.CreateHealthcheck(hc, "", version))
372+
373+
_, _, err := clientTest.GetHealthcheck(name, "")
374+
require.Error(t, err)
375+
v, err := clientTest.GetVersion("")
376+
require.NoError(t, err)
377+
require.Equal(t, version, v)
378+
}

0 commit comments

Comments
 (0)