OCPBUGS-26498: Add UnservableInFutureVersions route status condition type

[gcs278]

Add UnservableInFutureVersions as a route status condition type. This change allows the router to indicate that a route is using an unsupported configuration and will be unservable in the future. The ingress operator may use this condition to block upgrades.

More details in forum-ocp-updates slack thread and latest updates discussion in forum-api-review slack thread

[openshift-ci-robot]

@gcs278: This pull request references Jira Issue OCPBUGS-26498, which is invalid:

• expected the bug to target the "4.16.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Add Upgradeable as a route status condition type. This functionality allows the router to indicate that a route is not upgradeable. The ingress operator will use this condition to set Upgradeable=False in the cluster operator status.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Hello @gcs278! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

[deads2k]

please describe what happens if this is false. Is the upgrade stopped? That's a lot of power to give a namespace editor compared a cluster-admin assessing a CVE for instance.

If the upgrade isn't stopped, how do we expect a cluster-admin to avoid accidentally causing this situation?

cc @sdodson

for discussion of how to handle upgradeability.

[gcs278]

please describe what happens if this is false. Is the upgrade stopped? That's a lot of power to give a namespace editor compared a cluster-admin assessing a CVE for instance.

I will update code comment soon - the cluster ingress operator will read the route's Upgradeable status and set the cluster operator ingress Upgradeable status which will prevent upgrades. So yes, the route owner can stop an upgrade, agreed.

See https://redhat-internal.slack.com/archives/CEGKQ43CP/p1704894930666179 discussion.

[openshift-ci-robot]

@gcs278: This pull request references Jira Issue OCPBUGS-26498, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

Details

In response to this:

Add Upgradeable as a route status condition type. This functionality allows the router to indicate that a route is not upgradeable. The ingress operator will use this condition to block upgrades.

More details in slack thread

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 openshift-eng/jira-lifecycle-plugin repository.

[gcs278]

@deads2k I'd like to push forward with this change. @Miciah and I discussed the API changes.

We currently have settled on a Upgradeable condition on the route after exploring using Deprecated because Deprecated doesn't exactly imply the route won't be upgradeable (and we want to imply that). We liked the flexibility of Deprecated, in that we could backport further to 4.14 and warn users even earlier. But this would require the ingress operator to parse the condition.reason field, to determine ingress upgradeable status, which seems kludgy.

The other option would be using annotation. That seemed quite kludgy too.

Please advise if you have any concerns or questions.

[gcs278]

/hold
Actually, NE discussed this again. We discussed in favor of using Deprecated due to its greater flexibility compared to Upgradeable. My assumption that we would need to parse the Deprecated message is not necessarily true (at least, for the initial implementation). I was designing a more generic solution in which we'd have "permanent" code (4.15+) in the ingress operator that would interpret Deprecated conditions and take upgradeable action based on their reason or message.

The solution is to just have the ingress operator upgradeable code temporary, so it only exists in the OCP version N-1 where N is the option (e.g. SHA1) is unsupported. The code would be make an assumption that any Deprecated route status in 4.15 is going to block upgrades via an admin ack.

It becomes more complex if we intend to reuse the deprecated status in the future to determine the co's ingress upgradeable=false because a user could have previously admin-ack'd an SHA1 route and it could still have the deprecated condition. In that case, we have to start parsing the deprecated condition's reason/message to ensure we don't pick up the old SHA1 deprecated condition, which would cause an admin to have to re-ack it. Not the biggest concern, but it's something we can handle later down the road if this scenario ever arises again. CC: @Miciah

[openshift-ci-robot]

@gcs278: This pull request references Jira Issue OCPBUGS-26498, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

Details

In response to this:

Add Deprecated as a route status condition type. This functionality allows the router to indicate that a route is using a deprecated configuration. The ingress operator may use this condition to block upgrades.

More details in slack thread

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 openshift-eng/jira-lifecycle-plugin repository.

[wking]

micro-nit, feel free to ignore.

If this set to true on any route, the ingress operator may block upgrades.

This seems like an internal implementation detail. In cases where the ingress operator decides to use Deprecated on Routes to drive Upgradeable=False, that Upgradeable ClusterOperator condition is the API talking to the CVO, and the CVO will in turn set Upgradeable=False on ClusterVersion, and that ClusterVersion condition is the API talking to the cluster-admin or other OCP-managing actor. I don't think we need to hint at that possible chain in this Route condition's docs. Instead, we can keep the first sentence explaining the deprecation aspect, which lets Route-admins know that they should be thinking about a config update. And then leave it to the cluster-admin-facing ClusterVersion API to talk about whether we're concerned about anything (including deprecated Route config) to talk to cluster-admins about that aspect.

But also, "may block upgrades" is wiggly enough that I doubt anyone will feel like we are surprising them regardless of whether the ingress operator decides to take any action based on this new Route condition. So no worries if you want to leave this sentence in as a semi-informative bread-crumb.

[gcs278]

thanks for the info - I remove it as I agree it's an implementation detail.

[gcs278]

Whoops, I forgot to remove this.

[openshift-ci-robot]

@gcs278: This pull request references Jira Issue OCPBUGS-26498, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @ShudiLi

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Add Deprecated as a route status condition type. This functionality allows the router to indicate that a route is using a deprecated configuration. The ingress operator may use this condition to block upgrades.

More details in forum-ocp-updates slack thread and latest updates discussion in forum-api-review slack thread

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 openshift-eng/jira-lifecycle-plugin repository.

[candita]

/assign @frobware
/assign @Miciah

[frobware]

/lgtm

[deads2k]

The condition should be more assertive of the problem. Deprecated is generally read in a "well someday I'll fix this" and we're actually highlight a security concern that we know will prevent this route from functioning in two versions.

UnservableInFutureVersions could carry a reason and message that we choose for each case where you need this case. The reason and message should emphasize the insecure aspect of the route.

[gcs278]

Done.

[gcs278]

There's enough discussion and agreement that the hold is no longer needed
/hold cancel
@deads2k @frobware @Miciah ready for review

[openshift-ci]

@gcs278: all tests passed!

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

[deads2k]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, frobware, gcs278

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [deads2k]

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

[openshift-ci-robot]

@gcs278: Jira Issue OCPBUGS-26498: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

• openshift/cluster-ingress-operator#1014 is open

These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with /jira refresh.

Jira Issue OCPBUGS-26498 has not been moved to the MODIFIED state.

Details

In response to this:

Add UnservableInFutureVersions as a route status condition type. This change allows the router to indicate that a route is using an unsupported configuration and will be unservable in the future. The ingress operator may use this condition to block upgrades.

More details in forum-ocp-updates slack thread and latest updates discussion in forum-api-review slack thread

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-bot]

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.16.0-202402021641.p0.g8b34b98.assembly.stream for distgit ose-cluster-config-api.
All builds following this will include this PR.