Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

also support logging in via kubernetes_asyncio #1010

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

asteven
Copy link
Contributor

@asteven asteven commented Mar 3, 2023

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 asteven requested a review from nolar as a code owner March 3, 2023 17:31

try:
kubernetes.config.load_incluster_config() # cluster env vars
# Load config from cluster env vars.
kmodule.config.load_incluster_config()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if kmodule is kubernetes_asyncio , I think await keyword is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer, but only load_kube_config is async. load_incluster_config is just a regular function.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for your reply

@cpnielsen
Copy link

@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.

@asteven
Copy link
Contributor Author

asteven commented Apr 12, 2023

@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.

@cpnielsen
Copy link

@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

@asteven
Copy link
Contributor Author

asteven commented Apr 12, 2023

@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
Copy link
Contributor Author

asteven commented Jun 27, 2023

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
Copy link
Contributor Author

asteven commented Jun 27, 2023

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
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentication does not work with kubernetes_asyncio as the only package installed
3 participants