[extension/observer/k8sobserver] add k8s.ingress endpoint #33005
Conversation
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
M0NsTeRRR
changed the title
|
/label extension/observer/k8sobserver |
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Not stale |
|
FYI @rmfitzpatrick, @dmitryax |
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Not stale |
There was a problem hiding this comment.
This one looks like a useful addition and overall the patch looks good to me!
@dmitryax @rmfitzpatrick @mx-psi any thoughts on this? What would be the best way to move this forward?
| if rule.HTTP != nil { | ||
| // Create endpoint for each ingress rule. | ||
| for _, path := range rule.HTTP.Paths { | ||
| endpointID := observer.EndpointID(fmt.Sprintf("%s/%s/%s%s", idNamespace, ingress.UID, rule.Host, path.Path)) |
There was a problem hiding this comment.
I think it makes sense to escape the backslashes here and be explicit with the ID format to avoid any kind of unlikely ID collision?
| endpointID := observer.EndpointID(fmt.Sprintf("%s/%s/%s%s", idNamespace, ingress.UID, rule.Host, path.Path)) | |
| endpointID := observer.EndpointID(fmt.Sprintf("%s/%s/%s/%s", idNamespace, ingress.UID, rule.Host, escBackslashes(path.Path))) |
There was a problem hiding this comment.
I don't know; that was my initial question. How do other components deal with that ?
There was a problem hiding this comment.
We just need to ensure the uniqueness of the endpoint. I don't think the escaping is required. We shouldn't run into any conflicts without it
Signed-off-by: Ludovic Ortega <ludovic.ortega@adminafk.fr>
| if rule.HTTP != nil { | ||
| // Create endpoint for each ingress rule. | ||
| for _, path := range rule.HTTP.Paths { | ||
| endpointID := observer.EndpointID(fmt.Sprintf("%s/%s/%s%s", idNamespace, ingress.UID, rule.Host, path.Path)) |
There was a problem hiding this comment.
We just need to ensure the uniqueness of the endpoint. I don't think the escaping is required. We shouldn't run into any conflicts without it
Description: Add support for k8s.ingress endpoint
As described in the issue, this will allow users to dynamically obtain Kubernetes ingress ressource, facilitating the monitoring of certificate expiration with the Blackbox Exporter for example.
I didn't create a global ingress resource with sub-endpoints for each path because I don't see a relevant 'target'. I'm uncertain about the impact of my decision regarding the 'UID' as it's structured as '{namespace}/{ingress UID}/{host}{path}'. Since a path automatically starts with a '/', and can contain other '/', should I escape them ?
Link to tracking Issue:
#32971
Testing:
Documentation:
I've updated the documentation I've found.
PS : I don't write a lot of go and it's also my first contribution to the project I've probably missed some points :)