Refactor Policy Enforcer to Use Unified PolicyPaths

- Updated the Policy Enforcer to consolidate policy source configuration under a single `policyPaths` key, allowing for auto-detection of URLs, directories, and files. - Removed deprecated keys such as `policyFile` and `policyUrls` from the configuration structure. - Adjusted related code and tests to ensure compatibility with the new configuration format. - Enhanced documentation to clarify the usage of `policyPaths` and provide examples for various configurations.
2026-03-03 18:49:17 +05:30
parent a806af3228
commit dfbaf5c6c5
6 changed files with 86 additions and 120 deletions
--- a/config/onix/adapter.yaml
+++ b/config/onix/adapter.yaml
@@ -51,6 +51,16 @@ modules:
        policyEnforcer:
          id: policyenforcer
          config:
+            # policyPaths: polymorphic, auto-detects each entry as URL, directory, or file
+            # Examples:
+            #   policyPaths: "./policies"                              # local directory
+            #   policyPaths: "https://example.com/compliance.rego"     # remote URL
+            #   policyPaths: "./policies/compliance.rego"              # local file
+            # For multiple sources, use YAML folded scalar (>-):
+            #   policyPaths: >-
+            #     https://example.com/compliance.rego,
+            #     https://example.com/safety.rego,
+            #     ./policies
            policyPaths: "./policies"
        signValidator:
          id: signvalidator
--- a/pkg/plugin/implementation/policyenforcer/README.md
+++ b/pkg/plugin/implementation/policyenforcer/README.md
@@ -5,7 +5,7 @@ OPA/Rego-based policy enforcement for beckn-onix adapters. Evaluates incoming be
 ## Overview

 The `policyenforcer` plugin is a **Step plugin** that:
- Loads `.rego` policy files from local directories, files, URLs, or local paths
+- Loads `.rego` policy files from URLs, local directories, or local files
 - Evaluates incoming messages against compiled OPA policies
 - Returns a `BadReqErr` (NACK) when policy violations are detected
 - Fails closed on evaluation errors (treats as NACK)
@@ -17,9 +17,7 @@ All config keys are passed via `map[string]string` in the adapter YAML config.

 | Key | Required | Default | Description |
 |-----|----------|---------|-------------|
-| `policyUrls` | At least one of `policyUrls`, `policyDir`, or `policyFile` required | — | Comma-separated list of URLs, local file paths, or directory paths to `.rego` files |
-| `policyPaths` | | `./policies` | Local directory or path containing `.rego` files |
-| `policyFile` | | — | Single local `.rego` file path |
+| `policyPaths` | Yes (at least one source required) | `./policies` (if dir exists) | Comma-separated list of policy sources — each entry is auto-detected as a **URL**, **directory**, or **file** |
 | `query` | No | `data.policy.violations` | Rego query returning violation strings |
 | `actions` | No | *(empty — all actions)* | Comma-separated beckn actions to enforce. When omitted, all actions are evaluated and the Rego policy itself decides which to gate. |
 | `enabled` | No | `true` | Enable/disable the plugin |
@@ -28,26 +26,34 @@ All config keys are passed via `map[string]string` in the adapter YAML config.

 ### Policy Sources

-`policyUrls` is the primary configuration key. It accepts a comma-separated list of:
- **Remote URLs**: `https://policies.example.com/compliance.rego`
- **Local file paths**: `/etc/policies/local.rego`
- **Directory paths**: `/etc/policies/` (all `.rego` files loaded, `_test.rego` excluded)
+`policyPaths` is the single configuration key for all policy sources. Each comma-separated entry is **auto-detected** as:
+- **Remote URL** (`http://` or `https://`): fetched via HTTP at startup
+- **Local directory**: all `.rego` files loaded (`_test.rego` excluded)
+- **Local file**: loaded directly

 ```yaml
+# Single directory
 config:
-  policyUrls: "https://policies.example.com/compliance.rego,/etc/policies/,/local/safety.rego"
+  policyPaths: "./policies"
+
+# Single remote URL
+config:
+  policyPaths: "https://policies.example.com/compliance.rego"
+
+# Mix of URLs, directories, and files
+config:
+  policyPaths: "https://policies.example.com/compliance.rego,./policies,/local/safety.rego"
 ```

-When specifying many URLs, use the YAML folded scalar (`>-`) to keep the config readable:
+When specifying many sources, use the YAML folded scalar (`>-`) to keep the config readable:

 ```yaml
 config:
-  policyUrls: >-
+  policyPaths: >-
    https://policies.example.com/compliance.rego,
    https://policies.example.com/safety.rego,
-    https://policies.example.com/rate-limit.rego,
-    /local/policies/,
-    https://policies.example.com/auth.rego
+    ./policies,
+    /local/overrides/rate-limit.rego
 ```

 The `>-` folds newlines into spaces, so the value is parsed as a single comma-separated string.
@@ -68,7 +74,7 @@ Or specify a custom policy location:
 policyEnforcer:
  id: policyenforcer
  config:
-    policyUrls: "./policies/compliance.rego"
+    policyPaths: "./policies/compliance.rego"
 ```

 ### Air-Gapped Deployments
@@ -77,7 +83,7 @@ For environments without internet access, use local file paths or volume mounts:

 ```yaml
 config:
-  policyUrls: "/mounted-policies/compliance.rego,/mounted-policies/safety.rego"
+  policyPaths: "/mounted-policies/compliance.rego,/mounted-policies/safety.rego"
 ```

 ## Example Config
@@ -87,7 +93,9 @@ plugins:
  policyEnforcer:
    id: policyenforcer
    config:
-      policyUrls: "https://policies.example.com/compliance.rego,/local/policies/"
+      policyPaths: >-
+        /local/policies/,
+        https://policies.example.com/compliance.rego
      minDeliveryLeadHours: "4"
      debugLogging: "true"
 steps:
--- a/pkg/plugin/implementation/policyenforcer/config.go
+++ b/pkg/plugin/implementation/policyenforcer/config.go
@@ -8,17 +8,12 @@ import (

 // Config holds the configuration for the Policy Enforcer plugin.
 type Config struct {
-	// PolicyPaths is a local directory containing .rego policy files (all loaded).
-	// At least one policy source (PolicyPaths, PolicyFile, or PolicyUrls) is required.
-	PolicyPaths string
-
-	// PolicyFile is a single local .rego file path.
-	PolicyFile string
-
-	// PolicyUrls is a list of URLs (or local file paths) pointing to .rego files,
-	// fetched at startup or read from disk.
-	// Parsed from the comma-separated "policyUrls" config key.
-	PolicyUrls []string
+	// PolicyPaths is a list of policy sources. Each entry is auto-detected as:
+	//   - Remote URL (http:// or https://) → fetched via HTTP
+	//   - Local directory → all .rego files loaded (excluding _test.rego)
+	//   - Local file → loaded directly
+	// Parsed from the comma-separated "policyPaths" config key.
+	PolicyPaths []string

 	// Query is the Rego query that returns a set of violation strings.
 	// Default: "data.policy.violations".
@@ -43,8 +38,6 @@ type Config struct {
 // Known config keys that are handled directly (not forwarded to RuntimeConfig).
 var knownKeys = map[string]bool{
 	"policyPaths":  true,
-	"policyFile":   true,
-	"policyUrls":   true,
 	"query":        true,
 	"actions":      true,
 	"enabled":      true,
@@ -65,29 +58,22 @@ func DefaultConfig() *Config {
 func ParseConfig(cfg map[string]string) (*Config, error) {
 	config := DefaultConfig()

-	if dir, ok := cfg["policyPaths"]; ok && dir != "" {
-		config.PolicyPaths = dir
-	}
-	if file, ok := cfg["policyFile"]; ok && file != "" {
-		config.PolicyFile = file
-	}
-
-	// Comma-separated policyUrls (supports URLs, local files, and directory paths)
-	if urls, ok := cfg["policyUrls"]; ok && urls != "" {
-		for _, u := range strings.Split(urls, ",") {
-			u = strings.TrimSpace(u)
-			if u != "" {
-				config.PolicyUrls = append(config.PolicyUrls, u)
+	// Comma-separated policyPaths (each entry auto-detected as URL, directory, or file)
+	if paths, ok := cfg["policyPaths"]; ok && paths != "" {
+		for _, p := range strings.Split(paths, ",") {
+			p = strings.TrimSpace(p)
+			if p != "" {
+				config.PolicyPaths = append(config.PolicyPaths, p)
 			}
 		}
 	}

-	if config.PolicyPaths == "" && config.PolicyFile == "" && len(config.PolicyUrls) == 0 {
+	if len(config.PolicyPaths) == 0 {
 		// Fall back to the default ./policies directory if it exists on disk.
 		if info, err := os.Stat("./policies"); err == nil && info.IsDir() {
-			config.PolicyPaths = "./policies"
+			config.PolicyPaths = append(config.PolicyPaths, "./policies")
 		} else {
-			return nil, fmt.Errorf("at least one policy source is required (policyPaths, policyFile, or policyUrls)")
+			return nil, fmt.Errorf("at least one policy source is required (policyPaths)")
 		}
 	}

--- a/pkg/plugin/implementation/policyenforcer/enforcer.go
+++ b/pkg/plugin/implementation/policyenforcer/enforcer.go
@@ -24,7 +24,7 @@ func New(cfg map[string]string) (*PolicyEnforcer, error) {
 		return nil, fmt.Errorf("policyenforcer: config error: %w", err)
 	}

-	evaluator, err := NewEvaluator(config.PolicyPaths, config.PolicyFile, config.PolicyUrls, config.Query, config.RuntimeConfig)
+	evaluator, err := NewEvaluator(config.PolicyPaths, config.Query, config.RuntimeConfig)
 	if err != nil {
 		return nil, fmt.Errorf("policyenforcer: failed to initialize OPA evaluator: %w", err)
 	}
--- a/pkg/plugin/implementation/policyenforcer/enforcer_test.go
+++ b/pkg/plugin/implementation/policyenforcer/enforcer_test.go
@@ -37,7 +37,7 @@ func writePolicyDir(t *testing.T, filename, content string) string {
 func TestParseConfig_RequiresPolicySource(t *testing.T) {
 	_, err := ParseConfig(map[string]string{})
 	if err == nil {
-		t.Fatal("expected error when no policyPaths, policyFile, or policyUrls given")
+		t.Fatal("expected error when no policyPaths given")
 	}
 }

@@ -93,24 +93,21 @@ func TestParseConfig_CustomActions(t *testing.T) {
 	}
 }

-func TestParseConfig_PolicyUrls(t *testing.T) {
+func TestParseConfig_PolicyPaths(t *testing.T) {
 	cfg, err := ParseConfig(map[string]string{
-		"policyUrls": "https://example.com/a.rego, https://example.com/b.rego",
+		"policyPaths": "https://example.com/a.rego, https://example.com/b.rego",
 	})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	if len(cfg.PolicyUrls) != 2 {
-		t.Fatalf("expected 2 URLs, got %d: %v", len(cfg.PolicyUrls), cfg.PolicyUrls)
+	if len(cfg.PolicyPaths) != 2 {
+		t.Fatalf("expected 2 paths, got %d: %v", len(cfg.PolicyPaths), cfg.PolicyPaths)
 	}
-	if cfg.PolicyUrls[0] != "https://example.com/a.rego" {
-		t.Errorf("url[0] = %q", cfg.PolicyUrls[0])
+	if cfg.PolicyPaths[0] != "https://example.com/a.rego" {
+		t.Errorf("path[0] = %q", cfg.PolicyPaths[0])
 	}
 }

-// Note: policySources support was removed; we intentionally only support
-// comma-separated policyUrls and local paths via policyUrls entries.
-
 // --- Evaluator Tests (with inline policies) ---

 func TestEvaluator_NoViolations(t *testing.T) {
@@ -123,7 +120,7 @@ violations contains msg if {
 }
 `
 	dir := writePolicyDir(t, "test.rego", policy)
-	eval, err := NewEvaluator(dir, "", nil, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{dir}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator failed: %v", err)
 	}
@@ -147,7 +144,7 @@ violations contains msg if {
 }
 `
 	dir := writePolicyDir(t, "test.rego", policy)
-	eval, err := NewEvaluator(dir, "", nil, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{dir}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator failed: %v", err)
 	}
@@ -174,7 +171,7 @@ violations contains msg if {
 }
 `
 	dir := writePolicyDir(t, "test.rego", policy)
-	eval, err := NewEvaluator(dir, "", nil, "data.policy.violations", map[string]string{"maxValue": "100"})
+	eval, err := NewEvaluator([]string{dir}, "data.policy.violations", map[string]string{"maxValue": "100"})
 	if err != nil {
 		t.Fatalf("NewEvaluator failed: %v", err)
 	}
@@ -217,7 +214,7 @@ test_something if { count(policy.violations) > 0 }
 `
 	os.WriteFile(filepath.Join(dir, "policy_test.rego"), []byte(testFile), 0644)

-	eval, err := NewEvaluator(dir, "", nil, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{dir}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator should skip _test.rego files, but failed: %v", err)
 	}
@@ -238,7 +235,7 @@ import rego.v1
 violations := set()
 `
 	dir := writePolicyDir(t, "test.rego", policy)
-	eval, err := NewEvaluator(dir, "", nil, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{dir}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator failed: %v", err)
 	}
@@ -267,7 +264,7 @@ violations contains msg if {
 	}))
 	defer srv.Close()

-	eval, err := NewEvaluator("", "", []string{srv.URL + "/test_policy.rego"}, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{srv.URL + "/test_policy.rego"}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator with URL failed: %v", err)
 	}
@@ -295,14 +292,14 @@ func TestEvaluator_FetchURL_NotFound(t *testing.T) {
 	srv := httptest.NewServer(http.NotFoundHandler())
 	defer srv.Close()

-	_, err := NewEvaluator("", "", []string{srv.URL + "/missing.rego"}, "data.policy.violations", nil)
+	_, err := NewEvaluator([]string{srv.URL + "/missing.rego"}, "data.policy.violations", nil)
 	if err == nil {
 		t.Fatal("expected error for 404 URL")
 	}
 }

 func TestEvaluator_FetchURL_InvalidScheme(t *testing.T) {
-	_, err := NewEvaluator("", "", []string{"ftp://example.com/policy.rego"}, "data.policy.violations", nil)
+	_, err := NewEvaluator([]string{"ftp://example.com/policy.rego"}, "data.policy.violations", nil)
 	if err == nil {
 		t.Fatal("expected error for ftp:// scheme")
 	}
@@ -328,7 +325,7 @@ violations contains "remote_violation" if { input.remote_bad }
 	}))
 	defer srv.Close()

-	eval, err := NewEvaluator(dir, "", []string{srv.URL + "/remote.rego"}, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{dir, srv.URL + "/remote.rego"}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator failed: %v", err)
 	}
@@ -355,7 +352,7 @@ violations contains "from_file" if { input.bad }
 	policyPath := filepath.Join(dir, "local_policy.rego")
 	os.WriteFile(policyPath, []byte(policy), 0644)

-	eval, err := NewEvaluator("", "", []string{policyPath}, "data.policy.violations", nil)
+	eval, err := NewEvaluator([]string{policyPath}, "data.policy.violations", nil)
 	if err != nil {
 		t.Fatalf("NewEvaluator with local path failed: %v", err)
 	}
@@ -487,9 +484,9 @@ violations contains "blocked" if { input.context.action == "confirm" }
 	defer srv.Close()

 	enforcer, err := New(map[string]string{
-		"policyUrls": srv.URL + "/block_confirm.rego",
-		"query":      "data.policy.violations",
-		"actions":    "confirm",
+		"policyPaths": srv.URL + "/block_confirm.rego",
+		"query":       "data.policy.violations",
+		"actions":     "confirm",
 	})
 	if err != nil {
 		t.Fatalf("New failed: %v", err)
--- a/pkg/plugin/implementation/policyenforcer/evaluator.go
+++ b/pkg/plugin/implementation/policyenforcer/evaluator.go
@@ -40,63 +40,28 @@ const maxPolicySize = 1 << 20

 // NewEvaluator creates an Evaluator by loading .rego files from local paths
 // and/or URLs, then compiling them. runtimeConfig is passed to Rego as data.config.
-func NewEvaluator(policyPaths, policyFile string, policyUrls []string, query string, runtimeConfig map[string]string) (*Evaluator, error) {
+func NewEvaluator(policyPaths []string, query string, runtimeConfig map[string]string) (*Evaluator, error) {
 	modules := make(map[string]string)

-	// Load from local directory
-	if policyPaths != "" {
-		entries, err := os.ReadDir(policyPaths)
-		if err != nil {
-			return nil, fmt.Errorf("failed to read policy directory %s: %w", policyPaths, err)
-		}
-		for _, entry := range entries {
-			if entry.IsDir() {
-				continue
-			}
-			if !strings.HasSuffix(entry.Name(), ".rego") {
-				continue
-			}
-			// Skip test files — they shouldn't be compiled into the runtime evaluator
-			if strings.HasSuffix(entry.Name(), "_test.rego") {
-				continue
-			}
-			fpath := filepath.Join(policyPaths, entry.Name())
-			data, err := os.ReadFile(fpath)
+	// Load from policyPaths (each entry auto-detected as URL, directory, or file)
+	for _, source := range policyPaths {
+		if isURL(source) {
+			name, content, err := fetchPolicy(source)
 			if err != nil {
-				return nil, fmt.Errorf("failed to read policy file %s: %w", fpath, err)
-			}
-			modules[entry.Name()] = string(data)
-		}
-	}
-
-	// Load single local file
-	if policyFile != "" {
-		data, err := os.ReadFile(policyFile)
-		if err != nil {
-			return nil, fmt.Errorf("failed to read policy file %s: %w", policyFile, err)
-		}
-		modules[filepath.Base(policyFile)] = string(data)
-	}
-
-	// Load from URLs, local file paths, and directory paths (policyUrls)
-	for _, rawSource := range policyUrls {
-		if isURL(rawSource) {
-			name, content, err := fetchPolicy(rawSource)
-			if err != nil {
-				return nil, fmt.Errorf("failed to fetch policy from %s: %w", rawSource, err)
+				return nil, fmt.Errorf("failed to fetch policy from %s: %w", source, err)
 			}
 			modules[name] = content
-		} else if info, err := os.Stat(rawSource); err == nil && info.IsDir() {
-			// Treat as directory — load all .rego files inside
-			entries, err := os.ReadDir(rawSource)
+		} else if info, err := os.Stat(source); err == nil && info.IsDir() {
+			// Directory — load all .rego files inside
+			entries, err := os.ReadDir(source)
 			if err != nil {
-				return nil, fmt.Errorf("failed to read policy directory %s: %w", rawSource, err)
+				return nil, fmt.Errorf("failed to read policy directory %s: %w", source, err)
 			}
 			for _, entry := range entries {
 				if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".rego") || strings.HasSuffix(entry.Name(), "_test.rego") {
 					continue
 				}
-				fpath := filepath.Join(rawSource, entry.Name())
+				fpath := filepath.Join(source, entry.Name())
 				data, err := os.ReadFile(fpath)
 				if err != nil {
 					return nil, fmt.Errorf("failed to read policy file %s: %w", fpath, err)
@@ -104,12 +69,12 @@ func NewEvaluator(policyPaths, policyFile string, policyUrls []string, query str
 				modules[entry.Name()] = string(data)
 			}
 		} else {
-			// Treat as local file path
-			data, err := os.ReadFile(rawSource)
+			// Local file path
+			data, err := os.ReadFile(source)
 			if err != nil {
-				return nil, fmt.Errorf("failed to read local policy source %s: %w", rawSource, err)
+				return nil, fmt.Errorf("failed to read policy file %s: %w", source, err)
 			}
-			modules[filepath.Base(rawSource)] = string(data)
+			modules[filepath.Base(source)] = string(data)
 		}
 	}