-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
base: main
Are you sure you want to change the base?
Conversation
kopf/_core/intents/piggybacking.py
Outdated
|
||
try: | ||
kubernetes.config.load_incluster_config() # cluster env vars | ||
# Load config from cluster env vars. | ||
kmodule.config.load_incluster_config() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your reply
@asteven Have you tested this? We have this implemented locally using the 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 |
Yes, works for me. But did not test with BearerToken authentication.
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 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 |
308ec40
to
fe5066f
Compare
@cpnielsen thanks for putting that example together. I've reworked the patch to be more explicit.
vs
|
library Signed-off-by: Steven Armstrong <steven@armstrong.cc>
fe5066f
to
91c289c
Compare
After testing this more thoroughly it seems that none of the The created |
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
) |
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).