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

avro backwards-compatibility check is too permissive #16649

Closed
devinrsmith opened this issue Feb 20, 2024 · 6 comments · Fixed by #24032
Closed

avro backwards-compatibility check is too permissive #16649

devinrsmith opened this issue Feb 20, 2024 · 6 comments · Fixed by #24032
Labels
area/schema-registry Schema Registry service within Redpanda kind/bug Something isn't working

Comments

@devinrsmith
Copy link

devinrsmith commented Feb 20, 2024

Version & Environment

Redpanda version: (use rpk version):

Version:     v23.3.5
Git ref:     2c0d4bcfb1
Build date:  2024-02-09T06:03:08Z
OS/Arch:     linux/amd64
Go version:  go1.21.3

What went wrong?

A BACKWARDS compatible schema is allowing the registration of an avro schema that is not backwards compatible; adding a field that is ["null", <type>] without actually specifying a default is improperly getting validated as compatible. Ultimately, this causes a downstream error on the clients who assume backwards compatibility.

Caused by: org.apache.avro.AvroTypeException: Found test-validation, expecting test-validation, missing required field color
        at org.apache.avro.io.ResolvingDecoder.doAction(ResolvingDecoder.java:308)
        at org.apache.avro.io.parsing.Parser.advance(Parser.java:86)
        at org.apache.avro.io.ResolvingDecoder.readFieldOrder(ResolvingDecoder.java:127)
        at org.apache.avro.generic.GenericDatumReader.readRecord(GenericDatumReader.java:240)
        at org.apache.avro.generic.GenericDatumReader.readWithoutConversion(GenericDatumReader.java:180)
        at org.apache.avro.generic.GenericDatumReader.read(GenericDatumReader.java:161) 
        at org.apache.avro.generic.GenericDatumReader.read(GenericDatumReader.java:154) 
        at io.confluent.kafka.serializers.AbstractKafkaAvroDeserializer$DeserializationContext.read(AbstractKafkaAvroDeserializer.java:505)
        ... 15 more

Note: in testing, the type [<type>, "null"] without a default is properly getting blocked as invalid.

What should have happened instead?

The schema registration should have been blocked. The documentation for the schema registry and BACKWARDS compatibility mentions:

https://docs.redpanda.com/current/manage/schema-reg/schema-reg-api/#compatibility-uses-and-constraints

Add fields with default values

https://docs.confluent.io/platform/current/schema-registry/fundamentals/schema-evolution.html#compatibility-types

Add optional fields

How to reproduce the issue?

$ rpk registry schema create test-validation --schema car-v1.avro --type avro
SUBJECT          VERSION  ID    TYPE
test-validation  1        1     AVRO

$ rpk registry compatibility-level set test-validation --level BACKWARD                                            
SUBJECT          LEVEL     ERROR
test-validation  BACKWARD  

$ rpk registry schema check-compatibility test-validation --schema-version latest --schema car-v2a.avro --type avro
Schema is not compatible.

$ rpk registry schema check-compatibility test-validation --schema-version latest --schema car-v2b.avro --type avro
Schema is compatible.

Additional information

car-v1.avro:

{
    "type": "record",
    "name": "car",
    "fields": [
        {
            "name": "model",
            "type": "string"
        },
        {
            "name": "make",
            "type": "string"
        },
        {
            "name": "year",
            "type": "float"
        }
    ]
}

car-v2a.avro:

{
    "type": "record",
    "name": "car",
    "fields": [
        {
            "name": "model",
            "type": "string"
        },
        {
            "name": "make",
            "type": "string"
        },
        {
            "name": "year",
            "type": "float"
        },
        {
            "name": "color",
            "type": ["string", "null"]
        }
    ]
}

car-v2b.avro:

{
    "type": "record",
    "name": "car",
    "fields": [
        {
            "name": "model",
            "type": "string"
        },
        {
            "name": "make",
            "type": "string"
        },
        {
            "name": "year",
            "type": "float"
        },
        {
            "name": "color",
            "type": ["null", "string"]
        }
    ]
}

Potentially related to #4297?

JIRA Link: CORE-1796

@devinrsmith devinrsmith added the kind/bug Something isn't working label Feb 20, 2024
@devinrsmith
Copy link
Author

Additional note - in this case, I see the "correct" backwards-compatible schema as:

{
    "type": "record",
    "name": "car",
    "fields": [
        {
            "name": "model",
            "type": "string"
        },
        {
            "name": "make",
            "type": "string"
        },
        {
            "name": "year",
            "type": "float"
        },
        {
            "name": "color",
            "type": ["null", "string"],
            "default": null
        }
    ]
}

@devinrsmith
Copy link
Author

I've verified that the Confluent Schema Registry for Kafka correctly rejects car-v2b.avro as not backwards-compatible.

@BenPope BenPope added the area/schema-registry Schema Registry service within Redpanda label Feb 20, 2024
@dotnwat
Copy link
Member

dotnwat commented Feb 24, 2024

cc @michael-redpanda

@michael-redpanda michael-redpanda removed their assignment Feb 26, 2024
Copy link

github-actions bot commented Oct 2, 2024

This issue hasn't seen activity in 3 months. If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in two weeks.

@github-actions github-actions bot added the stale label Oct 2, 2024
@devinrsmith
Copy link
Author

I haven't heard any more activity on this ticket; I think it is still an issue.

@michael-redpanda
Copy link
Contributor

Hi @devinrsmith, thanks for reflagging this. We'll take a look at it and see if we can get a fix for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema-registry Schema Registry service within Redpanda kind/bug Something isn't working
Projects
None yet
4 participants