cloudfoundry-community/cf-python-client

UAA endpoint retrieval results in KeyError

ktreese opened this issue · 11 comments

In TAS 2.7.32, the UAA endpoint cannot be retrieved with the key/value combo that was updated in 0b2e898

This results in:

  File "/Users/reesek/opt/miniconda3/envs/tas/lib/python3.9/site-packages/cloudfoundry_client/client.py", line 196, in _get_info
    root_links["login"]["href"]

CF_TRACE shows the response is:

{
  "links": {
    "app_ssh": {
      "href": "ssh.devcf01.example.com:2222",
      "meta": {
        "host_key_fingerprint": "fingerprint",
        "oauth_client": "ssh-proxy"
      }
    },
    "bits_service": null,
    "cloud_controller_v2": {
      "href": "https://api.devcf01.example.com/v2",
      "meta": {
        "version": "2.139.0"
      }
    },
    "cloud_controller_v3": {
      "href": "https://api.devcf01.example.com/v3",
      "meta": {
        "version": "3.74.0"
      }
    },
    "credhub": null,
    "log_stream": {
      "href": "https://log-stream.devcf01.example.com"
    },
    "logging": {
      "href": "wss://doppler.devcf01.example.com:443"
    },
    "network_policy_v0": {
      "href": "https://api.devcf01.example.com/networking/v0/external"
    },
    "network_policy_v1": {
      "href": "https://api.devcf01.example.com/networking/v1/external"
    },
    "routing": {
      "href": "https://api.devcf01.example.com/routing"
    },
    "self": {
      "href": "https://api.devcf01.example.com"
    },
    "uaa": {
      "href": "https://uaa.devcf01.example.com"
    }
  }
}

Updating client.py to root_links["uaa"]["href"] corrects the error.

@ktreese thank you for the feedback.

From what I see from the documentation - not tha clear for me -, the uaa section and login one seem to be optional.

I provided a fix. Would it fix your issue?

@antechrestos
The question is more to me how old cloud controllers do we want to support in this library ?
The CC that is used in TAS 2.7.32 is a patched 2 years old release https://github.com/cloudfoundry/capi-release/releases/tag/1.84.0. And also document that we just test against the new CC and if one wants to use this lib with a 2 year old cf api we cannot make any promises and one maybe needs to stick with an old version of the lib ?

@antechrestos Thank you. After reinstalling with the proposed fix, I'm encountering the following:

(tas)  ➜  cf python cf-client.py                             
Traceback (most recent call last):
  File "/Users/reesek/cf/cf-client.py", line 10, in <module>
    client = CloudFoundryClient(target_endpoint, proxy=proxy, verify=False)
  File "/Users/reesek/opt/miniconda3/envs/tas/lib/python3.9/site-packages/cloudfoundry_client-1.20.3-py3.9.egg/cloudfoundry_client/client.py", line 136, in __init__
  File "/Users/reesek/opt/miniconda3/envs/tas/lib/python3.9/site-packages/cloudfoundry_client-1.20.3-py3.9.egg/cloudfoundry_client/client.py", line 196, in _get_info
KeyError: 'login'
(tas)  ➜  cf 

What about something like this?

    @staticmethod
    def _get_info(target_endpoint: str, proxy: Optional[dict] = None, verify: bool = True) -> Info:
        root_response = CloudFoundryClient._check_response(
            requests.get("%s/" % target_endpoint, proxies=proxy if proxy is not None else dict(http="", https=""), verify=verify)
        )
        root_info = root_response.json()

        def find_uaa(root_links):
            keys = ["login", "uaa", "self"]
            for key in keys:
                if key in root_links:
                    break

            return root_links[key]

        root_links = root_info["links"]
        logging = root_links.get("logging")
        log_stream = root_links.get("log_stream")
        uaa = find_uaa(root_links)

        return Info(
            root_links["cloud_controller_v2"]["meta"]["version"],
            uaa.get("href"),
            target_endpoint,
            logging.get("href") if logging is not None else None,
            log_stream.get("href") if log_stream is not None else None,
        )

@ktreese your're right I coded a fix like a damned morron, using the [] operator while wanting to invoke get that does not fail when no key present.

As a fix, I isolated resolution of login endpoint in a static method. As a consequence, people using the library against a old installation as @FloThinksPi pointed it will be able to extend the class and override the static methode (called with self reference).

@antechrestos sounds good. I'm a bit of a novice python programmer, but wanted to contribute my thought process. As such, I understand and agree with @FloThinksPi position, though it might be worth mentioning that TAS 2.7 is an LTS release, and is technically supported until April 2022. TAS 2.11 (the next LTS release) goes even further until 2024. I know this is an community driven project and don't know if features should align with supportability from the vendor. Just a thought!

@ktreese agreed. Is the fix ok now?

I had to step out but will give it a try this evening.

@ktreese no worry can wait for monday. Beer time here 😏

@antechrestos I was able to test and it's working great! Thanks for taking a look!

Released in 1.20.4