confusion around OutboundTrafficPolicy.Mode default; protobuf enum default makes this difficult

[jmazzitelli]

Bug Description

<TL;DR>
The protobuf default for OutboundTrafficPolicy.Mode is REGISTRY_ONLY. The docs, however, say the default is ALLOW_ANY. And because protobuf serialization eliminates the "mode" field even when it is explicitly set to the default value, the user is now confused. The docs and the code are not in agreement with what the default should be. In some places the docs explicitly declare ALLOW_ANY as the default (which, according to protobuf, it is not) and in some places the docs infer (and protobuf concurs) the default is REGISTRY_ONLY.
</TL;DR>

I'm not even sure what can be done here, other than perhaps enhanced documentation? I could argue that perhaps some code change is needed (to reverse the order of the protobuf enum OutboundTrafficPolicy.Mode so ALLOW_ANY is index #0 and thus the default value that protobuf uses). But I need to first explain a confusion that is being reported by users; hopefully someone knows how this can be solved.

This occurs when using Kial, but I believe it ultimately boils down to protobuf default value handling.

First, please see #43643 for the initial background of where the confusion is happening (someone creates a Sidecar resource, explicitly sets Sidecar.OutboundTrafficPolicy.Mode to REGISTRY_ONLY but then when looking at the YAML of that resource in Kiali, the mode field is missing). This has to do with the fact that protobuf does not serialize enum values (even if they are explicitly set) when those values are the defaults.

The question the user now asks is, "Well, if mode is missing, then I assume the system will use the default - what is that default?"

Here is where the confusion lies.

First, see https://istio.io/latest/docs/tasks/traffic-management/egress/egress-control/#envoy-passthrough-to-external-services where it says, "Istio has an installation option, meshConfig.outboundTrafficPolicy.mode, that configures the sidecar handling of external services ... ALLOW_ANY is the default value, allowing you to start evaluating Istio quickly`

OK, so the docs say ALLOW_ANY is the default.

Now go to the Sidecar resource docs - look in the outboundTrafficPolicy row in the table at https://istio.io/latest/docs/reference/config/networking/sidecar/#Sidecar where it says, "Configuration for the outbound traffic policy. . . . If not specified, inherits the system detected defaults from the namespace-wide or the global default Sidecar."

OK, fair enough. If the entire outboundTrafficPolicy isn't specified in the Sidecar resource, then it just defaults to that global setting. But the outboundTrafficPolicy is set, it is just that protobuf has unset mode (even though it was explicitly set to REGISTRY_ONLY - because that is the protobuf default, it has been removed when the object was serialized).

But now go to that link the first doc referenced: "Istio has an installation option, meshConfig.outboundTrafficPolicy.mode" and look at the table - specifically the ordering of the rows in the table (i.e. notice REGISTRY_ONLY is first in the list):

Notice no where in the docs here does it explicitly indicate which is the default. That infers the the default is REGISTRY_ONLY (since it is first in the list). Indeed, that is exactly how protobuf determines the default value for enums and it is how the generated source shows it in sidecar.pb.go:

type OutboundTrafficPolicy_Mode int32

const (
	// Outbound traffic will be restricted to services defined in the
	// service registry as well as those defined through `ServiceEntry` configurations.
	OutboundTrafficPolicy_REGISTRY_ONLY OutboundTrafficPolicy_Mode = 0
	// Outbound traffic to unknown destinations will be allowed, in case
	// there are no services or `ServiceEntry` configurations for the destination port.
	OutboundTrafficPolicy_ALLOW_ANY OutboundTrafficPolicy_Mode = 1
)

// Enum value maps for OutboundTrafficPolicy_Mode.
var (
	OutboundTrafficPolicy_Mode_name = map[int32]string{
		0: "REGISTRY_ONLY",
		1: "ALLOW_ANY",
	}
	OutboundTrafficPolicy_Mode_value = map[string]int32{
		"REGISTRY_ONLY": 0,
		"ALLOW_ANY":     1,
	}
)

Hopefully you can see the problem. The protobuf code default is REGISTRY_ONLY. The docs say the default is ALLOW_ANY. And because the protobuf serialization eliminates the "mode" field even when it is explicitly set to the default value, the user is now confused. They do not see the value in the YAML, so they assume it is the default, and the docs aren't clear what that default is! In some places it explicitly declares ALLOW_ANY as the default (which, according to protobuf, it is not) and in some places it infers (and protobuf concurs) the default is REGISTRY_ONLY.

Chaos ensues.

How do we solve this so users are not confused?

Version

This is related to the API, not any runtime. But this is applicable to at least 1.17 if not prior.

Additional Information

No response

[jmazzitelli]

Here's an illustration of the confusion that this causes.

Create a Sidecar resource with outboundTrafficPolicy.mode explicitly set to REGISTRY_ONLY. In Kiali, it looks like this (because protobuf removes the mode value, you will see mode is missing from the outboundTrafficPolicy):

But notice that the mode value is explicitly defined:

$ kubectl get sidecars.networking.istio.io default -n mazz -o jsonpath='{.spec.outboundTrafficPolicy}{"\n"}'
{"mode":"REGISTRY_ONLY"}

Now, in the Kiali UI editor (or any editor), unset the mode - so there is literally no value for outboundTrafficPolicy.mode. After you edit it, you should see that outboundTrafficPolicy is empty:

$ kubectl get sidecars.networking.istio.io default -n mazz -o jsonpath='{.spec.outboundTrafficPolicy}{"\n"}'
{}

Now go to Kiali UI and look at the resource YAML - it is identical. You still see this:

And therein lies the problem. The YAML representation is outboundTrafficPolicy: {} BOTH when mode is set to REGISTRY_ONLY and when mode is unset (which, according to the documentation, Istio will assume a default value of ALLOW_ANY).

So the user is now confused - she sees an unset mode and really doesn't know what default will take effect. It could be REGISTRY_ONLY, or it could be ALLOW_ANY. There is no way to tell looking at the YAML.

[howardjohn]

when mode is unset (which, according to the documentation, Istio will assume a default value of ALLOW_ANY).

Not quite true I think

outboundTrafficPolicy: {}: REGISTRY_ONLY (due to proto)
outboundTrafficPolicy: null (or more likely, not present at all): ALLOW_ANY, due to Istio specific code

I don't think this is a good state, which is why normally the 0 enum is UNSPECIFIED, but its hard to change an API at this point

[istio-policy-bot]

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2023-02-28. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.