Skip to content

Commit 2995225

Browse files
tonistiigitonistiigi
committed
policy: update unknown keys normalization
Make the code more unified between validation and test command. Normalize to key without the input prefix. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
1 parent fb5a831 commit 2995225

File tree

4 files changed

+103
-28
lines changed

4 files changed

+103
-28
lines changed

‎commands/policy/test.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func runTest(ctx context.Context, out io.Writer, path string, opts policy.TestOp
8484
_, _ = fmt.Fprintln(out, "decision: <nil>")
8585
}
8686
if len(result.MissingInput) > 0 {
87-
_, _ = fmt.Fprintf(out, "missing_input: %s\n", strings.Join(result.MissingInput, ", "))
87+
_, _ = fmt.Fprintf(out, "missing_input: %s\n", strings.Join(withInputPrefix(result.MissingInput), ", "))
8888
}
8989
if len(result.MetadataNeeded) > 0 {
9090
_, _ = fmt.Fprintf(out, "metadata_resolve: %s\n", strings.Join(result.MetadataNeeded, ", "))
@@ -106,6 +106,14 @@ func writeJSON(out io.Writer, label string, v any) {
106106
_, _ = fmt.Fprintf(out, "%s:\n%s\n", label, string(dt))
107107
}
108108

109+
func withInputPrefix(keys []string) []string {
110+
out := make([]string, len(keys))
111+
for i, k := range keys {
112+
out[i] = "input." + k
113+
}
114+
return out
115+
}
116+
109117
type policyTestResolver struct {
110118
dockerCli command.Cli
111119
builderName *string

‎policy/tester.go‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ func runPolicyTest(ctx context.Context, policyModules map[string]*ast.Module, te
311311
result.Allow = allow
312312
result.DenyMessages = deny
313313

314-
missing := missingInputRefs(policyPackageModules, effectiveInput)
314+
missing := missingInputRefs(policyPackageModules, effectiveInput, runtimeUnknownInputRefs(testState), runtimeUnknownInputRefs(decisionState))
315315
result.MissingInput = uniqueSortedStrings(missing)
316316
result.MetadataNeeded = summarizeMetadataRequests(result.MissingInput)
317317

@@ -534,7 +534,7 @@ func hasEnv(env Env) bool {
534534
func filterResolvableMissing(missing []string) []string {
535535
out := make([]string, 0, len(missing))
536536
for _, m := range missing {
537-
if strings.HasPrefix(m, "input.image.") || strings.HasPrefix(m, "input.git.") {
537+
if strings.HasPrefix(m, "image.") || strings.HasPrefix(m, "git.") {
538538
out = append(out, m)
539539
}
540540
}
@@ -595,24 +595,27 @@ func modulesForPackage(modules map[string]*ast.Module, pkgPath string) []*ast.Mo
595595
return out
596596
}
597597

598-
func missingInputRefs(mods []*ast.Module, input *Input) []string {
598+
func missingInputRefs(mods []*ast.Module, input *Input, extraRefs ...[]string) []string {
599599
if len(mods) == 0 {
600600
return nil
601601
}
602602
inputMap := normalizeInput(input)
603603
refs := collectUnknowns(mods, nil)
604+
for _, er := range extraRefs {
605+
refs = append(refs, er...)
606+
}
607+
seen := map[string]struct{}{}
604608
missing := make([]string, 0, len(refs))
605-
for _, ref := range refs {
606-
key := strings.TrimPrefix(ref, "input.")
607-
if key == ref {
609+
for _, key := range refs {
610+
if key == "" {
608611
continue
609612
}
610-
key = trimKey(key)
611-
if key == "" {
613+
if _, ok := seen[key]; ok {
612614
continue
613615
}
616+
seen[key] = struct{}{}
614617
if !inputHasPath(inputMap, strings.Split(key, ".")) {
615-
missing = append(missing, "input."+key)
618+
missing = append(missing, key)
616619
}
617620
}
618621
return missing
@@ -703,11 +706,7 @@ func summarizeMetadataRequests(missing []string) []string {
703706
return nil
704707
}
705708
req := &gwpb.ResolveSourceMetaRequest{}
706-
trimmed := make([]string, 0, len(missing))
707-
for _, m := range missing {
708-
trimmed = append(trimmed, strings.TrimPrefix(m, "input."))
709-
}
710-
if err := AddUnknowns(req, trimmed); err != nil {
709+
if err := AddUnknowns(req, missing); err != nil {
711710
return nil
712711
}
713712
var out []string

‎policy/utils_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package policy
33
import (
44
"testing"
55

6+
"github.com/open-policy-agent/opa/v1/ast"
67
"github.com/stretchr/testify/require"
78
)
89

@@ -18,12 +19,16 @@ func TestTrimKey(t *testing.T) {
1819
// one separator → stays as-is
1920
{"git.tag", "git.tag"},
2021
{"git[tag", "git[tag"},
22+
{"input.git.tag", "git.tag"},
23+
{"input.git[tag", "git[tag"},
2124

2225
// multiple separators → cut before second one
2326
{"git.tag.author", "git.tag"},
2427
{"git.tag.author.email", "git.tag"},
2528
{"git.tag[0][1]", "git.tag"},
2629
{"git.tag[0]", "git.tag"},
30+
{"input.git.tag.author", "git.tag"},
31+
{"input.git.tag[0]", "git.tag"},
2732

2833
{"a.b.c", "a.b"},
2934
}
@@ -34,3 +39,51 @@ func TestTrimKey(t *testing.T) {
3439
})
3540
}
3641
}
42+
43+
func TestCollectUnknowns(t *testing.T) {
44+
mod, err := ast.ParseModule("x.rego", `
45+
package x
46+
p if {
47+
input.git.tag[0].author == "a"
48+
input.image.signatures[_].signer.certificateIssuer != ""
49+
data.foo.bar == 1
50+
}
51+
`)
52+
require.NoError(t, err)
53+
54+
all := collectUnknowns([]*ast.Module{mod}, nil)
55+
require.ElementsMatch(t, []string{"git.tag", "image.signatures"}, all)
56+
57+
filtered := collectUnknowns([]*ast.Module{mod}, []string{"input.image.signatures"})
58+
require.Equal(t, []string{"image.signatures"}, filtered)
59+
}
60+
61+
func TestRuntimeUnknownInputRefs(t *testing.T) {
62+
require.Nil(t, runtimeUnknownInputRefs(nil))
63+
require.Nil(t, runtimeUnknownInputRefs(&state{}))
64+
65+
st := &state{
66+
Unknowns: map[string]struct{}{
67+
funcVerifyGitSignature: {},
68+
},
69+
}
70+
require.Equal(t, []string{"git.commit"}, runtimeUnknownInputRefs(st))
71+
}
72+
73+
func TestMissingInputRefsWithRuntimeUnknowns(t *testing.T) {
74+
mod, err := ast.ParseModule("x.rego", `
75+
package x
76+
p if {
77+
input.git.ref != ""
78+
}
79+
`)
80+
require.NoError(t, err)
81+
82+
in := &Input{
83+
Git: &Git{
84+
Ref: "refs/heads/main",
85+
},
86+
}
87+
missing := missingInputRefs([]*ast.Module{mod}, in, []string{"git.commit"})
88+
require.Equal(t, []string{"git.commit"}, missing)
89+
}

‎policy/validate.go‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,7 @@ func (p *Policy) CheckPolicy(ctx context.Context, req *policysession.CheckPolicy
224224
return nil, nil, err
225225
}
226226
unk := collectUnknowns(pq.Support, unknowns)
227-
if _, ok := st.Unknowns[funcVerifyGitSignature]; ok {
228-
unk = append(unk, "input.git.commit")
229-
}
227+
unk = append(unk, runtimeUnknownInputRefs(st)...)
230228

231229
if len(unk) > 0 {
232230
next := &gwpb.ResolveSourceMetaRequest{
@@ -621,14 +619,12 @@ func AddUnknowns(req *gwpb.ResolveSourceMetaRequest, unk []string) error {
621619
func AddUnknownsWithLogger(logf func(logrus.Level, string), req *gwpb.ResolveSourceMetaRequest, unk []string) error {
622620
unk2 := make([]string, 0, len(unk))
623621
for _, u := range unk {
624-
k := strings.TrimPrefix(u, "input.")
625-
k = trimKey(k)
626-
switch k {
622+
switch u {
627623
case "image", "git", "http", "local":
628624
// parents are returned as unknowns for some reason, ignore
629625
continue
630626
default:
631-
unk2 = append(unk2, k)
627+
unk2 = append(unk2, u)
632628
}
633629
}
634630
if len(unk2) == 0 {
@@ -680,8 +676,10 @@ func collectUnknowns(mods []*ast.Module, allowed []string) []string {
680676
for _, mod := range mods {
681677
ast.WalkRefs(mod, func(ref ast.Ref) bool {
682678
if ref.HasPrefix(ast.InputRootRef) {
683-
s := ref.String() // e.g. "input.request.path"
684-
s = "input." + trimKey(strings.TrimPrefix(s, "input."))
679+
s := trimKey(ref.String())
680+
if s == "" {
681+
return true
682+
}
685683
if _, ok := seen[s]; !ok {
686684
seen[s] = struct{}{}
687685
out = append(out, s)
@@ -696,6 +694,10 @@ func collectUnknowns(mods []*ast.Module, allowed []string) []string {
696694

697695
valid := map[string]struct{}{}
698696
for _, k := range allowed {
697+
k = trimKey(k)
698+
if k == "" {
699+
continue
700+
}
699701
valid[k] = struct{}{}
700702
}
701703

@@ -709,14 +711,25 @@ func collectUnknowns(mods []*ast.Module, allowed []string) []string {
709711
return filtered
710712
}
711713

714+
func runtimeUnknownInputRefs(st *state) []string {
715+
if st == nil || len(st.Unknowns) == 0 {
716+
return nil
717+
}
718+
var out []string
719+
if _, ok := st.Unknowns[funcVerifyGitSignature]; ok {
720+
out = append(out, "git.commit")
721+
}
722+
return out
723+
}
724+
712725
func summarizeUnknownsForLog(unk []string) []string {
713726
out := make([]string, 0, len(unk))
714727
seen := map[string]struct{}{}
715728
for _, u := range unk {
716-
if strings.HasPrefix(u, "input.image.signatures") {
717-
u = "input.image.signatures"
729+
if strings.HasPrefix(u, "image.signatures") {
730+
u = "image.signatures"
718731
}
719-
if u == "input.image" {
732+
if u == "image" {
720733
continue
721734
}
722735
if _, ok := seen[u]; ok {
@@ -730,14 +743,16 @@ func summarizeUnknownsForLog(unk []string) []string {
730743

731744
func hasHTTPUnknowns(unk []string) bool {
732745
for _, u := range unk {
733-
if strings.HasPrefix(u, "input.http.") {
746+
if strings.HasPrefix(u, "http.") {
734747
return true
735748
}
736749
}
737750
return false
738751
}
739752

740753
func trimKey(s string) string {
754+
s = strings.TrimPrefix(s, "input.")
755+
741756
const (
742757
dot = '.'
743758
sb = '['

0 commit comments

Comments
 (0)