projectsyn / lieutenant-operator

The Project Syn Inventory API Operator
https://docs.syn.tools/lieutenant-operator/
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

Remove unnecessary `url.PathEscape` for `GetNamespace` #240

Closed simu closed 2 years ago

simu commented 2 years ago

The new version of the GitLab library URL-escapes the provided repo path in GetNamespace, so we must not escape it ourselves anymore, as we otherwise get a double-URL-escaped value for the request, which then fails.

Also add a test case for creating a project in a subgroup.

Follow-up fix for #217

Checklist

simu commented 2 years ago

I'm a little confused by the test case. Shouldn't this code call /api/v4/namespaces/path%2Fto%2Fgroup2? I guess there is some magic somewhere I'm missing or am I misunderstanding the API?

I'm confused as well, but with /api/v4/namespaces/path%2Fto%2Fgroup2 the test panics because there's no handler for the requested path. I've not been able to figure out yet where the unescape happens.

glrf commented 2 years ago

Aah in Go's url package:

// Note that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.
// A consequence is that it is impossible to tell which slashes in the Path were
// slashes in the raw URL and which were %2f. This distinction is rarely important,
// but when it is, the code should use RawPath, an optional field which only gets
// set if the default encoding is different from Path.

Might be a bug (or at least unintuitive) in httptest's routing?