Returning NamedAddress type and marking it deprecated

[robscott]

What type of PR is this?
/kind bug
/kind cleanup

What this PR does / why we need it:
This returns our NamedAddress type and marks it deprecated. Unfortunately I think we went a step too far when we removed it altogether. Although we've left room to add new values to enums in new minor versions, we have not provided any precedent for removing supported enum values. Unfortunately I think removing "NamedAddress" from the list of supported values would make it so HTTPRoutes that were valid in v0.4.x would not be valid in v0.5.x, which is something we should strive to avoid.

I think the best we can do here is continue to allow the value to be set but document that it has been deprecated in favor of domain prefixed strings. Hopefully this approach makes sense.

Does this PR introduce a user-facing change?:

NamedAddress type is back to support backwards compatibility but is formally deprecated.

[robscott]

Although relatively tiny in scope, this undoes part of a previous change, so want to make sure there's some consensus here.

/hold

[shaneutt]

I'm not necessarily strongly for or against adding this back. For posterity how many implementations were wanting this to be added back and what was the reasoning if other than "need more time to update"?

[robscott]

For posterity how many implementations were wanting this to be added back and what was the reasoning if other than "need more time to update"?

My primary argument here is that removing it would break our backwards compatibility guarantees. In general we've said we can loosen validation or add new valid values, but if something was at some point considered valid, we will not prevent it from being set in the future unless it was very clearly a bug in our validation. In this case it represented a defined and valid value, so I don't think we can get away with removing it.

We have removed suggested reasons for status from the API, but I think the important distinction there is that we still allow implementations to populate those reasons, we just don't define them as suggested reasons anymore.

In upstream Kubernetes we've sometimes tweaked defaults between API versions, but I don't think we've ever tightened validation.

[shaneutt]

My primary argument here is that removing it would break our backwards compatibility guarantees. In general we've said we can loosen validation or add new valid values, but if something was at some point considered valid, we will not prevent it from being set in the future unless it was very clearly a bug in our validation. In this case it represented a defined and valid value, so I don't think we can get away with removing it.

I understand. I'm going to approve as I understand and can appreciate your reasoning, and I don't have a reason to block. I do sometimes wonder if it would be better to take a harder and faster approach at these earlier maturity states of a project, but this still ends up being a pretty minor thing in either case.

[shaneutt]

/approve

[robscott]

I do sometimes wonder if it would be better to take a harder and faster approach at these earlier maturity states of a project, but this still ends up being a pretty minor thing in either case.

Yeah I generally agree with that approach, but unfortunately my understanding of k8s API guidelines is that although we could make a breaking change in alpha, that would need to be followed by another alpha API version, not a beta API version. I don't think this is significant enough of a change to merit another alpha API release.

[robscott]

Discussed this at today's community meeting and the change had broad consensus. As mentioned above, this change is to preserve backwards compatibility, the type should still be considered deprecated. Removing the hold on this.

/hold cancel

[shaneutt]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nathancoleman, robscott, shaneutt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [robscott,shaneutt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment