tliron / puccini

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

Path references in CSAR file starting with a slash are not supported #141

Closed jpc4242 closed 5 months ago

jpc4242 commented 5 months ago

Some preliminary context:

When a file_URI starts with a dash, what does this mean? I was part of a discussion about that in ETSI SOL group, which ended in a formal request for clarification to OASIS TOSCA committee. The answer, (which is now explicitly incorporated into TOSCA 2.0 but was intended also for tosca 1.2 or 1.3) , goes likes this:

•   If the <file_URI> does not include a URL scheme, it is a considered a relative path URL. The TOSCA orchestrator or processor SHOULD handle such a <file_URI> as follows:
    –   If the import definition also specifies a <repository_name> (using the repository keyname), then <file_URI> refers to the path name of a file relative to the root of the named repository 
    –   If the import definition does not specify a <profile_name> then <file_URI> refers to a TOSCA service template located in the repository that contains the Service Template file that includes the import definition. If the importing service template is located in a CSAR file, then that CSAR file should be treated as the repository in which to locate the service template file that must be imported.
        •   If <file_URI> starts with a leading slash (‘/’) then <file_URI> specifies a path name starting at the root of the repository.
        •   If <file_URI> does not start with a leading slash, then <file_URI> specifies a path that is relative to the importing document’s location within the repository. Double dot notation (‘../’) can be used to refer to parent directories in a file path name.

So "if the URI starts with a slash, it refers to the root of the repository" and "CSAR file should be treated as the repository" mean that, whenver you find a URI in the form of /hello/world when processing a tempalte inside a CSAR file, it must be interpreted as the /hello/world file inside the CSAR (zip) file

Puccini is not honoring this. I am attaching 2 very simple casr files. the Only difference is that in one case (the one whose name starts with 10) I am using an URI staring with ../ and in the other case (the one whose name starts with 20) I am using an URI starting with /.

The first one works OK when passed to pucchini-tosca compile. About the second one:

goroutine 1 [running]: github.com/tliron/exturl.(FileURL).Key(...) /home/emblemparade/go/pkg/mod/github.com/tliron/exturl@v0.4.0/file.go:128 github.com/tliron/exturl.(FileURL).String(0xc0000ccd80?) /home/emblemparade/go/pkg/mod/github.com/tliron/exturl@v0.4.0/file.go:84 +0xe github.com/tliron/puccini/tosca/grammars/tosca_v2_0.(Artifact).Normalize(0xc000074100, 0xc000141b00) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/artifact.go:124 +0x43d github.com/tliron/puccini/tosca/grammars/tosca_v2_0.Artifacts.Normalize(...) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/artifact.go:167 github.com/tliron/puccini/tosca/grammars/tosca_v2_0.(NodeTemplate).Normalize(0xc0000c1220, 0xc00004e780) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/node-template.go:111 +0x6f6 github.com/tliron/puccini/tosca/grammars/tosca_v2_0.NodeTemplates.Normalize({0xc000074118, 0x1, 0x10c8a30?}, 0xc00004e780) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/node-template.go:124 +0x5e github.com/tliron/puccini/tosca/grammars/tosca_v2_0.(ServiceTemplate).Normalize(0xc0000ccc60, 0xc00004e780) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/service-template.go:127 +0x3c7 github.com/tliron/puccini/tosca/grammars/tosca_v2_0.(ServiceFile).NormalizeServiceTemplate(0xc00004c5f0) /tmp/tmp.6Q1ipEWz48/tosca/grammars/tosca_v2_0/service-file.go:60 +0x22f github.com/tliron/puccini/normal.NormalizeServiceTemplate.func1({0xe40480?, 0xc00004c5f0?}) /tmp/tmp.6Q1ipEWz48/normal/service-template.go:52 +0x42 github.com/tliron/kutil/reflection.TraverseEntities({0xe40480, 0xc00004c5f0}, 0x0?, 0xc0005739b0) /home/emblemparade/go/pkg/mod/github.com/tliron/kutil@v0.2.11/reflection/traverse.go:21 +0xda github.com/tliron/puccini/normal.NormalizeServiceTemplate({0xe40480?, 0xc00004c5f0?}) /tmp/tmp.6Q1ipEWz48/normal/service-template.go:50 +0x49 github.com/tliron/puccini/tosca/parser.(Context).Normalize(0xc0000c0320) /tmp/tmp.6Q1ipEWz48/tosca/parser/phase6-normalization.go:11 +0x9b github.com/tliron/puccini/puccini-tosca/commands.Parse({0x10d3a68, 0x18ddf20}, {0x7ffeb489400e, 0x11}) /tmp/tmp.6Q1ipEWz48/puccini-tosca/commands/parse.go:220 +0xb0c github.com/tliron/puccini/puccini-tosca/commands.Compile({0x10d3a68, 0x18ddf20}, {0x7ffeb489400e?, 0x0?}) /tmp/tmp.6Q1ipEWz48/puccini-tosca/commands/compile.go:57 +0x37 github.com/tliron/puccini/puccini-tosca/commands.glob..func1(0xc0001dce00?, {0xc00004c170?, 0x4?, 0xf1a776?}) /tmp/tmp.6Q1ipEWz48/puccini-tosca/commands/compile.go:51 +0x6c github.com/spf13/cobra.(Command).execute(0x18a3960, {0xc00004c140, 0x1, 0x1}) /home/emblemparade/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:944 +0x863 github.com/spf13/cobra.(Command).ExecuteC(0x18a3f20) /home/emblemparade/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5 github.com/spf13/cobra.(Command).Execute(...) /home/emblemparade/go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992 github.com/tliron/puccini/puccini-tosca/commands.Execute() /tmp/tmp.6Q1ipEWz48/puccini-tosca/commands/root.go:40 +0x1a main.main()



EDIT: below the 2 sample CSAR files

[10_with_dots.zip](https://github.com/tliron/puccini/files/14834922/10_with_dots.zip)

[20_with_slash.zip](https://github.com/tliron/puccini/files/14834924/20_with_slash.zip)
tliron commented 5 months ago

Many thanks for this very detailed bug report!

jpc4242 commented 5 months ago

Thank you for maintaining puccini, and for fixing the issue in record time!!

tliron commented 5 months ago

Haha, but you found two bugs:

1) A very subtle bug about Go interface pointers, that took some time to debug and understand. 2) The TOSCA feature: actually, the only way to solve this properly is to implement it as it is defined, specifically adding an optional "implicit repository" to the entire parser. This is only set when using CSAR, but it's effect should be exactly as described, the root for absolute file paths.

abhishek-kumar0409 commented 2 months ago

hi @tliron - Which specific release of Puccini will address this issue ? Unfortunately, some end-users are uncomfortable in building the binary from the source code.

tliron commented 2 months ago

Thanks, I will do an official release soon. Building the binary should be very easy!

tliron commented 2 months ago

Released

abhishek-kumar0409 commented 2 months ago

Thanks @tliron