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

Adding new fields to a provider and verifying against an older contract without the new fields seems to fail #53

Closed
jdem4ine opened this issue Mar 28, 2024 · 10 comments
Labels
bug Something isn't working smartbear-supported Issues with this label have been added to the Pactflow team's Jira backlog

Comments

@jdem4ine
Copy link

We have an initial proto file like this:

message GetUserResponse {
    string id = 1;
    string display_name = 2;
    string first_name = 3;
    string surname = 4;
    string created_at = 5;
    string updated_at = 6;
}
message GetUserRequest {
    string id = 1;
}
service UserService {
    rpc GetUser(GetUserRequest) returns (GetUserResponse);
}

Contracts have been generated for this interaction between a JS consumer and a go provider. These have been merged to main and deployed against one of our environments.

We then changed the structure of the response from the provider to be

message GetUserResponse {
    string id = 1;
    string display_name = 2;
    string first_name = 3;
    string surname = 4;
    string created_at = 5;
    string updated_at = 6;
    string last_name = 7;
    string email = 8;
    bool email_verified = 9;
}

This was done to ensure that the provider will support the change in contract i.e. the new email, email_verified and the change from surname to last_name as well as retain the fields from the previous contract such that it can be deployed to an environment where the old contract exists.

When we tried to verify this contract against consumer versions using selectors (matching-branch, deployed and mainbranch)

  • The matching branch one passed as the contract changes have been made on a branch with the same name on the consumer side to support the new expectations and the deprecated fields have been removed i.e:
response: {
          id: matchingRegex(uuidRegex, '89b9475a-09d0-47a9-a4bc-2b6b9d361db6'),
          display_name: matchingType('xX5im0n-P3ggXx'),
          first_name: matchingType('Simon'),
          last_name: matchingType('Pegg'),
          email: matchingType('simon@pegg.com'),
          email_verified: matchingType(true),
        },

NOTE: Here we are expecting less data than the provider provides (not expecting created_at updated_at and surname)and all seems fine (like I would expect)

  • The one that is deployed to our environment and in main (setup per the initial protofile) failed and spat out a couple of different errors:
    2024-03-28T10:04:36.189948Z ERROR ThreadId(18) pact_verifier: Individual mismatch is an error: failed to decode Protobuf message: invalid tag value: 0
2925Z  WARN tokio-runtime-worker request{method=POST uri=http://127.0.0.1:43899/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}:decode_message: pact_protobuf_plugin::message_decoder: Was not able to decode field: Did not find a field with number 7 in the descriptor
2024-03-27T16:22:07.932940Z DEBUG          tokio-runtime-worker pact_plugin_driver::child_process: Plugin(protobuf, 247654, STDOUT) || 2024-03-27T16:22:07.932935Z  WARN tokio-runtime-worker request{method=POST uri=http://127.0.0.1:43899/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}:decode_message: pact_protobuf_plugin::message_decoder: Was not able to decode field: Did not find a field with number 8 in the descriptor
2024-03-27T16:22:07.932948Z DEBUG          tokio-runtime-worker pact_plugin_driver::child_process: Plugin(protobuf, 247654, STDOUT) || 2024-03-27T16:22:07.932944Z  WARN tokio-runtime-worker request{method=POST uri=http://127.0.0.1:43899/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}:decode_message: pact_protobuf_plugin::message_decoder: Was not able to decode field: Did not find a field with number 9 in the descriptor

My suspicion is that when the contract comes from the broker for the main/deployed versions it is somehow wanting the provider to support the exact response it expects rather than just those fields as a minimum

@jdem4ine
Copy link
Author

On the broker we can see that for the new contract, the fields that were not required by the consumer are correctly skipped:
image

For reference this is the old message response
image

@jmanson377
Copy link

Is there any movement on either a workaround for this or alternative approach as this is a key blocker to us using the pact protobuf plugin

@jdem4ine
Copy link
Author

jdem4ine commented Apr 4, 2024

Upon further inspection of the logs, the new fields are being treadted as unknown by the plugin:

2024-04-04T09:05:47.635288Z DEBUG tokio-runtime-worker pact_plugin_driver::child_process: Plugin(protobuf, 50819, STDOUT) || 2024-04-04T09:05:47.635198Z DEBUG tokio-runtime-worker request{method=POST uri=http://127.0.0.1:32977/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}: pact_protobuf_plugin::verification: Received response from gRPC server - Response { metadata: MetadataMap { headers: {"content-type": "application/grpc", "grpc-accept-encoding": "gzip", "date": "Thu, 04 Apr 2024 09:05:47 GMT", "grpc-message": "", "grpc-status": "0"} }, message: DynamicMessage { fields: [ProtobufField { field_num: 1, field_name: "id", wire_type: LengthDelimited, data: String("89b9475a-09d0-47a9-a4bc-2b6b9d361db6") }, ProtobufField { field_num: 2, field_name: "display_name", wire_type: LengthDelimited, data: String("xX5im0n-P3ggXx") }, ProtobufField { field_num: 3, field_name: "first_name", wire_type: LengthDelimited, data: String("Simon") }, ProtobufField { field_num: 4, field_name: "surname", wire_type: LengthDelimited, data: String("Pegg") }, ProtobufField { field_num: 5, field_name: "created_at", wire_type: LengthDelimited, data: String("0001-01-01 00:00:00 +0000 UTC") }, ProtobufField { field_num: 6, field_name: "updated_at", wire_type: LengthDelimited, data: String("0001-01-01 00:00:00 +0000 UTC") }, ProtobufField { field_num: 7, field_name: "unknown", wire_type: LengthDelimited, data: Unknown([4, 80, 101, 103, 103]) }, ProtobufField { field_num: 8, field_name: "unknown", wire_type: LengthDelimited, data: Unknown([14, 115, 105, 109, 111, 110, 64, 112, 101, 103, 103, 46, 99, 111, 109]) }, ProtobufField { field_num: 9, field_name: "unknown", wire_type: Varint, data: Unknown([1, 0, 0, 0, 0, 0, 0, 0]) }], descriptors: FileDescriptorSet { file: [FileDescriptorProto { name: Some("user.proto"), package: Some("user.v1"), dependency: [], public_dependency: [], weak_dependency: [], message_type: [DescriptorProto { name: Some("GetUserRequest"), field: [FieldDescriptorProto { name: Some("id"), number: Some(1), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("id"), options: None, proto3_optional: None }], extension: [], nested_type: [], enum_type: [], extension_range: [], oneof_decl: [], options: None, reserved_range: [], reserved_name: [] }, DescriptorProto { name: Some("GetUserResponse"), field: [FieldDescriptorProto { name: Some("id"), number: Some(1), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("id"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("display_name"), number: Some(2), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("displayName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("first_name"), number: Some(3), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("firstName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("surname"), number: Some(4), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("surname"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("created_at"), number: Some(5), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("createdAt"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("updated_at"), number: Some(6), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("updatedAt"), options: None, proto3_optional: None }], extension: [], nested_type: [], enum_type: [], extension_range: [], oneof_decl: [], options: None, reserved_range: [], reserved_name: [] }, DescriptorProto { name: Some("CreateUserRequest"), field: [FieldDescriptorProto { name: Some("display_name"), number: Some(2), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("displayName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("first_name"), number: Some(3), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("firstName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("surname"), number: Some(4), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("surname"), options: None, proto3_optional: None }], extension: [], nested_type: [], enum_type: [], extension_range: [], oneof_decl: [], options: None, reserved_range: [], reserved_name: [] }, DescriptorProto { name: Some("CreateUserResponse"), field: [FieldDescriptorProto { name: Some("id"), number: Some(1), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("id"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("display_name"), number: Some(2), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("displayName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("first_name"), number: Some(3), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("firstName"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("surname"), number: Some(4), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("surname"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("created_at"), number: Some(5), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("createdAt"), options: None, proto3_optional: None }, FieldDescriptorProto { name: Some("updated_at"), number: Some(6), label: Some(Optional), r#type: Some(String), type_name: None, extendee: None, default_value: None, oneof_index: None, json_name: Some("updatedAt"), options: None, proto3_optional: None }], extension: [], nested_type: [], enum_type: [], extension_range: [], oneof_decl: [], options: None, reserved_range: [], reserved_name: [] }], enum_type: [], service: [ServiceDescriptorProto { name: Some("UserService"), method: [MethodDescriptorProto { name: Some("GetUser"), input_type: Some(".user.v1.GetUserRequest"), output_type: Some(".user.v1.GetUserResponse"), options: None, client_streaming: None, server_streaming: None }, MethodDescriptorProto { name: Some("CreateUser"), input_type: Some(".user.v1.CreateUserRequest"), output_type: Some(".user.v1.CreateUserResponse"), options: None, client_streaming: None, server_streaming: None }], options: None }], extension: [], options: Some(FileOptions { java_package: None, java_outer_classname: None, java_multiple_files: None, java_generate_equals_and_hash: None, java_string_check_utf8: None, optimize_for: None, go_package: Some("generated/user/v1;userv1"), cc_generic_services: None, java_generic_services: None, py_generic_services: None, php_generic_services: None, deprecated: None, cc_enable_arenas: None, objc_class_prefix: None, csharp_namespace: None, swift_prefix: None, php_class_prefix: None, php_namespace: None, php_metadata_namespace: None, ruby_package: None, uninterpreted_option: [] }), source_code_info: None, syntax: Some("proto3") }] } }, extensions: Extensions }

Per the proto3 spec these unkown fields seem to be legitimate even though they may or may not be used by the older schema https://protobuf.dev/programming-guides/proto3/#unknowns

@jdem4ine
Copy link
Author

jdem4ine commented Apr 4, 2024

After having a flick through the source code, it seems like 2024-04-04T09:05:47.635693Z DEBUG tokio-runtime-worker pact_plugin_driver::child_process: Plugin(protobuf, 50819, STDOUT) || 2024-04-04T09:05:47.635686Z INFO tokio-runtime-worker request{method=POST uri=http://127.0.0.1:32977/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}:decode_message: pact_protobuf_plugin::message_decoder: return=Err(failed to decode Protobuf message: invalid tag value: 0) is actually causing the failure and the warns about unknown fields might be a red herring, but I can't quite find where that's coming from in the message decoder 🤔

@jdem4ine
Copy link
Author

jdem4ine commented Apr 4, 2024

After a bit more digging we seem to have found the issue:
As above, we have the email_verified boolean added to the new schema. When the pact plugin tries to decode this, it sees this field as:
ProtobufField { field_num: 9, field_name: "unknown", wire_type: Varint, data: Unknown([1, 0, 0, 0, 0, 0, 0, 0]) }]
later in the logs I can see it being written as:
2024-04-04T09:05:47.635469Z DEBUG tokio-runtime-worker pact_plugin_driver::child_process: Plugin(protobuf, 50819, STDOUT) || 2024-04-04T09:05:47.635462Z DEBUG tokio-runtime-worker request{method=POST uri=http://127.0.0.1:32977/io.pact.plugin.PactPlugin/VerifyInteraction version=HTTP/2.0 headers={"te": "trailers", "content-type": "application/grpc", "authorization": Sensitive, "user-agent": "tonic/0.10.2"}}: pact_protobuf_plugin::dynamic_message: Writing unknown field 0100000000000000

When the unknown field is written there seems to be an extra zero prepended to the field which appears to be causing the protobuf decoder to fail because it thinks it has found a field with a tag value of 0.

@jdem4ine
Copy link
Author

jdem4ine commented Apr 8, 2024

Attaching full provider verification run for context
pact-plugin-logs.txt

@rholshausen rholshausen added bug Something isn't working smartbear-supported Issues with this label have been added to the Pactflow team's Jira backlog labels Apr 9, 2024
Copy link

github-actions bot commented Apr 9, 2024

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-1942). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.

@rholshausen
Copy link
Contributor

The issue was that the varint value for the boolean field was being captured as a u64 value (8 bytes), when a varint can be 1 to 10 bytes depending on the value. For a boolean value of true, it should be just 1 byte. This added an extra 7 zero bytes to the end of the message making it an invalid Protobuf message.

@YOU54F
Copy link
Member

YOU54F commented Apr 26, 2024

Thanks Ron!

@jdem4ine are you able to retest with 0.3.14 please?

@YOU54F
Copy link
Member

YOU54F commented Sep 26, 2024

Going to assume this is complete, feel free to let us know if not

@YOU54F YOU54F closed this as completed Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working smartbear-supported Issues with this label have been added to the Pactflow team's Jira backlog
Projects
None yet
Development

No branches or pull requests

4 participants