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]>
  • Loading branch information
dukanto committed Nov 22, 2024
1 parent 09cf0d6 commit 50e1d4d
Show file tree
Hide file tree
Showing 4 changed files with 121 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
16 changes: 13 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,11 @@ type Function struct {
log logging.Logger
}

const (
START = "^"

Check failure on line 26 in fn.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported const START should have comment (or a comment on this block) or be unexported (revive)
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 +69,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, START) {
// 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 50e1d4d

Please sign in to comment.