os_images visibility fix #38
Conversation
| @@ -115,7 +115,7 @@ | |||
| ramdisk: "{{ item.2.id if is_baremetal else omit }}" | |||
| vars: | |||
| is_baremetal: "{{ item.0.elements is defined and 'baremetal' in item.0.elements }}" | |||
| visibility: "{{ item.0.visibility | default(item.0.is_public | ternary('public', 'private') if item.0.is_public is defined else os_images_visibility) }}" | |||
| visibility: "{{ os_images_visibility | default(item.0.visibility | default(item.0.is_public | default(true) | ternary('public', 'private', true))) }}" | |||
There was a problem hiding this comment.
base on role defaults -
it will be public even if item's visibility/is_public would be private
IMO is_public should override os_images_visibility, since it has been specified on a per-image basis, whereas os_images_visibility is global. If you don't want is_public to be used, remove it from the image definition. |
| @@ -32,7 +32,7 @@ | |||
| filename: "{{ os_images_cache }}/{{ item.name }}/{{ item.name }}.vmlinuz" | |||
| with_items: "{{ os_images_list | list }}" | |||
| vars: | |||
| visibility: "{{ item.visibility | default(item.is_public | ternary('public', 'private') if item.is_public is defined else os_images_visibility) }}" | |||
| visibility: "{{ os_images_visibility | default(item.0.visibility | default(item.0.is_public | default(true) | ternary('public', 'private', true))) }}" | |||
There was a problem hiding this comment.
per-item values should take precedence over global values.
There was a problem hiding this comment.
I agree
I've misunderstood the intent.
Since os_images_visibility is being set as role defaults it is not it's purpose to override per-item values.
Closing.
Correcting visibility variable logic, to account for edge case when
is_publicis defined but we want to override it withos_images_visibility.Based on discussions here:
stackhpc/ansible-role-os-images#77 (comment)
It looks like the intention was to support deprecated
is_publicalong withvisibilityand still have the ability to override with role definedos_images_visibility.This PR handles that.
Fixes : #37