Delete field for instance and binding

cmd/svcat/testdata/output/get-binding.json

@@ -23,7 +23,8 @@
          }
       },
       "secretName": "ups-binding",
-      "externalID": "061e1d78-d27e-4958-97b8-e9f5aa2f99d7"
+      "externalID": "061e1d78-d27e-4958-97b8-e9f5aa2f99d7",
+      "deletionRequested": null
    },
    "status": {
       "conditions": [

cmd/svcat/testdata/output/get-binding.yaml

@@ -9,6 +9,7 @@ metadata:
   selfLink: /apis/servicecatalog.k8s.io/v1beta1/namespaces/test-ns/servicebindings/ups-binding
   uid: 7f2aefa0-f712-11e7-aa44-0242ac110005
 spec:
+  deletionRequested: null
   externalID: 061e1d78-d27e-4958-97b8-e9f5aa2f99d7
   instanceRef:
     name: ups-instance

cmd/svcat/testdata/output/get-bindings.json

@@ -23,7 +23,8 @@
             },
             "parameters": {},
             "secretName": "ups-binding",
-            "externalID": "061e1d78-d27e-4958-97b8-e9f5aa2f99d7"
+            "externalID": "061e1d78-d27e-4958-97b8-e9f5aa2f99d7",
+            "deletionRequested": null
          },
          "status": {
             "conditions": [

cmd/svcat/testdata/output/get-bindings.yaml

@@ -10,6 +10,7 @@ items:
     selfLink: /apis/servicecatalog.k8s.io/v1beta1/namespaces/test-ns/servicebindings/ups-binding
     uid: 7f2aefa0-f712-11e7-aa44-0242ac110005
   spec:
+    deletionRequested: null
     externalID: 061e1d78-d27e-4958-97b8-e9f5aa2f99d7
     instanceRef:
       name: ups-instance

cmd/svcat/testdata/output/get-instance.json

@@ -28,7 +28,8 @@
          }
       },
       "externalID": "7e2c42f3-6d94-4409-bb15-7610d60af544",
-      "updateRequests": 0
+      "updateRequests": 0,
+      "deletionRequested": null
    },
    "status": {
       "conditions": [

cmd/svcat/testdata/output/get-instance.yaml

@@ -15,6 +15,7 @@ spec:
   clusterServicePlanExternalName: default
   clusterServicePlanRef:
     name: 86064792-7ea2-467b-af93-ac9694d96d52
+  deletionRequested: null
   externalID: 7e2c42f3-6d94-4409-bb15-7610d60af544
   parameters:
     param1: value1

cmd/svcat/testdata/output/get-instances.json

@@ -28,7 +28,8 @@
             },
             "parameters": {},
             "externalID": "7e2c42f3-6d94-4409-bb15-7610d60af544",
-            "updateRequests": 0
+            "updateRequests": 0,
+            "deletionRequested": null
          },
          "status": {
             "conditions": [

cmd/svcat/testdata/output/get-instances.yaml

@@ -16,6 +16,7 @@ items:
     clusterServicePlanExternalName: default
     clusterServicePlanRef:
       name: 86064792-7ea2-467b-af93-ac9694d96d52
+    deletionRequested: null
     externalID: 7e2c42f3-6d94-4409-bb15-7610d60af544
     parameters: {}
     updateRequests: 0

docs/proposals/2018-05-declarative-delete-field.md

@@ -0,0 +1,83 @@
+# Declarative Delete Field
+
+Concept committed to [during 2018 April 12F2F](https://docs.google.com/document/d/1O7_fws7hwZ6qV3okAjbV5qdFaEq4bt1LYJ9bTu-IwvU)
+
+This document 2018-05.
+
+## Abstract
+
+Add a field to instance and binding to indicate that they have been asked to
+send a DELETE to the backend.
+
+## Motivation
+
+When the Originating-Identity header is used, the broker and refuse a delete
+call when the user info in the Originating-Identity header does not have
+permissions to delete the instance or binding. As this happens after kubernetes
+has accepted the DELETE as an true indication to do a delete, it has set the
+DeletionTimestamp, which is an irreversible step on the way to carrying out the
+necessary actions to delete a kubernetes resource.
+
+## Proposed Design
+
+ - Add a field to indicate deletion
+ - Add a condition to indicate permanent end state of deletion success.
+
+Once the field is activated the controller deprovisions the broker
+resource. When successfully deleted the controller then deletes the
+kubernetes resource.
+
+A feature flag to enable/disable this behavior.
+
+RBAC changes to allow the controller to issue deletes.
+
+Helm chart changes to enable the flag and the rbac rules.
+
+### API Resource Changes
+
+Add a condition for ServiceInstanceConditionType, ServiceBindingConditionType.
+```go
+  ServiceInstanceConditionDeleted ServiceBindingConditionType = "Deleted"
+  ServiceBindingConditionDeleted ServiceBindingConditionType = "Deleted"
+```
+
+Add a field to spec of ServiceInstance, ServiceBinding.
+```go
+  ExistsExternally bool `json:"existsExternally,omitempty"`
+```
+
+This field is set to true by default. The user indicates that it wants
+to delete the backing resource by setting this field to false. The
+controller will see this update and attempt to delete the backing
+broker resource. If the broker delete succeeds, it sets the deleted
+status condition.
+
+If the field is set to false, no updates are allowed besides
+setting the field back to true. 
+
+Once the status is changed to deleted the controller should issue the
+DELETE to the apiserver removing the resource.
+
+### API Server Changes
+
+Additional field results in generated code changes.
+
+Validation/Strategy to prevent changes after a deletion is started and is successful or
+until the deletion is rolled back.
+
+### Controller-Manager Changes
+
+Additional condition to determine delete flow is added. This condition is
+dependent on the state of the new field. Reuse all applicable status
+objects as appropriate.
+
+Proceed through the normal deletion flow and if the broker allows the delete,
+set a new condition indicating the delete has occurred. If the delete has
+occurred, when running the finalizer, allow the delete to finish and occur
+without any additional actions involving broker communication.
+
+
+
+### SVCat CLI Changes
+
+New types result in test input/output changes to golden files.

pkg/apis/servicecatalog/types.go

@@ -774,6 +774,12 @@ type ServiceInstanceSpec struct {
 	// allows for parameters to be updated with any out-of-band changes that have
 	// been made to the secrets from which the parameters are sourced.
 	UpdateRequests int64
+
+	// DeletionRequested provides a mechanism for requesting a delete without
+	// performing a direct delete (which cannot be undone in cases where the
+	// delete is rejected by the broker). This is the recommended way to
+	// delete, to avoid resources becoming stuck in the deletion phase.
+	DeletionRequested metav1.Time
 }
 
 // ServiceInstanceStatus represents the current status of an Instance.
@@ -1025,6 +1031,12 @@ type ServiceBindingSpec struct {
 	// settable by the end-user. User-provided values for this field are not saved.
 	// +optional
 	UserInfo *UserInfo
+
+	// DeletionRequested provides a mechanism for requesting a delete without
+	// performing a direct delete (which cannot be undone in cases where the
+	// delete is rejected by the broker). This is the recommended way to
+	// delete, to avoid resources becoming stuck in the deletion phase.
+	DeletionRequested metav1.Time
 }
 
 // ServiceBindingStatus represents the current status of a ServiceBinding.

pkg/apis/servicecatalog/v1beta1/types.go

@@ -865,6 +865,12 @@ type ServiceInstanceSpec struct {
 	// been made to the secrets from which the parameters are sourced.
 	// +optional
 	UpdateRequests int64 `json:"updateRequests"`
+
+	// DeletionRequested provides a mechanism for requesting a delete without
+	// performing a direct delete (which cannot be undone in cases where the
+	// delete is rejected by the broker). This is the recommended way to
+	// delete, to avoid resources becoming stuck in the deletion phase.
+	DeletionRequested metav1.Time `json:"deletionRequested,omitempty"`
 }
 
 // ServiceInstanceStatus represents the current status of an Instance.
@@ -1127,6 +1133,12 @@ type ServiceBindingSpec struct {
 	// settable by the end-user. User-provided values for this field are not saved.
 	// +optional
 	UserInfo *UserInfo `json:"userInfo,omitempty"`
+
+	// DeletionRequested provides a mechanism for requesting a delete without
+	// performing a direct delete (which cannot be undone in cases where the
+	// delete is rejected by the broker). This is the recommended way to
+	// delete, to avoid resources becoming stuck in the deletion phase.
+	DeletionRequested metav1.Time `json:"deletionRequested,omitempty"`
 }
 
 // ServiceBindingStatus represents the current status of a ServiceBinding.