Merge pull request #97 from dengsh12/NLB5028

Add `DirectiveSources` field to `ParseOptions`

Author: dengsh12

analyze.go

@@ -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
+				}
 			}
 		}
 	}

analyze_test.go

@@ -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")
+			}
+		})
+	}
+}

parse.go

@@ -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
 }