-
Notifications
You must be signed in to change notification settings - Fork 257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Moving to structured JSON policy decisions. #1718
Conversation
This commit makes two major changes: 1. All policy enforcement points now receive a context objects and use it to log policy errors and denial decisions. 2. Policy denials are now conveyed as structured JSON objects. Whereas previously policy denial was surfaced as a text error message, the policy now generates a bracketed base64 encoded string: policydecision< (base64) >policydecision When decoded, this will be a JSON object with the following structure: ```json { "input": <redacted input object>, "decision": "deny", "reason": <reason object explaining decision>, "policyError": <optional error field> } ``` NB: the `"policyError"` field above is only present if the denial was triggered by an actual error in the Rego policy. Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
if len(rawResult) == 0 { | ||
return result, nil | ||
return nil, errors.New("emtpy result from Rego query") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way to get context around why the rego query is empty or what to do about it?
|
||
errorBytes, err := base64.RawURLEncoding.DecodeString(matches[1]) | ||
if err != nil { | ||
return "", err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to also return the policy decision error so that action can be taken despite the fact that it's not properly base64 encoded?
} | ||
|
||
if versionMissing { | ||
return nil, errors.New(noAPIVersionError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than using errors.New
here, if you make the error types exported and define the global variables with errors.New
above, it would be possible in the future to use errors.Is
to take different actions based on different issues and decouple from the exact wording in the error message.
This commit makes two major changes: 1. All policy enforcement points now receive a context objects and use it to log policy errors and denial decisions. 2. Policy denials are now conveyed as structured JSON objects. Whereas previously policy denial was surfaced as a text error message, the policy now generates a bracketed base64 encoded string: policydecision< (base64) >policydecision When decoded, this will be a JSON object with the following structure: ```json { "input": <redacted input object>, "decision": "deny", "reason": <reason object explaining decision>, "policyError": <optional error field> } ``` NB: the `"policyError"` field above is only present if the denial was triggered by an actual error in the Rego policy. Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>
This PR makes two major changes:
Whereas previously policy denial was surfaced as a text error message, the policy now generates a bracketed base64 encoded string:
policydecision< (base64) >policydecision
When decoded, this will be a JSON object with the following structure:
NB: the
"policyError"
field above is only present if the denial was triggered by an actual error in the Rego policy.