From e765664e8e682c7395106d686c1323833c82b2d2 Mon Sep 17 00:00:00 2001 From: swh Date: Wed, 12 Jun 2024 13:46:42 -0700 Subject: [PATCH] Policy test case fix for restricted destination, prevent panic when return is missing (#965) * Fix restricted_destinations policy test * Prevent panic if return type is missing on config --- policy/compiler_test.go | 6 ++ policy/config.go | 4 + policy/config_test.go | 10 +++ .../restricted_destinations/tests.yaml | 83 ++++++++++++++++++- 4 files changed, 99 insertions(+), 4 deletions(-) diff --git a/policy/compiler_test.go b/policy/compiler_test.go index 59ad0375..07e33b6c 100644 --- a/policy/compiler_test.go +++ b/policy/compiler_test.go @@ -156,6 +156,12 @@ func (r *runner) run(t *testing.T) { } else if testOut.Equal(optOut.GetValue()) != types.True { t.Errorf("policy eval got %v, wanted %v", out, testOut) } + } else if boolOut, ok := out.(types.Bool); ok { + if testOut.Equal(boolOut) != types.True { + t.Errorf("policy eval got %v, wanted %v", boolOut, testOut) + } + } else { + t.Errorf("unexpected policy output type %v", out) } }) } diff --git a/policy/config.go b/policy/config.go index 51a36e82..3bbc1b43 100644 --- a/policy/config.go +++ b/policy/config.go @@ -210,6 +210,10 @@ func (od *OverloadDecl) AsFunctionOption(baseEnv *cel.Env) (cel.FunctionOpt, err return nil, err } } + + if od.Return == nil { + return nil, fmt.Errorf("missing return type on overload: %v", od.OverloadID) + } result, err := od.Return.AsCELType(baseEnv) if err != nil { return nil, err diff --git a/policy/config_test.go b/policy/config_test.go index 52f780de..0b4a9abe 100644 --- a/policy/config_test.go +++ b/policy/config_test.go @@ -203,6 +203,16 @@ functions: type_name: "null_type"`, err: "undefined type name: unknown", }, + { + config: ` +functions: + - name: "missing_return" + overloads: + - id: "unary_global" + args: + - type_name: "null_type"`, + err: "missing return type on overload: unary_global", + }, } baseEnv, err := cel.NewEnv(cel.OptionalTypes()) if err != nil { diff --git a/policy/testdata/restricted_destinations/tests.yaml b/policy/testdata/restricted_destinations/tests.yaml index 70183f7a..1cf59fe6 100644 --- a/policy/testdata/restricted_destinations/tests.yaml +++ b/policy/testdata/restricted_destinations/tests.yaml @@ -16,9 +16,9 @@ description: Restricted destinations conformance tests. section: - name: "valid" tests: - - name: "allowed" + - name: "ip_allowed" input: - "spec.origin": + "spec.origin": value: "us" "spec.restricted_destinations": value: @@ -27,9 +27,9 @@ section: - "kp" - "sd" - "sy" - "destination.ip": + "destination.ip": value: "10.0.0.1" - "origin.ip": + "origin.ip": value: "10.0.0.1" request: value: @@ -40,4 +40,79 @@ section: name: "/company/acme/secrets/doomsday-device" labels: location: "us" + output: "false" # false means unrestricted + - name: "nationality_allowed" + input: + "spec.origin": + value: "us" + "spec.restricted_destinations": + value: + - "cu" + - "ir" + - "kp" + - "sd" + - "sy" + "destination.ip": + value: "10.0.0.1" + request: + value: + auth: + claims: + nationality: "us" + resource: + value: + name: "/company/acme/secrets/doomsday-device" + labels: + location: "us" + output: "false" + - name: "invalid" + tests: + - name: "destination_ip_prohibited" + input: + "spec.origin": + value: "us" + "spec.restricted_destinations": + value: + - "cu" + - "ir" + - "kp" + - "sd" + - "sy" + "destination.ip": + value: "123.123.123.123" + "origin.ip": + value: "10.0.0.1" + request: + value: + auth: + claims: {} + resource: + value: + name: "/company/acme/secrets/doomsday-device" + labels: + location: "us" + output: "true" # true means restricted + - name: "resource_nationality_prohibited" + input: + "spec.origin": + value: "us" + "spec.restricted_destinations": + value: + - "cu" + - "ir" + - "kp" + - "sd" + - "sy" + "destination.ip": + value: "10.0.0.1" + request: + value: + auth: + claims: + nationality: "us" + resource: + value: + name: "/company/acme/secrets/doomsday-device" + labels: + location: "cu" output: "true"