kubernetes / kube-openapi

Kubernetes OpenAPI spec generation & serving
Apache License 2.0
317 stars 205 forks source link

Fix FilterSpecByPathsWithoutSideEffects to handle cases without slash #495

Open Jefftree opened 2 months ago

Jefftree commented 2 months ago

The current FilterSpecByPathsWithoutSideEffects behavior is wrong for paths without a trailing slash. See comment thread for more details: https://github.com/kubernetes/kubernetes/pull/123770/files#r1516319191

cc @liggitt

liangyuanpeng commented 1 month ago

I agree that the safest thing to achieve the goal is to exclude exact matches on prefix and prefix+slash. The required modification looks small.

Is this something you want? If so I'll open a PR.

+++ prefixSet := sets.NewString(keepPathPrefixes...)
    for path, pathItem := range sp.Paths.Paths {
        if !prefixes.HasPrefix(path) {
            continue
        }
+++     if !strings.HasSuffix(path, "/") {
+++         continue
+++     }
+++     if !prefixSet.Has(path) {
+++         continue
+++     }
        ret.Paths.Paths[path] = pathItem
    }
Jefftree commented 3 weeks ago

From a quick glance I don't think it's correct but seems close. Here's a useful test case

FilterSpecByPathsWithoutSideEffects([]{"/apis/group/v1beta1/"}, []{"/apis/group/v1"}, ...} should return "" because despite v1 being a prefix of v1beta1, we should really be matching for "/apis/group/v1/...".