-
Notifications
You must be signed in to change notification settings - Fork 388
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
feat: implement Lua EnvoyExtensionPolicy #5171
base: main
Are you sure you want to change the base?
Conversation
8c8ef7b
to
4b9c54c
Compare
@arkodg @zhaohuabing Wanted to get an initial buy in regarding the approach while I add more unit tests and E2E, please do review. Thanks! |
4b9c54c
to
93c9c44
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5171 +/- ##
==========================================
+ Coverage 66.85% 66.88% +0.02%
==========================================
Files 210 212 +2
Lines 32998 33207 +209
==========================================
+ Hits 22061 22209 +148
- Misses 9595 9645 +50
- Partials 1342 1353 +11 ☔ View full report in Codecov by Sentry. |
hey @rudrakhp the code looks great ! please feel free to continue with unit tests and e2e |
b256946
to
9e01f0e
Compare
c83a97b
to
db990b9
Compare
// +optional | ||
// +unionMember | ||
ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"` | ||
ValueRef *gwapiv1.SecretObjectReference `json:"valueRef,omitempty"` |
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.
any reason why this is secret ?
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.
SecretObjectReference
is not only for Secret
. It's for referring to any K8s object, by default it's Secret type. Here is it's documentation:
// SecretObjectReference identifies an API object including its namespace,
// defaulting to Secret.
//
// The API object must be valid in the cluster; the Group and Kind must
// be registered in the cluster for this reference to be valid.
//
// References to objects with invalid Group and Kind are not valid, and must
// be rejected by the implementation, with appropriate Conditions set
// on the containing object.
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.
Reason for preferring this is to reuse the function for processing config maps:
gateway/internal/provider/kubernetes/controller.go
Lines 753 to 764 in 54ca51b
// processConfigMapRef adds the referenced ConfigMap to the resourceTree if it's valid. | |
// - If it exists in the same namespace as the owner. | |
// - If it exists in a different namespace, and there is a ReferenceGrant. | |
func (r *gatewayAPIReconciler) processConfigMapRef( | |
ctx context.Context, | |
resourceMap *resourceMappings, | |
resourceTree *resource.Resources, | |
ownerKind string, | |
ownerNS string, | |
ownerName string, | |
configMapRef gwapiv1.SecretObjectReference, | |
) error { |
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.
check out the code flow for
gateway/api/v1alpha1/shared_types.go
Line 746 in 36fdc0b
ValueRef *gwapiv1.LocalObjectReference `json:"valueRef,omitempty"` |
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.
@arkodg will do. One additional caveat here is LocalObjectReference
let's you locate resources in same namespace only, do we want to have that restriction?
// LocalObjectReference contains enough information to let you locate the
// referenced object inside the same namespace.
db990b9
to
fc6d094
Compare
Signed-off-by: Rudrakh Panigrahi <[email protected]>
fc6d094
to
8e967bb
Compare
What type of PR is this?
feat: implement Lua feature in EnvoyExtensionPolicy
What this PR does / why we need it:
Implement API introduced in #4932
Which issue(s) this PR fixes:
Related #4627
Release Notes: No