add no_proxy support to k8s*

[itaru2622]

SUMMARY

close #271

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

plugins/module_utils/args_common.py
plugins/modules/k8s*

ADDITIONAL INFORMATION

It requires latest kubernetes library(>=19.15.0) to use this feature.

pip install kubernetes>=19.15.0
then, use following snippet yaml:

  - k8s:
      state: present
      src: "deployment.yaml"
      proxy:      "http://proxy.yourdomain.com:8080/"
      no_proxy:   "localhost,.yourdomain.com,127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192,168.0.0/16"

or use environment variable K8S_AUTH_NO_PROXY as well as K8S_AUTH_PROXY.

[itaru2622]

@abikouo @gravesm Could you review this PR, please.

[abikouo]

Thanks @itaru2622 for taking the time to submit this pull request. Some comments on this:

• file *.rst should not be part of this pull request
• Please add a comment to specify the required kubernetes version for this feature
• Module should proper failed when the kubernetes version is not as expected
• Are you able to add some tests (units or integration) for this ?

[itaru2622]

@abikouo thank you for your review.

1. file *.rst should not be part of this pull request

OK, I will revert them.

2. Please add a comment to specify the required kubernetes version for this feature

How should I add it?
Is it enough for you to add comment in plugins/doc_fragments/k8s_auth_options.py or you prefer others?

3. Module should proper failed when the kubernetes version is not as expected

Yes, you are right. the ansible statement will be failed when kubernetes<19.15.0

4. Are you able to add some tests (units or integration) for this ?

I will try to implement test code. but unfortunately, I cannot imagine the code itself and also how to run it yet.

about test code:

I think the most closer test code is tests/unit/module_utils/test_discoverer.py,
but it has just below A but not B, comparing my test case needs both of A and B. how I can implement about B?
A) api instance created by ansible_collections.kubernetes.core.plugins.module_utils.common.get_api_client
B) and it (api instance) is invoked from ansible statements ( such as yaml)

actually, I implemented test code for no_proxy feature in kubernetes library as below (it has A but not B):
https://github.com/kubernetes-client/python/blob/e0e60faa8f79c469641de36a9d65d6c851b74052/kubernetes/test/test_api_client.py#L28

if test code needs to run on molecule, it takes time for my implementing since I am newbie on it.

about running test code:

when I tried running official test code by make test-unit, it raised below error.
where should I put my git cloned folder ?

ERROR: The current working directory must be at or below:
 - an Ansible collection: {...}/ansible_collections/{namespace}/{collection}/

[abikouo]

Thanks a lot @itaru2622 for your quick feedback

How should I add it? Is it enough for you to add comment in plugins/doc_fragments/k8s_auth_options.py or you prefer others?

a comment in the plugins/doc_fragments... is enough

I will try to implement test code. but unfortunately, I cannot imagine the code itself and also how to run it yet.

we are running molecule testing on a kind cluster, we can imagine that we update the cluster version to support this functionality, however if it is too complex to create an infrastructure to test it you can basically do it using unit tests and mock mechanism, if too complex we can keep this without unit tests

[itaru2622]

@abikouo Thank you for your reply.

your comments applied.

1 ) files *.rst => reverted
2,3 ) kubernetes version and 'version_added' => added to plugins/doc_fragments/k8s_auth_options.py.
I also added little modification, first sentence becomes similar to 'manual of curl', and fixed typo, add example value.
4) test code => not yet.

I guess it takes time for me to implement and run test code on molecule. so, please review and merge without test code, if possible.
I may submit another PR when the test code is successfully implemented and executed in future...

[abikouo]

@itaru2622 I just forgot to mention that you will also need a change log to report this change.

[itaru2622]

@abikouo I add changelogs/fragments. please review this PR.

[itaru2622]

@abikouo your comment applied. please review it. at current, some CI tests were failed but I could imagine what should I do.

was it caused by 'version_added:2.2.2' or do I need to fix?
I cannot imagine what is wrong because of less message in failure reason. Is there any way to test them in local?

[gravesm]

@itaru2622 Since this is a new feature and not a bugfix, the version_added needs to be 2.3.0.

[itaru2622]

@gravesm Thank you for your comment. I updated version_added to 2.3.0.

[gravesm]

recheck

[itaru2622]

@gravesm thank you for your help.

[itaru2622]

@gravesm @abikouo finally first CI test passed, but second CI does not start. what should I do to move this PR forward ? could you check this, please.

[gravesm]

@itaru2622 The second CI check will run once the PR is approved. Could you add a failure when the kubernetes version is not high enough? You can add it after the for block here if the no_proxy param has been set:

kubernetes.core/plugins/module_utils/common.py

Line 181 in 91b80b1

auth[true_name] = env_value

See

kubernetes.core/plugins/module_utils/common.py

Line 198 in 91b80b1

if LooseVersion(kubernetes.__version__) < LooseVersion("17.17"):

for how to do the failure in this function.

[itaru2622]

@gravesm thank you. I will apply them

[itaru2622]

@gravesm I applied your comment, please check it.

[gravesm]

Could you move this down to line 221, after the if auth_set("host") block? It's currently only checking if NO_PROXY has been set through the environment variable. It needs to check against the auth dict after we're done creating it. So something like:

if auth_set("no_proxy") and LooseVersion...

[itaru2622]

@gravesm I applied your comment about kubernetes version check, and rebased to latest main branch.

[gravesm]

Thanks for your work on this!

[ansible-zuul]

LGTM!

Change addressed

[gravesm]

recheck

[gravesm]

recheck

[gravesm]

recheck

[gravesm]

recheck