Feature/allow ipv6 disable annotation #647
Conversation
|
Hi Andrew. Thank you for your PR. We will review it as soon as possible. |
| @@ -56,18 +57,22 @@ type proxyConfig struct { | |||
| proxyIngressPort int64 | |||
| // UID used by proxy | |||
| proxyUID int64 | |||
| // todo | |||
| enableIPV6 *bool | |||
There was a problem hiding this comment.
Does this need to be a *bool? Why not just a bool?
There was a problem hiding this comment.
I found when using a bool instead of *bool, and enableIPV6 was not set explicitly in proxyConfig, that the default value was false. This resulted in unexpected value of APPMESH_ENABLE_IPV6: 0 in the proxyinit env vars.
This allows checking for m.proxyConfig.enableIPV6 != nil && !*m.proxyConfig.enableIPV6 ensuring that the default value of APPMESH_ENABLE_IPV6 remains 1.
There was a problem hiding this comment.
Ah understood. Can you add a comment explaining this? I want to make sure the next person who looks at the code understands this is to set the default to 1 when the variable is unset.
Issue #, if available:
Implements the feature described in #646
Description of changes:
Similar to how annotations on the pod are exposed to configure options on the proxyinit container, this feature adds an annotation to set
APPMESH_ENABLE_IPV6=0in the proxyinit container.Adds tests to ensure the default
APPMESH_ENABLE_IPV6=1is set when the annotation is unset as well as testsAPPMESH_ENABLE_IPV6=0when the annotation is set toappmesh.k8s.aws/ipv6: disabled.Additionally I ran
makelocally which made changes toapis/appmesh/v1beta2/zz_generated.deepcopy.go.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.