xlab-si / xopera-opera

xOpera orchestrator compliant with TOSCA YAML v1.3 in the making
https://xlab-si.github.io/xopera-docs/
Apache License 2.0
35 stars 14 forks source link

Multiple requirement or capability names should not be allowed in get_property and get_attribute functions #101

Closed alexmaslenn closed 3 years ago

alexmaslenn commented 4 years ago

Description

When defining get_property or get_attribute functions multiple requirement or capability names are allowed, e.g.

public_address:       { default: { get_attribute: [ SELF, host, host, public_address ] } }

would be a valid definition from xOpera perspective.

According to 4.4.2.1 Grammar in Property functions only one <optional_req_or_cap_name> is allowed

get_property: [ <modelable_entity_name>, <optional_req_or_cap_name>, <property_name>, 
<nested_property_name_or_index_1>, ..., <nested_property_name_or_index_n> ]

Same applies to get_attribute function

Current behaviour

xOpera would proceed without errors.

Expected results

xOpera should return a parsing error.

anzoman commented 4 years ago

Good find @alexmaslenn! We will need to fix this.

anzoman commented 3 years ago

After looking a little closer to TOSCA functions I can share some more things. It's true that only one requirement or capability name should be allowed within get_attribute or get_property and it would be really nice to determine that. On the other hand we have a little problem because optional_req_or_cap_name is as the name says optional, which is really annoying. Why would you even allow an optional value within a YAML list and why not use a YAML map instead. It's even worse because this optional keyname is between the two required values and not for instance at the beginning. It's also good to know that there can be an unlimited number of parameters within TOSCA function's YAML list because of the nested attributes/properties. The only limit is that we will always have two values or more - and if we have more this can mean that we used optional_req_or_cap_name or that we have some nested property name.

So, it's relatively hard to check all these possibilities. With the latest PR #215, opera will try to extend the support for the two aforementioned TOSCA functions, so that these kind of problems would not go through unnoticed.