Skip to content
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

Update current_inventory_sources.j2 to fix the empty credential scenario #812

Merged
merged 4 commits into from
Apr 16, 2024

Conversation

iamgini
Copy link

@iamgini iamgini commented Apr 13, 2024

What does this PR do?

This is related to the issue #811.

If no credential is assigned to the inventory sources, still the inventory_source.credential will be defined but with [] value. Which will cause an issue as the actual inventory_source.summary_fields.credential will be empty. So need to ensure the inventory_source.summary_fields.credential is also defined and not empty.

How should this be tested?

Is there a relevant Issue open for this?

Resolves #811.

Other Relevant info, PRs, etc

Changelog

  • Fix the empty credential scenario where the playbook is looking for credential names and failed with undefied value.

@@ -22,7 +22,7 @@ controller_inventory_sources:
inventory: "{{ inventory_source.summary_fields.inventory.name }}"
update_on_launch: "{{ inventory_source.update_on_launch }}"
overwrite: "{{ inventory_source.overwrite }}"
{% if inventory_source.credential is defined %}
{% if inventory_source.credential is defined and inventory_source.summary_fields.credential is defined %}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic still seems slightly wrong here. Should it not just be if inventory_source.summary_fields.credential is defined? Perhaps should the value set be different if it is inventory_source.credential defined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you are right.
I didn't remove the original check as I thought its good to have a double check :)
Feel free to amend it and proceed.

Thanks in advance

Copy link
Collaborator

@Tompage1994 Tompage1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a question. Could you also please add a changelog fragment explaining the bugfix please.

@iamgini
Copy link
Author

iamgini commented Apr 15, 2024

Added a question. Could you also please add a changelog fragment explaining the bugfix please.
Amended

@Tompage1994 Tompage1994 enabled auto-merge (squash) April 15, 2024 08:33
@Tompage1994 Tompage1994 merged commit 0be9740 into redhat-cop:devel Apr 16, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filetree_create/templates/current_inventory_sources.j2 complains about an undefined variable and fails.
3 participants