Skip to content

Commit e24d411

Browse files
fix(logs): use correct variable for netobserv code path (#832)
1 parent 5bbdc9d commit e24d411

File tree

2 files changed

+54
-3
lines changed

2 files changed

+54
-3
lines changed

api/logs/v1/labels_enforcer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func enforceValuesOnLogQL(mInfo AuthzResponseData, v url.Values, paramName strin
8989
paramValue := v.Get(paramName)
9090
if paramValue == "" {
9191
// For query endpoints, we don't always to enforce the authorization
92-
// label matchers, so weskip it if the query is empty.
92+
// label matchers, so we skip it if the query is empty.
9393
if queryEndpoint {
9494
return v.Encode(), nil
9595
}
@@ -107,7 +107,7 @@ func enforceValuesOnLogQL(mInfo AuthzResponseData, v url.Values, paramName strin
107107
}
108108

109109
// Logical "OR" only applies to query expressions, not for filter params
110-
if mInfo.MatcherOp == logicalOr && paramValue == queryParam {
110+
if mInfo.MatcherOp == logicalOr && paramName == queryParam {
111111
// Logical "OR" to combine multiple matchers needs to be done via LogQueryExpr > LogPipelineExpr
112112
expr.Walk(func(expr interface{}) {
113113
if le, ok := expr.(*logqlv2.LogQueryExpr); ok {

api/logs/v1/labels_enforcer_test.go

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ func TestEnforceValuesOnQuery(t *testing.T) {
9797
tt := []struct {
9898
desc string
9999
accessMatchers []*labels.Matcher
100+
matcherOp string
100101
urlValues url.Values
101102
expValues url.Values
102103
}{
@@ -160,6 +161,45 @@ func TestEnforceValuesOnQuery(t *testing.T) {
160161
"query": []string{"{kubernetes_namespace_name=~\"ns-name|another-ns-name|openshift-.*\", kubernetes_container_name=\"logger\", kubernetes_pod_name=\"pod-name\"}"},
161162
},
162163
},
164+
{
165+
desc: "logical OR with query parameter",
166+
accessMatchers: []*labels.Matcher{
167+
{
168+
Type: labels.MatchRegexp,
169+
Name: "kubernetes_namespace_name",
170+
Value: "ns-1|ns-2",
171+
},
172+
},
173+
matcherOp: logicalOr,
174+
urlValues: url.Values{
175+
"query": []string{`{app="test"}`},
176+
},
177+
expValues: url.Values{
178+
"query": []string{"{app=\"test\"} | kubernetes_namespace_name =~ \"ns-1|ns-2\""},
179+
},
180+
},
181+
{
182+
desc: "logical OR with multiple matchers",
183+
accessMatchers: []*labels.Matcher{
184+
{
185+
Type: labels.MatchRegexp,
186+
Name: "kubernetes_namespace_name",
187+
Value: "ns-1|ns-2",
188+
},
189+
{
190+
Type: labels.MatchEqual,
191+
Name: "cluster",
192+
Value: "prod",
193+
},
194+
},
195+
matcherOp: logicalOr,
196+
urlValues: url.Values{
197+
"query": []string{`{service="api"}`},
198+
},
199+
expValues: url.Values{
200+
"query": []string{"{service=\"api\"} | kubernetes_namespace_name =~ \"ns-1|ns-2\" or cluster = \"prod\""},
201+
},
202+
},
163203
}
164204

165205
for _, tc := range tt {
@@ -169,7 +209,12 @@ func TestEnforceValuesOnQuery(t *testing.T) {
169209
ou, err := url.Parse(fmt.Sprintf("/loki/api/v1/query_range?%s", tc.urlValues.Encode()))
170210
testutil.Ok(t, err)
171211

172-
v, err := enforceValues(AuthzResponseData{Matchers: tc.accessMatchers}, ou)
212+
authzData := AuthzResponseData{Matchers: tc.accessMatchers}
213+
if tc.matcherOp != "" {
214+
authzData.MatcherOp = tc.matcherOp
215+
}
216+
217+
v, err := enforceValues(authzData, ou)
173218
testutil.Ok(t, err)
174219

175220
if len(tc.urlValues.Encode()) == 0 {
@@ -205,6 +250,12 @@ func TestEnforceValuesOnQuery(t *testing.T) {
205250
})
206251

207252
testutil.Equals(t, matchersToStrings(mExp), matchersToStrings(m))
253+
254+
if tc.matcherOp != "" {
255+
expQuery := tc.expValues.Get("query")
256+
actualQuery := smExpr.String()
257+
testutil.Equals(t, expQuery, actualQuery)
258+
}
208259
})
209260
}
210261
}

0 commit comments

Comments
 (0)