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

Configure SSL CA CERTIFICATE with REQUESTS_CA_BUNDLE env var before certifi #1131

Open
nirousseau opened this issue Mar 31, 2020 · 9 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@nirousseau
Copy link

nirousseau commented Mar 31, 2020

What is the feature and why do you need it:

The idea is to configure ssl_ca_cert using the REQUESTS_CA_BUNDLE env var before falling back to certifi if no specific configuration has been provided.

Some applications that are using kubernetes-client / python do not provide a parameter to client/configuration.py#L83. Using an env var before certifi will help such use cases.

Finally, it can be very useful in a container context, as we can pass this configuration via, once again, env vars.

Describe the solution you'd like to see:

In :
client/rest.py#L70

At the moment, the code is the following :

        # ca_certs
        if configuration.ssl_ca_cert:
            ca_certs = configuration.ssl_ca_cert
        else:
            # if not set certificate file, use Mozilla's root certificates.
            ca_certs = certifi.where()

We can add a new condition of the form :

        # ca_certs
        if configuration.ssl_ca_cert:
            ca_certs = configuration.ssl_ca_cert
        elif 'REQUESTS_CA_BUNDLE' in os.environ:
            ca_certs = environ.get('REQUESTS_CA_BUNDLE')
        else:
            # if not set certificate file, use Mozilla's root certificates.
            ca_certs = certifi.where()

The env var REQUESTS_CA_BUNDLE seems to be a good candidate as it is a common practice.

Related issues:

@nirousseau nirousseau added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 31, 2020
@palnabarun
Copy link
Member

@nirousseau Why not pass the ssl_ca_cert while initializing configuration?

import os
from kubernetes import client, config

client_configuration = client.configuration.Configuration()
client_configuration.ssl_ca_cert = os.environ.get('REQUESTS_CA_BUNDLE')

config.load_kube_config(client_configuration=client_configuration)

This creates a separation of concern and solves the problem at hand.

Also, changing anything in https://github.com/kubernetes-client/python/blob/master/kubernetes/client/rest.py requires a change in the OpenAPI generator since the module is automatically generated from OpenAPI spec.

@nirousseau
Copy link
Author

nirousseau commented Apr 15, 2020

@palnabarun Hello, thank your for your answer.

For more context, this proposal is connected with this issue from apache / airflow : apache/airflow#8019.

I agree that the responsibility of setting this configuration belongs to the software that is using this library. But as a system administrator, I would like to change this setting, even if the developers of apache / airflow have not integrated the ability to change it.

I want to discuss the idea that both developers and system administrators should be able to change the value of the certificate authority path, or at least, its default value.

Currently : user_given_setting -> certifi.

Developer oriented.

Pros :

  • Separation of concern, as only developers can operate the change by editing the code calling the library.

Cons :

  • I have to make a fork of apache / airflow and alter the logic
  • System administrators are relying on developers to expose this setting

I suggest : user_given_setting -> system_env -> certifi.

Developers + Operators can control the behavior.

Pros :

  • Both developers and system administrators can set the default CA path.

Cons :

  • Add a few lines in the OpenAPI generator

Now, the question "should a library inspect its environment in order to set default settings ?" can be legitimately asked. This proposal adds more control to system administrators, but it can be a breaking change for systems that have this env var miss configured.

@splashx
Copy link

splashx commented May 19, 2020

This code snippet:

import os
from kubernetes import client, config

client_configuration = client.configuration.Configuration()
client_configuration.ssl_ca_cert = os.environ.get('REQUESTS_CA_BUNDLE')

config.load_kube_config(client_configuration=client_configuration)

Unfortunately doesn't load the kube config config as expected - for example, the host is set as localhost instead of that of the kubeconfig/context.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 17, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 16, 2020
@palnabarun
Copy link
Member

/lifecycle frozen
/assign

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Oct 12, 2020
@mhumpula
Copy link

For anyone with custom CAs, trying to use this library: I was able to get away with this snippet

config.load_kube_config() # load everything from .kube/config
cc = client.Configuration.get_default_copy()
cc.ssl_ca_cert = os.environ.get('REQUESTS_CA_BUNDLE') # use proper CA list
client.Configuration.set_default(cc)

But still, this is kinda sad. The .kube/config already contains valid cluster CAs, so why does it work in golang but not in python? 🤔

@Stef16Robbe
Copy link

This is still a necessary workaround sadly... For anyone else requiring it, do note that cc.ssl_ca_cert needs a filename, not the contents ^^, I misunderstood this at first.

@tj-smith47
Copy link

Perhaps this is a pyright issue, but it is reading from the class definition of Configuration. Not sure if anyone else has experienced this warning, but a solution to the problem was posed by @palnabarun so posting it here despite being arguably a different topic. I would think this could be an instantiation kwarg and then the if ssl_ca_cert: approach could be used as it is with api_key and api_key_prefix.
Screenshot 2024-09-09 at 14 33 00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

8 participants