ansible-collections / servicenow.itsm

Ansible Collection for ServiceNow ITSM
GNU General Public License v3.0
89 stars 78 forks source link

Add now inventory vars column option #358

Closed miyuk closed 2 months ago

miyuk commented 4 months ago
SUMMARY

Add vars_column option for now inventory plugin for defining host_vars.

I want to realize the feature like a NetBox Inventory plugin config_context.

ISSUE TYPE
COMPONENT NAME

servicenow.itsm.now

ADDITIONAL INFORMATION

As a promise, defined u_host_vars string column in ServiceNow. (maybe this column is as customed one) u_host_vars column contains host_vars data formatted JSON string.

inventory plugin will get this column, and parse JSON and flatten variables.

now.yml
---
plugin: servicenow.itsm.now
vars_column: u_host_vars
~~~
api
{
  "name": "XXXX",
  "u_host_vars": "{\"ansible_network_os\": \"cisco.ios\", \"ansible_connection\": \"network_cli\"}"
}
rendered inventory
all:
  hosts:
    XXXX:
      ansible_network_os: cisco.ios
      ansible_connection: network_cli
mhjacks commented 3 months ago

We've discussed this a fair bit internally, and we are inclined not to do this. The main reason for this is that SNOW already supports the idea of adding multiple configuration columns, and we support using multiple configuration columns to the inventory this way.

Meanwhile, embedding JSON in a column this way would be fragile. What should happen, for example, if invalid JSON is stored in the column? Should that host be excluded from inventory? Should inventory fail overall? Further, the JSON support would could provide would be limited to a single level of nesting.

We'll leave this open for a bit if you have more feedback, especially if we have misunderstood the intent here or you think we have missed something.

miyuk commented 3 months ago

@mhjacks thanks for your honest discussion. I understood the idea has risk.

SNOW already supports the idea of adding multiple configuration columns,

What is this? compose directive?

the JSON support would could provide would be limited to a single level of nesting.

I don't think that, JSON can have nested variable when user defined.

mhjacks commented 2 months ago

SNOW already supports the idea of adding multiple configuration columns,

What is this? compose directive?

I was thinking primarily of adding custom columns to your instance, and then using the columns directive to include those columns in the inventory. Compose could also be used to combine columns if you need to do that.

the JSON support would could provide would be limited to a single level of nesting.

I don't think that, JSON can have nested variable when user defined.

JSON can indeed, but the current inventory implementation will not nest such data structures arbitrarily as inventory variables. Parsing each potential field as JSON and recursively building data structures could get very expensive in terms of compute time; we want to be very conscientious of the time it takes to construct large and complex inventories.

What do you think?