Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uniqueness of Type Names #341

Open
milesstoetzner opened this issue Aug 23, 2024 · 2 comments
Open

Uniqueness of Type Names #341

milesstoetzner opened this issue Aug 23, 2024 · 2 comments

Comments

@milesstoetzner
Copy link

Are type names unique per type collection (node types, artifact types, interface types, etc.) or per service template?

I am currently developing my own normative types.
Thereby, I also define some root types for each type collection and call them all root.
I assumed that even though these types have the same name, they are distinct since they are from different collections.
However, this does not seem to be the case in Unfurl.

I did not find anything regarding the uniqueness of type names in the TOSCA spec.

Nevertheless, the current behaviour of Unfurl seems odd to me.
There should be at least an error when two types have the same name (and not overridden?).

Here is a simple example.

# Ensemble A
apiVersion: unfurl/v1alpha1
kind: Ensemble
spec:
  service_template:
    artifact_types:
      root: 
        derived_from: tosca.artifacts.Root

      my_artifact_type:
        derived_from: root

    interface_types:
      root: 
        derived_from: tosca.interfaces.Root
        metadata: 
          key: value

    node_types:
      my_app:
        derived_from: tosca.nodes.Root

    topology_template:
      node_templates:
        my_app:
          type: my_app
          
          artifacts:
            my_artifact:
              type: my_artifact_type
              file: my_artifact_file

Validation will fail as follows since due to some reason interface_types.root is treated as artifact_types.root and, hence, throws the validation error mentioned in #340.

(.venv) stoetzms@storm:~/unfurl-issue$ unfurl validate
   INFO   UNFURL Using home project at: "/home/stoetzms/.unfurl_home/unfurl.yaml"
   INFO   UNFURL Loaded project at /home/stoetzms/unfurl-issue/unfurl.yaml
   INFO   UNFURL Vault password found, configuring vault ids: ['unfurl-issueVM', '.unfurl_homeF0']
   INFO   UNFURL Validating TOSCA template at /home/stoetzms/unfurl-issue/ensemble/ensemble.yaml
Exiting with error: TOSCA validation failed for /home/stoetzms/unfurl-issue/ensemble/ensemble.yaml:
UnknownFieldError: Artifacttype "root" contains unknown field "metadata". Refer to the definition to verify valid values. in node template "my_app"
UnknownFieldError: Artifacttype "root" contains unknown field "metadata". Refer to the definition to verify valid values. in node template "my_app"

Having unique names solves this issue.

# Ensemble B
apiVersion: unfurl/v1alpha1
kind: Ensemble
spec:
  service_template:
    artifact_types:
      root: 
        derived_from: tosca.artifacts.Root

      my_artifact_type:
        derived_from: root

    interface_types:
      interfaces.root:  # <------------ renamed this
        derived_from: tosca.interfaces.Root
        metadata: 
          key: value

    node_types:
      my_app:
        derived_from: tosca.nodes.Root

    topology_template:
      node_templates:
        my_app:
          type: my_app
          
          artifacts:
            my_artifact:
              type: my_artifact_type
              file: my_artifact_file

I tried this on Unfurl v1.0.0 and v1.1.0.

Best regards
Miles

@aszs
Copy link
Member

aszs commented Oct 12, 2024

The tosca parser puts them all in one namespace and it isn't an error to reassign, though it should probably be a warning at least.
If you think of tosca as a programing language, most languages declaring name binding in one scope will just shadow or override the same name declared in another scope. I think that's the reasoning at least... Regardless, changing this would be a breaking change, but I'll keep this issue open to at least warn, or maybe there could be a strict mode that you could opt-in to, maybe via metadata on the service template.

@milesstoetzner
Copy link
Author

Thanks for the insights.

though it should probably be a warning at least

A warning would be great.

but I'll keep this issue open to at least warn

I am also fine with closing this issue as long as you do not intend to implement anything regarding this issue, e.g., the warning.
A closed issue can still be found via the search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants