-
Notifications
You must be signed in to change notification settings - Fork 3.6k
AAP-68210 Use FQCN throughout collection, and reduce dynamic templating in favor of release-based #16347
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
base: devel
Are you sure you want to change the base?
AAP-68210 Use FQCN throughout collection, and reduce dynamic templating in favor of release-based #16347
Changes from all commits
2fc1eca
c5396a9
92cc32d
7646c16
314251e
b235b84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,12 +48,12 @@ | |
|
|
||
| class ControllerModule(AnsibleModule): | ||
| url = None | ||
| AUTH_ARGSPEC = dict( | ||
|
Check warning on line 51 in awx_collection/plugins/module_utils/controller_api.py
|
||
| controller_host=dict( | ||
| required=False, | ||
| aliases=['tower_host', 'aap_hostname'], | ||
| fallback=(env_fallback, ['CONTROLLER_HOST', 'TOWER_HOST', 'AAP_HOSTNAME'])), | ||
| controller_username=dict( | ||
|
Check warning on line 56 in awx_collection/plugins/module_utils/controller_api.py
|
||
| required=False, | ||
| aliases=['tower_username', 'aap_username'], | ||
| fallback=(env_fallback, ['CONTROLLER_USERNAME', 'TOWER_USERNAME', 'AAP_USERNAME'])), | ||
|
|
@@ -92,12 +92,14 @@ | |
| 'password': 'controller_password', | ||
| 'verify_ssl': 'validate_certs', | ||
| 'request_timeout': 'request_timeout', | ||
| 'api_prefix': 'api_prefix', | ||
| } | ||
| host = '127.0.0.1' | ||
| username = None | ||
| password = None | ||
| verify_ssl = True | ||
| request_timeout = 10 | ||
| api_prefix = None | ||
| authenticated = False | ||
| config_name = 'tower_cli.cfg' | ||
| version_checked = False | ||
|
|
@@ -178,7 +180,7 @@ | |
|
|
||
| return url | ||
|
|
||
| def load_config_files(self): | ||
|
Check failure on line 183 in awx_collection/plugins/module_utils/controller_api.py
|
||
| # Load configs like TowerCLI would have from least import to most | ||
| config_files = ['/etc/tower/tower_cli.cfg', join(expanduser("~"), ".{0}".format(self.config_name))] | ||
| local_dir = getcwd() | ||
|
|
@@ -308,13 +310,22 @@ | |
| # TODO: Move the collection version check into controller_module.py | ||
| # This gets set by the make process so whatever is in here is irrelevant | ||
| _COLLECTION_VERSION = "0.0.1-devel" | ||
| _COLLECTION_TYPE = "awx" | ||
| # This maps the collections type (awx/tower) to the values returned by the API | ||
| # The FQCN is the canonical identifier for this collection. | ||
| # On upstream (awx) this is "awx.awx"; on downstream (tower) the | ||
| # replace_fqcn.sh script converts it to "ansible.controller". | ||
| # _COLLECTION_TYPE is derived from the FQCN automatically. | ||
| _COLLECTION_FQCN = "awx.awx" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This syntax change allows this line to get swept up in the general find+replace. It's dumb logic that takes all "awx.awx" and turns it into "ansible.controller". This still leaves a special case for the version when packaging a collection. So the collection FQCN is bulk-replaced by the script and it must go in a commit. Then the version is inserted in before building, every time. |
||
| # This maps the collections type (awx/controller) to the values returned by the API | ||
| # Those values can be found in awx/api/generics.py line 204 | ||
| collection_to_version = { | ||
| 'awx': 'AWX', | ||
| 'controller': 'Red Hat Ansible Automation Platform', | ||
| } | ||
|
|
||
| @property | ||
| def _COLLECTION_TYPE(self): | ||
| return self._COLLECTION_FQCN.split('.')[1] | ||
|
|
||
| session = None | ||
| IDENTITY_FIELDS = {'users': 'username', 'workflow_job_template_nodes': 'identifier', 'instances': 'hostname'} | ||
| ENCRYPTED_STRING = "$encrypted$" | ||
|
|
@@ -488,7 +499,7 @@ | |
| def resolve_name_to_id(self, endpoint, name_or_id): | ||
| return self.get_exactly_one(endpoint, name_or_id)['id'] | ||
|
|
||
| def make_request(self, method, endpoint, *args, **kwargs): | ||
|
Check failure on line 502 in awx_collection/plugins/module_utils/controller_api.py
|
||
| # In case someone is calling us directly; make sure we were given a method, let's not just assume a GET | ||
| if not method: | ||
| raise Exception("The HTTP method must be defined") | ||
|
|
@@ -630,6 +641,8 @@ | |
|
|
||
| def api_path(self, app_key=None): | ||
|
|
||
| if self.api_prefix: | ||
| return self.api_prefix | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. api_prefix bypasses URL path normalization in api_pathMedium Severity When
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather not mess with it and let the user figure it out via the value of |
||
| default_api_path = "/api/" | ||
|
Comment on lines
+644
to
646
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check how api_prefix is used in tests and whether normalization is expected
rg -n "api_prefix" awx_collection/ -A3 -B3Repository: ansible/awx Length of output: 2622 🏁 Script executed: #!/bin/bash
# Get the complete api_path method to see the trailing slash normalization
sed -n '642,680p' awx_collection/plugins/module_utils/controller_api.py | cat -nRepository: ansible/awx Length of output: 1857 🏁 Script executed: #!/bin/bash
# Also search for how api_path and api_prefix are used in the codebase
rg -n "\.api_path\(|api_prefix" awx_collection/ --type py -A2 -B1 | head -100Repository: ansible/awx Length of output: 2038 🏁 Script executed: #!/bin/bash
# Search for test cases and usage of api_prefix to understand expected format
rg -n "api_prefix\s*=" awx_collection/ --type py -A2 -B2 | head -60Repository: ansible/awx Length of output: 473 🏁 Script executed: #!/bin/bash
# Check if there are any tests that demonstrate the issue or expected behavior
find awx_collection -name "*test*.py" -type f -exec grep -l "api_prefix" {} \;Repository: ansible/awx Length of output: 37 Add trailing slash normalization to When Either normalize 🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't wanna There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if self._COLLECTION_TYPE != "awx" or app_key is not None: | ||
| if app_key is None: | ||
|
|
@@ -717,7 +730,7 @@ | |
| return self.json_output | ||
| else: | ||
| if 'json' in response and '__all__' in response['json']: | ||
| self.fail_json(msg="Unable to delete {0} {1}: {2}".format(item_type, item_name, response['json']['__all__'][0])) | ||
|
Check failure on line 733 in awx_collection/plugins/module_utils/controller_api.py
|
||
| elif 'json' in response: | ||
| # This is from a project delete (if there is an active job against it) | ||
| if 'error' in response['json']: | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add language specifier to fenced code block.
The code block on line 49 is missing a language specifier. For consistency with other code blocks in this file (which use no language but contain shell/ini), consider adding
yamlsince this example shows YAML format.📝 Suggested fix
📝 Committable suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is templated from the prior README.md.j2
This is not in the scope of the PR. All this does it materialize content already in the template to the duplicated file. This is the content already being used when building the collection.
I didn't start this to fix this problem, and it's noise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlanCoding, understood — this is pre-existing content from the template that the PR simply materializes, and fixing the missing language specifier is out of scope here. I'll note this for future reference.