Merge pull request #11 from pusher/fix-template-input

Check status exists before trying to remove it

Author: JoelSpeed

pkg/quack/quack.go

@@ -106,7 +106,7 @@ func (ah *AdmissionHook) Admit(req *admissionv1beta1.AdmissionRequest) *admissio
 
 	templateInput, err := getTemplateInput(req.Object.Raw)
 	if err != nil {
-		return errorResponse(resp, "")
+		return errorResponse(resp, "Error creating template input: %v", err)
 	}
 	// Run Templating
 	glog.V(6).Infof("Input for %s: %s", requestName, templateInput)
@@ -194,12 +194,18 @@ func getTemplateInput(data []byte) ([]byte, error) {
 	}
 
 	// We should not modify the status of objects
-	patch := []byte(fmt.Sprintf(`[
-		{"op": "remove", "path": "/status"}
-	]`))
-	data, err = applyPatch(data, patch)
+	hasStatus, err := requestHasStatus(data)
 	if err != nil {
-		return nil, fmt.Errorf("error removing status: %v", err)
+		return nil, fmt.Errorf("error reading object status: %v", err)
+	}
+	if hasStatus {
+		patch := []byte(fmt.Sprintf(`[
+			{"op": "remove", "path": "/status"}
+		]`))
+		data, err = applyPatch(data, patch)
+		if err != nil {
+			return nil, fmt.Errorf("error removing status: %v", err)
+		}
 	}
 
 	for annotation := range objectMeta.Annotations {
@@ -252,6 +258,17 @@ func getObjectMeta(raw []byte) (metav1.ObjectMeta, error) {
 	return requestMeta.ObjectMeta, nil
 }
 
+func requestHasStatus(raw []byte) (bool, error) {
+	requestStatus := struct {
+		Status map[string]interface{} `json:"status"`
+	}{}
+	err := json.Unmarshal(raw, &requestStatus)
+	if err != nil {
+		return false, fmt.Errorf("failed to unmarshal input: %v", err)
+	}
+	return requestStatus.Status != nil, nil
+}
+
 func applyPatch(data, patchBytes []byte) ([]byte, error) {
 	patch, err := mergepatch.DecodePatch(patchBytes)
 	if err != nil {

pkg/quack/quack_test.go

@@ -342,3 +342,22 @@ func TestGetDelims(t *testing.T) {
 	assert.Equal(t, delimiters{}, withEmptyDelimeters, "Object with empty delimiter should return empty delimiters")
 	assert.NotNil(t, emptyErr, "Object with empty left delimiter should return error")
 }
+
+func TestRequestHasStatus(t *testing.T) {
+	withStatus := `{
+			"status": {
+				"foo": "bar",
+				"baz": 3
+			}
+		}`
+	hasStatus, err := requestHasStatus([]byte(withStatus))
+	assert.Equal(t, nil, err, "Error should not have occurred")
+	assert.Equal(t, true, hasStatus, "Expected object with status to return true")
+
+	withoutStatus := `{
+				"foo": "bar"
+			}`
+	hasStatus, err = requestHasStatus([]byte(withoutStatus))
+	assert.Equal(t, nil, err, "Error should not have occurred")
+	assert.Equal(t, false, hasStatus, "Expected object without status to return false")
+}