Detect possible inventory plugin files #4171
Conversation
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
awx/main/utils/ansible.py
Outdated
| if not valid_inventory_re.match(line): | ||
| return None | ||
| else: | ||
| return None |
There was a problem hiding this comment.
This would fail to include files such as
Because the plugin: keyword is not in the first 10 lines of the file.
the regex IIRC is fairly specifically targeted toward ini-type files (including those that don't actually have the .ini extension explicitly). Moving forward, the only thing that makes sense to me is to break this logic up for different extension types (or lack thereof). I would still prefer to avoid doing a json.loads on all YAML files in source due to performance concerns. Maybe larger YAML files should just get included by default. Maybe we specifically exclude only YAML files that are under 10 lines and don't have the "plugin:" syntax.
There was a problem hiding this comment.
I'd not noticed that there was such a large inventory plugin file there! It seems weird to me that the plugin key isn't at the beginning, but sure, there's no reason why it can't be at the end. My idea was to make just a tiny nudge to help improve the situation without making a large adjustment, and not expecting to catch everything. But I am ok with making this a deeper change. I'll update the PR.
Regardless, this isn't the best solution, as the UI still limits the total results to 50 (and I have a project with >1000 inventory files!) I think the best way to handle it is the same way we pick projects -- a modal box that is filterable, and additionally, able to drill down into directories. But I don't think I can spend the time to implement that.
There was a problem hiding this comment.
It seems weird to me that the plugin key isn't at the beginning, but sure, there's no reason why it can't be at the end.
The standard python YAML library sorts keys alphabetically, so that's why in this case.
@jlmitch5 might have some opinions on the UI comments. I think the API itself has some limit. I don't think playbook / inventory files I don't think have ever had filtering applied to them, like with the related items you mention.
|
Thanks for the contribution! I hope we can get this merged with some modifications. To get it out of the way now, next time you push, it would help to get the https://github.com/ansible/awx/blob/d8c4f118c23fde9f59f38334974381264d22f120/DCO_1_1.md |
|
I've been caught up in my primary work, but I'll |
|
@AlanCoding @philipsd6 either of you have any updates on this PR? |
|
This is definitely on my agenda at some point. There are just too many other things ahead of it. I can't get to it soon. |
philipsd6
force-pushed
the
feature/detect_inventory_plugin_files
branch
3 times, most recently
from
e8a4b2d
to
67aa4f8
Compare
|
I just rebased add added my sign-off, but didn't change the logic of the original PR for now.
I agree @AlanCoding, and I'm thinking a potential way to keep the existing login, if we need to potentially read the entire file, we could try using mmap to relieve the performance concerns of reading more than 10 lines? |
jessicamack
force-pushed
the
feature/detect_inventory_plugin_files
branch
from
67aa4f8
to
a1c1a0b
Compare
|
it looks like there are a couple of failing tests that will need to be resolved for this: |
philipsd6
force-pushed
the
feature/detect_inventory_plugin_files
branch
from
a1c1a0b
to
371b10e
Compare
I'm not sure when or how this ever could have worked honestly. The logic is based around opening up access to json/yaml inventory files, and checks for a couple of inventory related keywords to confirm validity, but the regex also passes anything, and since we won't/can't actually do a yaml/json load on the files to really confirm for sure, a valid yaml file that is NOT an inventory is always going to match the regex, even the old one. So the best I think we can do for now is fall back to the original |
philipsd6
force-pushed
the
feature/detect_inventory_plugin_files
branch
from
a23f2fa
to
d0cb117
Compare
Signed-off-by: Philip Douglass <philip@philipdouglass.com>
Signed-off-by: Philip Douglass <philip@philipdouglass.com>
Signed-off-by: Philip Douglass <philip@philipdouglass.com>
Signed-off-by: Philip Douglass <philip@philipdouglass.com>
dmzoneill
force-pushed
the
feature/detect_inventory_plugin_files
branch
from
d0cb117
to
12e49a2
Compare
SUMMARY
AWX ignores inventory plugin files when choosing inventory files from a Project source. This PR lets yaml and json files slip through after a very basic check for a couple of keywords that would be in a yaml or json inventory file or an inventory plugin file.
related #3491
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION