API with an unknown path should return an error #2190
Comments
|
The above bug also affects |
|
Hey @nopcoder I haven't dug into the code specifics yet but I am hoping to take this up if that is fine, ok? Thanks in advance! Best, |
|
sure, thanks! |
|
@DataDavD let me know if you have any questions or need help in this one. |
|
Ok great, thank you @nopcoder. I'll take a close look this weekend and return with any questions I'll probably have. Thanks again!!!! |
|
hey @nopcoder just wanted to give you an update. I was able to take a look at the code and believe I understand the specific issue at hand. I intend to look much more closely tomorrow or the following day now that I pinpointed the code at issue; I will most likely send you some clarifying questions then. Thank you, |
|
Thanks for the updates @DataDavD , here for anything that will help you get this one moving. |
|
hey @nopcoder I am assuming the issue is centered around this line of code. I've been having a little trouble reading through the code because a number of objects/types like Also to clarify, for any route past /api/v1 return json 401 error instead of UIHandler, correct? |
|
Hi @DataDavD, make build should run The main issue is any route under |
|
Ahh ok that makes sense; maybe my issue was maybe go 1.15 related, not totally sure though. Thanks for confirming the issue; I thought that was it but want to verify before I start writing code to handle that. Thank you!!!! |
|
hey @nopcoder took another look/try and still getting the same error for some reason. I'll try and dig in more tomorrow, but in meantime here is the full error For some reason it freaks out over the full error: |
|
@DataDavD looks from the output that you get a newer version of the |
nopcoder
changed the title
|
@DataDavD any luck build the generated code with the specific oapi-codegen library? |
|
hey @nopcoder sorry for the delayed response. Was very busy over weekend so didn't get to touch/look at the code, but I should tonight. I'll let ya know how it goes. Thanks again for helping me debug! |
|
Hey @nopcoder, I feel dumb as I realized for some reason my Go version tried to update the oapi-codegen library when I must have done a run a bit ago. Your suggested fixed the issue and now everything appears fine. I will start working on this issue now that I'm in a non-buggy state in my local repo. I can't thank you enough for being patient and very helpful. Thanks again for allowing me to contribute. best, |
|
Hi dd, did you had any progress with this issue? would like to have this bug fix for the next release if possible. Will it be possible to frame it for the following week or two? |
|
hey @nopcoder sorry for the delayed response. I thought I had messaged you prior about being out of town last week. I actually just got back in so I should be able to get this complete or at least a good place within the next week or two. If I have any further issues or think that it won't be doable by EOD Wednesday I'll let ya know and seek your guidance. Thanks again for your help; and sorry for the delay. best, |
|
Appreciate it, thank you! I can post a test code that will break that you can use if it will help. |
|
Yea sure anything will help. Currently, I have something of the sorts, but I'm not too sure the specific place to put it although I believe it should be placed in |
|
hey @nopcoder I was wondering if you saw my last message. I'm still having a little trouble, but am hoping/wanting to finish this before the end of the weekend although I'm a bit unsure of myself, so if this is holding y'all up please tell me as I don't want to do that. In the meantime I will carry on trying to get something working, properly. Thank you. Best, |
|
@DataDavD yea, I missed your previous message - sorry about that. Regarding test code - hope this one helps: // part of serve_test.go func TestInvalidRoute(t *testing.T) {
handler, _ := setupHandler(t, "")
server := setupServer(t, handler)
clt := setupClientByEndpoint(t, server.URL, "", "")
cred := createDefaultAdminUser(t, clt)
// setup client with invalid endpoint base url
basicAuthProvider, err := securityprovider.NewSecurityProviderBasicAuth(cred.AccessKeyID, cred.SecretAccessKey)
if err != nil {
t.Fatal("basic auth security provider", err)
}
clt, err = api.NewClientWithResponses(server.URL+api.BaseURL+"//", api.WithRequestEditorFn(basicAuthProvider.Intercept))
if err != nil {
t.Fatal("failed to create api client:", err)
}
ctx := context.Background()
resp, err := clt.ListRepositoriesWithResponse(ctx, &api.ListRepositoriesParams{})
if err != nil {
t.Fatalf("failed to get lakefs server version")
}
if resp.JSONDefault == nil {
t.Fatalf("client api call expected default error, got nil")
}
expectedErrMsg := api.ErrInvalidAPIEndpoint.Error()
errMsg := resp.JSONDefault.Message
if errMsg != expectedErrMsg {
t.Fatalf("client response error message: %s, expected: %s", errMsg, expectedErrMsg)
}
}// part of serve.go var ErrInvalidAPIEndpoint = errors.New("invalid API endpoint") |
|
Thank you for the info and help @nopcoder !!! I'll try and get this finalized tonight/tomorrow. If I still don't have something PR ready I'll hold off to avoid delaying this getting in the next release. Thanks again!! dd |
|
Hey @nopcoder I believe I have (at least close to) what is required for the issue fix, but for some reason I have 2 tests failing. One f the failed tests is Last, here is my existing code for this issue fix DataDavD@dfc3b78 |
|
@nopcoder FYI I submitted a PR (#2382). The tests passed, but I still had one test, Thank you for your patience on this and letting me contribute. Best, |
|
@DataDavD thanks! looking into the PR and I'll add my comments there. |
This commit works to close issue treeverse#2190 by creating handler that returns ErrInvalidAPIEndpoint when called. This will be used by chi Mount'ing the handler to a specified route so that any calls to a route downstream the Mount'd pattern will respond with the ErrInvalidAPIEndpoint error. Also add corresponding test, TestInvalidRoute, to test InvalidAPIEndpointHandler. Create middleware to catch downstream routes from BaseURL This commit works to close issue treeverse#2190 by creating middleware to ensure users that call routes downstream of the BaseURL (i.e. /api/v1) are return an invalid endpoint error and internal error status. Remove BaseURLHandler
guy-har
changed the title
Currently lakeFS register openapi handlers and handle all specific routes.
In case of a call to
/api/v1/test, the unknown path under the API prefix, the mux will serve the request by the UI handler and return a valid HTML (UI) page.The expected behaviour is to return a non-2xx status code with JSON error - prefered the internal error format, so the developer will handle an error and not fail to parse the response in case of a bad API call.
The text was updated successfully, but these errors were encountered: