redhat-cop / controller_configuration

A collection of roles to manage Ansible Controller and previously Ansible Tower
https://galaxy.ansible.com/infra/controller_configuration
GNU General Public License v3.0
259 stars 134 forks source link

Change default value for tags in workflows #824

Open ephracis opened 2 months ago

ephracis commented 2 months ago

Summary

When enforcing defaults (ie controller_configuration_workflows_enforce_defaults is set to true), the value for skip_tags and job_tags are set to empty strings. This causes these values to override whatever value is set on the individual nodes. The correct default value should instead be null as that value will not take precedence over the values on the nodes in the workflow.

Issue Type

Ansible, Collection, Controller details

ansible --version
ansible [core 2.15.5]
  config file = None
  configured module search path = ['/Users/christoffer/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/homebrew/lib/python3.11/site-packages/ansible
  ansible collection location = /Users/christoffer/.ansible/collections:/usr/share/ansible/collections
  executable location = /opt/homebrew/bin/ansible
  python version = 3.11.6 (main, Oct  2 2023, 13:45:54) [Clang 15.0.0 (clang-1500.0.40.1)] (/opt/homebrew/opt/python@3.11/bin/python3.11)
  jinja version = 3.0.3
  libyaml = True

ansible-galaxy collection list
Collection              Version
----------------------- -------
ansible.controller 4.5.1
ansible.content_builder 0.0.0
ansible.netcommon       6.0.0
ansible.utils           3.0.0
azure.azcollection      1.18.1
community.general       7.1.0
infra.controller_configuration 2.3.0

Controller version
4.5.6

Desired Behavior

When enforcing defaults it should be possible to assign different tags on different nodes in workflows. This means that job_tags in the workflow must be set to null.

Actual Behavior

When enforcing defaults, the value for job_tags in workflows are set to an empty string. This overrides whatever is set on the individual nodes in the workflow and causes them to run without any value for job_tags.

STEPS TO REPRODUCE

controller_configuration_enforce_defaults: true
controller_workflows:
- name: My Workflow
  simplified_workflow_nodes:
    - identifier: My Node
      unified_job_template: Some Template With Prompt For Tags
      job_tags: ignored_tag
ephracis commented 2 months ago

I think this applies to even more fields such as limit and scm_branch.

djdanielsson commented 2 months ago

To be clear are you saying that you have tags in your config that are being ignored or are you saying if tags have been set in UI that this removes them?

ephracis commented 2 months ago

When setting fields such as job_tags to "nothing" in the UI it is set to null. This is what the user wants as it means that values from the nodes take precedence.

There is a good explanation in this comment in another issue: https://github.com/ansible/awx/issues/12991#issuecomment-1425904775

There are 4 sources for the limit's value (in the order of precedence):

job template default (possible values: string or "") job template node prompt (possible values: string or "") workflow template default (possible values: string or null) workflow template prompt (possible values: string or "")

An empty string ("") means that no limit will be applied and a null means that a limit of lower precedence will be applied.

So I guess what I'm saying is sort of "I have tags in my config that are being ignored". They are ignored since the enforcement of defaults sets empty field to "" instead of null, making it take precedence over the node prompt.

djdanielsson commented 2 months ago

if you do not have tags in your code it's self then yes that is what is supposed to happen, enforce defaults is to allow people to catch changes that are made in the UI that isn't in code and make it match the code.

ephracis commented 2 months ago

Yes, but the default value that should be enforced on the workflow template should be null and not an empty string, as an empty string in this case is actually a value that is applied on the nodes, which is not what we want. So the default value that is enforced should be changed for values such as job_tags, skip_tags, limit, scm_branch and perhaps more.

sean-m-sullivan commented 2 months ago

@ephracis This is a limitation of the python modules that if we pass null it will ignore it, and keep any value set in the gui, and not enforce the defaults. This is by design in this iteration. The next iteration will move these out of the default value, and into the python of the modules, but its taken some time, and needs some tweaks to the API itself to work.

So at the moment this is working as intended, functionally, while the default is technically null, it becomes tricky at the python level.

ephracis commented 2 months ago

Ok, I see. Too bad it is not possible to send in null since that is what the GUI sets and what is required to make it possible to have nodes with different tags in a workflow. An empty string breaks this functionality.

I suggest keeping this issue open until the new approach is implemented since this is pretty obviously a bug as it breaks functionality which is expected to work.

Until then the only workaround is to disable enforcing defaults for workflows by setting controller_configuration_workflows_enforce_defaults: false.