From c9a68d5d8a6a098f094d5026e0f2d2891520d5fc Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 10:49:59 +0100 Subject: [PATCH 1/5] BCH-1290: Include certificate domain name in evidence title Suffixes each evidence title with the certificate domain name so users can identify which ACM certificate a piece of evidence relates to. Example: "ACM certificate must carry all required tags [api.example.com]" Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/eval.go b/internal/eval.go index 6874abd..343a835 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -133,6 +133,9 @@ func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, po ) evidence, perr := processor.GenerateResults(ctx, policyPath, input) + for _, ev := range evidence { + ev.Title = fmt.Sprintf("%s [%s]", ev.Title, cert.DomainName) + } evidences = append(evidences, evidence...) if perr != nil { accumulatedErrors = errors.Join(accumulatedErrors, perr) From 19a382c66a886712a2d1ab772589f6fe1e009ab8 Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:20:03 +0100 Subject: [PATCH 2/5] BCH-1290: Load bundle data.json once per eval cycle, not once per cert loadBundleRootData (now exported as LoadBundleRootData) was called inside PolicyEvaluator.Eval which is invoked once per certificate, causing N_certs x N_policies redundant disk reads per evaluation cycle. Move the load+merge step to main.go's Eval, before the cert loop, so data.json is read once per policy path per evaluation. Eval now takes pre-merged data keyed by policy path instead of raw operator overrides. Add eval_test.go with three tests: - operator overrides win over bundle defaults (config primacy) - no data.json returns overrides unchanged - data.json one directory up from policyPath is found correctly Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 22 ++++++------- internal/eval_test.go | 73 +++++++++++++++++++++++++++++++++++++++++++ main.go | 13 +++++++- 3 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 internal/eval_test.go diff --git a/internal/eval.go b/internal/eval.go index 343a835..89b6c50 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -32,7 +32,7 @@ func NewPolicyEvaluator(ctx context.Context, logger hclog.Logger, stepActivities // extraLabels (e.g. PolicyLabels from config) are merged with cert-derived labels; // SeededUUID derives the evidence UUID from ALL resulting labels, so label keys must // not change between runs. -func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, policyPaths []string, policyData map[string]interface{}, extraLabels map[string]string) ([]*proto.Evidence, error) { +func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, policyPaths []string, policyDataByPath map[string]map[string]interface{}, extraLabels map[string]string) ([]*proto.Evidence, error) { var accumulatedErrors error evidences := make([]*proto.Evidence, 0) @@ -117,10 +117,7 @@ func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, po } for _, policyPath := range policyPaths { - rootData, err := loadBundleRootData(policyPath, policyData) - if err != nil { - return nil, fmt.Errorf("loading bundle data for %s: %w", policyPath, err) - } + rootData := policyDataByPath[policyPath] processor := policyManager.NewPolicyProcessor( pe.logger, labels, @@ -156,12 +153,13 @@ func certificateBaseLabels() map[string]string { } } -// loadBundleRootData reads data.json from the OPA bundle root and merges it -// with base. When the agent downloads a policy OCI artifact it returns the +// LoadBundleRootData reads data.json from the OPA bundle root and merges it +// with overrides. When the agent downloads a policy OCI artifact it returns the // policies/ subdirectory as policyPath; the bundle's data.json lives one level // up in the bundle root. For local source trees the data.json lives inside the -// policies/ directory itself, so we check both locations. -func loadBundleRootData(policyPath string, base map[string]interface{}) (map[string]interface{}, error) { +// policies/ directory itself, so both locations are checked. overrides win on +// conflict, so operator-supplied policy_data takes precedence over bundle defaults. +func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (map[string]interface{}, error) { candidates := []string{ filepath.Join(filepath.Dir(policyPath), "data.json"), filepath.Join(policyPath, "data.json"), @@ -178,16 +176,16 @@ func loadBundleRootData(policyPath string, base map[string]interface{}) (map[str if err := json.Unmarshal(raw, &bundleData); err != nil { return nil, fmt.Errorf("parsing bundle data %s: %w", p, err) } - merged := make(map[string]interface{}, len(bundleData)+len(base)) + merged := make(map[string]interface{}, len(bundleData)+len(overrides)) for k, v := range bundleData { merged[k] = v } - for k, v := range base { + for k, v := range overrides { merged[k] = v } return merged, nil } - return base, nil + return overrides, nil } // arnCertID extracts the certificate UUID from an ACM ARN. diff --git a/internal/eval_test.go b/internal/eval_test.go new file mode 100644 index 0000000..c32384a --- /dev/null +++ b/internal/eval_test.go @@ -0,0 +1,73 @@ +package internal + +import ( + "os" + "path/filepath" + "testing" +) + +func TestLoadBundleRootData_OverridesWinOverBundleDefaults(t *testing.T) { + dir := t.TempDir() + bundleJSON := `{"expiry_warning_days":30,"required_certificate_tags":["Environment"]}` + if err := os.WriteFile(filepath.Join(dir, "data.json"), []byte(bundleJSON), 0644); err != nil { + t.Fatal(err) + } + + overrides := map[string]interface{}{ + "expiry_warning_days": float64(90), + } + + result, err := LoadBundleRootData(dir, overrides) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Operator override (90) must win over bundle default (30). + if got := result["expiry_warning_days"]; got != float64(90) { + t.Errorf("expiry_warning_days: got %v, want 90", got) + } + + // Bundle key absent from overrides must still be present. + if _, ok := result["required_certificate_tags"]; !ok { + t.Error("required_certificate_tags from bundle missing from merged result") + } +} + +func TestLoadBundleRootData_NoBundleDataJsonReturnsOverrides(t *testing.T) { + dir := t.TempDir() // no data.json written + + overrides := map[string]interface{}{ + "expiry_warning_days": float64(60), + } + + result, err := LoadBundleRootData(dir, overrides) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := result["expiry_warning_days"]; got != float64(60) { + t.Errorf("expiry_warning_days: got %v, want 60", got) + } +} + +func TestLoadBundleRootData_FindsDataJsonOneDirectoryUp(t *testing.T) { + root := t.TempDir() + bundleJSON := `{"expiry_warning_days":30}` + if err := os.WriteFile(filepath.Join(root, "data.json"), []byte(bundleJSON), 0644); err != nil { + t.Fatal(err) + } + policiesDir := filepath.Join(root, "policies") + if err := os.Mkdir(policiesDir, 0755); err != nil { + t.Fatal(err) + } + + // policyPath is the policies/ subdirectory; data.json is one level up. + result, err := LoadBundleRootData(policiesDir, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := result["expiry_warning_days"]; got != float64(30) { + t.Errorf("expiry_warning_days: got %v, want 30", got) + } +} diff --git a/main.go b/main.go index 7842485..23f29c8 100644 --- a/main.go +++ b/main.go @@ -72,12 +72,23 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api }, fmt.Errorf("failed to fetch data: %w", err) } + // Pre-load and merge bundle data.json defaults with operator overrides once per + // policy path, rather than re-reading from disk on every per-certificate Eval call. + policyDataByPath := make(map[string]map[string]interface{}, len(request.GetPolicyPaths())) + for _, path := range request.GetPolicyPaths() { + merged, err := internal.LoadBundleRootData(path, l.policyData) + if err != nil { + return &proto.EvalResponse{Status: proto.ExecutionStatus_FAILURE}, fmt.Errorf("loading bundle data for %s: %w", path, err) + } + policyDataByPath[path] = merged + } + policyEvaluator := internal.NewPolicyEvaluator(ctx, l.logger, activities) var allEvidences []*proto.Evidence var evalErrors error for _, cert := range certs { - certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), l.policyData, l.config.PolicyLabels) + certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), policyDataByPath, l.config.PolicyLabels) allEvidences = append(allEvidences, certEvidences...) if err != nil { evalErrors = errors.Join(evalErrors, fmt.Errorf("evaluating cert %s: %w", cert.CertificateArn, err)) From d2b1aa2ba96450c25746b66484774fda4d599d1b Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:21:53 +0100 Subject: [PATCH 3/5] =?UTF-8?q?BCH-1290:=20Address=20PR=20review=2013=20?= =?UTF-8?q?=E2=80=94=20use=20GetTitle()=20proto=20getter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use ev.GetTitle() instead of ev.Title when reading the proto field to follow protobuf conventions and handle nil cases correctly. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/eval.go b/internal/eval.go index 89b6c50..9ce213e 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -131,7 +131,7 @@ func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, po evidence, perr := processor.GenerateResults(ctx, policyPath, input) for _, ev := range evidence { - ev.Title = fmt.Sprintf("%s [%s]", ev.Title, cert.DomainName) + ev.Title = fmt.Sprintf("%s [%s]", ev.GetTitle(), cert.DomainName) } evidences = append(evidences, evidence...) if perr != nil { From a2283f70be63ba7f0a7e8361357b58a05dee5687 Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:24:05 +0100 Subject: [PATCH 4/5] =?UTF-8?q?BCH-1290:=20Simplify=20policyData=20?= =?UTF-8?q?=E2=80=94=20single=20map,=20not=20map-of-maps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All policy paths share one bundle so per-path data maps were unnecessary. Load bundle data.json once using the first path, pass the single merged map through to Eval and directly into NewPolicyProcessor. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 5 ++--- main.go | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/eval.go b/internal/eval.go index 9ce213e..b1365c0 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -32,7 +32,7 @@ func NewPolicyEvaluator(ctx context.Context, logger hclog.Logger, stepActivities // extraLabels (e.g. PolicyLabels from config) are merged with cert-derived labels; // SeededUUID derives the evidence UUID from ALL resulting labels, so label keys must // not change between runs. -func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, policyPaths []string, policyDataByPath map[string]map[string]interface{}, extraLabels map[string]string) ([]*proto.Evidence, error) { +func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, policyPaths []string, policyData map[string]interface{}, extraLabels map[string]string) ([]*proto.Evidence, error) { var accumulatedErrors error evidences := make([]*proto.Evidence, 0) @@ -117,7 +117,6 @@ func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, po } for _, policyPath := range policyPaths { - rootData := policyDataByPath[policyPath] processor := policyManager.NewPolicyProcessor( pe.logger, labels, @@ -126,7 +125,7 @@ func (pe *PolicyEvaluator) Eval(ctx context.Context, cert CertificateContext, po inventory, actors, pe.stepActivities, - rootData, + policyData, ) evidence, perr := processor.GenerateResults(ctx, policyPath, input) diff --git a/main.go b/main.go index 23f29c8..eda4ddb 100644 --- a/main.go +++ b/main.go @@ -72,15 +72,15 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api }, fmt.Errorf("failed to fetch data: %w", err) } - // Pre-load and merge bundle data.json defaults with operator overrides once per - // policy path, rather than re-reading from disk on every per-certificate Eval call. - policyDataByPath := make(map[string]map[string]interface{}, len(request.GetPolicyPaths())) - for _, path := range request.GetPolicyPaths() { - merged, err := internal.LoadBundleRootData(path, l.policyData) + // Load bundle data.json defaults and merge with operator overrides once per + // evaluation cycle. All policy paths share the same bundle so one load suffices. + policyData := l.policyData + if paths := request.GetPolicyPaths(); len(paths) > 0 { + merged, err := internal.LoadBundleRootData(paths[0], l.policyData) if err != nil { - return &proto.EvalResponse{Status: proto.ExecutionStatus_FAILURE}, fmt.Errorf("loading bundle data for %s: %w", path, err) + return &proto.EvalResponse{Status: proto.ExecutionStatus_FAILURE}, fmt.Errorf("loading bundle data for %s: %w", paths[0], err) } - policyDataByPath[path] = merged + policyData = merged } policyEvaluator := internal.NewPolicyEvaluator(ctx, l.logger, activities) @@ -88,7 +88,7 @@ func (l *CompliancePlugin) Eval(request *proto.EvalRequest, apiHelper runner.Api var allEvidences []*proto.Evidence var evalErrors error for _, cert := range certs { - certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), policyDataByPath, l.config.PolicyLabels) + certEvidences, err := policyEvaluator.Eval(ctx, cert, request.GetPolicyPaths(), policyData, l.config.PolicyLabels) allEvidences = append(allEvidences, certEvidences...) if err != nil { evalErrors = errors.Join(evalErrors, fmt.Errorf("evaluating cert %s: %w", cert.CertificateArn, err)) From fb30406388727ff68b29d4f6467d6f9cc21a8dd0 Mon Sep 17 00:00:00 2001 From: James Salt Date: Wed, 3 Jun 2026 11:28:59 +0100 Subject: [PATCH 5/5] =?UTF-8?q?BCH-1290:=20Address=20PR=20review=2014=20?= =?UTF-8?q?=E2=80=94=20fix=20data.json=20lookup=20order=20and=20add=20prec?= =?UTF-8?q?edence=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LoadBundleRootData was checking the parent directory first, meaning a local source tree (where data.json lives inside policies/) would silently fall through to an unrelated parent data.json when both exist. Swap the order so policyPath/data.json wins, with parent as the OCI-bundle fallback. Add TestLoadBundleRootData_PolicyPathDataJsonWinsOverParent to lock this precedence: when both locations exist, the file inside policyPath is used. Co-Authored-By: Claude Sonnet 4.6 --- internal/eval.go | 2 +- internal/eval_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/internal/eval.go b/internal/eval.go index b1365c0..9fe74b2 100644 --- a/internal/eval.go +++ b/internal/eval.go @@ -160,8 +160,8 @@ func certificateBaseLabels() map[string]string { // conflict, so operator-supplied policy_data takes precedence over bundle defaults. func LoadBundleRootData(policyPath string, overrides map[string]interface{}) (map[string]interface{}, error) { candidates := []string{ - filepath.Join(filepath.Dir(policyPath), "data.json"), filepath.Join(policyPath, "data.json"), + filepath.Join(filepath.Dir(policyPath), "data.json"), } for _, p := range candidates { raw, err := os.ReadFile(p) diff --git a/internal/eval_test.go b/internal/eval_test.go index c32384a..1edadb8 100644 --- a/internal/eval_test.go +++ b/internal/eval_test.go @@ -50,6 +50,35 @@ func TestLoadBundleRootData_NoBundleDataJsonReturnsOverrides(t *testing.T) { } } +func TestLoadBundleRootData_PolicyPathDataJsonWinsOverParent(t *testing.T) { + root := t.TempDir() + parentJSON := `{"expiry_warning_days":30,"source":"parent"}` + if err := os.WriteFile(filepath.Join(root, "data.json"), []byte(parentJSON), 0644); err != nil { + t.Fatal(err) + } + policiesDir := filepath.Join(root, "policies") + if err := os.Mkdir(policiesDir, 0755); err != nil { + t.Fatal(err) + } + policiesJSON := `{"expiry_warning_days":60,"source":"policyPath"}` + if err := os.WriteFile(filepath.Join(policiesDir, "data.json"), []byte(policiesJSON), 0644); err != nil { + t.Fatal(err) + } + + // When both data.json locations exist, policyPath/data.json must win. + result, err := LoadBundleRootData(policiesDir, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := result["source"]; got != "policyPath" { + t.Errorf("source: got %v, want policyPath", got) + } + if got := result["expiry_warning_days"]; got != float64(60) { + t.Errorf("expiry_warning_days: got %v, want 60", got) + } +} + func TestLoadBundleRootData_FindsDataJsonOneDirectoryUp(t *testing.T) { root := t.TempDir() bundleJSON := `{"expiry_warning_days":30}`