Skip to content

Commit

Permalink
Policy test case fix for restricted destination, prevent panic when r…
Browse files Browse the repository at this point in the history
…eturn is missing (#965)

* Fix restricted_destinations policy test
* Prevent panic if return type is missing on config
  • Loading branch information
l46kok authored Jun 12, 2024
1 parent a555792 commit e765664
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 4 deletions.
6 changes: 6 additions & 0 deletions policy/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down
4 changes: 4 additions & 0 deletions policy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions policy/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
83 changes: 79 additions & 4 deletions policy/testdata/restricted_destinations/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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"

0 comments on commit e765664

Please sign in to comment.