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

Schema Validation changes after upgrading to v2.7.0 from v1.27.9 #485

Closed
gautam-goudar opened this issue Nov 12, 2024 · 8 comments
Closed

Comments

@gautam-goudar
Copy link

Have subsequent upgrades to the Terraform provider changed the schema validation mechanism?

When using v1.27.0, we are able to upgrade the schema with BACKWARD compatibility by

  • adding a field, where the field type was a new user defined message type

Example

v1
-----------
message MyProtoSchema_v1 {
  google.protobuf.StringValue id = 1;
  google.protobuf.Timestamp issuedAt = 2;
}


v2
-----------
message MyProtoSchema_v1 {
  google.protobuf.StringValue id = 1;
  google.protobuf.Timestamp issuedAt = 2;
  TemporaryOperatingPermitCalculation calculation = 3;
}

message TemporaryOperatingPermitCalculation {
...........................
}

With v1.27.0, we were able to deploy v2 with the above changes by adding the field calculation of type TemporaryOperatingPermitCalculation.

Now after upgrading to v2.7.0, the same pipeline returns the below error (without any changes)

Error: error validating Schema: error validating a schema: [{errorType:"MESSAGE_REMOVED", description:"The new schema is missing a MESSAGE type at path '#/TemporaryOperatingPermitCalculation' in the old schema"} {oldSchemaVersion: 2} {oldSchema: 'syntax = "proto3";

What is the solution to this error? Please help.

Thanks

@gautam-goudar
Copy link
Author

If this helps, below is the line in the source code (like 459) that prints out the error we are seeing in the logs

return fmt.Errorf("error validating Schema: error validating a schema: %s", schemaNotCompatibleErrorMessage)

The function “schemaValidateCheck” was added to the code base on May 31st, 2023

@cryoshida
Copy link
Member

Hi @gautam-goudar !

Forward and full compatibility are not recommended for Protobuf. The compatibility matrix remained the same across SR releases.

Note that best practice for Protobuf is to use BACKWARD_TRANSITIVE, as adding new message types is not forward compatible.

Here are some links that might be helpful: https://docs.confluent.io/platform/current/schema-registry/fundamentals/schema-evolution.html#summary and https://www.confluent.io/blog/best-practices-for-confluent-schema-registry/ .

That being said, I'm double checking with our team to see if there is anything else that we might have missed.

@gautam-goudar
Copy link
Author

Thanks.

We are using "BACKWARD" compatibility for out protobuf schema. For this very same reason, we are not sure why we are seeing the error when a new message is added to evolve schema.

@linouk23
Copy link
Contributor

linouk23 commented Nov 19, 2024

@gautam-goudar, it's a bit surprising to see that it worked in the first place, considering the validation fails. 🤔

Could you try setting skip_validation_during_plan = true and see whether terraform plan work (assuming the goal is to fix terraform plan for already deployed schemas)?

Given that this flag was likely introduced after we added validation to terraform plan, and your terraform plan fails already, you might have to add skip_validation_during_plan = true. Additionally, you may need to patch the TF state file manually to include this line. Otherwise, your update from skip_validation_during_plan = false (default) to true may not go through. This is because terraform apply will implicitly call terraform plan (that fails already), which will cause the apply to fail as well.

@linouk23
Copy link
Contributor

@gautam-goudar did you get a chance to try out our suggestion above?

@gautam-goudar
Copy link
Author

I haven't tried this yet. I'll let you know when I do (sometime this week).

Thanks for checking

@linouk23
Copy link
Contributor

@gautam-goudar did you get a chance to try out our suggestion above?

@linouk23
Copy link
Contributor

linouk23 commented Mar 4, 2025

I'm closing this one due to inactivity, but feel free to reopen it if the suggested fix doesn't work.

@linouk23 linouk23 closed this as completed Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants