-
Notifications
You must be signed in to change notification settings - Fork 14
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
Limitador Service Settings #19
Conversation
20d2107
to
c9346ce
Compare
f42564c
to
03ff076
Compare
2fafa43
to
a3525a6
Compare
* Created NamespacedName type just to add serialization details
* It gets the URL from Limitador.Status * It retrieves the right Limitador from its Spec or Default
* Listener instead of Service, to be aligned with other projects * Defining a TransferProtocol for Ports, allowing for future TLS features * Getting the name of the Limitador Obj for the Service Name
a3525a6
to
c3112ad
Compare
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.
minor issues.
Code looks good.
@@ -62,6 +64,11 @@ type RateLimitList struct { | |||
Items []RateLimit `json:"items"` | |||
} | |||
|
|||
type NamespacedName struct { |
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.
it exists in apimachinery https://pkg.go.dev/k8s.io/apimachinery/pkg/types#NamespacedName
// Simple enough now, we could implement a thorough check like with RateLimit with ObservedGeneration in the future | ||
builtServiceUrl := buildServiceUrl(limitadorObj) | ||
if builtServiceUrl != limitadorObj.Status.ServiceURL { | ||
logger.V(1).Info("Updating the Status", "Name", limitadorObj.Name) |
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.
no need, already includes the name
Closes #20
This PR introduces a couple of changes that aim to fix the hardcoded values taken for creating/discovering Limitador Services. The Limitador Operator is meant to have the "ownership" of the Limitador Instance (the actual service), which means it will have the source of truth regarding the Service params, and it's the place to define those.
On the
Limitador
controllerStatus
out of a newSpec.Listener
type andLimitador.Name
Spec.Listener
is marked asoptional
and when it's not present, the values are retrieved from defaultconst
On the
RateLimit
controllerLimitadorRef
within theRateLimit
specoptional
too, will get the values out of default const (as before)namespacedName
to fetch itsStatus.ServiceURL