vacuum icon indicating copy to clipboard operation
vacuum copied to clipboard

path parameters are not resolved correctly prior to validation

Open LukeHagar opened this issue 4 months ago • 1 comments

I may have found the source a bug we have been troubleshooting internally, but I suspect it may actually be sourced in the vacuum rule logic.

I first had the issue on my own, in the plex-api-spec the sectionKey parameter detailed there is what the vacuum rule flagged on.

I've dug through and managed to replicate the error in the path_parameters_test.go file in vacuum as well with this snippet:

components: 
  parameters:
    chicken:
      in: path
      required: true
      name: chicken
paths:
 /musical/{melody}/{pizza}/{cake}:
   parameters:
     - in: path
       name: melody
       required: true
   get:
     parameters:
       - in: path
         name: pizza
         required: true
 /dogs/{chicken}/{ember}:
   get:
     parameters:
       - in: path
         name: ember
         required: true
       - $ref: '#/components/parameters/chicken'
   post:
     parameters:
       - in: path
         name: ember
       - $ref: '#/components/parameters/chicken'
 /dogs/{chicken}/{ember}/pizza:
   get:
     parameters:
       - $ref: "./test-param.yaml"
       - $ref: '#/components/parameters/chicken'

The test-param.yaml file in this context is just the contents of the ember param.

name: ember
in: path
required: true

This successfully replicates the error:

--- FAIL: TestPathParameters_RunRule_MultiplePaths_TopAndVerbParams (0.00s)
    path_parameters_test.go:430: 
            Error Trace:    /home/luke/github/vacuum/functions/openapi/path_parameters_test.go:430
            Error:          "[{`GET` must define parameter `cake` as expected by path `/musical/{melody}/{pizza}/{cake}` {{0 0} {0 0}} $.paths['/musical/{melody}/{pizza}/{cake}']   <nil> <nil> 0xc0003cb0e0 0xc0004a2640 <nil> <nil>} {`GET` must define parameter `ember` as expected by path `/dogs/{chicken}/{ember}/pizza` {{0 0} {0 0}} $.paths['/dogs/{chicken}/{ember}/pizza']   <nil> <nil> 0xc0003db0e0 0xc0004a2f00 <nil> <nil>}]" should have 1 item(s), but has 2
            Test:           TestPathParameters_RunRule_MultiplePaths_TopAndVerbParams

I believe the issue is that in this setup/vacuum flow the rolodex is not properly being initialized prior to lookups occurring.

Running through debug I was able to trace it down to this error message when a lookup attempt is made:

"rolodex has not been initialized, cannot open file '/home/luke/github/vacuum/functions/openapi/test-param.yaml'"

Here is that call stack at that point: image

Digging into the test setup, This line seems to imply the Rolodex should already be initialized at this point

But even adding a nil check and manual Rolodex initialization

if config.Rolodex == nil {
    config.Rolodex = NewRolodex(config)
}
index.rolodex = config.Rolodex

The Rolodex created still throws errors that no file systems have been configured

{"time":"2024-10-01T21:17:26.857755475Z","level":"ERROR","msg":"unable to open the rolodex file, check specification references and base path","file":"/home/luke/github/vacuum/functions/openapi/test-param.yaml","error":"rolodex has no file systems configured, cannot open '/home/luke/github/vacuum/functions/openapi/test-param.yaml'. Add a BaseURL or BasePath to your configuration so the rolodex knows how to resolve references"}

So I suspect there is a little more setup needed here.

LukeHagar avatar Oct 01 '24 21:10 LukeHagar