camunda / bpmnlint-plugin-camunda-compat

A bpmnlint plug-in for Camunda BPMN compatibility.
MIT License
0 stars 2 forks source link

feat(rule): add version compatibility rules #163

Closed Oleksiivanov closed 3 months ago

Oleksiivanov commented 4 months ago

related :

Changes Include:

Example:

Imagine there's a new feature in version 8.4 that isn't in version 8.3. If a user gets a new connector template with a property that version 8.3 doesn't support, they can still use it. But, they will see a warning that tells them about the version issue:

The property '${propertyName}' is incompatible with the current camunda runtime version. Please upgrade to version ${version} or later to use this feature.,

How to add a new property to the rule :


const propertyVersionCompatibility = [
  {
    minimalVersion: '8.6',
    extensionElement: 'zeebe:Properties',
    subExtensionElement: 'properties',
    key: 'name',
    propertyNames: ['messageTtl', 'consumeUnmatchedEvents', 'deduplicationModeManualFlag']
  },
  // Example for outbound connectors:
  {
    minimalVersion: '8.6',
    extensionElement: 'zeebe:IoMapping',
    subExtensionElement: 'inputParameters',
    key: 'target',
    propertyNames: ['newPropertyName']
  }
];
nikku commented 4 months ago

@Oleksiivanov Could you elaborate a little more on how this works and how you ensure that you do not interfere with other non-connector use-cases?

I see that we match for certain properties (zeebe:property but also zeebe:ioMapping). These can be used by arbitary tasks, for arbitrary use-cases. The least thing we want to do is to ensure that a certain zeebe:topic (task worker) is configured I think, before we go on to validate generic properties that anyone can technically use.

Oleksiivanov commented 4 months ago

@Oleksiivanov Could you elaborate a little more on how this works and how you ensure that you do not interfere with other non-connector use-cases?

I see that we match for certain properties (zeebe:property but also zeebe:ioMapping). These can be used by arbitary tasks, for arbitrary use-cases. The least thing we want to do is to ensure that a certain zeebe:topic (task worker) is configured I think, before we go on to validate generic properties that anyone can technically use.

Hi, @nikku, Sorry for the late answer, I was OOO the past days. Thank you. good point, and to make sure that this is related only to our connectors, I can add to the property list of connectors, or paths to the example : const propertyVersionCompatibility = [

  {
    minimalVersion: '8.6',
    extensionElement: 'zeebe:Properties',
    subExtensionElement: 'properties',
    key: 'name',
    propertyNames: [ 'messageTtl', 'consumeUnmatchedEvents', 'deduplicationModeManualFlag' ],
    modelerTemplate : [ 'io.camunda.connectors.inbound.KAFKA.v1', 'io.camunda.connectors.outbound' ]
  }
];

WDYT?

nikku commented 4 months ago

@Oleksiivanov I see two options:

Either one would accomplish different things:

If you ask me for a recommendation, then I'd go with :b:, because it reflects the run-time constraints independent of our OOTB element templates.

Oleksiivanov commented 4 months ago

@Oleksiivanov I see two options:

  • 🅰️ You could match elements with a specific (connector) element template assigned
  • 🅱️ You could match elements (service tasks) with a specific zeebe:taskDefinition#type assigned or another property (for inbound) that identifies the element from the connectors run-time point of view

Either one would accomplish different things:

  • 🅰️ stops to work when users customize the template, most strict (least false positives when matching)
  • 🅱️ is less strict; allows users to change the element template while still benefiting from the warnings; validates also when element templates are unbound; may cause false positives in the rare case where folks do not use connectors, but re-use the technical bindings that connectors build upon (I honestly think we can ignore this!)

If you ask me for a recommendation, then I'd go with 🅱️, because it reflects the run-time constraints independent of our OOTB element templates.

Thank you. I agree with you that checking by type looks better. For outbound connectors, it can be <zeebe:taskDefinition type="needed value"/>, and for inbound connectors, it will be <zeebe:property name="inbound.type" value="needed value"/>. So, I will introduce a commit with these changes soon

Oleksiivanov commented 4 months ago

Added matching by definition types for inbound and outbound connector

nikku commented 3 months ago

@philippfromme Please review this PR.

philippfromme commented 3 months ago

Looking at this and thinking about the long term I'd argue we want

Otherwise I'm worried about the maintainability of this plugin. Also, this plugin is strictly about engine compatibility and you could argue that the connector template rules are about engine plus connectors runtime compatibility.

@nikku What do you think? Overkill?

nikku commented 3 months ago

@philippfromme If we keep it here then we have good practices under control.

The least thing we could do is to prefix / namespace connector related rules, so that pulling them out will be possible in the future.

philippfromme commented 3 months ago

Okay, let's keep the rule(s) in this plugin but reorganize a bit. I'm looking into this now.

philippfromme commented 3 months ago

I made some changes:

brave_bLsnAwEBYM

philippfromme commented 3 months ago

I will go ahead and merge. @Oleksiivanov feel free to have a look.

nikku commented 3 months ago

Great job both :clap: