operator: add missing markers to condition type

[ironcladlou]

@sttts @deads2k @damemi @Miciah

See openshift/cluster-ingress-operator#301 (review).

I think the ingress and dns APIs are the only consumers of this type, and we've been persisting them as optional in 4.1 (example) and switched them to required in 4.2 (example).

Since we own the only known consumers, we can deal with any cleanup (and upgrade testing has not revealed any issues).

[ironcladlou]

I'm not sure I know how to assess just how risky this change is.

[ironcladlou]

@deads2k @Miciah if these have to stay optional then I think we need to fix https://github.com/openshift/cluster-ingress-operator/blob/release-4.2/manifests/00-custom-resource-definition.yaml#L708 in 4.2?

[Miciah]

/lgtm

[ironcladlou]

@sttts @deads2k can you please look at this one? Don't want to let it get away from us, and I especially need to understand if there are 4.2 implications

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ironcladlou, Miciah
To complete the pull request process, please assign eparis
You can assign the PR to them by writing /assign @eparis in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[sttts]

Very important to add: these fields are not omitempty and are only encoded as JSON. Hence, at worst we have "" as values for these two fields when generated from (typed) code.

[ironcladlou]

@sttts

Very important to add: these fields are not omitempty and are only encoded as JSON. Hence, at worst we have "" as values for these two fields when generated from (typed) code.

Hmm, does that mean it's safe to mark them required now?

[sttts]

Hmm, does that mean it's safe to mark them required now?

It means that it is safe if we can assume that neither a dynamic client has been involved in creating those conditions and no user manually removed those fields. Both assumptions are unlikely to be wrong.

[sttts]

@deads2k want to hear your opinion on #445 (comment)

[deads2k]

Hmm, does that mean it's safe to mark them required now?

It means that it is safe if we can assume that neither a dynamic client has been involved in creating those conditions and no user manually removed those fields. Both assumptions are unlikely to be wrong.

Don't tighten validation.

/close

[openshift-ci-robot]

@deads2k: Closed this PR.

In response to this:

Hmm, does that mean it's safe to mark them required now?

It means that it is safe if we can assume that neither a dynamic client has been involved in creating those conditions and no user manually removed those fields. Both assumptions are unlikely to be wrong.

Don't tighten validation.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.