Skip to content

Add DirectiveSources field to ParseOptions #97

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,32 @@ func enterBlockCtx(stmt *Directive, ctx blockCtx) blockCtx {

//nolint:gocyclo,funlen,gocognit
func analyze(fname string, stmt *Directive, term string, ctx blockCtx, options *ParseOptions) error {
masks, knownDirective := directives[stmt.Directive]
var masks []uint
knownDirective := false

currCtx, knownContext := contexts[ctx.key()]
directiveName := stmt.Directive

// Find all bitmasks from the sources invoker provides.
for _, matchFn := range options.DirectiveSources {
if masksInFn, found := matchFn(directiveName); found {
masks = append(masks, masksInFn...)
knownDirective = true
}
}

if !knownDirective {
for _, matchFn := range options.MatchFuncs {
if masks, knownDirective = matchFn(stmt.Directive); knownDirective {
break
// If DirectiveSources was not provided, try to find the directive in legacy way.
// We want to use DirectiveSources to indicate the OSS/N+ version, and dynamic
// modules we want to include for validation. However, invokers used MatchFuncs
// and a directive map before. This is a transition plan. After MatchFuncs is deleted
// from ParseOptions, this if block should be deleted.
if len(options.DirectiveSources) == 0 {
masks, knownDirective = directives[directiveName]
if !knownDirective {
for _, matchFn := range options.MatchFuncs {
if masks, knownDirective = matchFn(stmt.Directive); knownDirective {
break
}
}
}
}
Expand Down
122 changes: 122 additions & 0 deletions analyze_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2427,3 +2427,125 @@ func TestAnalyze_headers_more(t *testing.T) {
})
}
}

//nolint:funlen
func TestAnalyze_directiveSources(t *testing.T) {
t.Parallel()
// two self defined maps and matchFn to ensure it is a separate test
testDirectiveMap1 := map[string][]uint{
"common_dir": {ngxAnyConf | ngxConfTake1},
"test_dir1": {ngxAnyConf | ngxConfTake1},
}
testSource1 := func(directive string) ([]uint, bool) {
masks, matched := testDirectiveMap1[directive]
return masks, matched
}

testDirectiveMap2 := map[string][]uint{
"common_dir": {ngxAnyConf | ngxConfTake2},
"test_dir2": {ngxAnyConf | ngxConfTake2},
}
testSource2 := func(directive string) ([]uint, bool) {
masks, matched := testDirectiveMap2[directive]
return masks, matched
}

testcases := map[string]struct {
stmt *Directive
ctx blockCtx
wantErr bool
}{
// The directive only found in source1 and satisfies the bitmask in it
"DirectiveFoundOnlyInSource1_pass": {
&Directive{
Directive: "test_dir1",
Args: []string{"arg1"},
Line: 5,
},
blockCtx{"http", "upstream"},
false,
},
// The directive only found in source2 and satisfies the bitmask in it
"DirectiveFoundOnlyInSource2_pass": {
&Directive{
Directive: "test_dir2",
Args: []string{"arg1", "arg2"},
Line: 5,
},
blockCtx{"http", "upstream"},
false,
},
// The directive only found in source2 but not satisfies the bitmask in it
"DirectiveFoundOnlyInsource2_fail": {
&Directive{
Directive: "test_dir2",
Args: []string{"arg1"},
Line: 5,
},
blockCtx{"http", "upstream"},
true,
},
// The directive found in both sources,
// but only satisfies bitmasks in source1 it should still pass validation
"DirectiveFoundInBothSources_pass_case1": {
&Directive{
Directive: "common_dir",
Args: []string{"arg1"},
Line: 5,
},
blockCtx{"http", "upstream"},
false,
},
// The directive found in both Sources,
// but only satisfies bitmasks in source2 it should still pass validation
"DirectiveFoundInBothSources_pass_case2": {
&Directive{
Directive: "common_dir",
Args: []string{"arg1", "arg2"},
Line: 5,
},
blockCtx{"http", "upstream"},
false,
},
// The directive found in both sources,
// but doesn't satisfy bitmask in any of them
"DirectiveFoundInBothSources_fail": {
&Directive{
Directive: "common_dir",
Args: []string{"arg1", "arg2", "arg3"},
Line: 5,
},
blockCtx{"http", "upstream"},
true,
},
// The directive not found in any source
"DirectiveNotFoundInAnySource_fail": {
&Directive{
Directive: "no_exist",
Args: []string{},
Line: 5,
},
blockCtx{"http", "location"},
true,
},
}

for name, tc := range testcases {
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()
err := analyze("nginx.conf", tc.stmt, ";", tc.ctx, &ParseOptions{
DirectiveSources: []MatchFunc{testSource1, testSource2},
ErrorOnUnknownDirectives: true,
})

if !tc.wantErr && err != nil {
t.Fatal(err)
}

if tc.wantErr && err == nil {
t.Fatal("expected error, got nil")
}
})
}
}
8 changes: 8 additions & 0 deletions parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,15 @@ type ParseOptions struct {
// encountered by the parser to determine the valid contexts and argument count of the
// directive. Set this option to enable parsing of directives belonging to non-core or
// dynamic NGINX modules that follow the usual grammar rules of an NGINX configuration.
// If DirectiveSources was provided, this will be ignored.
MatchFuncs []MatchFunc

// An array of MatchFunc, which is used to indicate the OSS/N+ version, and
// dynamic modules you want to include for validation. If a directive matches
// any of them, and satisfies the corresponding bitmask, it should pass the validation.
// If we provide this, MatchFuncs will be ignored
DirectiveSources []MatchFunc

LexOptions LexOptions
}

Expand Down
Loading