meshkit icon indicating copy to clipboard operation
meshkit copied to clipboard

feat: Add ConnectionDefinition support to registration utilities

Open HeerakKashyap opened this issue 5 months ago β€’ 29 comments

This PR adds support for ConnectionDefinition entities in the MeshKit registration system. Previously, when importing models that contained connection definitions, these entities were being silently ignored because the registration pipeline didn't know how to handle them.

What was broken

The main issue was in utils/utils.go - the FindEntityType function was missing a case for connections.meshery.io, so it couldn't identify connection definition files. This meant connection definitions were being skipped during model import.

What I fixed

  • Added the missing connections.meshery.io case to FindEntityType in utils/utils.go
  • Made getEntity public as GetEntity in models/registration/utils.go so it can be tested
  • Updated PackagingUnit in models/registration/register.go to include connections and handle their registration
  • Added connection file processing in models/registration/dir.go
  • Added connection support to the database schema in models/meshmodel/registry/registry.go
  • Created the ConnectionDefinition type in models/meshmodel/core/v1beta1/connection.go
  • Added ConnectionSchemaVersion constant in the corev1beta1 package

Based on reviewer feedback, I've made the following adjustments:

  • Reverted GetEntity back to unexported getEntity since it's not used outside the package
  • Moved ConnectionSchemaVersion constant to the corev1beta1 package
  • Removed unnecessary changes to OPA and OCI files that were unrelated to the ConnectionDefinition task
  • Kept only the essential changes needed for ConnectionDefinition support

Testing

I tested this with a sample ConnectionDefinition JSON file. The pipeline now works correctly:

  • FindEntityType properly identifies connection definition files
  • getEntity parses them into the right type
  • The registration process handles them end-to-end

Before this fix, connection definitions were just being ignored. Now the registry can properly handle all entity types: components, relationships, and connections.

fixes- #762


Notes for Reviewers

  • Signed commits
  • [x] Yes, I signed my commits.

HeerakKashyap avatar Jul 28 '25 14:07 HeerakKashyap

Thank you for your contribution!! If this work is complete, this is a good item to add to the weekly Meshery Development meeting agenda. You can add this item in the doc, attend, and present it. Meeting details at https://meet.layer5.io/meshery.

vr-varad avatar Aug 05 '25 21:08 vr-varad

Thank you for your contribution!! If this work is complete, this is a good item to add to the weekly Meshery Development meeting agenda. You can add this item in the doc, attend, and present it. Meeting details at https://meet.layer5.io/meshery.

Yeah sure, I guess there are some small errors which needs to be solved, if fixed timely - I'll try to present it in development meeting, thankss @vr-varad

HeerakKashyap avatar Aug 06 '25 03:08 HeerakKashyap

Thank you @HeerakKashyap for your contribution!! If this work is complete, this is a good item to add to the weekly Meshery Development meeting agenda. You can add this item in the doc, attend, and present it. Meeting details at https://meet.layer5.io/meshery.

vr-varad avatar Aug 12 '25 19:08 vr-varad

Thank you @HeerakKashyap for your contribution!! If this work is complete, this is a good item to add to the weekly Meshery Development meeting agenda. You can add this item in the doc, attend, and present it. Meeting details at https://meet.layer5.io/meshery.

Thank you @HeerakKashyap for your contribution!! If this work is complete, this is a good item to add to the weekly Meshery Development meeting agenda. You can add this item in the doc, attend, and present it. Meeting details at https://meet.layer5.io/meshery.

Yeah I'll let u know as soon as I am done with solving the bugs

HeerakKashyap avatar Aug 12 '25 19:08 HeerakKashyap

Hey @n2h9 @leecalcote, I've updated the PR with all the fixes for the issues we were seeing. The description now explains what was broken and how I fixed it. Would appreciate a review when you have time!

HeerakKashyap avatar Aug 14 '25 20:08 HeerakKashyap

Hey hey, @HeerakKashyap hello :wave: :innocent:

You need to take connection definition from the schema repo and not define it in meshkit. Similar how f.e. in getEntity: it is done for models, components and relationship.

n2h9 avatar Aug 16 '25 19:08 n2h9

Hey hey, @HeerakKashyap hello πŸ‘‹ πŸ˜‡

You need to take connection definition from the schema repo and not define it in meshkit. Similar how f.e. in getEntity: it is done for models, components and relationship.

Thanks for the feedback @n2h9! I looked into using the existing Connection struct from the schemas repo, but I found that it doesn't implement the entity.Entity interface that our registration system requires. The Connection struct's Create method signature is different from what our Entity interface expects.

For now, I've kept the ConnectionDefinition in MeshKit to get the registration functionality working. The proper long-term solution would be to create a ConnectionDefinition struct in the schemas repo that implements the Entity interface, similar to how ComponentDefinition and ModelDefinition work.

This way we can move forward with the registration system while maintaining the correct architectural pattern. Would you like me to create a separate PR to the schemas repo to add the proper ConnectionDefinition there?

HeerakKashyap avatar Aug 17 '25 15:08 HeerakKashyap

Hey hey, @HeerakKashyap hello πŸ‘‹ πŸ˜‡ You need to take connection definition from the schema repo and not define it in meshkit. Similar how f.e. in getEntity: it is done for models, components and relationship.

Thanks for the feedback @n2h9! I looked into using the existing Connection struct from the schemas repo, but I found that it doesn't implement the entity.Entity interface that our registration system requires. The Connection struct's Create method signature is different from what our Entity interface expects.

For now, I've kept the ConnectionDefinition in MeshKit to get the registration functionality working. The proper long-term solution would be to create a ConnectionDefinition struct in the schemas repo that implements the Entity interface, similar to how ComponentDefinition and ModelDefinition work.

This way we can move forward with the registration system while maintaining the correct architectural pattern. Would you like me to create a separate PR to the schemas repo to add the proper ConnectionDefinition there?

I think we need to add missing functions to connection in connection_helper.go to make it implement required interface. Use Component and Model entities (they also have xxx_helper.go file in schema repo. :innocent:

thank you :bowing_woman:

n2h9 avatar Aug 17 '25 19:08 n2h9

Hey hey, @HeerakKashyap hello πŸ‘‹ πŸ˜‡ You need to take connection definition from the schema repo and not define it in meshkit. Similar how f.e. in getEntity: it is done for models, components and relationship.

Thanks for the feedback @n2h9! I looked into using the existing Connection struct from the schemas repo, but I found that it doesn't implement the entity.Entity interface that our registration system requires. The Connection struct's Create method signature is different from what our Entity interface expects. For now, I've kept the ConnectionDefinition in MeshKit to get the registration functionality working. The proper long-term solution would be to create a ConnectionDefinition struct in the schemas repo that implements the Entity interface, similar to how ComponentDefinition and ModelDefinition work. This way we can move forward with the registration system while maintaining the correct architectural pattern. Would you like me to create a separate PR to the schemas repo to add the proper ConnectionDefinition there?

I think we need to add missing functions to connection in connection_helper.go to make it implement required interface. Use Component and Model entities (they also have xxx_helper.go file in schema repo. πŸ˜‡

thank you πŸ™‡β€β™€οΈ

Hey @n2h9! Thanks for the feedback! πŸ‘‹ πŸ˜…

I get ur point - we should definitely use the schemas repo definitions when possible. But I ran into a bit of a problem when I tried to add those Entity methods to the existing Connection struct.

Here's what I found: the Connection struct from the schemas repo is actually playing a different role in our system. It's acting as a registrant (like a host/connection that registers other entities), not as an entity definition itself.

If you look at RegistryManager.RegisterEntity(), you'll see that Connection is the first parameter - it's the thing doing the registering, not the thing being registered. Adding Type(), GetID(), and GetEntityDetail() methods to it would kinda break that role and create some confusion about what it's supposed to do.

HeerakKashyap avatar Aug 17 '25 20:08 HeerakKashyap

Hey hey, @HeerakKashyap hello πŸ‘‹ πŸ˜‡ You need to take connection definition from the schema repo and not define it in meshkit. Similar how f.e. in getEntity: it is done for models, components and relationship.

Thanks for the feedback @n2h9! I looked into using the existing Connection struct from the schemas repo, but I found that it doesn't implement the entity.Entity interface that our registration system requires. The Connection struct's Create method signature is different from what our Entity interface expects. For now, I've kept the ConnectionDefinition in MeshKit to get the registration functionality working. The proper long-term solution would be to create a ConnectionDefinition struct in the schemas repo that implements the Entity interface, similar to how ComponentDefinition and ModelDefinition work. This way we can move forward with the registration system while maintaining the correct architectural pattern. Would you like me to create a separate PR to the schemas repo to add the proper ConnectionDefinition there?

I think we need to add missing functions to connection in connection_helper.go to make it implement required interface. Use Component and Model entities (they also have xxx_helper.go file in schema repo. πŸ˜‡ thank you πŸ™‡β€β™€οΈ

Hey @n2h9! Thanks for the feedback! πŸ‘‹ πŸ˜…

I totally get where you're coming from - we should definitely use the schemas repo definitions when possible. But I ran into a bit of a problem when I tried to add those Entity methods to the existing Connection struct.

Here's what I found: the Connection struct from the schemas repo is actually playing a different role in our system. It's acting as a registrant (like a host/connection that registers other entities), not as an entity definition itself.

If you look at RegistryManager.RegisterEntity(), you'll see that Connection is the first parameter - it's the thing doing the registering, not the thing being registered. Adding Type(), GetID(), and GetEntityDetail() methods to it would kinda break that role and create some confusion about what it's supposed to do.

or maybe @leecalcote could help us give their opinion regarding this... πŸ˜ŠπŸ™‡

HeerakKashyap avatar Aug 17 '25 20:08 HeerakKashyap

@n2h9 kindly let me know if the changes I did works for us πŸ™‡

HeerakKashyap avatar Aug 22 '25 09:08 HeerakKashyap

@n2h9 kindly let me know if the changes I did works for us πŸ™‡

@HeerakKashyap, test your changes and provide confirmation of those test results.

leecalcote avatar Aug 22 '25 11:08 leecalcote

@leecalcote I have tested locally - it was working fine. I'll upload the test results shortly to confirm it πŸ‘

HeerakKashyap avatar Aug 22 '25 12:08 HeerakKashyap

https://github.com/user-attachments/assets/d34a5938-97f5-4430-913f-e4490c49014f

@leecalcote @n2h9

HeerakKashyap avatar Aug 22 '25 19:08 HeerakKashyap

Hey hey @HeerakKashyap hello :wave: :innocent:

  • you need to take Connection struct from schema, here.
  • you do not need to create ConnectionDefinition wrapper around it.
  • if you need to enhance Connection struct with functions, you can do it in connection_helper.go
  • You need to take this value const ConnectionSchemaVersion = "connections.meshery.io/v1beta1" from here;

Could you please check how it is right now done for model, component and relationship?

n2h9 avatar Aug 23 '25 09:08 n2h9

Could you please also remove no-lint around opa packages import, there is a separate pr which fixes the opa packages lint errors

n2h9 avatar Aug 23 '25 09:08 n2h9

Could you please also remove no-lint around opa packages import, there is a separate pr which fixes the opa packages lint errors

Sure...

HeerakKashyap avatar Aug 23 '25 10:08 HeerakKashyap

Could you please also remove no-lint around opa packages import, there is a separate pr which fixes the opa packages lint errors

So should I leave it for now as it is...?

HeerakKashyap avatar Aug 25 '25 13:08 HeerakKashyap

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ˜‡

  • you need to take Connection struct from schema, here.
  • you do not need to create ConnectionDefinition wrapper around it.
  • if you need to enhance Connection struct with functions, you can do it in connection_helper.go
  • You need to take this value const ConnectionSchemaVersion = "connections.meshery.io/v1beta1" from here;

Could you please check how it is right now done for model, component and relationship?

I understand we don't want a ConnectionDefinition wrapper, but I'm facing a technical constraint: The Issue: The Connection struct from schemas has a Type field (in connection.go under meshkit/models/meshmodel/core/v1beta1), which conflicts with the Entity interface's required Type() method. This prevents the Connection struct from directly implementing the Entity interface.I Used Connection struct from schemas, Used ConnectionSchemaVersion from schemas, Couldn't add Entity methods to Connection due to field/method name conflict. How should I handle this? The registration system expects entities that implement the Entity interface, but the Connection struct can't implement it directly due to the Type field conflict.

HeerakKashyap avatar Aug 25 '25 15:08 HeerakKashyap

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ˜‡

  • you need to take Connection struct from schema, here.
  • you do not need to create ConnectionDefinition wrapper around it.
  • if you need to enhance Connection struct with functions, you can do it in connection_helper.go
  • You need to take this value const ConnectionSchemaVersion = "connections.meshery.io/v1beta1" from here;

Could you please check how it is right now done for model, component and relationship?

I understand we don't want a ConnectionDefinition wrapper, but I'm facing a technical constraint: The Issue: The Connection struct from schemas has a Type field (in connection.go under meshkit/models/meshmodel/core/v1beta1), which conflicts with the Entity interface's required Type() method. This prevents the Connection struct from directly implementing the Entity interface.I Used Connection struct from schemas, Used ConnectionSchemaVersion from schemas, Couldn't add Entity methods to Connection due to field/method name conflict. How should I handle this? The registration system expects entities that implement the Entity interface, but the Connection struct can't implement it directly due to the Type field conflict.

@n2h9

HeerakKashyap avatar Aug 26 '25 12:08 HeerakKashyap

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ˜‡

  • you need to take Connection struct from schema, here.
  • you do not need to create ConnectionDefinition wrapper around it.
  • if you need to enhance Connection struct with functions, you can do it in connection_helper.go
  • You need to take this value const ConnectionSchemaVersion = "connections.meshery.io/v1beta1" from here;

Could you please check how it is right now done for model, component and relationship?

I understand we don't want a ConnectionDefinition wrapper, but I'm facing a technical constraint: The Issue: The Connection struct from schemas has a Type field (in connection.go under meshkit/models/meshmodel/core/v1beta1), which conflicts with the Entity interface's required Type() method. This prevents the Connection struct from directly implementing the Entity interface.I Used Connection struct from schemas, Used ConnectionSchemaVersion from schemas, Couldn't add Entity methods to Connection due to field/method name conflict. How should I handle this? The registration system expects entities that implement the Entity interface, but the Connection struct can't implement it directly due to the Type field conflict.

Hey hey @HeerakKashyap hello :wave: :hugs: !

Indeed there is a conflict :cry: , could be a blocker.

We want to achieve is that the result PackagingUnit will contain the list of Connection entities from schema. You may wish to think about how to re-arrange the code in meshkit to achieve this result in efficient way. F.e. rename the method name of an interface or the name of a Connection property (probably interface method will be easier to rename). Probably there is some easier and smarter way to achieve this.

Thank you :bowing_woman:

n2h9 avatar Aug 26 '25 19:08 n2h9

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ˜‡

  • you need to take Connection struct from schema, here.
  • you do not need to create ConnectionDefinition wrapper around it.
  • if you need to enhance Connection struct with functions, you can do it in connection_helper.go
  • You need to take this value const ConnectionSchemaVersion = "connections.meshery.io/v1beta1" from here;

Could you please check how it is right now done for model, component and relationship?

I understand we don't want a ConnectionDefinition wrapper, but I'm facing a technical constraint: The Issue: The Connection struct from schemas has a Type field (in connection.go under meshkit/models/meshmodel/core/v1beta1), which conflicts with the Entity interface's required Type() method. This prevents the Connection struct from directly implementing the Entity interface.I Used Connection struct from schemas, Used ConnectionSchemaVersion from schemas, Couldn't add Entity methods to Connection due to field/method name conflict. How should I handle this? The registration system expects entities that implement the Entity interface, but the Connection struct can't implement it directly due to the Type field conflict.

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ€— !

Indeed there is a conflict 😒 , could be a blocker.

We want to achieve is that the result PackagingUnit will contain the list of Connection entities from schema. You may wish to think about how to re-arrange the code in meshkit to achieve this result in efficient way. F.e. rename the method name of an interface or the name of a Connection property (probably interface method will be easier to rename). Probably there is some easier and smarter way to achieve this.

Thank you πŸ™‡β€β™€οΈ

Okayy sure, I'll look into this ☺️

HeerakKashyap avatar Aug 26 '25 21:08 HeerakKashyap

Hey hey @HeerakKashyap hello :wave: :hugs: !

You are validating data over the connectionDefinition, which is a wrapper over connection entity. I think we should validate directly connection entity, because it is supposed to receive connection entity (not a wrapper) inside a model folder / archive when we call to model import f.e.

n2h9 avatar Aug 29 '25 18:08 n2h9

Hey hey @HeerakKashyap hello πŸ‘‹ πŸ€— !

You are validating data over the connectionDefinition, which is a wrapper over connection entity. I think we should validate directly connection entity, because it is supposed to receive connection entity (not a wrapper) inside a model folder / archive when we call to model import f.e.

I am working on it, I'll let u know about the same asap @n2h9. Currently busy with college exams πŸ₯²

HeerakKashyap avatar Sep 02 '25 10:09 HeerakKashyap

Hey @n2h9, Let me know if this works...

HeerakKashyap avatar Sep 03 '25 09:09 HeerakKashyap

Hey @n2h9, Let me know if this works...

hey hey Heerak, hello :wave: . Thank you, validation over the connection model looks good :slightly_smiling_face: !

Couple points:

  • we do not need to define connection json schema in meshkit schemas/connections/connectionDefinition.json ;
  • we need to add Connection to resulting PackagingUnit ;

Thank you :hugs:

n2h9 avatar Sep 04 '25 20:09 n2h9

Hey @n2h9, Let me know if this works...

hey hey Heerak, hello πŸ‘‹ . Thank you, validation over the connection model looks good πŸ™‚ !

Couple points:

  • we do not need to define connection json schema in meshkit schemas/connections/connectionDefinition.json ;
  • we need to add Connection to resulting PackagingUnit ;

Thank you πŸ€—

okay, i'll fix these..

HeerakKashyap avatar Sep 06 '25 11:09 HeerakKashyap

Hey @n2h9 , is it the issue being resolved corresponding to this pr ?

HeerakKashyap avatar Sep 24 '25 09:09 HeerakKashyap

Hey @n2h9 , is it the issue being resolved corresponding to this pr ?

Hey hey, @HeerakKashyap :wave: hello :innocent:

I think that this part is still pending:

  • we need to add Connection to resulting PackagingUnit ;

n2h9 avatar Sep 26 '25 19:09 n2h9

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Nov 12 '25 00:11 stale[bot]