Skip to content

Commit

Permalink
fix: return better error if regexp is invalid
Browse files Browse the repository at this point in the history
Signed-off-by: Miguel Ángel Ducanto Hadad <[email protected]>
(cherry picked from commit 7cadaa2)
  • Loading branch information
dukanto committed Nov 26, 2024
1 parent 75fdc5e commit 3438209
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
6 changes: 5 additions & 1 deletion fn.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
// possibly avoid using regex for matching literal strings
strictPattern = fmt.Sprintf("%s%s%s", START, string(before), END)
}
re := regexp.MustCompile(strictPattern)
re, err := regexp.Compile(strictPattern)
if err != nil {
response.Fatal(rsp, errors.Wrapf(err, "cannot compile regex %s", strictPattern))
return rsp, nil
}
keys := []resource.Name{}
for k := range desiredComposed {
if re.MatchString(string(k)) {
Expand Down
74 changes: 73 additions & 1 deletion fn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ 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",
reason: "The function should not modify the sequence regex, since it's already prefixed",
args: args{
req: &fnv1beta1.RunFunctionRequest{
Input: resource.MustStructObject(&v1beta1.Input{
Expand Down Expand Up @@ -865,6 +865,78 @@ func TestRunFunction(t *testing.T) {
},
},
},
"SequenceRegexInvalidRegex": {
reason: "The function should return a fatal error because the regex is invalid",
args: args{
req: &fnv1beta1.RunFunctionRequest{
Input: resource.MustStructObject(&v1beta1.Input{
Rules: []v1beta1.SequencingRule{
{
Sequence: []resource.Name{
`^(`,
"second",
},
},
},
}),
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": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
},
},
},
},
want: want{
rsp: &fnv1beta1.RunFunctionResponse{
Meta: &fnv1beta1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)},
Results: []*fnv1beta1.Result{
{
Severity: fnv1beta1.Severity_SEVERITY_FATAL,
Message: "cannot compile regex ^(: error parsing regexp: missing closing ): `^(`",
},
},
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": {
Resource: resource.MustStructJSON(mr),
Ready: fnv1beta1.Ready_READY_TRUE,
},
},
},
},
},
},
}

for name, tc := range cases {
Expand Down

0 comments on commit 3438209

Please sign in to comment.