HCM overrides do not support 0-value overrides

[DoroNahari]

Gloo Edge Version

1.11.x (latest stable)

Kubernetes Version

1.22.x

Describe the bug

This feature #6414 presented an option to override settings with delegated gateways.
As described in the feature:
preventChildOverrides=false | {"parent":{"foo":"bar"}, "child":{"foo":"baz"}} --> {"foo":"baz"}

When setting a parent gateway with a boolean prop true and a child gateway with the same boolean prop false, the override is not working.

For example:
Parent gateway:

apiVersion: gateway.solo.io/v1
kind: Gateway
metadata:
  name: gateway-proxy-ssl
  namespace: gloo-system
spec:
  bindAddress: '::'
  bindPort: 8443
  hybridGateway:
    delegatedHttpGateways:
      httpConnectionManagerSettings:
        acceptHttp10: true
      selector:
        labels:
          hybrid-gateway: ssl
  proxyNames:
    - gateway-proxy
  ssl: true
  useProxyProto: false

Child gateway:

apiVersion: gateway.solo.io/v1
kind: MatchableHttpGateway
metadata:
  name: default-ssl-http-gateway-2
  namespace: apigw-gloo-ee
  labels:
    hybrid-gateway: ssl
spec:
  httpGateway:
    virtualServiceSelector:
      http-gateway: default-ssl
    options:
        httpConnectionManagerSettings:
          acceptHttp10: false

...

The observed behavior was that http1.0 yet was accepted

[gunnar-solo]

this is a real ...ish issue. It's mostly a logic problem. Consider the field SkipXffAppend, which has this autogenerated getter:

func (x *HttpConnectionManagerSettings) GetSkipXffAppend() bool {
	if x != nil {
		return x.SkipXffAppend
	}
	return false
}

because booleans cannot be nil, our current implementation gives us these possibilities:

parent | child  | outcome
------ | ------ | -------
true   | true   | true    # as-expected
true   | false  | true    # raises issue, as _optimally_, we want "false"
true   | unset  | true    # as-expected
false  | true   | true    # as-expected
false  | false  | false   # as-expected
false  | unset  | false   # as-expected
unset  | true   | true    # as-expected
unset  | false  | false   # as-expected
unset  | unset  | false   # as-expected

it is totally possible to logically write code to override true to false, exactly as we want. The problem comes when we consider all unset user fields will always be false in such a case.

consider such a case, where a child value of unset could override a parent's true:

parent | child  | outcome
------ | ------ | -------
true   | true   | true    # as-expected
true   | false  | true    # as-expected (we are now happy)
true   | unset  | false   # raises a _new_ issue
false  | true   | true    # as-expected
false  | false  | false   # as-expected
false  | unset  | false   # as-expected
unset  | true   | true    # as-expected
unset  | false  | false   # as-expected
unset  | unset  | false   # as-expected

[gunnar-solo]

mergo (the merging library we're currently using) calls this concept "overwriting with a 0 value" to be more data structure agnostic (consider enums, empty strings, the number 0...). Similar user issue.

By specifying mergo.WithOverwriteWithEmptyValue, we could adopt behavior that solves your original issue ...at the expense of unset values overwriting set values

[gunnar-solo]

Three possible approaches to resolve this issue:

• accept the limitations described above, on way or the other
• a large breaking change/mass deprecation where we convert all bools to *wrappers.BoolValue (so they can be nil)
• muck with our unmarshalling from crd code, such that we preserve/store the keys of resourceCrd.Spec as a list onto "some" field

[chrisgaun]

Related to #6532

[Tyzanol]

We have come across another scenario where this issue rises.
When setting a parent gateway with a non-0 integer prop, and a child gateway with a zero-valued prop, the override doesn't work.
This happens for instance when trying to override the 'HttpConnectionManagerSettings_CodecType' value.
The 0 valued child prop is not propagated, and therefore can't override the parent's non-0 value.

[LinMoskovitch]

We have come across another scenario where this issue rises. When setting a parent gateway with a non-0 integer prop, and a child gateway with a zero-valued prop, the override doesn't work. This happens for instance when trying to override the 'HttpConnectionManagerSettings_CodecType' value. The 0 valued child prop is not propagated, and therefore can't override the parent's non-0 value.
#6476 (comment)

Hi,
The issue when using HttpConnectionManagerSettings_CodecType = 0 still appears.

Seems that the type of HttpConnectionManagerSettings_CodecType is int32, instead of *int32

Thanks for your reference.