Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mutation failed: cannot decode incoming new object: json: unknown field "subresource" #11448

Closed
howardjohn opened this issue Jun 1, 2021 · 16 comments · Fixed by knative/pkg#2249
Labels
area/API API objects and controllers kind/bug Categorizes issue or PR as related to a bug. triage/needs-user-input Issues which are waiting on a response from the reporter

Comments

@howardjohn
Copy link

/area networking

What version of Knative?

0.23

Expected Behavior

Hello world works

Actual Behavior

Kservice stuck, logs are showing a bunch of errors like:

{"severity":"INFO","timestamp":"2021-06-01T18:52:01.271342634Z","logger":"webhook","caller":"webhook/admission.go:131","message":"remote admission controller audit annotations=map[string]string(nil)","commit":"35efb31","knative.dev/pod":"webhook-6f4d97c469-dvfg7","knative.dev/kind":"serving.knative.dev/v1, Kind=Route","knative.dev/namespace":"default","knative.dev/name":"hello","knative.dev/operation":"UPDATE","knative.dev/resource":"serving.knative.dev/v1, Resource=routes","knative.dev/subresource":"status","knative.dev/userinfo":"{system:serviceaccount:knative-serving:controller 515c0bce-588c-44d2-b9ee-cdade6b60f1d [system:serviceaccounts system:serviceaccounts:knative-serving system:authenticated] map[authentication.kubernetes.io/pod-name:[controller-7c697c6857-7r5zj] authentication.kubernetes.io/pod-uid:[aa02fc23-43e6-4519-9e28-002016b408e7]]}","admissionreview/uid":"daf45280-2ffd-441a-8bcd-2fe6ee9047cd","admissionreview/allowed":false,"admissionreview/result":"&Status{ListMeta:ListMeta{SelfLink:,ResourceVersion:,Continue:,RemainingItemCount:nil,},Status:Failure,Message:mutation failed: cannot decode incoming new object: json: unknown field \"subresource\",Reason:BadRequest,Details:nil,Code:400,}"}

Steps to Reproduce the Problem

kind create cluster
istioctl install -y
kubectl apply --filename https://github.com/knative/serving/releases/download/v0.23.0/serving-crds.yaml
kubectl wait --for=condition=Established --all crd
kubectl apply --filename https://github.com/knative/serving/releases/download/v0.23.0/serving-core.yaml
kubectl wait pod --timeout=-1s --for=condition=Ready -l '!job-name' -n knative-serving

kubectl apply --filename https://github.com/knative/net-istio/releases/download/v0.23.0/net-istio.yaml
kubectl replace --filename https://github.com/knative/serving/releases/download/v0.23.0/serving-default-domain.yaml


cat <<EOF | kubectl apply -f -
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: hello
spec:
  template:
    spec:
      containers:
        - image: gcr.io/knative-samples/helloworld-go
          ports:
            - containerPort: 8080
          env:
            - name: TARGET
              value: "Knative"
EOF

After manually deleting all the mutating and validating webhooks seems thing to start working

@howardjohn howardjohn added the kind/bug Categorizes issue or PR as related to a bug. label Jun 1, 2021
@nak3
Copy link
Contributor

nak3 commented Jun 2, 2021

/area API

I tried serving v0.23 but it worked fine on my end.
json: unknown field \"subresource\" seems to be API issue 🤔 Forwarding to API team.

@nak3 nak3 added area/API API objects and controllers and removed area/networking labels Jun 2, 2021
@julz
Copy link
Member

julz commented Jun 25, 2021

@howardjohn are you able to consistently reproduce this on a clean kind cluster? If so could you let us know version of kind you're using, OS, any other info that may be helpful

/needs-user-input

@julz
Copy link
Member

julz commented Jun 25, 2021

/triage needs-user-input

@knative-prow-robot knative-prow-robot added the triage/needs-user-input Issues which are waiting on a response from the reporter label Jun 25, 2021
@daisy-ycguo
Copy link
Member

I met with the same problem on Kubernetes 1.22. It used to work fine on Kubernetes 1.21.
@julz,could you check your Kubernetes version ?

@vdemeester
Copy link
Member

This is also something happening on tektoncd/pipeline with 1.22 (OCP 4.9 nightly and even k8s 1.22 it seems)

@julz
Copy link
Member

julz commented Aug 16, 2021

Created #11805 to get 1.22 in to CI and see if we can reproduce there.

@liggitt
Copy link

liggitt commented Aug 23, 2021

discussion with k8s api-machinery on the topic at https://kubernetes.slack.com/archives/C0EG7JC6T/p1629219036101100, wrapping up with:

@lavalamp:
I recommend webhooks be upgraded first, but when that's not possible (like in this case), the webhook absolutely must tolerate future k8s versions.

@lavalamp
Copy link

Let me repeat the other thing I said, too: There's not much point in having a webhook disallow unknown fields -- the only time you'll see "not allowed" fields from the server is if the server has a different idea about what is allowed than the webhook does.

@liggitt
Copy link

liggitt commented Aug 23, 2021

based on the description in #11848

Unfortunately this does leave us without a great solution for things such as Tekton where schema validation bumps against etcd limits

I think this was being used to prevent unknown fields from being persisted in scenarios where the webhook had a go type and the CRD did not define a schema, so in that case, for things outside the metadata field, the server did have a different idea about what is allowed than the webhook (the server would happily allow anything and the webhook had the go type definition)

@lavalamp
Copy link

I see, that makes sense. Yes, if you're going to do that, I'd remove metadata prior to validating and parse that separately.

If space is preventing adding in schemas, you can save a bit of space by not using client side apply, and if absolutely necessary you can send proto-encoded objects to apiserver, which may save a little more.

@julz
Copy link
Member

julz commented Aug 23, 2021

@liggitt that's exactly right. I think now we have schemas in Knative CRDs we can and should rely on those. Unfortunately Tekton doesn't have schemas yet (and we're worried the size of the needed schemas would exceed limits, based on previous investigations, but this needs revalidating).

@cowbon
Copy link

cowbon commented Aug 23, 2021

Based on the comments above

I met with the same problem on Kubernetes 1.22. It used to work fine on Kubernetes 1.21.
@julz,could you check your Kubernetes version?

This is caused by apiextensions.k8s.io/v1beta1 CustomResiurceDefinition being unavailable in Kubernetes v1.22+ found in net-istio.yaml. Migrating to apiextensions.k8s.io/v1 CustomResiurceDefinition` solves the issue.

@markusthoemmes
Copy link
Contributor

markusthoemmes commented Aug 24, 2021

FWIW, so it doesn't get lost in some Slack thread: The hybrid solution (allowing unknown fields only in metadata) could look somewhat like this:

func decodeIgnoringMetadata(decoder *json.Decoder, into *resourcesemantics.GenericCRD, disallowUnknownFields bool) error {
	var intermediate map[string]json.RawMessage
	if err := decoder.Decode(&intermediate); err != nil {
		return err
	}

	// Roundtrip via ObjectMeta to strip inconmpatible fields, if metadata is present.
	rawMeta := intermediate["metadata"]

	if len(rawMeta) != 0 {
		var meta metav1.ObjectMeta
		if err := json.Unmarshal(intermediate["metadata"], &meta); err != nil {
			return err
		}

		var err error
		intermediate["metadata"], err = json.Marshal(&meta)
		if err != nil {
			return err
		}
	}

	newBytes, err := json.Marshal(&intermediate)
	if err != nil {
		return err
	}

	newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes))
	if disallowUnknownFields {
		newDecoder.DisallowUnknownFields()
	}
	if err := newDecoder.Decode(into); err != nil {
		return err
	}
	return nil
}

@julz
Copy link
Member

julz commented Aug 24, 2021

could also do something along the lines of https://play.golang.org/p/pZmT-E53afz maybe. (Edit:) Actually you're already using RawMessage so the above seems fine to me 😊.

@ZhiminXiang
Copy link

Would like to confirm that: does this issue only happen on k8s 1.22? Is there any other k8s version that could also have this issue?

@dprotaso
Copy link
Member

Would like to confirm that: does this issue only happen on k8s 1.22

1.22+ - but it should be resolved now

Samze added a commit to Samze/service-bindings that referenced this issue Feb 11, 2022
Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.6 to the k8s testing matrix in Github actions.

fixes vmware-tanzu#214
Samze added a commit to Samze/service-bindings that referenced this issue Feb 11, 2022
Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.5 to the k8s testing matrix in Github actions.

fixes vmware-tanzu#214
Samze added a commit to Samze/service-bindings that referenced this issue Feb 14, 2022
Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

fixes vmware-tanzu#214
Samze added a commit to Samze/service-bindings that referenced this issue Feb 14, 2022
Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.5 to the k8s testing matrix in Github actions.

fixes vmware-tanzu#214
rashedkvm pushed a commit to vmware-tanzu/servicebinding that referenced this issue Feb 14, 2022
Kubernetes 1.22 added a new subresource field kubernetes/kubernetes#100970. The knative pkg which is used the webhook had an issue that was resolved here knative/serving#11448.

This change bumps the knative pkg dependency which includes the fix but keeps it pinned on the 0.22 release branch.

In addition it adds 1.22.5 to the k8s testing matrix in Github actions.

fixes #214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/bug Categorizes issue or PR as related to a bug. triage/needs-user-input Issues which are waiting on a response from the reporter
Projects
None yet