Repository navigation
Pass-through drops unknown protobuf fields that function-sdk-go keeps #31
Description
Found while building composition functions on function-sdk-rust 0.3.1.
Today. The server decodes RunFunctionRequest into prost types, and prost discards fields it does not know (tokio-rs/prost#2, open; the opt-in tokio-rs/prost#574 is not merged). response::to clones req.desired and req.context. When a newer Crossplane adds a field to State or Resource, every Rust function in the pipeline strips it from the desired state it returns, with no error.
Nothing is lost yet: the vendored run_function.proto matches Crossplane main. The risk starts with the first new field, for any function built on an older SDK.
Go. protobuf-go keeps unknown fields on every message by default (proto.UnmarshalOptions.DiscardUnknown is false, and grpc-go's codec uses the defaults) and writes them back on marshal. response.To passes the request's Desired and Context messages to the response, so:
- unknown fields on
Statealways survive; - unknown fields on a
Resourcesurvive unless the function replaces that entry.SetDesiredComposedResourcesandSetDesiredCompositeResourcebuild a fresh&v1.Resource{}, which drops them.
Proposed: keep unknown fields on State and Resource, as Go does. These are the two messages response::to carries forward. Context is a google.protobuf.Struct, a well-known type that will not grow fields. Add an unknown_fields field to both structs and hand-write their prost::Message impls: the prost-derive output, except that the fallback arm keeps the field's bytes instead of skipping them.
pub struct Resource {
pub resource: Option<pbjson_types::Struct>,
pub connection_details: HashMap<String, Vec<u8>>,
pub ready: i32,
/// Fields this SDK's proto does not know, kept and re-encoded.
pub unknown_fields: UnknownFields,
}
impl prost::Message for Resource {
fn merge_field(
&mut self,
tag: u32,
wire_type: WireType,
buf: &mut impl Buf,
ctx: DecodeContext,
) -> Result<(), DecodeError> {
match tag {
1 => encoding::message::merge(
wire_type,
self.resource.get_or_insert_with(Default::default),
buf,
ctx,
),
2 => encoding::hash_map::merge(
encoding::string::merge,
encoding::bytes::merge,
&mut self.connection_details,
buf,
ctx,
),
3 => encoding::int32::merge(wire_type, &mut self.ready, buf, ctx),
// prost-derive generates `_ => encoding::skip_field(...)` here.
_ => self.unknown_fields.merge(tag, wire_type, buf, ctx),
}
}
fn encode_raw(&self, buf: &mut impl BufMut) {
// Fields 1 to 3 exactly as prost-derive encodes them, then:
buf.put_slice(&self.unknown_fields.0);
}
// encoded_len adds self.unknown_fields.0.len(); clear resets the struct.
}UnknownFields re-encodes each field with prost's public encoding functions, so truncated input still returns prost's own decode errors:
/// Fields a newer Crossplane sent that this SDK's proto does not know, kept
/// as their wire bytes (key and value, in arrival order) and written back
/// on encode.
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct UnknownFields(Vec<u8>);
impl UnknownFields {
fn merge(
&mut self,
tag: u32,
wire_type: WireType,
buf: &mut impl Buf,
ctx: DecodeContext,
) -> Result<(), DecodeError> {
let out = &mut self.0;
match wire_type {
WireType::Varint => {
let mut v = 0u64;
encoding::uint64::merge(wire_type, &mut v, buf, ctx)?;
encoding::uint64::encode(tag, &v, out);
}
WireType::SixtyFourBit => {
let mut v = 0u64;
encoding::fixed64::merge(wire_type, &mut v, buf, ctx)?;
encoding::fixed64::encode(tag, &v, out);
}
WireType::ThirtyTwoBit => {
let mut v = 0u32;
encoding::fixed32::merge(wire_type, &mut v, buf, ctx)?;
encoding::fixed32::encode(tag, &v, out);
}
WireType::LengthDelimited => {
let mut v = Vec::<u8>::new();
encoding::bytes::merge(wire_type, &mut v, buf, ctx)?;
encoding::bytes::encode(tag, &v, out);
}
// Groups are proto2-only; run_function.proto is proto3.
WireType::StartGroup | WireType::EndGroup => {
encoding::skip_field(wire_type, tag, buf, ctx)?;
}
}
Ok(())
}
}Codegen keeps generating everything else. prost-build's extern_path for .apiextensions.fn.proto.v1.State and .Resource makes it skip the two messages and reference hand-written types, re-exported from proto::v1. The pbjson serde impls construct both structs, so they need unknown_fields: Default::default(), either through a two-line post-processing step in codegen or through hand-written serde impls for the two types. protojson has no representation for unknown fields, so serde ignores them.
In the prototype, a request that carries an unknown field on State and another on a Resource goes through response::to, encode and decode, and comes back as an equal State with both unknown fields present. Replacing the Resource entry drops its unknown fields, matching Go. The existing tests and the example compile unchanged because they build these structs with ..Default::default(). Downstream code that writes exhaustive State { .. } or Resource { .. } literals breaks, hence 0.4.0.
Scope. State and Resource are the whole pass-through path. The only request data a function returns is the desired State with its Resources, and context, which is a google.protobuf.Struct. Everything else in the response is built fresh: response::to creates the meta, as Go's response.To does, and the function creates its results, conditions and requirements. Unknown fields elsewhere in the request (meta, credentials, required resources) are read-only to a function: Go keeps them in memory but never sends them back, and a Go function cannot read them without protobuf reflection. Keeping unknown fields on State and Resource therefore gives Rust functions the behaviour Go functions have in practice.
Later: prost support. tokio-rs/prost#1340, a rebase of #574, adds prost_build::Config::include_unknown_fields(path, field_name), which can cover a whole package with a field of type prost::UnknownFieldList. It is open, with changes requested. If it lands in that shape, codegen can switch to it and drop the hand-written impls. To keep that switch non-breaking, the SDK's UnknownFields exposes only the traits UnknownFieldList derives (Debug, Default, Clone, PartialEq, Eq) and no methods. It can then become pub type UnknownFields = prost::UnknownFieldList;, and code that builds messages with ..Default::default() keeps compiling.
Alternatives I ruled out:
- A raw pass-through helper that splices the request's
desiredandcontextbytes into the response, as function-wasm does for a step with no module (noop_response). It only helps functions that return desired state unchanged, it needs a raw codec in the server, and it is not what Go does. - Splicing unknown fields back in the server by resource name. This needs no API change, but a resource the function replaced would keep stale unknown fields where Go drops them, and functions run outside
serve(tests, wasm guests) would not get it. - Waiting for Add opt-in support for unknown fields tokio-rs/prost#574.
The Rust function template in crossplane/cli (crossplane/cli#374, open) pins function-sdk-rust = "0.3", so 0.4.0 also needs a template bump there.
Activity