Skip to content

Fix 867 by rejecting unsupported OpenAPI specs#1708

Closed
mromaszewicz wants to merge 2 commits into
mainfrom
fix-867
Closed

Fix 867 by rejecting unsupported OpenAPI specs#1708
mromaszewicz wants to merge 2 commits into
mainfrom
fix-867

Conversation

@mromaszewicz

Copy link
Copy Markdown
Member

No description provided.

This kinda fixes #867 by rejecting specs that we know
we can't parse.

We only support 3.0.x openAPI specs, which must have the
`openapi` property as semver at the top level.

Exit with an error if `openapi` is not present. This will be
the case for Swagger specs.

Exit with an error if major version != 3

Exit with an error if minor version != 0

Stop using the word Swagger in function names where we can help it.
@mromaszewicz
mromaszewicz requested a review from a team as a code owner July 23, 2024 00:22
@jamietanna
jamietanna self-requested a review July 23, 2024 07:05

@jamietanna jamietanna left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not going to be a breaking change? Given we've had to update our test files

Does this impact cases we've got partial specs (ie for external refs)?

I'd seen that when I started with #713

@mromaszewicz

Copy link
Copy Markdown
Member Author

Is this not going to be a breaking change? Given we've had to update our test files

Does this impact cases we've got partial specs (ie for external refs)?

I'd seen that when I started with #713

Yes. It is indeed a breaking change, and one I wanted your opinion on.

A more compatible way would be to shim ourselves into the loading path in Kin somehow, and only complain if there's a known incompatible version present, and accept specs without versions. Let me have a second go at it...

swagger, err := util.LoadSwagger(flag.Arg(0))
spec, err := util.LoadOpenAPI(flag.Arg(0))
if err != nil {
errExit("error loading swagger spec in %s\n: %s\n", flag.Arg(0), err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be this?

Suggested change
errExit("error loading swagger spec in %s\n: %s\n", flag.Arg(0), err)
errExit("error loading OpenAPI spec in %s\n: %s\n", flag.Arg(0), err)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunately a wider issue #672

@mromaszewicz

Copy link
Copy Markdown
Member Author

I'm going to do this differently, without affecting the loading of broken specs.

@mromaszewicz
mromaszewicz deleted the fix-867 branch July 23, 2024 17:29
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

Successfully merging this pull request may close these issues.

3 participants