Revert to a string BindingPhase

This effectively reverts crossplane/crossplane#325.

I still think it would be ideal to represent BindingState as an int with a sane
zero value that marshaled to a JSON string, but it is currently impossible to
override the type of the field that is used when generating an OpenAPI spec per
kubernetes-sigs/controller-tools#155. Until that issue
is closed it seems better to simply make this a string with a meaningless zero
value.

Signed-off-by: Nic Cope <[email protected]>

Author: negz

pkg/apis/core/v1alpha1/bindingphase.go

@@ -14,60 +14,33 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-//go:generate go run ../../../../vendor/golang.org/x/tools/cmd/stringer/stringer.go ../../../../vendor/golang.org/x/tools/cmd/stringer/importer19.go -type=BindingPhase -trimprefix BindingPhase
-
 package v1alpha1
 
-import (
-	"encoding/json"
-
-	"github.com/pkg/errors"
-)
-
-// BindingPhase is to identify the current binding status of given resources
-type BindingPhase int
-
-// MarshalJSON returns a JSON representation of a BindingPhase.
-func (p BindingPhase) MarshalJSON() ([]byte, error) {
-	return json.Marshal(p.String())
-}
-
-// UnmarshalJSON returns a BindingPhase from its JSON representation.
-func (p *BindingPhase) UnmarshalJSON(b []byte) error {
-	var ps string
-	if err := json.Unmarshal(b, &ps); err != nil {
-		return err
-	}
-	switch ps {
-	case BindingPhaseUnbindable.String():
-		*p = BindingPhaseUnbindable
-	case BindingPhaseUnbound.String():
-		*p = BindingPhaseUnbound
-	case BindingPhaseBound.String():
-		*p = BindingPhaseBound
-	default:
-		return errors.Errorf("unknown binding state %s", ps)
-	}
-	return nil
-}
+// BindingPhase represents the current binding phase of a resource or claim.
+type BindingPhase string
 
 // Binding phases.
 const (
+	// BindingPhaseUnknown resources cannot be bound to another resource because
+	// they are in an unknown binding phase.
+	BindingPhaseUnknown BindingPhase = ""
+
 	// BindingPhaseUnbindable resources cannot be bound to another resource, for
 	// example because they are currently unavailable, or being created.
-	BindingPhaseUnbindable BindingPhase = iota
+	BindingPhaseUnbindable BindingPhase = "Unbindable"
 
 	// BindingPhaseUnbound resources are available for binding to another
 	// resource.
-	BindingPhaseUnbound
+	BindingPhaseUnbound BindingPhase = "Unbound"
 
 	// BindingPhaseBound resources are bound to another resource.
-	BindingPhaseBound
+	BindingPhaseBound BindingPhase = "Bound"
 )
 
 // A BindingStatus represents the bindability and binding of a resource.
 type BindingStatus struct {
 	// Phase represents the binding phase of the resource.
+	// +kubebuilder:validation:Enum=Unbindable,Unbound,Bound
 	Phase BindingPhase `json:"bindingPhase,omitempty"`
 }
 

pkg/apis/core/v1alpha1/bindingphase_string.go

@@ -351,7 +351,7 @@ func (r *ClaimReconciler) Reconcile(req reconcile.Request) (reconcile.Result, er
 		}
 	}
 
-	if managed.GetBindingPhase() == v1alpha1.BindingPhaseUnbindable {
+	if !IsBindable(managed) && !IsBound(managed) {
 		// If this claim was not already binding we'll be requeued due to the
 		// status update. Otherwise there's no need to requeue. We should be
 		// watching both the resource claims and the resources we own, so we'll
@@ -360,7 +360,7 @@ func (r *ClaimReconciler) Reconcile(req reconcile.Request) (reconcile.Result, er
 		return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, claim), errUpdateClaimStatus)
 	}
 
-	if managed.GetBindingPhase() == v1alpha1.BindingPhaseUnbound {
+	if IsBindable(managed) {
 		if err := r.managed.PropagateConnection(ctx, claim, managed); err != nil {
 			// If we didn't hit this error last time we'll be requeued implicitly
 			// due to the status update. Otherwise we want to retry after a brief

pkg/apis/core/v1alpha1/bindingphase_test.go

@@ -381,7 +381,7 @@ func TestReconcile(t *testing.T) {
 			},
 			want: want{result: reconcile.Result{RequeueAfter: aShortWait}},
 		},
-		"ManagedIsUnbindable": {
+		"ManagedIsInUnknownBindingPhase": {
 			args: args{
 				m: &MockManager{
 					c: &test.MockClient{
@@ -393,6 +393,9 @@ func TestReconcile(t *testing.T) {
 								*o = *cm
 								return nil
 							case *MockManaged:
+								// We do not explicitly set a BindingPhase here
+								// because the zero value of BindingPhase is
+								// BindingPhaseUnknown.
 								mg := &MockManaged{}
 								mg.SetCreationTimestamp(now)
 								*o = *mg
@@ -418,6 +421,44 @@ func TestReconcile(t *testing.T) {
 			},
 			want: want{result: reconcile.Result{Requeue: false}},
 		},
+		"ManagedIsInUnbindableBindingPhase": {
+			args: args{
+				m: &MockManager{
+					c: &test.MockClient{
+						MockGet: test.NewMockGetFn(nil, func(o runtime.Object) error {
+							switch o := o.(type) {
+							case *MockClaim:
+								cm := &MockClaim{}
+								cm.SetResourceReference(&corev1.ObjectReference{})
+								*o = *cm
+								return nil
+							case *MockManaged:
+								mg := &MockManaged{}
+								mg.SetCreationTimestamp(now)
+								mg.SetBindingPhase(v1alpha1.BindingPhaseUnbindable)
+								*o = *mg
+								return nil
+							default:
+								return errUnexpected
+							}
+						}),
+						MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error {
+							want := &MockClaim{}
+							want.SetResourceReference(&corev1.ObjectReference{})
+							want.SetConditions(Binding(), v1alpha1.ReconcileSuccess())
+							if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" {
+								t.Errorf("-want, +got:\n%s", diff)
+							}
+							return nil
+						}),
+					},
+					s: MockSchemeWith(&MockClaim{}, &MockManaged{}),
+				},
+				of:   ClaimKind(MockGVK(&MockClaim{})),
+				with: ManagedKind(MockGVK(&MockManaged{})),
+			},
+			want: want{result: reconcile.Result{Requeue: false}},
+		},
 		"PropagateConnectionError": {
 			args: args{
 				m: &MockManager{

pkg/resource/reconciler.go

@@ -115,3 +115,15 @@ func SetBindable(b Bindable) {
 	}
 	b.SetBindingPhase(v1alpha1.BindingPhaseUnbound)
 }
+
+// IsBindable returns true if the supplied Bindable is ready for binding to
+// another Bindable, such as a resource claim or managed resource.
+func IsBindable(b Bindable) bool {
+	return b.GetBindingPhase() == v1alpha1.BindingPhaseUnbound
+}
+
+// IsBound returns true if the supplied Bindable is bound to another Bindable,
+// such as a resource claim or managed resource.
+func IsBound(b Bindable) bool {
+	return b.GetBindingPhase() == v1alpha1.BindingPhaseBound
+}