-
Notifications
You must be signed in to change notification settings - Fork 588
virtualservice: change the directResponse.body.bytes format to byte
#3606
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
base: master
Are you sure you want to change the base?
Conversation
|
😊 Welcome @dgn! This is either your first contribution to the Istio api repo, or it's been You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines Thanks for contributing! Courtesy of your friendly welcome wagon. |
|
/retest |
26204eb to
fb6e837
Compare
howardjohn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably fix this at the source if binary is never valid
cmd/protoc-gen-crd/openapiGenerator.go
810- case descriptor.FieldDescriptorProto_TYPE_BYTES:
811- schema.Type = "string"
812: schema.Format = "binary"
813- schema.Description = g.generateDescription(field)
from istio/tools
| string string = 1; | ||
|
|
||
| // response body as base64 encoded bytes. | ||
| // +kubebuilder:validation:Format=byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh, in the current CRD do we really force users to base64 encode it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that's how we document it
Thanks for the pointer. See istio/tools#3315 for a fix |
fb6e837 to
a9efd5d
Compare
|
/retest |
252e275 to
a9efd5d
Compare
According to our documentation, base64-encoded values should be used in this field, in which case `byte` is the correct format rather than `binary`.
a9efd5d to
89e774a
Compare
According to our documentation, base64-encoded values should be used in this field, in which case
byteis the correct format rather thanbinary. In fact,binarymakes no sense as there's no way input raw binary data into CRDs/etcd. It is also not one of the allowed formats in CustomResourceDefinitions.As of K8s 1.34+, installing the VirtualService CRD will output a warning about this:
Fixes istio/istio#58461