kubernetes.core.kustomize lookup: support http proxy

bh-tt commented 1 month ago

SUMMARY

Kubernetes.core.kustomize lookup does not allow setting a HTTP proxy. This means we cannot use it to run kustomize with remote (e.g. github) bases/resources, since our ansible controller is on a system without direct access to the internet. I'd like to add a proxy (or maybe http_proxy) setting to the plugin to allow setting the proxy for use with this lookup.

ISSUE TYPE

Feature Idea

COMPONENT NAME

kubernetes.core.kustomize

ADDITIONAL INFORMATION

It allows ansible controllers behind a http proxy to access remote kustomize bases/resources. I've looked at several alternatives:

using the pipe plugin, as that allows setting the HTTPS_PROXY variable, but that is not really usable as the PWD of the lookup becomes the playbook file, whereas most of our lookups will happen in roles
simply using ansible.builtin.command, but again that results in PWD issues

adding a reverse proxy (say github.example.com) to our infrastructure and directing the calls there, but that seems needlessly complicated

- name: kustomize
ansible.builtin.set_fact:
  resources: "{{ lookup('kubernetes.core.kustomize', dir='../files/', proxy='http://proxy') }}"

yurnov commented 3 weeks ago

Hi @bh-tt,

Lookup plugin kubernetes.core.kustomize (please take a look to the code) is a ansible-wrapper on the kubectl or kustomize binary, so, as proxy is not supported by the kubectl is can't be added (at least to be easily added) to kubernetes.core.kustomize

bh-tt commented 3 weeks ago

Kubectl is a Go binary. By default, Go accepts the HTTP_PROXY, HTTPS_PROXY and NO_PROXY variable for the standard HTTP client. In my tests it has worked by simply adding the HTTP_PROXY or 'HTTPS_PROXY` environment variable.

For now I've managed to make this work using the pipe plugin and the 'playbook_dir' and 'role_path` ansible variables, like so:

- name: deploy
  kubernetes.core.k8s:
    definition: "{{ lookup('pipe', 'HTTPS_PROXY=http://proxy:3128 kubectl kustomize ' + role_path + '/files') | from_yaml_all }}"

If you want to implement the proxy setting that would be nice, but it is no longer an issue for us, so closing is fine as well.

yurnov commented 3 weeks ago

I see

Have you tried to use:

    - name: kustomize
      environment:
        HTTPS_PROXY: "http://proxy:3128"
      ansible.builtin.set_fact:
        resources: "{{ lookup('kubernetes.core.kustomize', dir='../files/') }}"

on the first look, it should work if kubectl supports HTTP_PROXY and HTTPS_PROXY environment variables.

bh-tt commented 3 weeks ago

Yes I have, but the plugin does not seem to pass the environment on to the command by default, since the environment is meant to be set on the remote host, whereas a lookup plugin runs on the ansible controller.

See https://github.com/ansible-collections/kubernetes.core/blob/c8a9326306e65c0edf945fb3e99a67937cbe9375/plugins/lookup/kustomize.py#L95

yurnov commented 3 weeks ago

Thanks @bh-tt for the clarification, I know how to improve it and will create PR.

yurnov commented 3 weeks ago

Hi @bh-tt,

could you please check this one, I don't have env to test it on my side right now.

BR/Yurii

bh-tt commented 3 weeks ago

Same result, I'm afraid this method does not work, as the environment keyword only sets the remote environment. The docs explicitly say this has no impact on the execution of filters or lookups: 'The environment: keyword does not affect Ansible itself, Ansible configuration settings, the environment for other users, or the execution of other plugins like lookups and filters.'

You could add an 'environment' variable to the task itself?

yurnov commented 3 weeks ago

I see.

But with PR #786 env HTTPS_PROXY="http://proxy:3128" ansible-playbook playbook.yaml should work with something like:

- hosts: localhost
  tasks:
    - name: kustomize
      ansible.builtin.set_fact:
        resources: "{{ lookup('kubernetes.core.kustomize', dir='../files/', use_local_env='True') }}"

Parameter use_local_env enable environment variable for kubectl or kustomize, please check:

        if use_local_env:
            environ = dict(os.environ)
        else:
            environ = None

        (ret, out, err) = run_command(command, environ=environ)

and:

def run_command(command, environ=None):
    cmd = subprocess.Popen(
        command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=environ
    )
    stdout, stderr = cmd.communicate()
    return cmd.returncode, stdout, stderr

So, the local variable should be considered by the command (in this case by the kubectl or kustomize.

Yep, I can add variable for the tasks, not sure what is better. Will do to see how it work

bh-tt commented 3 weeks ago

I can confirm that adding the parameter in the actual ansible-playbook call works. You understand we would prefer not to set this environment value for every ansible call, especially since some tasks will need it while others may crash as they only communicate with local (e.g. not needing the http proxy) endpoints. Additionally, that would require a very good memory to remember which playbooks need it and which do not.

yurnov commented 3 weeks ago

Hi @bh-tt,

now it should work in both ways, as the env HTTPS_PROXY="http://proxy:3128" ansible-playbook playbook.yaml and:

- hosts: localhost
  tasks:
    - name: kustomize
      ansible.builtin.set_fact:
        resources: "{{ lookup('kubernetes.core.kustomize', dir='../files/', enviroment='HTTPS_PROXY=http://proxy:3128') }}"

Same PR with this state

bh-tt commented 3 weeks ago

I think you're missing the definition of enviroment in the run function, I'm getting a not defined error. Also, there is a typo with the envrion variable. If I fix those 2 issues your example works.

Is environment a keyword in ansible, that you do not use it as parameter name for the plugin?

bh-tt commented 3 weeks ago

Then I'm satisfied, though I still think the typo'ed enviroment paramater name should be changed. Perhaps env or environ?

yurnov commented 3 weeks ago

Even 'environment` seems work when I fix one another typo:

TASK [ansible.builtin.set_fact] ***************************************************************************************************************************
task path: /app/ansible/playbooks/test.yaml:6
ok: [localhost] => {
    "ansible_facts": {
        "definition": "apiVersion: v1\ndata:\n  application.properties: |\n    FOO=Bar\nkind: ConfigMap\nmetadata:\n  name: example-configmap-1-g4hk9g2ff8\n"
    },
    "changed": false
}

PLAY RECAP ************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

eric-eo-cm-gr-7fbf495c9c-xz9cl:/app/ansible/playbooks$ cat test.yaml
---
- hosts: localhost
  gather_facts: false

  tasks:
    - ansible.builtin.set_fact:
        definition: "{{ lookup('kustomize', binary_path='/usr/bin/kubectl', enviroment='VAR=value') }}"

gravesm commented 3 weeks ago

Why not just set the environment variable before you run ansible-playbook?

bh-tt commented 3 weeks ago

Because we have about 200 playbooks and remembering which playbooks need which environment variable is not really something I'd like to do. Setting a default value is very hard for http proxy variables, since support varies a lot. Some applications support some form of wildcards, some do not, etc.

ansible-collections / kubernetes.core