tliron / puccini

Cloud topology management and deployment tools based on TOSCA
https://puccini.cloud
Apache License 2.0
88 stars 20 forks source link

Support for metadata attribute in the property_definitions #146

Closed abhishek-kumar0409 closed 3 weeks ago

abhishek-kumar0409 commented 2 months ago

Issue: Puccni is rejecting the descriptors having metadata attribute within the property definitions.

Feature request: Support for the metadata in the property definitions

Steps to reproduce issue:

Command Executed: puccini-tosca compile my_sample_nf_vnfd.yaml -x etsinfv

Command Output:
data_types["mycompanyXYZ.datatypes.nfv.VnfInfoModifiableAttributesExtensions"].properties["password"].metadata: unsupported keyname

VNFD-Snippet

data_types:
  mycompanyXYZ.datatypes.nfv.VnfInfoModifiableAttributes:
    derived_from: tosca.datatypes.nfv.VnfInfoModifiableAttributes
    properties:
      extensions:
        type:   mycompanyXYZ.datatypes.nfv.VnfInfoModifiableAttributesExtensions

    mycompanyXYZ.datatypes.nfv.VnfInfoModifiableAttributesExtensions:
    derived_from: tosca.datatypes.nfv.VnfInfoModifiableAttributesExtensions
    properties:

      security_group:
        type: string
        required: false
        default: ""

      password:
        type: string
        required: false
        metadata:
           sensitive: "true"

Per TOSCA-1.3 spec, the metadata seems to be an optional parameter.

image

Currently, Puccini only support few of the properties. Can the support be extended for the metadata attribute?

tosca/grammars/cloudify_v1_3/property-definition.go

type PropertyDefinition struct {
    *ParameterDefinition `name:"property definition"`
    Required *bool `read:"required"`
}
abhishek-kumar0409 commented 1 month ago

@tliron - FYI - This issue was reproduced only on TOSCA-1.2 packages. Let me know your thoughts.

tliron commented 1 month ago

What do you mean? Do you mean that this issue appears in TOSCA 1.2 but not in TOSCA 1.3?

abhishek-kumar0409 commented 1 month ago

Hey Tal - I meant - the issue was reproduced in VNF Packages conforming to ETSI SOL-001 v2.5.1 (i.e. TOSCA-1.2). At the moment, I am not sure if this issue is reproducible in ETSI SOL-001 v3.3.1 (TOSCA 1.3).
Nevertheless, this enhancement will help us to establish to have some parity with other opensource tosca parsers as well as we don't encounter the errors in those toolsets. We would like to see this feature in both TOSCA-1.2 and 1.3 grammars.

abhishek-kumar0409 commented 1 month ago

Hi @tliron - Would you be able to add a fix for this issue?

tliron commented 3 weeks ago

@abhishek-kumar0409 I try to follow the spec as literally as possible. Since this feature did not exist in TOSCA 1.2, it shouldn't normally be supported. However, I can add a quirk to specifically enable it. Would that suit you?

tliron commented 3 weeks ago

Actually, I am seeing that it is specified in TOSCA 1.2. Let me try to figure out what is going on.

abhishek-kumar0409 commented 3 weeks ago

@tliron - Appreciate your help. Is the fix available in the v0.22.5 version of the binary? Or, do you plan to release a patch version soon ?

tliron commented 3 weeks ago

I would appreciate if you could build and test from main branch before I release a version.

abhishek-kumar0409 commented 3 weeks ago

Thanks Tal. Can you suggest how to build the binary in mac ?

abhishek-kumar0409 commented 2 weeks ago

Ignore my above request. I was getting error because my go version was 1.21.

I compiled the package using the new binary and I'm getting the same error.

PROBLEMS (1)
    @410,9 data_types["mycompany.datatypes.nfv.VnfInfoModifiableAttributesExtensions"].properties["password"].metadata: unsupported keyname
tliron commented 2 weeks ago

Here is the example I just tried, and it works for me. Are you sure you are using the puccini-tosca executable you built and not the one you've downloaded?

tosca_definitions_version: tosca_simple_yaml_1_2

data_types:

  Planet:
    properties:
      radius:
        type: float
        metadata:
          galaxy: Milky Way

node_types:

  Server:
    derived_from: tosca:Compute
    metadata:
      galactic.planet: Jupiter
    version: 1.0.0
    properties:
      planet:
        type: Planet
        required: false
        metadata:
          galaxy: Andromeda
abhishek-kumar0409 commented 2 weeks ago

My bad. I messed up with the binary. The fix is working fine now. Thanks a zillion for your help.