Skip to content

Conversation

@dgn
Copy link
Contributor

@dgn dgn commented Dec 2, 2025

According to our documentation, base64-encoded values should be used in this field, in which case byte is the correct format rather than binary. In fact, binary makes 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:

CustomResourceDefinition "virtualservices.networking.istio.io": unrecognized format "binary"

Fixes istio/istio#58461

@dgn dgn requested a review from a team as a code owner December 2, 2025 11:12
@istio-policy-bot
Copy link

😊 Welcome @dgn! This is either your first contribution to the Istio api repo, or it's been
a while since you've been here.

You can learn more about the Istio working groups, Code of Conduct, and contribution guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 2, 2025
@dgn dgn added the release-notes-none Indicates a PR that does not require release notes. label Dec 2, 2025
@dgn
Copy link
Contributor Author

dgn commented Dec 2, 2025

/retest

@dgn dgn force-pushed the fix-binary-format branch from 26204eb to fb6e837 Compare December 2, 2025 12:03
@dgn dgn removed the release-notes-none Indicates a PR that does not require release notes. label Dec 2, 2025
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 2, 2025
Copy link
Member

@howardjohn howardjohn left a 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
Copy link
Member

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?

Copy link
Contributor Author

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

@dgn
Copy link
Contributor Author

dgn commented Dec 3, 2025

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

Thanks for the pointer. See istio/tools#3315 for a fix

@dgn dgn force-pushed the fix-binary-format branch from fb6e837 to a9efd5d Compare December 4, 2025 13:48
@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 4, 2025
@dgn dgn requested a review from a team as a code owner December 4, 2025 14:03
@istio-testing istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 4, 2025
@dgn
Copy link
Contributor Author

dgn commented Dec 4, 2025

/retest

@dgn dgn force-pushed the fix-binary-format branch from 252e275 to a9efd5d Compare December 4, 2025 15:02
@istio-testing istio-testing added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 4, 2025
According to our documentation, base64-encoded values should be used in
this field, in which case `byte` is the correct format rather than
`binary`.
@dgn dgn force-pushed the fix-binary-format branch from a9efd5d to 89e774a Compare December 4, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unrecognized format "binary" in VirtualService CRD

4 participants