silasbw / swagger-fluent

A fluent client for OpenAPI and Swagger
MIT License
8 stars 8 forks source link

Potential fix for custom metrics endpoints #107

Open svaranasi-corporate opened 3 years ago

svaranasi-corporate commented 3 years ago

This should fix https://github.com/silasbw/swagger-fluent/issues/55 and a few upstream project issues such as this: https://github.com/external-secrets/kubernetes-external-secrets/issues/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.

        },
        "/apis/custom.metrics.k8s.io/v1beta1/{resource}/{name}/{subresource}": {
            "get": {
                .
                .
                .
        },
        "/apis/custom.metrics.k8s.io/v1beta2/{resource}/{name}/{subresource}": {
            "get": {

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:

                "x-kubernetes-group-version-kind": {
                    "group": "discovery.k8s.io",
                    "kind": "EndpointSlice",
                    "version": "v1beta1"
                }

The fix in the PR assigns variable 'group' to list item [1] in an array that looks like 'splits' below:

  name: '/apis/custom.metrics.k8s.io/v1beta1/{resource}/{name}/{subresource}',
  splits: [
    'apis',
    'custom.metrics.k8s.io',
    'v1beta1',
    '{resource}',
    '{name}',
    '{subresource}'
  ],

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:

        "/apis/custom.metrics.k8s.io/v1beta1/{resource}/{name}/{subresource}": {
            "get": {
                "description": "list custom metrics describing an object or objects",
                "consumes": ["*/*"],
                "produces": ["application/json", "application/yaml", "application/vnd.kubernetes.protobuf", "application/json;stream=watch", "application/vnd.kubernetes.protobuf;stream=watch"],
                "schemes": ["https"],
                "tags": ["customMetrics_v1beta1"],
                "operationId": "listCustomMetricsV1beta1MetricValue",
                "responses": {
                    "200": {
                        "description": "OK",
                        "schema": {
                            "$ref": "#/definitions/io.k8s.metrics.pkg.apis.custom_metrics.v1beta1.MetricValueList"
                        }
                    }
                }
            },
            "parameters": [{
                "uniqueItems": true,
                "type": "boolean",
                "description": "allowWatchBookmarks requests watch events with type \"BOOKMARK\". Servers that do not implement bookmarks may ignore this flag and bookmarks are sent at the server's discretion. Clients should not assume bookmarks are returned at any specific interval, nor may they assume the server will send any BOOKMARK event during a session. If this is not a watch, this field is ignored. If the feature gate WatchBookmarks is not enabled in apiserver, this field is ignored.",
                "name": "allowWatchBookmarks",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "The continue option should be set when retrieving more results from the server. Since this value is server defined, clients may only use the continue value from a previous query result with identical query parameters (except for the value of continue) and the server may reject a continue value it does not recognize. If the specified continue value is no longer valid whether due to expiration (generally five to fifteen minutes) or a configuration change on the server, the server will respond with a 410 ResourceExpired error together with a continue token. If the client needs a consistent list, it must restart their list without the continue field. Otherwise, the client may send another list request with the token received with the 410 error, the server will respond with a list starting from the next key, but from the latest snapshot, which is inconsistent from the previous list results - objects that are created, modified, or deleted after the first list request will be included in the response, as long as their keys are after the \"next key\".\n\nThis field is not supported when watch is true. Clients may start a watch from the last resourceVersion value returned by the server and not miss any modifications.",
                "name": "continue",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "A selector to restrict the list of returned objects by their fields. Defaults to everything.",
                "name": "fieldSelector",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "A selector to restrict the list of returned objects by their labels. Defaults to everything.",
                "name": "labelSelector",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "integer",
                "description": "limit is a maximum number of responses to return for a list call. If more items exist, the server will set the `continue` field on the list metadata to a value that can be used with the same initial query to retrieve the next set of results. Setting a limit may return fewer than the requested amount of items (up to zero items) in the event all requested objects are filtered out and clients should only use the presence of the continue field to determine whether more results are available. Servers may choose not to support the limit argument and will return all of the available results. If limit is specified and the continue field is empty, clients may assume that no more results are available. This field is not supported if watch is true.\n\nThe server guarantees that the objects returned when using continue will be identical to issuing a single list call without a limit - that is, no objects created, modified, or deleted after the first request is issued will be included in any subsequent continued requests. This is sometimes referred to as a consistent snapshot, and ensures that a client that is using limit to receive smaller chunks of a very large result can ensure they see all possible objects. If objects are updated during a chunked list the version of the object that was present at the time the first list result was calculated is returned.",
                "name": "limit",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "name": "metricLabelSelector",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "name of the described resource",
                "name": "name",
                "in": "path",
                "required": true
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "If 'true', then the output is pretty printed.",
                "name": "pretty",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "the name of the resource",
                "name": "resource",
                "in": "path",
                "required": true
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "resourceVersion sets a constraint on what resource versions a request may be served from. See https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-versions for details.\n\nDefaults to unset",
                "name": "resourceVersion",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "resourceVersionMatch determines how resourceVersion is applied to list calls. It is highly recommended that resourceVersionMatch be set for list calls where resourceVersion is set See https://kubernetes.io/docs/reference/using-api/api-concepts/#resource-versions for details.\n\nDefaults to unset",
                "name": "resourceVersionMatch",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "string",
                "description": "the name of the subresource",
                "name": "subresource",
                "in": "path",
                "required": true
            }, {
                "uniqueItems": true,
                "type": "integer",
                "description": "Timeout for the list/watch call. This limits the duration of the call, regardless of any activity or inactivity.",
                "name": "timeoutSeconds",
                "in": "query"
            }, {
                "uniqueItems": true,
                "type": "boolean",
                "description": "Watch for changes to the described resources and return them as a stream of add, update, and remove notifications. Specify resourceVersion.",
                "name": "watch",
                "in": "query"
            }]
        },
tr-srij commented 3 years ago

@silasbw pretty please

silasbw commented 3 years ago

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.

svaranasi-corporate commented 3 years ago

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?

svaranasi-corporate commented 3 years ago

@silasbw

svaranasi-corporate commented 3 years ago

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.

adutchak-x commented 3 years ago

@svaranasi-traderev not sure either is this is right fix but it saved us from removing apiservice v1beta1.custom.metrics.k8s.io from our clusters in order to make external-secrets working. looking forward to get this or similar fix merged.

adutchak-x commented 3 years ago

@svaranasi-traderev not sure either is this is right fix but it saved us from removing apiservice v1beta1.custom.metrics.k8s.io from our clusters in order to make external-secrets working. looking forward to get this or similar fix merged.

Sorry, that changes led to "ERROR, kubeNamespace.get is not a function" errors, it seems broke the whole lib. Instead, I've temporarily excluded custom.metrics.k8s.io with statement like this: "if (template && !endpoint.name.includes("custom.metrics.k8s.io"))" {...

svaranasi-corporate commented 2 years ago

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.

mestuddtc commented 2 years ago

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.