pb33f / openapi-changes

The world's sexiest OpenAPI breaking changes detector. Discover what changed between two OpenAPI specs, or a single spec over time. Supports OpenAPI 3.1, 3.0 and Swagger
https://pb33f.io/openapi-changes/
Other
180 stars 16 forks source link

Errors while trying to use openapi-changes #66

Closed donquixote8 closed 11 months ago

donquixote8 commented 1 year ago

Hello, I made changes to openapi-changes to allow local (-x) and remote (-y) refs. Will raise a PR soon. I was testing using two simple openapi files each including another file (old/new_blah.json) via $ref. GitHub didn't let me upload json files, hence renamed as .txt

$ diff old_test_openapi.json new_test_openapi.json 18c18 < "$ref": "old_blah.json"

                    "$ref": "new_blah.json"

####################

$ diff old_blah.json new_blah.json 7c7 < "type": "string",

        "type": "int",

Here are my results. I feel the errors are coming from openapilib, or if I'm doing something not right.

  1. openapi-changes report -x -y old_test_openapi.json new_test_openapi.json panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x14949b4]

goroutine 1 [running]: github.com/pb33f/libopenapi/what-changed/reports.CreateOverallReport(0x0) /Users/XYZ/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/what-changed/reports/summary.go:26 +0x34 github.com/pb33f/openapi-changes/cmd.createReport(...) /Users/XYZ/generated_openapi/openapi-changes/cmd/report.go:349 github.com/pb33f/openapi-changes/cmd.runLeftRightReport({0x7ff7bfeff4d6, 0x15}, {0x7ff7bfeff4ec, 0x15}, 0x4?, 0x19b97a0?, 0x0?, 0x0?) /Users/XYZ/generated_openapi/openapi-changes/cmd/report.go:345 +0x659 github.com/pb33f/openapi-changes/cmd.GetReportCommand.func1(0xc0001d8200?, {0xc000134c40, 0x2, 0x16b88d3?}) /Users/XYZ/generated_openapi/openapi-changes/cmd/report.go:197 +0xb4d github.com/spf13/cobra.(Command).execute(0xc000004900, {0xc000134c00, 0x4, 0x4}) /Users/XYZ/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x87c github.com/spf13/cobra.(Command).ExecuteC(0x1ed2580) /Users/XYZ/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3a5 github.com/spf13/cobra.(*Command).Execute(...) /Users/XYZ/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:968 github.com/pb33f/openapi-changes/cmd.Execute({0x16b95ac?, 0x10013f0?}, {0x16b95ac?, 0x16c2b29?}, {0xc00002c840?, 0xc0000061a0?}) /Users/XYZ/generated_openapi/openapi-changes/cmd/root.go:52 +0xb0 main.main() /Users/XYZ/generated_openapi/openapi-changes/openapi-changes.go:26 +0x105

  1. openapi-changes summary -x -y old_test_openapi.json new_test_openapi.json

@@@@@@@ @@@@@@@ @@@@@@ @@@@@@ @@@@@@@@

openapi-changes version: latest | compiled: 2023-09-28 15:33:17 PDT INFO printing summary This is not showing any diffs, let alone breaking changes. Just to let you know, the following worked perfectly: openapi-changes console -x -y https://github.com/pb33f/openapi-changes/blob/main/sample-specs/petstorev3.json [old_test_openapi.json.txt](https://github.com/pb33f/openapi-changes/files/12754198/old_test_openapi.json.txt) [new_test_openapi.json.txt](https://github.com/pb33f/openapi-changes/files/12754205/new_test_openapi.json.txt) [new_blah.json.txt](https://github.com/pb33f/openapi-changes/files/12754211/new_blah.json.txt) [old_blah.json.txt](https://github.com/pb33f/openapi-changes/files/12754237/old_blah.json.txt)
daveshanley commented 1 year ago

Thank you, i'll investigate this on my next bug run.

daveshanley commented 1 year ago

in v0.0.43 there is a new feature that implements your -x and -y flags, but in a way that matches vacuum (docs are here: https://pb33f.io/openapi-changes/command-arguments/#unresolved-references)

You should be able to use exploded/unresolved specs fully now.

However if you re-try your test in the current state, it will still fail. The reason why you don't see any changes with the new_blah etc.. is that those parameter objects are not valid definitions of a parameter. You need to put the properties under a schema.

For example:

Currently you have:

{
    "title": "blah",
    "description": "The blah details.",
    "type": "object",
    "properties": {
        "id": {
            "type": "int",
            "description": "The unique blah-generated ID for the blah.",
            "minLength": 4,
            "maxLength": 60,
            "readOnly": true
        },
        "name": {
            "type": "string",
            "description": "The blah name.",
            "minLength": 5,
            "maxLength": 127
        }
    }
}

It should be changed to:

{
    "title": "blah",
    "description": "The blah details.",
    "type": "object",
    "schema": {
        "properties": {
            "id": {
                "type": "int",
                "description": "The unique blah-generated ID for the blah.",
                "minLength": 4,
                "maxLength": 60,
                "readOnly": true
            },
            "name": {
                "type": "string",
                "description": "The blah name.",
                "minLength": 5,
                "maxLength": 127
            }
        }
    }
}

And then you will see this:

Screenshot 2023-10-06 at 5 02 40 PM

LegendWojciechPolowniak commented 11 months ago

Hey Dave πŸ‘‹ Hope you're alright

here is a new feature that implements your -x and -y flags, but in a way that matches vacuum

What's the exact commad structure that one is expected to follow with the -x -y approach? I can't see anything in --help about those

Overall I'm getting

β–€  Building original model for commit e5aa9bpanic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x98 pc=0x102602658]

goroutine 1 [running]:
github.com/pb33f/libopenapi/datamodel/low.GenerateHashString({0x0, 0x0})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/extraction_functions.go:707 +0x68
github.com/pb33f/libopenapi/datamodel/low/base.(*Schema).Hash(0x14001275c00)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/base/schema.go:249 +0x1418
github.com/pb33f/libopenapi/datamodel/low/v3.(*Parameter).Hash(0x14000ed1b80)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/v3/parameter.go:129 +0x654
github.com/pb33f/libopenapi/datamodel/low.GenerateHashString({0x102c93560, 0x14000ed1b80})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/extraction_functions.go:703 +0x194
github.com/pb33f/libopenapi/datamodel/low/v3.(*Operation).Hash(0x140001d4b00)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/v3/operation.go:200 +0x9f0
github.com/pb33f/libopenapi/datamodel/low.GenerateHashString({0x102c88180, 0x140001d4b00})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/extraction_functions.go:703 +0x194
github.com/pb33f/libopenapi/datamodel/low/v3.(*PathItem).Hash(0x1400018c000)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/v3/path_item.go:52 +0x200
github.com/pb33f/libopenapi/datamodel/low.GenerateHashString({0x102c4bbc0, 0x1400018c000})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/extraction_functions.go:703 +0x194
github.com/pb33f/libopenapi/datamodel/low/v3.(*Paths).Hash(0x14000770000)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/v3/paths.go:167 +0x284
github.com/pb33f/libopenapi/datamodel/low.AreEqual({0x102cb6b20?, 0x14000770000?}, {0x102cb6b20, 0x14000771ce0})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/datamodel/low/extraction_functions.go:695 +0x44
github.com/pb33f/libopenapi/what-changed/model.ComparePaths({0x102c5b1e0?, 0x14000770000?}, {0x102c5b1e0?, 0x14000771ce0?})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/what-changed/model/paths.go:143 +0x41c
github.com/pb33f/libopenapi/what-changed/model.CompareDocuments({0x102c100c0?, 0x140006a8d80?}, {0x102c100c0?, 0x14000f98b40?})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/what-changed/model/document.go:234 +0x1340
github.com/pb33f/libopenapi/what-changed.CompareOpenAPIDocuments(...)
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/what-changed/what_changed.go:28
github.com/pb33f/libopenapi.CompareDocuments({0x102cbce70, 0x1400070b500}, {0x102cbce70, 0x14000721ad0})
    C:/Users/runneradmin/go/pkg/mod/github.com/pb33f/libopenapi@v0.9.4/document.go:299 +0x228
github.com/pb33f/openapi-changes/git.BuildCommitChangelog({0x1400126fb08, 0x2, 0x140001c9b18?}, 0x1?, 0x1?, {0x0, 0x0})
    D:/a/openapi-changes/openapi-changes/git/read_local.go:182 +0x564
github.com/pb33f/openapi-changes/cmd.runLeftRightSummary({0x16db0b960, 0x8}, {0x16db0b969, 0x8}, 0x4?, 0x102cb2030?, {0x0, 0x0})
    D:/a/openapi-changes/openapi-changes/cmd/summary.go:280 +0x474
github.com/pb33f/openapi-changes/cmd.GetSummaryCommand.func1(0x140001d8100?, {0x14000110c60, 0x2, 0x1027f3bcb?})
    D:/a/openapi-changes/openapi-changes/cmd/summary.go:195 +0x720
github.com/spf13/cobra.(*Command).execute(0x140001e8300, {0x14000110c00, 0x2, 0x2})
    C:/Users/runneradmin/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x658
github.com/spf13/cobra.(*Command).ExecuteC(0x103106500)
    C:/Users/runneradmin/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x320
github.com/spf13/cobra.(*Command).Execute(...)
    C:/Users/runneradmin/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:968
github.com/pb33f/openapi-changes/cmd.Execute({0x102b75b50?, 0x0?}, {0x102b76220?, 0x10232dc60?}, {0x102b76010?, 0x140000021a0?})
    D:/a/openapi-changes/openapi-changes/cmd/root.go:52 +0xc0
main.main()
    D:/a/openapi-changes/openapi-changes/openapi-changes.go:26 +0x128

When trying to run openapi-changes console old.yaml api.yaml to compare two files locally. I have run vacuum on those spec, and while they're not the best (autogenerated from code), they do pass without errors.


β–ˆβ–ˆβ•—   β–ˆβ–ˆβ•— β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•—  β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•—β–ˆβ–ˆβ•—   β–ˆβ–ˆβ•—β–ˆβ–ˆβ•—   β–ˆβ–ˆβ•—β–ˆβ–ˆβ–ˆβ•—   β–ˆβ–ˆβ–ˆβ•—
β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•”β•β•β–ˆβ–ˆβ•—β–ˆβ–ˆβ•”β•β•β•β•β•β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ–ˆβ–ˆβ•— β–ˆβ–ˆβ–ˆβ–ˆβ•‘
β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘     β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•”β–ˆβ–ˆβ–ˆβ–ˆβ•”β–ˆβ–ˆβ•‘
β•šβ–ˆβ–ˆβ•— β–ˆβ–ˆβ•”β•β–ˆβ–ˆβ•”β•β•β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘     β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘   β–ˆβ–ˆβ•‘β–ˆβ–ˆβ•‘β•šβ–ˆβ–ˆβ•”β•β–ˆβ–ˆβ•‘
 β•šβ–ˆβ–ˆβ–ˆβ–ˆβ•”β• β–ˆβ–ˆβ•‘  β–ˆβ–ˆβ•‘β•šβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•—β•šβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•”β•β•šβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ–ˆβ•”β•β–ˆβ–ˆβ•‘ β•šβ•β• β–ˆβ–ˆβ•‘
  β•šβ•β•β•β•  β•šβ•β•  β•šβ•β• β•šβ•β•β•β•β•β• β•šβ•β•β•β•β•β•  β•šβ•β•β•β•β•β• β•šβ•β•     β•šβ•β•

version: 0.4.3 | compiled: 2023-11-05T14:43:04Z
πŸ”— https://quobix.com/vacuum | https://github.com/daveshanley/vacuum

 INFO  Linting against 42 rules: https://quobix.com/vacuum/rulesets/recommended

Category     | Errors | Warnings | Info
Operations   | 0      | 2        | 0
Tags         | 0      | 45       | 0
Descriptions | 0      | 151      | 305
Examples     | 0      | 409      | 0

Linting passed, but with 607 warnings and 305 informs

Yet, openapi-changes crashes. It would probably be very hard for me to provide you with a simplified OAS file to reproduce this, as this spec is really long and I don't know which fields are causing that. Not sure if that's enough insights to put some further discovery into this or not, but if we can somehow resolve that together lmk. Thanks again for the great work with pb33f tooling

zoidyzoidzoid commented 11 months ago

I'm also getting this issue. I'll see if I can figure out a smaller example that still breaks.

daveshanley commented 11 months ago

I have some good news. I've been working on libopenapi for the past month to completely re-write how remote references are handled. There is a super powerful new engine called 'the rolodex' that can be configured to look things up, just about any which way you like.

There are a couple of options now

in openapi-changes v0.0.44 There are two flags you can use when enabling remote references (http or file). There is the global -p / --base flag that sets either a working/base directory to look from, or it sets a base URL to resolve from. This means that ALL http refs will be resolved from this base.

The second is to use the new global -r / --remote flag, that turns on local and remote handling. openapi-changes will use the working directory as the base, and will attempt to resolve all http links and continue resolving those relative links as they are found remotely.

This new code has been heavily tested, but you may still find some gaps.

Here is what I see when I run openapi-changes v0.0.44

Screenshot 2023-11-09 at 7 36 41β€―AM

LegendWojciechPolowniak commented 11 months ago

I can confirm that v0.0.44 works for my definitions!

LegendWojciechPolowniak commented 11 months ago

@daveshanley could we update the docker image too please?

zoidyzoidzoid commented 11 months ago

Yay, for mine too! πŸŽ‰

daveshanley commented 11 months ago

Yeah the docker build failed because I had to move the entire stack up a few versions of golang. I will have to update the dockerfile and push another release.

daveshanley commented 11 months ago

The docker image has been updated with the latest release.