Add status subresource to HTTPProxy

This commit enables the status subresource for HTTPProxy.
This means that:
- normal HTTPProxy changes that include updates to the `status` stanza will have the status updates dropped
- updates to `status` can only be made using requests to the status subresource.

Also changes the HTTPProxy status updating to use the status subresource,
and adds RBAC for Contour to be able to update it.

Having the status subresource enabled is best-practice for CRDs and will be a foundation for further status
work.

It also means that HTTPProxy objects no longer need to include the `status` stanza to be valid.

Updates #2495

Signed-off-by: Nick Young <ynick@vmware.com>

apis/projectcontour/v1/httpproxy.go

@@ -421,6 +421,7 @@ type Status struct {
 // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.currentStatus",description="The current status of the HTTPProxy"
 // +kubebuilder:printcolumn:name="Status Description",type="string",JSONPath=".status.description",description="Description of the current status"
 // +kubebuilder:resource:scope=Namespaced,path=httpproxies,shortName=proxy;proxies,singular=httpproxy
+// +kubebuilder:subresource:status
 type HTTPProxy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`

examples/contour/01-crds.yaml

@@ -479,7 +479,8 @@ spec:
     - proxies
     singular: httpproxy
   scope: Namespaced
-  subresources: {}
+  subresources:
+    status: {}
   validation:
     openAPIV3Schema:
       description: HTTPProxy is an Ingress CRD specification

examples/contour/02-rbac.yaml

@@ -79,6 +79,12 @@ rules:
   - put
   - post
   - patch
+- apiGroups:
+  - "projectcontour.io"
+  resources:
+  - "httpproxies/status"
+  verbs:
+  - update
 - apiGroups: ["networking.x.k8s.io"]
   resources: ["gatewayclasses", "gateways", "httproutes", "tcproutes"]
   verbs:

internal/k8s/status.go

@@ -21,8 +21,11 @@ import (
 	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	jsonpatch "github.com/evanphx/json-patch"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 
@@ -177,23 +180,27 @@ func (irs *StatusWriter) setIngressRouteStatus(existing, updated *ingressroutev1
 }
 
 func (irs *StatusWriter) setHTTPProxyStatus(existing, updated *projcontour.HTTPProxy) error {
-	existingBytes, err := json.Marshal(existing)
-	if err != nil {
-		return err
-	}
-	// Need to set the resource version of the updated endpoints to the resource
-	// version of the current service. Otherwise, the resulting patch does not
-	// have a resource version, and the server complains.
-	updated.ResourceVersion = existing.ResourceVersion
-	updatedBytes, err := json.Marshal(updated)
+
+	usUpdated, err := toUnstructured(updated, projcontour.SchemeGroupVersion.WithKind("HTTPProxy"))
 	if err != nil {
 		return err
 	}
-	patchBytes, err := jsonpatch.CreateMergePatch(existingBytes, updatedBytes)
+
+	_, err = irs.Client.Resource(projcontour.HTTPProxyGVR).Namespace(existing.GetNamespace()).UpdateStatus(context.TODO(), usUpdated, metav1.UpdateOptions{})
+	return err
+}
+
+func toUnstructured(obj runtime.Object, gvk schema.GroupVersionKind) (*unstructured.Unstructured, error) {
+
+	obj.GetObjectKind().SetGroupVersionKind(gvk)
+	objMsi, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
-	_, err = irs.Client.Resource(projcontour.HTTPProxyGVR).Namespace(existing.GetNamespace()).Patch(context.TODO(), existing.GetName(), types.MergePatchType, patchBytes, metav1.PatchOptions{})
-	return err
+	us := &unstructured.Unstructured{}
+	us.SetUnstructuredContent(objMsi)
+
+	return us, nil
+
 }

internal/k8s/status_test.go

@@ -131,29 +131,33 @@ func TestSetIngressRouteStatus(t *testing.T) {
 
 func TestSetHTTPProxyStatus(t *testing.T) {
 	type testcase struct {
-		msg           string
-		desc          string
-		existing      *projectcontour.HTTPProxy
-		expectedPatch string
-		expectedVerbs []string
+		msg      string
+		desc     string
+		existing *projectcontour.HTTPProxy
+		expected *projectcontour.HTTPProxy
 	}
 
 	run := func(t *testing.T, name string, tc testcase) {
 		t.Helper()
 
 		t.Run(name, func(t *testing.T) {
 			t.Helper()
-			var gotPatchBytes []byte
+			var gotObj runtime.Object
 			s := runtime.NewScheme()
 			projcontour.AddKnownTypes(s)
+			usc, err := NewUnstructuredConverter()
+			if err != nil {
+				t.Fatal(err)
+			}
+
 			client := fake.NewSimpleDynamicClient(s, tc.existing)
 
-			client.PrependReactor("patch", "httpproxies", func(action k8stesting.Action) (bool, runtime.Object, error) {
-				switch patchAction := action.(type) {
+			client.PrependReactor("*", "httpproxies", func(action k8stesting.Action) (bool, runtime.Object, error) {
+				switch updateAction := action.(type) {
 				default:
 					return true, nil, fmt.Errorf("got unexpected action of type: %T", action)
-				case k8stesting.PatchActionImpl:
-					gotPatchBytes = patchAction.GetPatch()
+				case k8stesting.UpdateActionImpl:
+					gotObj = updateAction.GetObject()
 					return true, tc.existing, nil
 				}
 			})
@@ -165,12 +169,19 @@ func TestSetHTTPProxyStatus(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			if len(client.Actions()) != len(tc.expectedVerbs) {
-				t.Fatalf("Expected verbs mismatch: want: %d, got: %d", len(tc.expectedVerbs), len(client.Actions()))
+			toProxy, err := usc.Convert(gotObj)
+			if err != nil {
+				t.Fatal(err)
 			}
 
-			if tc.expectedPatch != string(gotPatchBytes) {
-				t.Fatalf("expected patch: %s, got: %s", tc.expectedPatch, string(gotPatchBytes))
+			if toProxy == nil && tc.expected == nil {
+				return
+			}
+
+			assert.Equal(t, toProxy, tc.expected)
+
+			if toProxy == nil && tc.expected != nil {
+				t.Fatalf("Did not get expected update, %#v", tc.expected)
 			}
 		})
 	}
@@ -188,8 +199,16 @@ func TestSetHTTPProxyStatus(t *testing.T) {
 				Description:   "",
 			},
 		},
-		expectedPatch: `{"status":{"currentStatus":"valid","description":"this is a valid HTTPProxy"}}`,
-		expectedVerbs: []string{"patch"},
+		expected: &projcontour.HTTPProxy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test",
+				Namespace: "default",
+			},
+			Status: projcontour.Status{
+				CurrentStatus: "valid",
+				Description:   "this is a valid HTTPProxy",
+			},
+		},
 	})
 
 	run(t, "no update", testcase{
@@ -205,8 +224,7 @@ func TestSetHTTPProxyStatus(t *testing.T) {
 				Description:   "this is a valid HTTPProxy",
 			},
 		},
-		expectedPatch: ``,
-		expectedVerbs: []string{},
+		expected: nil,
 	})
 
 	run(t, "replace existing status", testcase{
@@ -222,8 +240,16 @@ func TestSetHTTPProxyStatus(t *testing.T) {
 				Description:   "boo hiss",
 			},
 		},
-		expectedPatch: `{"status":{"currentStatus":"valid","description":"this is a valid HTTPProxy"}}`,
-		expectedVerbs: []string{"patch"},
+		expected: &projcontour.HTTPProxy{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test",
+				Namespace: "default",
+			},
+			Status: projcontour.Status{
+				CurrentStatus: "valid",
+				Description:   "this is a valid HTTPProxy",
+			},
+		},
 	})
 }