argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
16.75k stars 5.08k forks source link

OpenAPI document does not match actual responses for /v1 of the API. #14604

Open jm-bairesdev opened 11 months ago

jm-bairesdev commented 11 months ago

Checklist:

Describe the bug

The spec file downloaded from /swagger-ui does not match the actual responses of the API.

Screenshot 2023-07-19 at 8 31 17

To Reproduce

{
  "hosts": [...],
  "nodes": [
    "version": "v1",
    "kind": "string",
    "namespace": "string",
    "name": "string",
    "uid": "string",
    "parentRefs": [...],
    "info": [...],
    "networkingInfo": {...},
    "resourceVersion": "string",
    "images": [...],
    "health": {...},
    "createdAt": "iso-date-string"
  ],
  "orphanedNodes": [...]
},

Expected behavior

The API spec matches what's actually coded. Being able to rely on the documentation. Being able to generate a reliable client (services, models, etc) based on the spec.

Fix

Update the OpenAPI specification of the /v1 API to reflect what's actually returned in the code.

crenshaw-dev commented 11 months ago

Can you double-check the "actual` example provided? As it's written, it's not valid JSON (keys nested under an array).

I've just tested in 2.8.0-rc2, and the response seems to match the OpenAPI spec:

image

jm-bairesdev commented 11 months ago

Thanks for the quick reply @crenshaw-dev.

Sorry, didn't run it through a JSON validator; added missing commas, removed the ellipses (were just illustrative), and added the curly braces to the example node.

Leaving only nodes in this snippet as I want to focus on the differences there, not that those are the only ones.

Expected structure:

{
    "hosts": [],
    "nodes": [{
        "createdAt": {
            "nanos": 0,
            "seconds": "string"
        },
        "health": {

        },
        "images": [],
        "info": [],
        "networkingInfo": {

        },
        "parentRefs": [],
        "resourceRef": {
            "group": "string",
            "kind": "string",
            "name": "string",
            "namespace": "string",
            "uid": "string",
            "version": "string"
        },
        "resourceVersion": "string"
    }],
    "orphanedNodes": []
}

Actual response:

{
    "hosts": [],
    "nodes": [{
        "version": "v1",
        "kind": "string",
        "namespace": "string",
        "name": "string",
        "uid": "string",
        "parentRefs": [],
        "info": [],
        "networkingInfo": {

        },
        "resourceVersion": "string",
        "images": [],
        "health": {

        },
        "createdAt": "iso-date-string"
    }],
    "orphanedNodes": []
}

As for your test, if that image is the actual returned data, then yes, that's also what I see when I GET /api/v1/applications/{applicationName}/resource-tree. The problem is that that's not what the OpenAPI spec documents.

So maybe I'm using the wrong spec? I would like to use /v1 as documented in swagger. I am downloading the OpenAPI spec file from /swagger-ui, as described here.

Please let me know if I'm using the right one.

Thanks

crenshaw-dev commented 11 months ago

Ah, yep. tbh this doesn't surprise me. The code that generates our OpenAPI doc often disagrees with our actual API code, due to differences in expected vs. actual JSON marshaling. Here's an example of an attempt to solve one: https://github.com/argoproj/argo-cd/pull/13046/files

jq hacks are the current fix. If we can identify a common "class" of issues which this example demonstrates, maybe we can eliminate quite a few inaccuracies with a single hack.

jm-bairesdev commented 11 months ago

@crenshaw-dev looking through that example, I'm not sure we're talking about the same level of different...

You can see in my example, the differences are quite beyond something like string vs number.

For example, the docs say about createdAt:

"createdAt":  {
  "nanos": 0,
  "seconds": "string"
}

and about resourceRef:

"resourceRef": {
  "group": "string",
  "kind": "string",
  "name": "string",
  "namespace": "string",
  "uid": "string",
  "version": "string"
}

Whereas in actuality, we get:

I mean, complete disassociation between the spec and the code.

So, yeah, I just wanted to bring this up because it renders the spec file completely useless, sadly, and we could be using it with tools like openapi-typescript-codegen as we do we other APIs we consume to auto generate our client πŸ˜ƒ (instead of manually as we currently do with ArgoCD 😒).

Thanks again

crenshaw-dev commented 11 months ago

Believe it or not, each of those is pretty easily explained with just a few lines of golang. :-P We have a custom marshaller that flattens timestamps into string, and a JSON tag that flattens the resourceRef fields into the parent object.

jm-bairesdev commented 11 months ago

Ok... I'll take your word for it πŸ˜„.

Well, I hope this is fixed... sometime... πŸ₯²

crenshaw-dev commented 11 months ago

If you can write a jq query that can correct these two issues in the openapi spec, I'd happily review!

jm-bairesdev commented 11 months ago

Honestly, this is uncharted territory for me, but it sure sounds interesting. I'll give it a try when I have time. Can I throw technical questions for you in this thread if I have any?

crenshaw-dev commented 11 months ago

For sure! I'm here and on CNCF Slack.