ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
93 stars 19 forks source link

Module-specific variable namespacing #81

Open dagwieers opened 6 years ago

dagwieers commented 6 years ago

Proposal: Module-specific variable namespacing

Author: Dag Wieers (@dagwieers)

Date: 2017-10-18

Motivation

Currently there's no standardized way to pass credential information and session tokens from/to modules. Especially REST-based modules currently require the user to add the credential information (but also proxy and other information) for each individual task. Each module takes care of logging on and performing its intended action(s), but a single session could be used for all these tasks, not requiring a log on for each task.

Some REST-based back-ends also have a limited number of simultaneous sessions they can handle, which makes it very important to reuse existing sessions, rather than to rely on a module to always properly log out on exit. Since Ansible can be interrupted at any time, such session would go stale and takes a long timeout before it can be reused. (So we need a means of returning session information and caching this information would be crucial in the new design)

Problems

What problems exist that this proposal will solve?

Solution proposal

Anything else?

The network team introduced something called provider which is specific to a selected set of CLI network modules. This proposal is a more generic solution, not necessarily related to connection related information only, but anything that makes sense within the technology domain (e.g. ACI tenant name, VMware resource_pool, etc...).

Alternatives

An alternative way of doing the same is not having the module subscribe to a specific bucket, but define the bucket as part of the task. This would make it more explicit that some information comes from the inventory. (And would be closer to YAML anchors in that regard) This would also allow to make the bucket-name variable, or iterate over different buckets. The drawback would be that we are adding new directives to tasks, blocks, plays, ...

  - name: Ensure our tenant exists
    aci_tenant:
      state: present
    bucket: aci

  - name: Ensure VRF exists
    aci_vrf:
      vrf_name: vrf_ansible
      state: present
    bucket: aci
willthames commented 6 years ago

This would be incredibly handy for AWS modules too.

christ2016 commented 6 years ago

Still learning so feel free to disregard:

This sounds great; I am using the provider method extensively already to manage my network equipment and it seems very flexible and functional.

For me it would be very important that it were possible to override a subset of the values in a bucket, as it could be problematic if a bucket became an atomic entity; there are often reasons to override.

privateip commented 6 years ago

It seems to me it would make more sense to elevate API based implementations to being true connection plugins instead of passing connection values via playbook arguments. With the persistence framework that is being put in place now, we should have the ability to do that. We would then be able to do things like have connection: aci for instance, and have the module use the underlying REST connection. By implementing over the persist connection plugin framework, the session cookies can be held by the connection and persisted from task to task.

dagwieers commented 6 years ago

@privateip The proposed functionality solves problems beyond persistent connections (i.e. not in scope of the existing persistence framework). Having to implement separate connection plugins for each of the tens of REST-based APIs seems overly complex compared to the well-known module concept.

Wrt. the persistence framework I also have concerns to influencing the connection pugin (e.g. adding proxy support, proxy credentials, delegation, become, ...) but since I didn't find any good documentation of the persistence framework (only the release announcement) I don't know how it was designed for this or how it is expected to work for REST-based APIs. (i.e. is this even intended for UCS, IMC or AWS, or network-centric ?)

dagwieers commented 6 years ago

@privateip Can you please give any information about the persist connection plugin ? Because at this rate we won't have a solution by Ansible v2.5.

itdependsnetworks commented 6 years ago

I know this is a bit late to the game, but I think this allows module writers to generically implement solutions that does not require bespoke plugin changes, as @dagwieers put it:

Having to implement separate connection plugins for each of the tens of REST-based APIs seems overly complex compared to the well-known module concept.

bcoca commented 6 years ago

You end up implementing the connection handling inside the modules, this does not make it less work, but more IMHO.

I had a proposal of 'auth plugins' that overlaps with this, but similar concept. Allow to define auth info 1 time and reuse across other plugins.

dagwieers commented 6 years ago

@bcoca It's a library, and yes, at the moment that is happening for a lot of modules already (HTTP, REST, ...)

bcoca commented 6 years ago

yes, we've been good on making these things common in module_utils, but there is still a lot of duplication across other plugin types that can/might want to reuse these.

itdependsnetworks commented 6 years ago

Anything that requires you to keep an object, e.g. sql alchemy would be able to use this generic method. The distribution of plugins is not easy, and this solves a problem.

agaffney commented 6 years ago

I've put together https://github.com/ansible/ansible/pull/22648, which seems to implement this proposal's goals (if not in the proposed form). Can anyone think of a use case that is not covered by my PR?

caphrim007 commented 6 years ago

I too would use something like this (particularly the caching of the token upon the first module's return). though I am impartial where it is implemented (buckets as this proposal suggests or connection plugins as other suggest). Agreed that it would be valuable beyond network modules though

dagwieers commented 6 years ago

@agaffney As I discussed in ansible/ansible#22648 it's not practical to have the same parameters set for all ACI modules. Unless you copy them for an entry for each (50+). Which is not covered by your PR.

agaffney commented 6 years ago

I already showed how to handle that use case in my PR. It may be a bit cumbersome, but it works, and it doesn't require potentially major architectural changes (or at least duplicated/bifurcated module metadata) like the "bucket" idea.

dagwieers commented 6 years ago

Let's continue this discussion in your PR.

orthanc commented 6 years ago

This is definatly something I would like, I was looking for exactly this mechanism the other day trying to work out if there was a better way to deal with AWS credentials.

There are a couple of points that don't seem to be explicitly covered by the proposal that I think would need explicit definitions, but these are just details.

Contention for Naming "Buckets" There needs to be a defined convention for naming the "buckets" probably somehow related to the module namespacing. WIthout this there's likely to be early sprawl of naming that needs to be cleaned up later.

I'd suggest naming the buckets after the module namespacing. E.g. the aws modules would generally use a bucket called cloud.amazon or similar.

Support for set_facts and other fact setting mechanisms I can think of at least one case where it would be useful to be able to set "bucket" facts using a set_facts or include_vars type tasks. It would be good to ensure this is possible.

The example I have is two factor auth in AWS. Currently I use the sts_token module to get AWS session tokens as I require MFA. These session tokens are then used by all subsequent AWS tokens. It's better to keep this set_facts a bit seperate from the module since it involves user interaction.

But it would still be great to be able to do a set_facts to set the "bucket" facts then not have to pass to all subsequent AWS tasks.

bcoca commented 3 years ago

@orthanc see module_defaults