istio: No visibility into bad IOP yaml

Apply yaml like

apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  namespace: istio-system
spec:
  profile: default
  revision: 16

note: revision is an int not a string

IOP gets no status.

Only info about this is operator logs showing

2020-06-19T17:07:21.829451Z     error   k8s.io/client-go@v0.18.0/tools/cache/reflector.go:125: Failed to list *v1alpha1.IstioOperator: v1alpha1.IstioOperatorList.ListMeta: v1.ListMeta.TypeMeta: Kind: Items: []v1alpha1.IstioOperator: v1alpha1.IstioOperator.Spec: unmarshalerDecoder: json: cannot unmarshal number into Go value of type string, error found in #10 byte of ...|":false}}}}],"kind":|..., bigger context ...|tyEnabled":true}},"prometheus":{"enabled":false}}}}],"kind":"IstioOperatorList","metadata":{"continu|...

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 15 (13 by maintainers)

Most upvoted comments

Or how about not parsing directly to IstioOperator? We could parse it to Unstructured or map[string]interface{}. Then with something like tpath we can pick the fields and validate. That way, we can give the user context and avoid printing huge YAML. As we have given the field name, we don’t have to show that in YAML.

We can locate spec.revision then perform whatever validation we want to perform. In case of arrays like gateways, we could probably iterate through each one of them and apply this technique. After all, everything is a tree here. We could convert to IstioOperator and perform more validations later.

Per https://github.com/istio/istio/issues/24819#issuecomment-707900293 I don’t think it is feasible to address this by 1.8. Moved this to 1.9.

To clarify: the original intent of this issue is nothing to do with the error. The problem is when there is any error, the user has no way of understanding what went wrong. We should never have to look at operator logs.

Or how about not parsing directly to IstioOperator? We could parse it to Unstructured or map[string]interface{}.

We are already doing that.

Then with something like tpath we can pick the fields and validate.

I think we can try this but I’m more interested in doing something like the following before unmarshlling to IstioOperator.

https://github.com/istio/istio/blob/d5893dc37b2c1ab9d05d839173017b35990c8e92/operator/pkg/validate/validate.go#L33-L42

I agree with @su225 and my own comment https://github.com/istio/istio/issues/24819#issuecomment-676375377 . I am willing to tackle this but I got frustrated that I couldn’t get a review https://github.com/istio/istio/pull/26308 which I wanted to merge first.

@ostromart Can we put this on BOTH the Environments and UX roadmap for 1.9? This is a P0 and deserves tracking.