Skip to content

Commit

Permalink
Escape underscores and percent signs in filter matches in Vai (#94)
Browse files Browse the repository at this point in the history 
* Escape underscores and percent signs in filter matches in Vai

* Add cases to integration tests
  • Loading branch information
@maxsokolovsky
maxsokolovsky authored Sep 18, 2024
1 parent 6b2576c commit 0106849
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 19 deletions.
15 changes: 11 additions & 4 deletions pkg/cache/sql/informer/listoption_indexer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
"strings"

"github.com/pkg/errors"
"github.com/rancher/lasso/pkg/cache/sql/db"
"github.com/rancher/lasso/pkg/cache/sql/partition"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/tools/cache"

"github.com/rancher/lasso/pkg/cache/sql/db"
"github.com/rancher/lasso/pkg/cache/sql/partition"
)

// ListOptionIndexer extends Indexer by allowing queries based on ListOption
Expand Down Expand Up @@ -417,12 +418,18 @@ func (l *ListOptionIndexer) buildORClauseFromFilters(orFilters OrFilter) (string
return "", nil, err
}

orWhereClause += fmt.Sprintf(`f."%s" %s ?`, columnName, opString)
orWhereClause += fmt.Sprintf(`f."%s" %s ? ESCAPE '\'`, columnName, opString)
format := strictMatchFmt
if filter.Partial {
format = matchFmt
}
params = append(params, fmt.Sprintf(format, filter.Match))
match := filter.Match
// To allow matches on the backslash itself, the character needs to be replaced first.
// Otherwise, it will undo the following replacements.
match = strings.ReplaceAll(match, `\`, `\\`)
match = strings.ReplaceAll(match, `_`, `\_`)
match = strings.ReplaceAll(match, `%`, `\%`)
params = append(params, fmt.Sprintf(format, match))
if index == len(orFilters.Filters)-1 {
continue
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/cache/sql/informer/listoption_indexer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import (
"reflect"
"testing"

"github.com/rancher/lasso/pkg/cache/sql/partition"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/rancher/lasso/pkg/cache/sql/partition"
)

func TestNewListOptionIndexer(t *testing.T) {
Expand Down Expand Up @@ -279,7 +280,7 @@ func TestListByOptions(t *testing.T) {
expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o
JOIN "something_fields" f ON o.key = f.key
WHERE
(f."metadata.somefield" LIKE ?) AND
(f."metadata.somefield" LIKE ? ESCAPE '\') AND
(FALSE)
ORDER BY f."metadata.name" ASC `,
expectedStmtArgs: []any{"somevalue"},
Expand Down Expand Up @@ -307,7 +308,7 @@ func TestListByOptions(t *testing.T) {
expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o
JOIN "something_fields" f ON o.key = f.key
WHERE
(f."metadata.somefield" NOT LIKE ?) AND
(f."metadata.somefield" NOT LIKE ? ESCAPE '\') AND
(FALSE)
ORDER BY f."metadata.name" ASC `,
expectedStmtArgs: []any{"somevalue"},
Expand Down Expand Up @@ -335,7 +336,7 @@ func TestListByOptions(t *testing.T) {
expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o
JOIN "something_fields" f ON o.key = f.key
WHERE
(f."metadata.somefield" LIKE ?) AND
(f."metadata.somefield" LIKE ? ESCAPE '\') AND
(FALSE)
ORDER BY f."metadata.name" ASC `,
expectedStmtArgs: []any{"%somevalue%"},
Expand Down Expand Up @@ -372,7 +373,7 @@ func TestListByOptions(t *testing.T) {
expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o
JOIN "something_fields" f ON o.key = f.key
WHERE
(f."metadata.somefield" LIKE ? OR f."metadata.somefield" LIKE ? OR f."metadata.somefield" NOT LIKE ?) AND
(f."metadata.somefield" LIKE ? ESCAPE '\' OR f."metadata.somefield" LIKE ? ESCAPE '\' OR f."metadata.somefield" NOT LIKE ? ESCAPE '\') AND
(FALSE)
ORDER BY f."metadata.name" ASC `,
expectedStmtArgs: []any{"%somevalue%", "someothervalue", "somethirdvalue"},
Expand Down Expand Up @@ -414,8 +415,8 @@ func TestListByOptions(t *testing.T) {
expectedStmt: `SELECT o.object, o.objectnonce, o.dekid FROM "something" o
JOIN "something_fields" f ON o.key = f.key
WHERE
(f."metadata.somefield" LIKE ? OR f."status.someotherfield" NOT LIKE ?) AND
(f."metadata.somefield" LIKE ?) AND
(f."metadata.somefield" LIKE ? ESCAPE '\' OR f."status.someotherfield" NOT LIKE ? ESCAPE '\') AND
(f."metadata.somefield" LIKE ? ESCAPE '\') AND
(f."metadata.namespace" = ?) AND
(FALSE)
ORDER BY f."metadata.name" ASC `,
Expand Down Expand Up @@ -705,7 +706,6 @@ func TestListByOptions(t *testing.T) {
}
stmt := &sql.Stmt{}
rows := &sql.Rows{}
assert.Nil(t, err)
objType := reflect.TypeOf(testObject)
store.EXPECT().GetName().Return("something").AnyTimes()
store.EXPECT().Prepare(test.expectedStmt).Do(func(a ...any) {
Expand Down
57 changes: 50 additions & 7 deletions pkg/cache/sql/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import (
"testing"
"time"

"github.com/rancher/lasso/pkg/cache/sql/informer"
"github.com/rancher/lasso/pkg/cache/sql/informer/factory"
"github.com/rancher/lasso/pkg/cache/sql/partition"
"github.com/stretchr/testify/suite"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -20,6 +17,10 @@ import (
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
"sigs.k8s.io/controller-runtime/pkg/envtest"

"github.com/rancher/lasso/pkg/cache/sql/informer"
"github.com/rancher/lasso/pkg/cache/sql/informer/factory"
"github.com/rancher/lasso/pkg/cache/sql/partition"
)

const testNamespace = "sql-test"
Expand Down Expand Up @@ -88,7 +89,9 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
matches := configMapWithAnnotations("matches-filter", map[string]string{"somekey": "somevalue"})
// partial match for somekey == somevalue (different suffix)
partialMatches := configMapWithAnnotations("partial-matches", map[string]string{"somekey": "somevaluehere"})
createConfigMaps(matches, partialMatches)
specialCharacterMatch := configMapWithAnnotations("special-character-matches", map[string]string{"somekey": "c%%l_value"})
backSlashCharacterMatch := configMapWithAnnotations("backslash-character-matches", map[string]string{"somekey": `my\windows\path`})
createConfigMaps(matches, partialMatches, specialCharacterMatch, backSlashCharacterMatch)

cache, cacheFactory, err := i.createCacheAndFactory(fields, nil)
require.NoError(err)
Expand All @@ -100,7 +103,7 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
missing := configMapWithAnnotations("missing", nil)
createConfigMaps(notMatches, missing)

configMapNames := []string{matches.Name, partialMatches.Name, notMatches.Name, missing.Name}
configMapNames := []string{matches.Name, partialMatches.Name, notMatches.Name, missing.Name, specialCharacterMatch.Name, backSlashCharacterMatch.Name}
err = i.waitForCacheReady(configMapNames, testNamespace, cache)
require.NoError(err)

Expand Down Expand Up @@ -136,6 +139,46 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
}),
wantNames: []string{"matches-filter", "partial-matches"},
},
{
name: "no matches for filter with underscore as it is interpreted literally",
filters: orFiltersForFilters(informer.Filter{
Field: []string{`metadata`, `annotations[somekey]`},
Match: "somevalu_",
Op: informer.Eq,
Partial: true,
}),
wantNames: nil,
},
{
name: "no matches for filter with percent sign as it is interpreted literally",
filters: orFiltersForFilters(informer.Filter{
Field: []string{`metadata`, `annotations[somekey]`},
Match: "somevalu%",
Op: informer.Eq,
Partial: true,
}),
wantNames: nil,
},
{
name: "match with special characters",
filters: orFiltersForFilters(informer.Filter{
Field: []string{`metadata`, `annotations[somekey]`},
Match: "c%%l_value",
Op: informer.Eq,
Partial: true,
}),
wantNames: []string{"special-character-matches"},
},
{
name: "match with literal backslash character",
filters: orFiltersForFilters(informer.Filter{
Field: []string{`metadata`, `annotations[somekey]`},
Match: `my\windows\path`,
Op: informer.Eq,
Partial: true,
}),
wantNames: []string{"backslash-character-matches"},
},
{
name: "not eq filter",
filters: orFiltersForFilters(informer.Filter{
Expand All @@ -144,7 +187,7 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
Op: informer.NotEq,
Partial: false,
}),
wantNames: []string{"partial-matches", "not-matches-filter", "missing"},
wantNames: []string{"partial-matches", "not-matches-filter", "missing", "special-character-matches", "backslash-character-matches"},
},
{
name: "partial not eq filter",
Expand All @@ -154,7 +197,7 @@ func (i *IntegrationSuite) TestSQLCacheFilters() {
Op: informer.NotEq,
Partial: true,
}),
wantNames: []string{"not-matches-filter", "missing"},
wantNames: []string{"not-matches-filter", "missing", "special-character-matches", "backslash-character-matches"},
},
{
name: "multiple or filters match",
Expand Down

0 comments on commit 0106849

Please sign in to comment.