Skip to content

Commit

Permalink
fix: make regex strict only if not explicitly open-ended
Browse files Browse the repository at this point in the history
Signed-off-by: Miguel Ángel Ducanto Hadad <[email protected]>
(cherry picked from commit 5765b24)
  • Loading branch information
dukanto committed Nov 26, 2024
1 parent 2ea63be commit 75fdc5e
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 6 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ For example, the pipeline step below, will ensure that `second-resource` is not
- second-resource
```

You can write the regex as strict as you want, but keep in mind that it defaults to strict matching (start and end are enforced).
In other words, the following rules apply:
```yaml
- resource # this has no explicit start or end, so it will match EXACTLY ^resource$ (normal behaviour)
- a-group-.* # this has no explicit start or end, so it will match EXACTLY ^a-group-.*$
- ^a-group # this has an explicit start, so it will match EVERYTHING that starts with a-group
- a-group$ # this has an explicit end, so it will match EVERYTHING that ends with a-group
```

See `example/composition-regex.yaml` for a complete example.

## Installation
Expand Down
22 changes: 19 additions & 3 deletions example/composition-regex.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,27 @@ spec:
- time: 10s
conditionType: Ready
conditionStatus: "True"
- name: second-resource
- name: second-object
base:
apiVersion: nop.crossplane.io/v1alpha1
kind: NopResource
metadata:
name: second-resource
name: second-object
spec:
forProvider:
conditionAfter:
- time: 5s
conditionType: Ready
conditionStatus: "False"
- time: 10s
conditionType: Ready
conditionStatus: "True"
- name: third-resource
base:
apiVersion: nop.crossplane.io/v1alpha1
kind: NopResource
metadata:
name: third-resource
spec:
forProvider:
conditionAfter:
Expand All @@ -79,4 +94,5 @@ spec:
rules:
- sequence:
- first-subresource-.*
- second-resource
- object$ # this will match everything that ends with "object"
- third-resource
18 changes: 15 additions & 3 deletions fn.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"regexp"
"strings"

"github.com/crossplane/crossplane-runtime/pkg/errors"
"github.com/crossplane/crossplane-runtime/pkg/logging"
Expand All @@ -21,6 +22,13 @@ type Function struct {
log logging.Logger
}

const (
// START marks the start of a regex pattern.
START = "^"
// END marks the end of a regex pattern.
END = "$"
)

// RunFunction runs the Function.
func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequest) (*fnv1beta1.RunFunctionResponse, error) { //nolint:gocyclo // This function is unavoidably complex.
f.log.Info("Running function", "tag", req.GetMeta().GetTag())
Expand Down Expand Up @@ -63,9 +71,13 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
continue
}
for _, before := range sequence[:i] {
// add the regex with ^ & $ to match the entire string
// possibly avoid using regex for matching literal strings
strictPattern := fmt.Sprintf("^%s$", string(before))
strictPattern := string(before)
if !strings.HasPrefix(strictPattern, START) && !strings.HasSuffix(strictPattern, END) {
// if the user provides a delimited regex, we'll use it as is
// if not, add the regex with ^ & $ to match the entire string
// possibly avoid using regex for matching literal strings
strictPattern = fmt.Sprintf("%s%s%s", START, string(before), END)
}
re := regexp.MustCompile(strictPattern)
keys := []resource.Name{}
for k := range desiredComposed {
Expand Down
80 changes: 80 additions & 0 deletions fn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,86 @@ func TestRunFunction(t *testing.T) {
},
},
},
"SequenceRegexAlreadyPrefixed": {
reason: "The function should delay the creation of second and fourth resources because the first and third are not ready",
args: args{
req: &fnv1beta1.RunFunctionRequest{
Input: resource.MustStructObject(&v1beta1.Input{
Rules: []v1beta1.SequencingRule{
{
Sequence: []resource.Name{
"^first-.*$",
"^second-.*",
"third-.*$",
"fourth",
},
},
},
}),
Observed: &fnv1beta1.State{
Composite: &fnv1beta1.Resource{
Resource: resource.MustStructJSON(xr),
},
Resources: map[string]*fnv1beta1.Resource{},
},
Desired: &fnv1beta1.State{
Composite: &fnv1beta1.Resource{
Resource: resource.MustStructJSON(xr),
},
Resources: map[string]*fnv1beta1.Resource{
"first-0": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"first-1": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"second-0": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"third-0": {
Resource: resource.MustStructJSON(mr),
},
},
},
},
},
want: want{
rsp: &fnv1beta1.RunFunctionResponse{
Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)},
Results: []*fnv1beta1.Result{
{
Severity: fnv1beta1.Severity_SEVERITY_NORMAL,
Message: "Delaying creation of resource \"fourth\" because \"third-.*$\" is not fully ready (0 of 1)",
},
},
Desired: &fnv1beta1.State{
Composite: &fnv1beta1.Resource{
Resource: resource.MustStructJSON(xr),
},
Resources: map[string]*fnv1beta1.Resource{
"first-0": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"first-1": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"second-0": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
"third-0": {
Resource: resource.MustStructJSON(mr),
},
},
},
},
},
},
}

for name, tc := range cases {
Expand Down

0 comments on commit 75fdc5e

Please sign in to comment.