Validation to prevent deletion of actively referenced Revisions

[whaught]

/area API

Intended as follow up to: #8208

Describe the feature

We will start labelling revisions with pending/active/retired states such that all in-progress deployments and actively serving revisions should be marked. We also recently added DELETE support to the webhooks for validation.

We should

• Create a feature flag for prevent-actively-serving-delete validation
• Create a new hook that fails validation when pending/active are present on a revision
• e2e test trying to delete an active revision
• e2e test that cascading deletions are not blocked

This feature can remain optional, but it is likely that users don't intend to delete revisions that are active/pending. This will make it important to ensure pending states are resolved within the pending deployment time. We will want to give users a nice error message about why their deletion request has failed and may want to consider a --force option to bypass validation (actually we could consider such a header generically for bypassing validation, maybe that's a separate issue).

[whaught]

/assign @whaught

[whaught]

/assign @nthnca

[vagababov]

Did we bikeshed this anywhere?

[whaught]

Did we bikeshed this anywhere?

I've been busy with populating the new states and the new GC alg, but @nthnca expressed a desire to pick something up. We could bring this up in the WG sometime if we want to discuss/propose the details for this one

[vagababov]

Yeah, since it's not 100% clear that we actually want this feature? (I mean it's a reasonable thing to do, but 🤷 )

[whaught]

The motivation behind this proposal is that knative's goal is to make deployment easy. It might be in our interest to, by default, prevent customers from shooting themselves in the foot by deleting their serving revision (but leave a path to do so if you really really meant to)

[duglin]

I would think normal Knative users won't touch the underlying objects much (if at all) and instead only deal with the ksvc. So if they're going out of their way to delete a revision I suspect they probably have a good reason to do so.

This feels a bit like we're crossing the line into being too parental.

[nthnca]

Discussed in Serving API working group: https://docs.google.com/document/d/1NC4klOdNaU-N-PsKLyXBqDKgNSHtxCDep29Ta2b5FK0/edit#heading=h.e2ybf6vi6e2z

General notes:

• Is this a good idea: Yes.
• Concerns: We don't want to change the behaviour of kubectl. A kubectl user shouldn't have to dig into understanding knative to be able to do what they can normally do. This validation being the new default and then asking for a force flag if you want to override will change the default behavior of kubectl.
• Thoughts: Instead of using a force, use a --want-validation type of flag. This would allow the knative client and other 3rd party clients to opt-in to this validation if they would like and would leave kubectl alone.
• Thoughts: instead of making deletion a command, what if make deletion a part of the spec, then the revision will just get automatically deleted once it is safe. -- This seems a little awkward, not clear that this is an improvement.

[nthnca]

I haven't had time to work on this. As a result unassigning myself for now to let others pick it up if they would like.