Conversation
hown3d
requested review from
dergeberl,
robinschneider and
timebertt
as code owners
hown3d
force-pushed
the
refactor/types
branch
2 times, most recently
from
859f14e to
9e7beaf
Compare
dergeberl
left a comment
dergeberl
left a comment
There was a problem hiding this comment.
Very nice to have this objects typed! 🎉
We added a few comments.
I will have a second look incl. the cluster on the ondemand after resolving our comments.
| if err := json.Unmarshal([]byte(originalFilter.Raw), &originalFilterMap); err != nil { | ||
| return admission.Errored(http.StatusInternalServerError, err) | ||
| var originalFilter *structpb.Struct | ||
| for _, configpatch := range filter.Spec.ConfigPatches { |
There was a problem hiding this comment.
In the gjson we only checked the ConfigPatches[0].
There was a problem hiding this comment.
We could break the outer loop when we find the first occurence of"envoy.filters.network.tcp_proxy" in a filterList. This way we are safe if the configPatches changes and something is added before the tcp_proxy filter
hown3d
force-pushed
the
refactor/types
branch
2 times, most recently
from
bdf5b81 to
3c0bb13
Compare
| - remote_ip: | ||
| address_prefix: 10.96.0.0 | ||
| prefix_len: 11 | ||
| stat_prefix: envoyrbac |
There was a problem hiding this comment.
This was previously a bug, there is not stat_prefix field in http.rbac.v3.RBAC message.
Ref: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/rbac/v3/rbac.proto
Signed-off-by: Lukas Hoehl <lukas.hoehl@stackit.cloud>
2 participants
What this PR does / why we need it:
Refactors the code to use the proto definitions of envoy instead of relying on map[string]interface{}.
This ensures type safety and increases developer experience a lot, since you no longer have to dig deep in the envoy documentation in regards to syntax.
Special notes for your reviewer:
/cc @timebertt
Tested on
ond-lambwith shootacl