kopf icon indicating copy to clipboard operation
kopf copied to clipboard

also support logging in via kubernetes_asyncio

Open asteven opened this issue 1 year ago • 6 comments

Since kubernetes and kubernetes_asyncio are generated from the same openapi spec they can both be used interchangeably to provide the login_via_client activity.

closes: #1008

I did not find any tests for the existing login_via_client. Let me know if/how this could be done (or is needed at all).

asteven avatar Mar 03 '23 17:03 asteven

@asteven Have you tested this? We have this implemented locally using the @kopf.on.login decorator, and I found that the async library deviates a little bit, and you need to change the header line to:

header: Optional[str] = cfg.get_api_key_with_prefix("BearerToken")

Beyond that it works the same. I would also suggest that you seperate sync from async (combining import and configure in one try-catch), and then factor out the shared implementation to create the kopf.ConnectionInfo object.

cpnielsen avatar Apr 11 '23 14:04 cpnielsen

@asteven Have you tested this?

Yes, works for me. But did not test with BearerToken authentication.

We have this implemented locally using the @kopf.on.login decorator, and I found that the async library deviates a little bit, and you need to change the header line to:

header: Optional[str] = cfg.get_api_key_with_prefix("BearerToken")

Beyond that it works the same. I would also suggest that you seperate sync from async (combining import and configure in one try-catch), and then factor out the shared implementation to create the kopf.ConnectionInfo object.

I agree. I'll see what I can do.

But - I'm not even sure @nolar would accept this patch anyway as from my understanding the whole login via client lib is seen as leftover from the past as noted in kopf/_core/intents/piggybacking.py

 # We keep the official client library auto-login only because it was
 # an implied behavior before switching to pykube -- to keep it so (implied).
def login_via_client(
...

@nolar are you willing to accept a (nicer) patch to support the async client? As it seems not supporting the standard kuberentes library (async or not) causes some regressions. So this might be a simple way to fix that for many people.

@cpnielsen Thanks for the hints. If you could paste your on-login decorator I could work from there. Or at least update the docs with an example if nolar does not want this in code.

asteven avatar Apr 12 '23 06:04 asteven

@asteven I had to collect it from bits and pieces (as it was mingled with our internal code), but the below code should work as-is. It's made to try to configure both with in-cluster credentials as well as local kube config (making testing easier). Adjust as needed.

from kubernetes_asyncio import client, config

import kopf

from typing import Any, Optional, Sequence


@kopf.on.login()
async def authenticate(**_: Any) -> kopf.ConnectionInfo:
    """This method is needed, as the default kopf.login_via_client is based on the sync kubernetes library.
    We also use "BearerToken" for the api key lookup instead of "authorization", see differences:
    - Sync lib: https://github.com/kubernetes-client/python-base/blob/master/config/kube_config.py#L570
    - Async lib: https://github.com/tomplus/kubernetes_asyncio/blob/master/kubernetes_asyncio/config/kube_config.py#L370
    """
    try:
        config.load_incluster_config()
    except config.ConfigException:
        await config.load_kube_config()

    cfg = client.Configuration.get_default_copy()

    # Taken from kopf.piggybacking for sync kubernetes library
    header: Optional[str] = cfg.get_api_key_with_prefix("BearerToken")
    parts: Sequence[str] = header.split(" ", 1) if header else []
    scheme, token = (
        (None, None) if len(parts) == 0 else (None, parts[0]) if len(parts) == 1 else (parts[0], parts[1])
    )  # RFC-7235, Appendix C.

    ci = kopf.ConnectionInfo(
        server=cfg.host,
        ca_path=cfg.ssl_ca_cert,
        insecure=not cfg.verify_ssl,
        username=cfg.username or None,
        password=cfg.password or None,
        scheme=scheme,
        token=token,
        certificate_path=cfg.cert_file,
        private_key_path=cfg.key_file,
        priority=1,
    )

    return ci

cpnielsen avatar Apr 12 '23 08:04 cpnielsen

@cpnielsen thanks for putting that example together.

I've reworked the patch to be more explicit. There is some code duplication but it's probably cleaner this way. It is now also visible in the logs which client library was used to authenticate, e.g.

[2023-04-12 23:28:43,441] kopf.activities.auth [INFO    ] Activity 'login_via_client' succeeded.

vs

[2023-04-12 23:31:50,394] kopf.activities.auth [INFO    ] Activity 'login_via_async_client' succeeded.

asteven avatar Apr 12 '23 21:04 asteven

After testing this more thoroughly it seems that none of the login_* functions from kopf/_core/intents/piggybacking.py work properly since #933 was merged.

The created credentials.ConnectionInfo instances are created without an explicit expiration kwarg. As they have no expiration set, they never expire and the login_* handlers are never called again.

asteven avatar Jun 27 '23 14:06 asteven

The following on.login handler works properly

@kopf.on.login()
async def authenticate(
        *,
        logger: kopf.Logger,
        **_: Any,
) -> Optional[kopf.ConnectionInfo]:
    # Keep imports in the function, as module imports are mocked in some tests.
    try:
        import kubernetes_asyncio.config
    except ImportError:
        return None

    try:
        kubernetes_asyncio.config.load_incluster_config()  # cluster env vars
        logger.debug("Async client is configured in cluster with service account.")
    except kubernetes_asyncio.config.ConfigException as e1:
        try:
            await kubernetes_asyncio.config.load_kube_config()  # developer's config files
            logger.debug("Async client is configured via kubeconfig file.")
        except kubernetes_asyncio.config.ConfigException as e2:
            raise kopf.LoginError("Cannot authenticate the async client library "
                                         "neither in-cluster, nor via kubeconfig.")

    # We do not even try to understand how it works and why. Just load it, and extract the results.
    # For kubernetes client >= 12.0.0 use the new 'get_default_copy' method
    if callable(getattr(kubernetes_asyncio.client.Configuration, 'get_default_copy', None)):
        config = kubernetes_asyncio.client.Configuration.get_default_copy()
    else:
        config = kubernetes_asyncio.client.Configuration()

    # For auth-providers, this method is monkey-patched with the auth-provider's one.
    # We need the actual auth-provider's token, so we call it instead of accessing api_key.
    # Other keys (token, tokenFile) also end up being retrieved via this method.
    header: Optional[str] = config.get_api_key_with_prefix('BearerToken')
    parts: Sequence[str] = header.split(' ', 1) if header else []
    scheme, token = ((None, None) if len(parts) == 0 else
                     (None, parts[0]) if len(parts) == 1 else
                     (parts[0], parts[1]))  # RFC-7235, Appendix C.

    expiration = datetime.datetime.utcnow() + datetime.timedelta(minutes=1)
    return kopf.ConnectionInfo(
        server=config.host,
        ca_path=config.ssl_ca_cert,  # can be a temporary file
        insecure=not config.verify_ssl,
        username=config.username or None,  # an empty string when not defined
        password=config.password or None,  # an empty string when not defined
        scheme=scheme,
        token=token,
        certificate_path=config.cert_file,  # can be a temporary file
        private_key_path=config.key_file,  # can be a temporary file
        priority=1,
        expiration=expiration
    )

asteven avatar Jun 27 '23 14:06 asteven