Skip to content

Reserve core endpoints#231

Merged
roosterfish merged 4 commits into
canonical:v3from
MggMuggins:reserve-core-endpoints
Aug 14, 2024
Merged

Reserve core endpoints#231
roosterfish merged 4 commits into
canonical:v3from
MggMuggins:reserve-core-endpoints

Conversation

@MggMuggins
Copy link
Copy Markdown
Contributor

Prevent CoreAPI extension servers from creating endpoints under /core.

Fixes #227

@MggMuggins MggMuggins marked this pull request as ready for review August 9, 2024 21:08
Copy link
Copy Markdown
Contributor

@roosterfish roosterfish left a comment

Choose a reason for hiding this comment

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

Looks good thanks, only some smaller comments.

Comment thread internal/rest/resources/resources.go Outdated
Comment thread internal/rest/resources/resources.go Outdated
Comment thread internal/rest/resources/resources.go Outdated
...for CoreAPI servers

Fixes canonical#227

Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
@MggMuggins MggMuggins force-pushed the reserve-core-endpoints branch from 188d71c to b5a8dfb Compare August 12, 2024 20:47
@MggMuggins
Copy link
Copy Markdown
Contributor Author

It occurred to me that only checking the PathPrefix wouldn't catch resources where PathPrefix: "".

We also should probably require at least one endpoint in every resource. Happy to drop those commits if that's the wrong assumption.

@MggMuggins MggMuggins requested a review from roosterfish August 12, 2024 20:51
Copy link
Copy Markdown
Contributor

@masnax masnax left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @MggMuggins!

@roosterfish roosterfish merged commit 8dbf50e into canonical:v3 Aug 14, 2024
@MggMuggins MggMuggins deleted the reserve-core-endpoints branch August 14, 2024 14:03
@MggMuggins MggMuggins restored the reserve-core-endpoints branch August 16, 2024 16:55
@MggMuggins MggMuggins deleted the reserve-core-endpoints branch August 16, 2024 17:22
masnax added a commit that referenced this pull request Aug 16, 2024
Backport of #231

Not sure if I did it right but also backported #236 so that the tests
pass
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.

Reserve all of the top level API paths

3 participants