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

Integer key inside dictionary imported as string when inventory is sourced from a project #991

Closed
bcbrockway opened this issue Jan 16, 2018 · 9 comments

Comments

@bcbrockway
Copy link

ISSUE TYPE
  • Bug Report
COMPONENT NAME
  • API
SUMMARY

When you have a dictionary defined in your project which has keys that are integers, AWX imports them as strings when using a project as the inventory source.

ENVIRONMENT
  • AWX version: 1.0.2-350-gae06cff
  • AWX install method: docker on linux
  • Ansible version: 2.4.2.0
  • Operating System: CentOS 7.4.1708
  • Web Browser: Chrome 63.0.3239.132 (Official Build) (64-bit)
STEPS TO REPRODUCE
  1. Create a project in source control containing a dictionary like the following in host_vars/group_vars
    loader1_var:
      subvar1:
        1:
          - foo
        2:
          - bar
  2. Add the project in AWX and sync
  3. Create an inventory that uses this project as a source and sync
EXPECTED RESULTS

Dictionary is imported as laid out in the source file with the keys as integers:

loader1_var:
  subvar1:
    1:
      - foo
    2:
      - bar
ACTUAL RESULTS

Dictionary is imported with the keys converted to strings:

loader1_var:
  subvar1:
    '1':
      - foo
    '2':
      - bar
@AlanCoding
Copy link
Member

For any questions like these, we have to be extremely careful that the behavior of AWX is congruent with Ansible core, itself. If Ansible core does something wrong, we may actually prefer to pass-through that wrong behavior and ask that Ansible core fixes the issue. I believe this is one of those cases.

Here is a test set inside of my inventory examples.

AlanCoding/Ansible-inventory-file-examples@c2ef46d

Inventory content can be tested using the ansible-inventory tool. This represents the data in the same way that it is used inside of playbooks (which is ultimately what we want). I get the following output for the --list options with this example.

{
    "_meta": {
        "hostvars": {
            "foobar": {
                "loader1_var": {
                    "subvar1": {
                        "1": [
                            "foo"
                        ], 
                        "2": [
                            "bar"
                        ]
                    }
                }
            }
        }
    }, 
    "all": {
        "children": [
            "ungrouped"
        ]
    }, 
    "ungrouped": {
        "hosts": [
            "foobar"
        ]
    }
}

Feel free to raise an issue inside of https://github.com/ansible/ansible/issues if you want this fixed, and they will triage it (although it may result in it being closed, depending on their expectation).

@cchurch
Copy link
Contributor

cchurch commented Feb 13, 2018

If an inventory containing these host/group vars is used directly by ansible, it preserves the type of the dictionary keys:

$ ansible host -m debug -a "msg='{{loader1_var.subvar1.keys()[0]|type_debug}}'"
host | SUCCESS => {
    "msg": "int"
}

However, since we use ansible-inventory --list to output JSON (which requires that dictionary keys are strings) and then parse that JSON to import into AWX, those keys are converted to strings in the process. The only way it would work within AWX would be to use ansible-inventory --list --yaml and update the inventory import to also parse YAML -- not something we've planning on adding so far.

There's always the |int filter if you need to ensure an integer is used.

@AlanCoding
Copy link
Member

The example that @cchurch somewhat violates the principle of ansible-inventory - that it yields the same behavior as ansible-playbook. That's the point, and the entire reason we use ansible-inventory.

However, I see the problem with compatibility as we cross over multiple formats. The python JSON implementation (and possibly the standard itself) does not support this type structure.

The following example really shows the crux of the issue.

ansible-inventory -i issues/AWX991/inv --list --yaml
all:
  children:
    ungrouped:
      hosts:
        foobar:
          loader1_var:
            subvar1:
              1:
              - foo
              2:
              - bar

I have tested this in the sense that I pipe that output to a file, and then use the python yaml library to parse it and obtain...

>>> [type(k) for k in adict['all']['children']['ungrouped']['hosts']['foobar']['loader1_var']['subvar1'].keys()]
[<type 'int'>, <type 'int'>]

Thus this issue may actually be resolvable by having AWX always use the yaml option for inventory imports.

@steaksauce-
Copy link

steaksauce- commented Feb 22, 2018

@AlanCoding
Copy link
Member

No, that's a special script for importing inventory from another server.

https://github.com/ansible/awx/blob/1ccdb305e3497f1958fa0229bf82edc262686836/awx/main/management/commands/inventory_import.py

This would require finding the ansible-inventory use there, change it to output YAML format, and read it as YAML.

Not sure if this breaks anything else unexpected, might be good to ping the Ansible core mailing list about that. They are more savvy about JSON vs. YAML than I am.

@cchurch
Copy link
Contributor

cchurch commented Feb 22, 2018

@steaksauce- The changes needed to support your use case will need to be made in https://github.com/ansible/awx/blob/devel/awx/main/management/commands/inventory_import.py#L165, like @AlanCoding said.

The file you mentioned will need to be modified to support pulling inventory from another AWX/Tower instance while preserving integer dictionary keys, and we'll also need to update the /api/v2/inventories/N/script/ endpoint to be able to output YAML.

@steaksauce-
Copy link

D'oh! Thanks for letting me know!

@AlanCoding
Copy link
Member

The use of YAML looks like it may have some overlap with other requests I'm dealing with, so I probably need to carry out some of this basic research myself.

@AlanCoding
Copy link
Member

From what I was finding, there's no way this will work without some other substantial concessions made by Ansible core, and there's nothing on the horizon for this.

The YAML inventory format would give the correct types, but the structure would destroy the performance of our imports to make many large inventories unusable, even assuming the importing algorithm is updated correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants