Apicurio / apicurio-registry

An API/Schema registry - stores APIs and Schemas.
https://www.apicur.io/registry/
Apache License 2.0
587 stars 260 forks source link

Apicurio 2.0.3 : ccompat/v6 bad version return #2796

Open mpumd opened 2 years ago

mpumd commented 2 years ago

Hi guys,

I have a question about the version management inside the apicurio registry. Inside the "version": field, we are free to set any alphanumeric character. As you can see below, we have the version 1 and 3 which are a string.

curl http://dev.company.com/apis/registry/v2/groups/default/artifacts/com.compagny.movie/versions | jq
{
  "count": 2,
  "versions": [
    {
      "name": "company-movie",
      "description": "super movie avro",
      "createdOn": "2022-09-14T13:29:20+0000",
      "createdBy": "",
      "type": "AVRO",
      "labels": [
        "avro",
        "kafka"
      ],
      "state": "ENABLED",
      "globalId": 56,
      "version": "1",
      "properties": {
        "custom1": "bubu",
        "custom2": "lala"
      },
      "contentId": 16
    },
    {
      "name": "company-movie",
      "description": "super movie avro",
      "createdOn": "2022-09-14T13:36:28+0000",
      "createdBy": "",
      "type": "AVRO",
      "labels": [
        "avro",
        "kafka"
      ],
      "state": "ENABLED",
      "globalId": 57,
      "version": "3",
      "properties": {
        "custom1": "bubu",
        "custom2": "lala"
      },
      "contentId": 20
    }
  ]
}

We found the same versions by the ccompat/v6 so consistent.

curl http://dev.company.com/apis/ccompat/v6/subjects/com.compagny.movie/versions |  jq 
[
    1,
    3
] 

Below, we have a version 2. Where is it from ? With the schema, I recognize the version 3 of above. So how this version is calculate in ccompat/v6 ? Our clients, GUI and other, uses this version 2 which create an ambiguous understanding on schema versionning.

curl http://dev.company.com/apis/ccompat/v6/subjects/com.compagny.movie/versions/latest | jq 
{                                                                                                                                                                                      
  "id": 20,
  "subject": "com.compagny.movie",
  "version": 2, 
  "schema": "{\"namespace\":\"com.company\",\"type\":\"record\",\"name\":\"Movie\",\"fields\":[{\"name\":\"title2\",\"type\":\"string\"},{\"name\":\"year\",\"type\":\"int\"}]}"
}

Can you explain how this version 2 is calculated ?
Thanks in advance.

apicurio-bot[bot] commented 2 years ago

Thank you for reporting an issue!

Pinging @EricWittmann to respond or triage.

EricWittmann commented 1 year ago

Oh this is perhaps an interesting bug. I'll have to dig into this to verify what I'm about to tell you is correct, but here's what I think is happening:

Registry supports user-defined versions. So when you create or update an artifact, you can (optionally) provide a version. It doesn't have to be a number. It can be whatever string value you want. So instead of "1" and "2" you could have instead used "1.0.0" and "1.0.1" (as an example). However, for a variety of reasons, we also have a versionId that is automatically calculated for each version. That value is always numeric. And we use that numeric calculated value if you choose to not provide a value.

If you're using our core API, everything should work nicely. If you use the Confluent compatibility API, it appears there may be a bug.

Just checking the code now. For the "Get Versions" endpoint, we are getting back a list of the string versions (the ones optionally provided by the user) and returning those. This is why you see 1, 3 as you expect. However, the implementation of "Get Subject With Version" returns the versionId instead of the version. I would argue this is a simple bug, and it's very easily fixable.

Please note that if you were to use non-numeric versions, you would then no longer be able to use the ccompat API. We would try to convert the version numbers to numeric and it would fail.

mpumd commented 1 year ago

Hi Eric,

Thanks for your answer. You confirm what I've investigated :). I think it's not a technical bug because you must be compliant with confluent api. But my problem is more complex and is out of apicurio. Some kafka registry frontends exist and use the ccompat/v6 api like akhq. We have also a redhat amqstream frontend that use registry/v2. For a developer, it's confuse de have potentially two differents versions format for a schema.

I resume your example :

label       versionId
1.0.0           1
1.0.1           2
1.1.0           3
2.0.0           4       latest

Our workaround consists to force an incremental number inserted in order.

label       versionId
"1"         1
"2"         2
"3"         3
"4"         4       latest

Moreover, latest version is confuse too. For a developer, latest is the bigger number that identify a version. In apicurio it's the last inserted version. Below, 3 is the latest version.

label       versionId
"1"         1
"4"         2
"2"         3
"3"         4       latest

So we must create each versions in ascending order like the second example when you considered error recovery like flushing apicurio db by production crash for example. It's very unyielding. Finally we have an ergonomic problem which is to have same number identifier for a schema version on ccompat/v6 and registry/v2.

So like you said, we have a choice of no use the label version ? This api /apis/registry/v2/groups/default/artifacts/com.compagny.movie/versions could return a same version numbers of ccompat/v6 like "version": 1 ?

Thanks for your investigations.

EricWittmann commented 1 year ago

I understand and agree with everything you've said here. We definitely have challenges with versions. Part of the challenge is with ccompat, but that's not all (as you've observed). We're discussing now how to support GitOps workflows, and the ordering of versions is of particular interest in that use case. It's something we'll need to solve, but we're not quite sure yet.

I would also add that if you use the core v2 API and simply don't provide a version string (e.g. do not send the X-Registry-Version header), then the version will simply be a string conversion of the versionId. In that case, the list of versions you get back from the core v2 endpoint you referenced would be the same as what you get back from the ccompat api (except formatted as string rather than numbers, but with the same values).

vsevel commented 1 year ago

Hello @EricWittmann Your last comment relates also to https://github.com/Apicurio/apicurio-registry-content-sync-operator/issues/101 Our intention is to use the operator and a pure gitops approach. So I would rather not use the core API directly, and be explicit about the versions in the CR, rather than trusting that the order of apply will do the right thing.

During normal operations (today I create a version, and next month another version) this may be livable. But it won't be as soon as you start dealing with unexpected situations:

I should be able to apply CRs in any order. and any suite of actions leading to a certain state from the CR perspective should lead to the same corresponding state in apicurio. for instance I should end up with the same result in apicurio if I do any one of these 3 scenarios:

If an action is invalid, then the operation (e.g. delete v1) should be immediately vetoed, or (for create/update) operations the status should be in error.

let's summarize this by saying that I would rather by explicit about the version number.

EricWittmann commented 1 year ago

Yes understood. I don't have a viable solution right now, but we're in agreement about the problem.

@andreaTP @carlesarnal @jsenko anything to add to this conversation?

vsevel commented 1 year ago

any news on this?

EricWittmann commented 1 year ago

Not yet. We think the right way to handle this type of scenario is by adding semver support. However, that has all sorts of complicated consequences. For example, it would be nice to have the Compatibility rule configured differently if the patch version changes vs. if the major version changes.

But we could start with just using semver for ordering purposes.

Also note that we likely will have some gitops requirements coming soon, which will make us feel your pain and perhaps bump the priority of this.

rmarting commented 1 year ago

I would like to visualize an issue related to the different version types in the integration of Apicurio Registry with a ccompat compliant application, like Kafka-UI. Basically, as soon the schemas in Apicurio use a SemVer approach (or anything with a dot .) then the ccompac api fails.

Totally agree with all the comments of this issue, and it is an not easy way to fix it, however, I am thinking about a workaround to avoid breaking the ccompact api. An idea could be check the conversion of the version of the artifact, and in the case of failure because it is not an integer, use the internal versionId of the artifact, that always is an integer. I know that idea will not change the behavior described in the comment, but at least the api will not break with an exception because the version is not an integer.

What do you think?

References:

vsevel commented 1 year ago

Also note that we likely will have some gitops requirements coming soon, which will make us feel your pain and perhaps bump the priority of this.

has this happened @EricWittmann ?

vsevel commented 1 year ago

hello any news on this? thanks.

carlesarnal commented 1 year ago

We do have some news on this. @jsenko has been working on a gitops approach and there is a PR #3611 opened with it. At the same time, I've been working on improving the compatibility API, mostly addressing problems around enabling/disabling different versions. Hopefully, with those two PRs, the problems mentioned here will be addressed, especially thanks to @jsenko's work.