Skip to content

Commit

Permalink
BUG/MEDIUM: fix HTTPConnectionMode vs HTTPServerClose HTTPKeepAlive a…
Browse files Browse the repository at this point in the history 
…nd Httpclose fields
  • Loading branch information
@hdurand0710
hdurand0710 committed Aug 1, 2023
1 parent a0b117c commit 2081a28
Show file tree
Hide file tree
Showing 4 changed files with 311 additions and 19 deletions.
233 changes: 232 additions & 1 deletion configuration/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,8 +836,8 @@ func TestCreateEditDeleteBackend(t *testing.T) {
Secure: true,
Type: "rewrite",
},
// Use non deprecated option only
HTTPConnectionMode: "httpclose",
Httpclose: "enabled",
ConnectTimeout: &tOut,
StickTable: &models.ConfigStickTable{
Expire: &e,
Expand Down Expand Up @@ -887,6 +887,8 @@ func TestCreateEditDeleteBackend(t *testing.T) {
Level: "warning",
Mailers: misc.StringP("localmailer1"),
},
// Use deprecated option only
Httpclose: "enabled",
Originalto: &models.Originalto{
Enabled: misc.StringP("enabled"),
Header: "X-Client-Dst",
Expand Down Expand Up @@ -1182,5 +1184,234 @@ func compareBackends(x, y *models.Backend, t *testing.T) bool { //nolint:gocogni
x.Originalto = nil
y.Originalto = nil

// Due to deprecated fields Httpclose, HTTPKeepAlive, HTTPServerClose
// in favor of HTTPConnectionMode
// If HTTPConnectionMode is set in original backend
// - Httpclose, HTTPKeepAlive, HTTPServerClose will be set in updated backend, even if not present in original backend
// If HTTPConnectionMode is unset in original backend:
// - it will be set in updated backend
switch y.HTTPConnectionMode {
case "http-keep-alive":
if x.HTTPKeepAlive != "enabled" {
return false
}
x.HTTPKeepAlive = ""
case "http-server-close":
if x.HTTPServerClose != "enabled" {
return false
}
x.HTTPServerClose = ""
case "httpclose":
if x.Httpclose != "enabled" {
return false
}
x.Httpclose = ""
case "":
x.HTTPConnectionMode = ""
}

return reflect.DeepEqual(x, y)
}

func TestCreateEditDeleteBackendHTTPConnectionMode(t *testing.T) {
// TestCreateBackend
tOut := int64(5)

// Backend with HTTPConnectionMode only
b := &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPConnectionMode: "http-keep-alive",
}

err := clientTest.CreateBackend(b, "", version)
if err != nil {
t.Error(err.Error())
} else {
version++
}

v, backend, err := clientTest.GetBackend("special-httpconnectionmode", "")
if err != nil {
t.Error(err.Error())
}

if backend.HTTPConnectionMode != "http-keep-alive" {
t.Errorf("Created backend is not correct for HTTPConnectionMode: %s", backend.HTTPConnectionMode)
}
if backend.HTTPKeepAlive != "enabled" {
t.Errorf("Created backend is not correct for HTTPKeepAlive: %s", backend.HTTPConnectionMode)
}

if v != version {
t.Errorf("Version %v returned, expected %v", v, version)
}

err = clientTest.CreateBackend(b, "", version)
if err == nil {
t.Error("Should throw error bck already exists")
version++
}

type testinput struct {
backend *models.Backend
expectedHTTPConnectionMode string
expectedHTTPKeepAlive string
expectedHttpclose string
exptectedHTTPServerClose string
}
// TestEditBackend
inputs := []testinput{
{
// Update HTTPConnectionMode
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPConnectionMode: "httpclose",
},
expectedHTTPConnectionMode: "httpclose",
expectedHTTPKeepAlive: "",
exptectedHTTPServerClose: "",
expectedHttpclose: "enabled",
},
{
// Use only deprecated option
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPServerClose: "enabled",
},
expectedHTTPConnectionMode: "http-server-close",
expectedHTTPKeepAlive: "",
exptectedHTTPServerClose: "enabled",
expectedHttpclose: "",
},
{
// Use both - Priority on HTTPConnection
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPConnectionMode: "http-keep-alive",
HTTPServerClose: "enabled",
},
expectedHTTPConnectionMode: "http-keep-alive",
expectedHTTPKeepAlive: "enabled",
exptectedHTTPServerClose: "",
expectedHttpclose: "",
},
{
// no option with deprecated option
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPServerClose: "disabled",
},
expectedHTTPConnectionMode: "", // not possible to set no option with this field
expectedHTTPKeepAlive: "",
exptectedHTTPServerClose: "disabled",
expectedHttpclose: "",
},
{
// set back with HTTPConnectionMode
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPConnectionMode: "httpclose",
},
expectedHTTPConnectionMode: "httpclose",
expectedHTTPKeepAlive: "",
exptectedHTTPServerClose: "",
expectedHttpclose: "enabled",
},
{
// remove option
backend: &models.Backend{
Name: "special-httpconnectionmode",
Mode: "http",
DefaultServer: &models.DefaultServer{
ServerParams: models.ServerParams{
Fall: &tOut,
Inter: &tOut,
},
},
HTTPConnectionMode: "",
},
expectedHTTPConnectionMode: "",
expectedHTTPKeepAlive: "",
exptectedHTTPServerClose: "",
expectedHttpclose: "",
},
}

for i, input := range inputs {
err := clientTest.EditBackend("special-httpconnectionmode", input.backend, "", version)
if err != nil {
t.Error(err.Error())
} else {
version++
}

_, backend, err := clientTest.GetBackend("special-httpconnectionmode", "")
if err != nil {
t.Error(err.Error())
}

if backend.HTTPConnectionMode != input.expectedHTTPConnectionMode {
t.Errorf("Updated backend %d is not correct for HTTPConnectionMode: %s", i, backend.HTTPConnectionMode)
}
if backend.HTTPKeepAlive != input.expectedHTTPKeepAlive {
t.Errorf("Updated backend %d is not correct for HTTPKeepAlive: %s", i, backend.HTTPConnectionMode)
}
if backend.HTTPServerClose != input.exptectedHTTPServerClose {
t.Errorf("Updated backend %d is not correct for HTTPServerClose: %s", i, backend.HTTPServerClose)
}
if backend.Httpclose != input.expectedHttpclose {
t.Errorf("Updated backend %d is not correct for Httpclose: %s", i, backend.Httpclose)
}

}

// TestDeleteBackend
err = clientTest.DeleteBackend("special-httpconnectionmode", "", version)
if err != nil {
t.Error(err.Error())
} else {
version++
}
}
91 changes: 73 additions & 18 deletions configuration/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,11 @@ type SectionObject struct {
Section parser.Section
Name string
Parser parser.Parser
// In the context of the deprecation of the fields:
// HTTPKeepAlive, HTTPServerClose and Httpclose.
// This flag is used to set a priority on HTTPConnectionMode field over
// the deprecated ones.
httpConnectionModeFlag bool
}

// CreateEditSection creates or updates a section in the parser based on the provided object
Expand All @@ -1424,6 +1429,7 @@ func CreateEditSection(object interface{}, section parser.Section, pName string,
Name: pName,
Parser: p,
}
so.setHTTPConnectionModeFlag()
return so.CreateEditSection()
}

Expand Down Expand Up @@ -1526,6 +1532,11 @@ func (s *SectionObject) checkSpecialFields(fieldName string, field reflect.Value
case "DefaultBackend":
return true, s.defaultBackend(field)
case "HTTPConnectionMode":
// if HTTPConnectionMode is not set, skip HTTPConnectionMode
// Write only options (Httpclose, HTTPKeepAlive, HTTPServerClose)
if !s.httpConnectionModeFlag {
return true, nil
}
return true, s.httpConnectionMode(field)
case "HTTPReuse":
return true, s.httpReuse(field)
Expand Down Expand Up @@ -1596,8 +1607,70 @@ func (s *SectionObject) checkTimeouts(fieldName string, field reflect.Value) (ma
return false, nil
}

// setHTTPConnectionModeFlag set the httpConnectionModeFlag flag if:
//
// HTTPConnectionMode is present, false otherwise.
//
// This check is needed due to the deprecation of deprecated options:
//
// HTTPKeepAlive, HTTPServerClose and Httpclose.
func (s *SectionObject) setHTTPConnectionModeFlag() {
objValue := reflect.ValueOf(s.Object)
if objValue.Kind() == reflect.Ptr {
objValue = reflect.ValueOf(s.Object).Elem()
}

deprecatedFieldPresent := false
atLeastOneDeprecatedFieldNotEmpty := false
httpConnectionModePresentSet := false
for i := 0; i < objValue.NumField(); i++ {
typeField := objValue.Type().Field(i)
field := objValue.FieldByName(typeField.Name)
if typeField.Name == "HTTPConnectionMode" && !valueIsNil(field) {
httpConnectionModePresentSet = true
}
if typeField.Name == "HTTPKeepAlive" || typeField.Name == "HTTPServerClose" || typeField.Name == "Httpclose" {
deprecatedFieldPresent = true
if !valueIsNil(field) {
atLeastOneDeprecatedFieldNotEmpty = true
}
}
}
// Backend
if deprecatedFieldPresent {
// if HTTPConnectionMode is present is not empty => has priority
if httpConnectionModePresentSet {
s.httpConnectionModeFlag = true
return
}

s.httpConnectionModeFlag = !atLeastOneDeprecatedFieldNotEmpty
return
}
// For Default and Frontend,
// HTTPKeepAlive, HTTPServerClose and Httpclose do not exist in the Object
// We are always using HTTPConnectionMode
s.httpConnectionModeFlag = true
}

// isHTTPConnectionModeDeprecatedField returns, in regards to the deprecation of HTTPConectionMode option fields:
//
// HTTPKeepAlive, HTTPServerClose and Httpclose.
//
// - true if it's a deprecated field, false otherwise
func isHTTPConnectionModeDeprecatedField(fieldName string) bool {
if fieldName == "HTTPKeepAlive" || fieldName == "HTTPServerClose" || fieldName == "Httpclose" {
return true
}
return false
}

func (s *SectionObject) checkOptions(fieldName string, field reflect.Value) (match bool, err error) {
if pName := fmt.Sprintf("option %s", misc.DashCase(fieldName)); s.Parser.HasParser(s.Section, pName) {
// Skip this field if it's a deprecated one and HTTPConnectionMode is present
if isHTTPConnectionModeDeprecatedField(fieldName) && s.httpConnectionModeFlag {
return true, nil
}
if valueIsNil(field) {
if err := s.set(pName, nil); err != nil {
return true, err
Expand Down Expand Up @@ -1638,20 +1711,6 @@ func (s *SectionObject) checkSingleLine(fieldName string, field reflect.Value) (
return false, nil
}

func (s *SectionObject) isNoOption(attribute string) bool {
data, err := s.Parser.Get(s.Section, s.Name, attribute)
if err != nil {
return false
}

simpleOpt, ok := data.(*types.SimpleOption)
if !ok {
return false
}

return simpleOpt.NoOption
}

func (s *SectionObject) set(attribute string, data interface{}) error {
return s.Parser.Set(s.Section, s.Name, attribute, data)
}
Expand Down Expand Up @@ -1784,10 +1843,6 @@ func (s *SectionObject) httpConnectionMode(field reflect.Value) error {
for _, opt := range []string{"httpclose", "http-server-close", "http-keep-alive"} {
attribute := fmt.Sprintf("option %s", opt)

if s.isNoOption(attribute) {
continue
}

if err := s.set(attribute, nil); err != nil {
return err
}
Expand Down
Loading

0 comments on commit 2081a28

Please sign in to comment.