Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions pkg/registry/file/dynamicpathdetector/analyze_endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,22 @@ func splitEndpointPortAndPath(endpoint string) (string, string) {
}

// MergeDuplicateEndpoints folds duplicates and merges same-path specific-port
// endpoints into a wildcard-port (:0) sibling. The folding is symmetric:
// endpoints into a wildcard-port (:0) sibling. Folding is symmetric and is
// keyed on the same triple HTTPEndpoint.Equal compares — (Endpoint,
// Direction, Internal). An Internal=false endpoint will therefore NOT merge
// with an Internal=true sibling even if their path and direction match.
//
// - If a specific-port endpoint is encountered AFTER its :0 sibling, the
// specific-port methods/headers are merged INTO the wildcard entry.
// - If a specific-port endpoint is encountered BEFORE its :0 sibling, it
// is initially recorded; when the wildcard arrives we sweep `seen` for
// same-(path, direction) specific-port siblings, fold them into the
// wildcard, and remove them from the output.
// same-(path, direction, Internal) specific-port siblings, fold them
// into the wildcard, and remove them from the output.
//
// This contract was tightened on the back of upstream review on
// kubescape/storage#316 — a single :0 entry must NOT cause unrelated
// concrete-port endpoints to be wildcarded; only same-path siblings fold.
// concrete-port endpoints to be wildcarded; only same-path same-Internal
// siblings fold.
func MergeDuplicateEndpoints(endpoints []*types.HTTPEndpoint) []*types.HTTPEndpoint {
seen := make(map[string]*types.HTTPEndpoint)
var newEndpoints []*types.HTTPEndpoint
Expand All @@ -137,30 +141,30 @@ func MergeDuplicateEndpoints(endpoints []*types.HTTPEndpoint) []*types.HTTPEndpo

if isWildcardPort(port) {
// Wildcard arriving after specific-port siblings — sweep `seen`
// for any same-(path, direction) specific-port entries already
// recorded, fold them into the wildcard, then drop them from
// the output slice.
absorbed := false
// for any same-(path, direction, Internal) specific-port entries
// already recorded, fold them into the wildcard, then drop them
// from the output slice.
for k, e := range seen {
ePort, ePath := splitEndpointPortAndPath(e.Endpoint)
if isWildcardPort(ePort) || ePath != pathPart || e.Direction != endpoint.Direction {
if isWildcardPort(ePort) || ePath != pathPart ||
e.Direction != endpoint.Direction || e.Internal != endpoint.Internal {
continue
}
endpoint.Methods = MergeStrings(endpoint.Methods, e.Methods)
mergeHeaders(endpoint, e)
delete(seen, k)
newEndpoints = removeEndpoint(newEndpoints, e)
absorbed = true
}
seen[key] = endpoint
newEndpoints = append(newEndpoints, endpoint)
_ = absorbed
continue
}

// Specific port: if a wildcard sibling for the same (path, direction)
// is already in `seen`, fold this entry into it.
wildcardKey := fmt.Sprintf(":0%s|%s", pathPart, endpoint.Direction)
// Specific port: if a wildcard sibling for the same
// (path, direction, Internal) is already in `seen`, fold this entry
// into it. The wildcardKey shape MUST match getEndpointKey exactly so
// the lookup hits the same map slot the wildcard was inserted under.
wildcardKey := fmt.Sprintf(":0%s|%s|%t", pathPart, endpoint.Direction, endpoint.Internal)
if existing, found := seen[wildcardKey]; found {
existing.Methods = MergeStrings(existing.Methods, endpoint.Methods)
mergeHeaders(existing, endpoint)
Expand All @@ -186,8 +190,12 @@ func removeEndpoint(s []*types.HTTPEndpoint, target *types.HTTPEndpoint) []*type
return s
}

// getEndpointKey returns a key that uniquely identifies an HTTPEndpoint by
// the same fields HTTPEndpoint.Equal compares: Endpoint, Direction, Internal.
// Keep this in sync with the wildcardKey shape constructed in
// MergeDuplicateEndpoints — the two MUST hash identical entries identically.
func getEndpointKey(endpoint *types.HTTPEndpoint) string {
return fmt.Sprintf("%s|%s", endpoint.Endpoint, endpoint.Direction)
return fmt.Sprintf("%s|%s|%t", endpoint.Endpoint, endpoint.Direction, endpoint.Internal)
}

func mergeHeaders(existing, new *types.HTTPEndpoint) {
Expand Down
116 changes: 116 additions & 0 deletions pkg/registry/file/dynamicpathdetector/tests/analyze_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,3 +449,119 @@ func TestMergeDuplicateEndpoints_NoWildcardKeepsAllSpecificPorts(t *testing.T) {

assert.Equal(t, 3, len(result), "no wildcard sibling => all specific-port endpoints must be kept")
}

// ---------------------------------------------------------------------------
// Internal-field isolation tests.
//
// `HTTPEndpoint.Equal` distinguishes endpoints by (Endpoint, Direction,
// Internal). The merge key in `MergeDuplicateEndpoints` and the wildcard
// sweep must therefore also distinguish Internal — otherwise an
// internally-originating endpoint can absorb an externally-originating one
// (or vice versa) just because they share path + direction.
//
// Flagged by upstream review on kubescape/storage#316 (matthyx).
// ---------------------------------------------------------------------------

// TestMergeDuplicateEndpoints_InternalFieldDistinguishesDuplicates asserts
// that two endpoints differing ONLY in Internal are NOT collapsed by the
// duplicate-key check at the top of the merge loop.
func TestMergeDuplicateEndpoints_InternalFieldDistinguishesDuplicates(t *testing.T) {
external := &types.HTTPEndpoint{
Endpoint: ":443/login",
Methods: []string{"POST"},
Direction: "outbound",
Internal: false,
}
internal := &types.HTTPEndpoint{
Endpoint: ":443/login",
Methods: []string{"GET"},
Direction: "outbound",
Internal: true,
}

result := dynamicpathdetector.MergeDuplicateEndpoints([]*types.HTTPEndpoint{external, internal})

assert.Equal(t, 2, len(result),
"endpoints with different Internal must NOT merge (HTTPEndpoint.Equal distinguishes Internal)")
// Each output must keep its own Internal value and methods.
for _, ep := range result {
switch ep.Internal {
case false:
assert.Equal(t, []string{"POST"}, ep.Methods, "external endpoint methods must not be polluted")
case true:
assert.Equal(t, []string{"GET"}, ep.Methods, "internal endpoint methods must not be polluted")
}
}
}

// TestMergeDuplicateEndpoints_InternalFieldGuardsWildcardAbsorbingPrior
// pins the wildcard-after-specific path: an Internal=false :0 wildcard must
// NOT sweep up a previously-recorded Internal=true specific-port sibling.
func TestMergeDuplicateEndpoints_InternalFieldGuardsWildcardAbsorbingPrior(t *testing.T) {
specificInternal := &types.HTTPEndpoint{
Endpoint: ":443/login",
Methods: []string{"POST"},
Direction: "outbound",
Internal: true,
}
wildcardExternal := &types.HTTPEndpoint{
Endpoint: ":0/login",
Methods: []string{"GET"},
Direction: "outbound",
Internal: false,
}

result := dynamicpathdetector.MergeDuplicateEndpoints([]*types.HTTPEndpoint{specificInternal, wildcardExternal})

assert.Equal(t, 2, len(result),
"wildcard with Internal=false must NOT absorb a specific-port sibling with Internal=true")
}

// TestMergeDuplicateEndpoints_InternalFieldGuardsSpecificFoldingIntoWildcard
// pins the wildcard-first path: a previously-recorded Internal=true wildcard
// must NOT absorb a later Internal=false specific-port sibling.
func TestMergeDuplicateEndpoints_InternalFieldGuardsSpecificFoldingIntoWildcard(t *testing.T) {
wildcardInternal := &types.HTTPEndpoint{
Endpoint: ":0/login",
Methods: []string{"GET"},
Direction: "outbound",
Internal: true,
}
specificExternal := &types.HTTPEndpoint{
Endpoint: ":443/login",
Methods: []string{"POST"},
Direction: "outbound",
Internal: false,
}

result := dynamicpathdetector.MergeDuplicateEndpoints([]*types.HTTPEndpoint{wildcardInternal, specificExternal})

assert.Equal(t, 2, len(result),
"specific-port with Internal=false must NOT fold into a wildcard sibling with Internal=true")
}

// TestMergeDuplicateEndpoints_InternalFieldMatching_StillFolds is the
// positive sanity check: when Internal DOES match, the existing
// path+direction merge contract still holds. A regression here would mean
// the Internal guard accidentally blocks legitimate folding.
func TestMergeDuplicateEndpoints_InternalFieldMatching_StillFolds(t *testing.T) {
wildcard := &types.HTTPEndpoint{
Endpoint: ":0/login",
Methods: []string{"GET"},
Direction: "outbound",
Internal: true,
}
specific := &types.HTTPEndpoint{
Endpoint: ":443/login",
Methods: []string{"POST"},
Direction: "outbound",
Internal: true,
}

result := dynamicpathdetector.MergeDuplicateEndpoints([]*types.HTTPEndpoint{wildcard, specific})

assert.Equal(t, 1, len(result), "matching Internal => specific-port sibling still folds into wildcard")
assert.Equal(t, ":0/login", result[0].Endpoint)
assert.True(t, result[0].Internal, "merged endpoint must preserve Internal=true")
assert.ElementsMatch(t, []string{"GET", "POST"}, result[0].Methods)
}
Loading