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
OPA filter implementation && doc reference #856
Conversation
Codecov ReportBase: 77.09% // Head: 77.05% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #856 +/- ##
==========================================
- Coverage 77.09% 77.05% -0.05%
==========================================
Files 108 109 +1
Lines 12241 12345 +104
==========================================
+ Hits 9437 9512 +75
- Misses 2278 2305 +27
- Partials 526 528 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| input.request.path_parts | array | The current http request URL path parts | ["a","b","c"] | | ||
| input.request.raw_query | string | The current http request raw query | "a=1&b=2&c=3" | | ||
| input.request.query | map | The current http request query map | {"a":1,"b":2,"c":3} | | ||
| input.request.headers | map | The current http request header map targeted by<br/> includedHeaders | {"Content-Type":"application/json"} | |
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.
typo, <br/>
return o.opaError(w, err) | ||
} | ||
|
||
if len(results) == 0 { |
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.
do you think it is nessesary to check if len(results) > 1 and then report error?
pkg/filters/opafilter/opafilter.go
Outdated
|
||
var body string | ||
if o.spec.ReadBody { | ||
_ = r.FetchPayload(0) |
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.
this has already been done in the HTTPServer.
_ = r.FetchPayload(0) |
pkg/filters/opafilter/opafilter.go
Outdated
if a, exists := resMap["allow"]; exists { | ||
allow = cast.ToBool(a) | ||
} |
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.
if we are sure resMap["allow"]
is a string, propose using strconv.ParseBool
to avoid introducing a new 3rd party package.
pkg/filters/opafilter/opafilter.go
Outdated
} else { | ||
if !allow { | ||
w.SetStatusCode(isc) | ||
} | ||
} |
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.
to reduce nesting.
} else { | |
if !allow { | |
w.SetStatusCode(isc) | |
} | |
} | |
} else if !allow { | |
w.SetStatusCode(isc) | |
} |
Hey, this is really neat! If you'd like to submit your integration to the OPA ecosystem page, we'd be happy to see it there. |
@anderseknert Thanks for the information, we have submitted a PR: open-policy-agent/opa#5421 |
* OPA filter implementation && doc reference * Doc reference changed & using type assertion for performance issue * remove extra opa functions, only retain allow result * remove extra r.FetchPayload(0) * remove unused unit test cases
OPA filter implementation && doc reference #832