-
Notifications
You must be signed in to change notification settings - Fork 8
Potential fix for custom metrics endpoints #107
base: master
Are you sure you want to change the base?
Conversation
@silasbw pretty please |
I've been a really horrible open source steward for this project. I'm sorry about that, but I haven't had the time. I did a quick test of this locally and it doesn't appear to solve the issue and therefore I can't merge it. If someone wants to pick it up and add additional verification, I would happily accept a refined PR. |
I noticed I'd committed with 'const' instead of 'let'. Can you please try again? |
Okay, for anyone else looking at this, we went ahead and built our custom Kube External Secrets docker image with this code change. Everything's working as expected. |
@svaranasi-traderev not sure either is this is right fix but it saved us from removing |
Sorry, that changes led to "ERROR, kubeNamespace.get is not a function" errors, it seems broke the whole lib. |
I found the previous fix wasn't as reliable as we expected it to be, as @adutchak-x discovered. So, I worked on a more reliable fix that has stood the test of time for us. Unsure why Git's showing so many lines as changed while vscode's diff is absolutely normal. |
You indented most/all of the file by one space. See #117 without the indentation change. |
This should fix #55 and a few upstream project issues such as this: external-secrets/kubernetes-external-secrets#576
Fair bit of WARNING that I'm nowhere even remotely close to having any JS knowledge, so while this code works for me, it might either be a terrible implementation or a buggy one for other projects. Please feel free to fix as you see fit.
The reason this fix is required is due to projects such as 'kube-metrics-adapter' creating a couple of new API endpoints. If you fetch the openapi spec:
kubectl get --raw /openapi/v2
you'll see that two problematic custom.metrics endpoints have a templated parameter right after 'version'. No other API in the spec has this behaviour.
Side note, I noticed none of the custom.metrics or external.metrics endpoints carry a version object either. Other endpoints carry this information in the
"x-kubernetes-group-version-kind"
field, for example:The fix in the PR assigns variable 'group' to list item [1] in an array that looks like 'splits' below:
The fix then ignores if the parent of the current templated 'split' matches the 'group' variable, since a 'group' would never be a templated split.
Here's the entire json for one of the two problematic endpoints, in case you were curious: