feat: Add ConnectionDefinition support to registration utilities
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.iocase toFindEntityTypeinutils/utils.go - Made
getEntitypublic asGetEntityinmodels/registration/utils.goso it can be tested - Updated
PackagingUnitinmodels/registration/register.goto 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
ConnectionDefinitiontype inmodels/meshmodel/core/v1beta1/connection.go - Added
ConnectionSchemaVersionconstant in the corev1beta1 package
Based on reviewer feedback, I've made the following adjustments:
- Reverted
GetEntityback to unexportedgetEntitysince it's not used outside the package - Moved
ConnectionSchemaVersionconstant 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:
FindEntityTypeproperly identifies connection definition filesgetEntityparses 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.
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.
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
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.
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
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!
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.
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?
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
Connectionstruct from the schemas repo, but I found that it doesn't implement theentity.Entityinterface that our registration system requires. TheConnectionstruct'sCreatemethod signature is different from what ourEntityinterface expects.For now, I've kept the
ConnectionDefinitionin MeshKit to get the registration functionality working. The proper long-term solution would be to create aConnectionDefinitionstruct in the schemas repo that implements theEntityinterface, similar to howComponentDefinitionandModelDefinitionwork.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
ConnectionDefinitionthere?
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:
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
Connectionstruct from the schemas repo, but I found that it doesn't implement theentity.Entityinterface that our registration system requires. TheConnectionstruct'sCreatemethod signature is different from what ourEntityinterface expects. For now, I've kept theConnectionDefinitionin MeshKit to get the registration functionality working. The proper long-term solution would be to create aConnectionDefinitionstruct in the schemas repo that implements theEntityinterface, similar to howComponentDefinitionandModelDefinitionwork. 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 properConnectionDefinitionthere?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.
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
Connectionstruct from the schemas repo, but I found that it doesn't implement theentity.Entityinterface that our registration system requires. TheConnectionstruct'sCreatemethod signature is different from what ourEntityinterface expects. For now, I've kept theConnectionDefinitionin MeshKit to get the registration functionality working. The proper long-term solution would be to create aConnectionDefinitionstruct in the schemas repo that implements theEntityinterface, similar to howComponentDefinitionandModelDefinitionwork. 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 properConnectionDefinitionthere?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
Connectionstruct.Here's what I found: the
Connectionstruct 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 thatConnectionis the first parameter - it's the thing doing the registering, not the thing being registered. AddingType(),GetID(), andGetEntityDetail()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... ππ
@n2h9 kindly let me know if the changes I did works for us π
@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 I have tested locally - it was working fine. I'll upload the test results shortly to confirm it π
https://github.com/user-attachments/assets/d34a5938-97f5-4430-913f-e4490c49014f
@leecalcote @n2h9
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?
Could you please also remove no-lint around opa packages import, there is a separate pr which fixes the opa packages lint errors
Could you please also remove no-lint around opa packages import, there is a separate pr which fixes the opa packages lint errors
Sure...
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...?
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 π π
- 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
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:
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
Connectionentities 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 βΊοΈ
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.
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 π₯²
Hey @n2h9, Let me know if this works...
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:
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..
Hey @n2h9 , is it the issue being resolved corresponding to this pr ?
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 ;
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.