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

Use original fact names instead of injected ones #147

Merged
merged 1 commit into from
Jan 10, 2024
Merged

Use original fact names instead of injected ones #147

merged 1 commit into from
Jan 10, 2024

Conversation

Apollo3zehn
Copy link
Contributor

Thank you very much for this very useful role. In my setup I have to set INJECT_FACTS_AS_VARS=false in ansible.cfg which means that facts of the ansible_facts variable are not injected into the top level vars variable. To use this role I had to redefine the missing variables like this:

ansible_distribution: "{{ ansible_facts.distribution }}"
ansible_python: "{{ ansible_facts.python }}"

To avoid this kind of issues it would be better if this project uses the original variables names (e.g. ansible_facts.distribution instead of ansible_distribution). This PR tries to accomplish that.

@stefangweichinger
Copy link
Owner

@Apollo3zehn thanks for the PR. I will have a closer look soon. Sounds valid ...

@stefangweichinger
Copy link
Owner

Sorry for the delay, holidays and now I am ill ...
So you reference to something like https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_vars_facts.html , right? Sounds good to me. I am hesitating to merge such a fundemental change ;-) but consider doing it soon.

@Apollo3zehn
Copy link
Contributor Author

More specifically I am referencing to this section ("ansible-facts"): https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_vars_facts.html#ansible-facts

Normally, all collected facts are accessible under the ansible_facts variable but if INJECT_FACTS_AS_VARS is set to TRUE (default), then they are also available as top level variables using the ansible_ prefix.

In my case INJECT_FACTS_AS_VARS is set to false due to security reasons and therefore this role stops working as it relies on INJECT_FACTS_AS_VARS=TRUE.

@Apollo3zehn
Copy link
Contributor Author

Get well soon!

@stefangweichinger
Copy link
Owner

More specifically I am referencing to this section ("ansible-facts"): https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_vars_facts.html#ansible-facts

Normally, all collected facts are accessible under the ansible_facts variable but if INJECT_FACTS_AS_VARS is set to TRUE (default), then they are also available as top level variables using the ansible_ prefix.

In my case INJECT_FACTS_AS_VARS is set to false due to security reasons and therefore this role stops working as it relies on INJECT_FACTS_AS_VARS=TRUE.

Understood. I might merge soon, currently I still have to look at the failing release-step in the pipeline. Your PR looks good to me.

@stefangweichinger
Copy link
Owner

Get well soon!

Thank you ... it still takes time and patience ...

@stefangweichinger stefangweichinger merged commit b5b470a into stefangweichinger:main Jan 10, 2024
37 of 38 checks passed
@stefangweichinger
Copy link
Owner

tried the merge yesterday, but the release-step fails: https://github.com/stefangweichinger/ansible-rclone/actions/runs/7472462092/job/20335076243#step:3:14

That isn't related to your PR, I have to research the issue. I already made sure to use the correct API Key in the gh-project. I am still not recovered again, so I have to see when I can solve this.

@Apollo3zehn
Copy link
Contributor Author

No worries, this is not time-critical for me as I have a workaround

@stefangweichinger
Copy link
Owner

this seems to be the issue: robertdebock/galaxy-action#13

@stefangweichinger
Copy link
Owner

I could try to release the role manually from the local shell ... but it would be better to have a valid CI/CD-run in the project history etc

stefangweichinger added a commit that referenced this pull request Jan 11, 2024
@stefangweichinger
Copy link
Owner

@Apollo3zehn I think I released 0.1.4 to galaxy right now. What an act ...

The release-action succeeded but I don't see it on https://galaxy.ansible.com/ui/standalone/roles/stefangweichinger/ansible_rclone/ yet.

maybe that takes a while

@Apollo3zehn
Copy link
Contributor Author

Thanks for your efforts! Unfortunately I cannot find version 0.1.4 on that page :-(

@stefangweichinger
Copy link
Owner

Yes, I know. I am not yet sure how to proceed. Maybe I try to export it again from my local shell ...
The automated release-process is cool, if it works. Since I added that semver/tag-stuff it doesn't fully work anymore, I think.
As there are only a few changes now and then, the manual export is doable as well.
Although it should work ;-)

@stefangweichinger
Copy link
Owner

Managed a manual export, now it's visible on https://galaxy.ansible.com/ui/standalone/roles/stefangweichinger/ansible_rclone/

pls check

It's not perfect, the export did a lint and threw these messages
( This is just a note for me, not relevant to your PR. I have to deal with this in a separate issue someday):

Successfully submitted import request 2061798368953129805301308511844670061
Starting import: task_id=2061798368953129805301308511844670061, pulp_id=018d1682-0b1f-7bb7-9026-5200a5619a6d

==== PARAMETERS ====
importer username: stefangweichinger
matched user: stefangweichinger id:18774
github_user: stefangweichinger
github_repo: ansible-rclone
github_reference: None
alternate_clone_url: None
alternate_namespace_name: None
alternate_role_name: None

==== CHECK FOR MATCHING ROLE(S) ====
user:stefangweichinger repo:ansible-rclone matched existing role stefangweichinger.ansible_rclone id:27200

===== CLONING REPO =====
cloning https://github.com/stefangweichinger/ansible-rclone ...

===== GIT ATTRIBUTES =====
github_reference(branch): main
github_commit: b5b470acccd3611345fd45d2a7aa7b0ab57b8e49
github_commit_message: Use original fact names instead of injected ones (#147)

Co-authored-by: Apollo3zehn <[email protected]>
github_commit_date: 2024-01-10T09:55:36+01:00

===== LOADING ROLE =====
Importing with galaxy-importer 0.4.18
Determined role name to be ansible_rclone
Linting role ansible_rclone via ansible-lint...
ansible-rclone/galaxy.yml:4: galaxy[version-incorrect]: collection version should be greater than or equal to 1.0.0
ansible-rclone/molecule/default/converge.yml:10: yaml[truthy]: Truthy value should be one of
ansible-rclone/tasks/beta.yml:7: yaml[octal-values]: Forbidden implicit octal value "0744"
ansible-rclone/tasks/main.yml:86: yaml[octal-values]: Forbidden implicit octal value "0600"
ansible-rclone/tasks/stable.yml:7: yaml[octal-values]: Forbidden implicit octal value "0744"
...ansible-lint run complete
Legacy role loading complete

===== PROCESSING LOADER RESULTS ====
enumerated role name ansible_rclone

===== COMPUTING ROLE VERSIONS ====
adding new version from tag: 0.1.4
tag: 0.1.0 version: 0.1.0
tag: 0.1.1 version: 0.1.1
tag: 0.1.2 version: 0.1.2
tag: 0.1.3 version: 0.1.3
tag: 0.1.4 version: 0.1.4

==== SAVING ROLE ====

Import completed

I hope your PR doesn't break anything for others ;-)
Thanks again.

@veltrupdev
Copy link

Thank you! Yes the version is available now and works fine for me :-)

@Apollo3zehn
Copy link
Contributor Author

Sorry, wrong account.

@tigattack tigattack mentioned this pull request Jan 17, 2024
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.

3 participants