Skip to content

Commit

Permalink
Migrate services.MatchLabels to parse.Matcher
Browse files Browse the repository at this point in the history
This should be backwards-compatible plus add the {{regexp.match(...)}}
and {{regexp.not_match(...)}} functions.
  • Loading branch information
Andrew Lytvynov committed Sep 29, 2020
1 parent 2a52efa commit 45cf28b
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 43 deletions.
22 changes: 15 additions & 7 deletions lib/services/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ func ApplyTraits(r Role, traits map[string][]string) Role {
// at least one value in case if return value is nil
func applyValueTraits(val string, traits map[string][]string) ([]string, error) {
// Extract the variable from the role variable.
variable, err := parse.Variable(val)
variable, err := parse.NewExpression(val)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -687,7 +687,7 @@ func (r *RoleV3) CheckAndSetDefaults() error {
for _, condition := range []RoleConditionType{Allow, Deny} {
for _, login := range r.GetLogins(condition) {
if strings.Contains(login, "{{") || strings.Contains(login, "}}") {
_, err := parse.Variable(login)
_, err := parse.NewExpression(login)
if err != nil {
return trace.BadParameter("invalid login found: %v", login)
}
Expand Down Expand Up @@ -1585,11 +1585,19 @@ func MatchLabels(selector Labels, target map[string]string) (bool, string, error
}

if !utils.SliceContainsStr(selectorValues, Wildcard) {
result, err := utils.SliceMatchesRegex(targetVal, selectorValues)
if err != nil {
return false, "", trace.Wrap(err)
} else if !result {
return false, fmt.Sprintf("no value match: got '%v' want: '%v'", targetVal, selectorValues), nil
matched := false
for _, expr := range selectorValues {
m, err := parse.NewMatcher(expr)
if err != nil {
return false, "", trace.Wrap(err)
}
if m.Match(targetVal) {
matched = true
break
}
}
if !matched {
return false, fmt.Sprintf("no value match: got %q want: %q", targetVal, selectorValues), nil
}
}
}
Expand Down
58 changes: 58 additions & 0 deletions lib/services/role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,64 @@ func (s *RoleSuite) TestCheckAccess(c *C) {
{server: serverC2, login: "admin", hasAccess: true},
},
},
{
name: "role matches a regexp label",
roles: []RoleV3{
RoleV3{
Metadata: Metadata{
Name: "name1",
Namespace: defaults.Namespace,
},
Spec: RoleSpecV3{
Options: RoleOptions{
MaxSessionTTL: Duration(20 * time.Hour),
},
Allow: RoleConditions{
Logins: []string{"admin"},
NodeLabels: Labels{"role": []string{`{{regexp.match("worker.*")}}`}},
Namespaces: []string{defaults.Namespace, namespaceC},
},
},
},
},
checks: []check{
{server: serverA, login: "root", hasAccess: false},
{server: serverA, login: "admin", hasAccess: false},
{server: serverB, login: "root", hasAccess: false},
{server: serverB, login: "admin", hasAccess: true},
{server: serverC, login: "root", hasAccess: false},
{server: serverC, login: "admin", hasAccess: false},
},
},
{
name: "role matches a negative regexp label",
roles: []RoleV3{
RoleV3{
Metadata: Metadata{
Name: "name1",
Namespace: defaults.Namespace,
},
Spec: RoleSpecV3{
Options: RoleOptions{
MaxSessionTTL: Duration(20 * time.Hour),
},
Allow: RoleConditions{
Logins: []string{"admin"},
NodeLabels: Labels{"role": []string{`{{regexp.not_match("db.*")}}`}},
Namespaces: []string{defaults.Namespace, namespaceC},
},
},
},
},
checks: []check{
{server: serverA, login: "root", hasAccess: false},
{server: serverA, login: "admin", hasAccess: false},
{server: serverB, login: "root", hasAccess: false},
{server: serverB, login: "admin", hasAccess: true},
{server: serverC, login: "root", hasAccess: false},
{server: serverC, login: "admin", hasAccess: false},
},
},
}
for i, tc := range testCases {

Expand Down
2 changes: 1 addition & 1 deletion lib/services/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ func (u *UserV1) Check() error {
return trace.BadParameter("user name cannot be empty")
}
for _, login := range u.AllowedLogins {
e, err := parse.Variable(login)
e, err := parse.NewExpression(login)
if err == nil && e.Namespace() != parse.LiteralNamespace {
return trace.BadParameter("role variables not allowed in allowed logins")
}
Expand Down
12 changes: 6 additions & 6 deletions lib/utils/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ var reVariable = regexp.MustCompile(
`(?P<suffix>[^}{]*)$`,
)

// Variable parses expressions like {{external.foo}} or {{internal.bar}}, or a
// literal value like "prod". Call Interpolate on the returned Expression to
// get the final value based on traits or other dynamic values.
func Variable(variable string) (*Expression, error) {
// NewExpression parses expressions like {{external.foo}} or {{internal.bar}},
// or a literal value like "prod". Call Interpolate on the returned Expression
// to get the final value based on traits or other dynamic values.
func NewExpression(variable string) (*Expression, error) {
match := reVariable.FindStringSubmatch(variable)
if len(match) == 0 {
if strings.Contains(variable, "{{") || strings.Contains(variable, "}}") {
Expand Down Expand Up @@ -165,7 +165,7 @@ type Matcher interface {
Match(in string) bool
}

// Match parses a matcher expression. Currently supported expressions:
// NewMatcher parses a matcher expression. Currently supported expressions:
// - string literal: `foo`
// - wildcard expression: `*` or `foo*bar`
// - regexp expression: `^foo$`
Expand All @@ -175,7 +175,7 @@ type Matcher interface {
//
// These expressions do not support variable interpolation (e.g.
// `{{internal.logins}}`), like Expression does.
func Match(value string) (Matcher, error) {
func NewMatcher(value string) (Matcher, error) {
match := reVariable.FindStringSubmatch(value)
if len(match) == 0 {
if strings.Contains(value, "{{") || strings.Contains(value, "}}") {
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/parse/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestVariable(t *testing.T) {

for _, tt := range tests {
t.Run(tt.title, func(t *testing.T) {
variable, err := Variable(tt.in)
variable, err := NewExpression(tt.in)
if tt.err != nil {
assert.IsType(t, tt.err, err)
return
Expand Down Expand Up @@ -269,7 +269,7 @@ func TestMatch(t *testing.T) {

for _, tt := range tests {
t.Run(tt.title, func(t *testing.T) {
matcher, err := Match(tt.in)
matcher, err := NewMatcher(tt.in)
if tt.err != nil {
assert.IsType(t, tt.err, err, err)
return
Expand Down
27 changes: 0 additions & 27 deletions lib/utils/replace.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,5 @@ func ReplaceRegexp(expression string, replaceWith string, input string) (string,
return expr.ReplaceAllString(input, replaceWith), nil
}

// SliceMatchesRegex checks if input matches any of the expressions. The
// match is always evaluated as a regex either an exact match or regexp.
func SliceMatchesRegex(input string, expressions []string) (bool, error) {
for _, expression := range expressions {
if !strings.HasPrefix(expression, "^") || !strings.HasSuffix(expression, "$") {
// replace glob-style wildcards with regexp wildcards
// for plain strings, and quote all characters that could
// be interpreted in regular expression
expression = "^" + GlobToRegexp(expression) + "$"
}

expr, err := regexp.Compile(expression)
if err != nil {
return false, trace.BadParameter(err.Error())
}

// Since the expression is always surrounded by ^ and $ this is an exact
// match for either a a plain string (for example ^hello$) or for a regexp
// (for example ^hel*o$).
if expr.MatchString(input) {
return true, nil
}
}

return false, nil
}

var replaceWildcard = regexp.MustCompile(`(\\\*)`)
var reExpansion = regexp.MustCompile(`\$[^\$]+`)

0 comments on commit 45cf28b

Please sign in to comment.