Add snipe it play #121
Add snipe it play #121
Conversation
WalkthroughAdds Snipe-IT asset registration: new encrypted API key, new common role task to collect hardware facts and POST assets, and expands inventory and play targets with new groups and hosts; Ansible config adjusted for remote temp path. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin
participant AC as Ansible Controller
participant TH as Target Host
participant SI as Snipe‑IT API
Admin->>AC: Run play (workstations, computenodes, servers, userservers, storageservers, vms)
AC->>TH: Execute common role (includes snipeIT.yml)
rect rgba(220,235,255,0.4)
TH->>TH: Run dmidecode / lshw to collect BIOS, board, CPU, GPU, serial
TH->>TH: Gather IP/MAC, RAM, OS, hostname, group_names
TH->>TH: Map group_names -> model_id
end
TH-->>AC: Set facts (_bios/_board/_cpu/_gpu/serial_number/model_id)
AC->>AC: Read `files/snipeIT.key` (vault) for Authorization
AC->>SI: POST /api/v1/hardware (JSON payload) [delegate_to: localhost]
alt 200 or 201
SI-->>AC: Success
else
SI-->>AC: Error (handled)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
inventory (3)
89-91: Interpreter pinning inconsistent within storageservers.Only cicss04 pins python3.8; cicss05/06 don’t. Standardize to /usr/bin/python3 or auto_silent at group level.
-cicss04 ansible_python_interpreter=/usr/bin/python3.8 +cicss04 ansible_python_interpreter=/usr/bin/python3Alternatively add group_vars/storageservers.yml with ansible_python_interpreter: auto_silent.
94-95: Hostname format consistency.vms use FQDNs while most other groups use short names. Aligning formats reduces surprises with inventory_hostname vs fqdn and host_vars file naming.
82-82: Avoid pinning ansible_python_interpreter to /usr/bin/python3.8
- inventory:82 (cicvmhost01) and inventory:89 (cicss04) — replace ansible_python_interpreter=/usr/bin/python3.8 with ansible_python_interpreter=/usr/bin/python3
- Better: centralize in group_vars/servers.yml: ansible_python_interpreter: auto_silent
site.yml (1)
30-41: storageservers/vms exclude common; Snipe‑IT won’t run there. Intentional?If you want asset registration on these groups, re‑enable common or carve out a slim snipeit‑only role.
Example to enable only Snipe‑IT tasks via tag:
- roles: - - bodsch.snapd -# - common + roles: + - bodsch.snapd + - { role: common, tags: ['snipeit'] }Then run with --tags snipeit to avoid other common tasks.
roles/common/tasks/snipeIT.yml (6)
17-20: Avoid shadowingbios_version; store the combined string under a new var.You register
bios_versionthen overwrite it viaset_fact. Rename tobios_infoand update the payload reference.- - name: Set BIOS info + - name: Set BIOS info set_fact: - bios_version: "{{ bios_vendor.stdout | default('') | trim }} {{ bios_version.stdout | default('') | trim }} {{ bios_revision.stdout | default('') | trim }}" + bios_info: "{{ bios_vendor.stdout | default('') | trim }} {{ bios_version.stdout | default('') | trim }} {{ bios_revision.stdout | default('') | trim }}" @@ - _snipeit_bios_15: "{{ bios_version }}" + _snipeit_bios_15: "{{ bios_info }}"Also applies to: 93-93
29-40: Harden fact composition with safe defaults.Prevent “undefined variable” errors and bad values.
- motherboard: "{{ baseboard_manufacturer.stdout | trim }} {{ baseboard_product_name.stdout | trim }}" + motherboard: "{{ baseboard_manufacturer.stdout | default('') | trim }} {{ baseboard_product_name.stdout | default('') | trim }}" @@ - cpu: "{{ processor_family.stdout | trim }} {{ processor_manufacturer.stdout | trim }} {{ processor_version.stdout | trim }}" + cpu: "{{ processor_family.stdout | default('') | trim }} {{ processor_manufacturer.stdout | default('') | trim }} {{ processor_version.stdout | default('') | trim }}" @@ - _snipeit_ip_address_12: "{{ ansible_all_ipv4_addresses | join(', ') }}" - _snipeit_mac_address_1: "{{ ansible_default_ipv4.macaddress }}" + _snipeit_ip_address_12: "{{ ansible_all_ipv4_addresses | default([]) | join(', ') }}" + _snipeit_mac_address_1: "{{ ansible_default_ipv4.macaddress | default('') }}"Also applies to: 53-56, 95-101
37-40: Use a resilient serial/asset_tag with fallbacks; filter bogus DMI values.Some vendors return “Not Specified”/“To Be Filled By O.E.M.”; also DMI can be blank in VMs.
- serial_number: "{{ baseboard_serial_number.stdout | trim }}" + serial_number: >- + {% set sn = (baseboard_serial_number.stdout | default('') | trim) %} + {% if sn and sn not in ['Not Specified','To Be Filled By O.E.M.','None'] %} + {{ sn }} + {% else %} + {{ ansible_product_serial | default(ansible_product_uuid | default(ansible_machine_id | default(ansible_facts.get('machine_id','unknown')))) }} + {% endif %} @@ - asset_tag: "{{ serial_number }}" - serial: "{{ serial_number }}" + asset_tag: "{{ serial_number }}" + serial: "{{ serial_number }}"Also applies to: 89-90
62-65: GPU parsing: tighten regex and add fallback.Current regex is greedy and brittle. Prefer a safer pattern and fallback to
lspciiflshwis absent.- gpu_model: "{{ gpu_info.stdout | regex_search('product: (.+)', '\\1') | default('Unknown') | replace('[', '') | replace(']', '') | replace('\"', '') | replace(\"'\", '') }}" + gpu_model: >- + {{ gpu_info.stdout | regex_search('product:\\s*(.*)', '\\1') | default('Unknown') | trim }}Optional fallback task:
- name: Fallback GPU model via lspci command: bash -lc "lspci | grep -iE 'vga|3d|2d' | head -1 | cut -d':' -f3-" register: lspci_gpu changed_when: false failed_when: false when: gpu_model == 'Unknown' - set_fact: gpu_model: "{{ lspci_gpu.stdout | default('Unknown') | trim }}" when: gpu_model == 'Unknown'
66-76: Externalizemodel_idmapping to vars; avoid hard‑coding IDs in tasks.Keep IDs in
group_vars/all.ymlor role defaults for portability and easier changes.Example:
# group_vars/all.yml (or roles/common/defaults/main.yml) snipeit_model_id_by_group: workstations: 1 computenodes: 3 vms: 4 userservers: 5 storageservers: 6 servers: 12 snipeit_default_model_id: 13Then replace the Jinja block with:
- model_id: >- - {% if 'workstations' in group_names %}1 - {% elif 'computenodes' in group_names %}3 - {% elif 'vms' in group_names %}4 - {% elif 'userservers' in group_names %}5 - {% elif 'storageservers' in group_names %}6 - {% elif 'servers' in group_names %}12 - {% else %}13 # Default model ID if not matched - {% endif %} + model_id: "{{ (group_names | map('extract', snipeit_model_id_by_group, default=None) | select('number') | first) | default(snipeit_default_model_id) }}"
78-106: Add sane HTTP options and pathing reliability.
- Add
timeoutto avoid hanging.- Ensure TLS verification (default is true; being explicit helps).
- If you keep file lookup elsewhere, prefer
{{ role_path }}/files/...for stable resolution.body_format: json + validate_certs: true + timeout: 30If you ever reintroduce file lookup:
Authorization: "Bearer {{ lookup('ansible.builtin.file', role_path + '/files/snipeIT.key') }}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
files/snipeIT.key(1 hunks)inventory(2 hunks)roles/common/tasks/main.yml(1 hunks)roles/common/tasks/snipeIT.yml(1 hunks)site.yml(1 hunks)update.yml(1 hunks)
🔇 Additional comments (4)
inventory (1)
19-19: New workstation host: confirm onboarding expectations.Ensure host-specific vars (e.g., owner/location for Snipe-IT) exist if the common role expects them.
site.yml (1)
18-23: Servers/userservers: OK to run common (brings Snipe‑IT).No action—just noting this will execute common (and snipeIT if enabled) on these groups. Looks intentional.
Confirm required facts/commands (dmidecode, lshw) exist on these hosts or are guarded in snipeIT.yml.
files/snipeIT.key (1)
1-55: Prefer vaulted variable for Snipe‑IT API token (avoid file lookup)
- Found: roles/common/tasks/snipeIT.yml:83 uses Authorization: "Bearer {{ lookup('file', 'files/snipeIT.key') }}". Move the token to a vaulted var (e.g., group_vars/all/vault.yml: snipeit_api_token) and build the header at runtime: Authorization: "Bearer {{ snipeit_api_token }}".
- If you keep files/snipeIT.key, ensure the path is resolved reliably from the role (use role_path when referencing role files) and confirm the file contains only the raw token (not a full "Bearer " header).
roles/common/tasks/main.yml (1)
21-21: Gate Snipe‑IT import behind a toggle (snipeit_enabled)File: roles/common/tasks/main.yml:21 — Prevent unintended API calls in routine runs; enable via -e snipeit_enabled=true or set in group_vars.
- - {import_tasks: snipeIT.yml, tags: snipeit} + - import_tasks: snipeIT.yml + tags: snipeit + when: snipeit_enabled | default(false)Ensure roles/common/tasks/snipeIT.yml is idempotent (lookup/GET before POST) to avoid creating duplicate assets.
| - name: Retrieve BIOS vendor | ||
| command: dmidecode -s bios-vendor | ||
| register: bios_vendor | ||
| ignore_errors: yes | ||
|
|
||
| - name: Retrieve BIOS version | ||
| command: dmidecode -s bios-version | ||
| register: bios_version | ||
| ignore_errors: yes | ||
|
|
||
| - name: Retrieve BIOS revision | ||
| command: dmidecode -s bios-revision | ||
| register: bios_revision | ||
| ignore_errors: yes | ||
|
|
There was a problem hiding this comment.
Run hardware probes with privilege; mark them idempotent.
dmidecode/lshw need root. Also mark these command tasks changed_when: false to avoid noisy diffs.
Apply this diff pattern to each command task:
- command: dmidecode -s bios-vendor
+ command: dmidecode -s bios-vendor
+ become: true
+ changed_when: false
...
- command: lshw -C display
+ command: lshw -C display
+ become: true
+ changed_when: falseAlso applies to: 21-28, 33-36, 41-52, 57-60
🤖 Prompt for AI Agents
In roles/common/tasks/snipeIT.yml around lines 2 to 16, the dmidecode command
tasks must run with elevated privileges and be marked idempotent: add become:
yes to ensure the probe runs as root and add changed_when: false to prevent
Ansible from reporting a change every run (keep existing register and
ignore_errors as-is); apply the same pattern (become: yes and changed_when:
false) to the other command tasks at lines 21-28, 33-36, 41-52, and 57-60.
| - name: Create asset in Snipe-IT | ||
| uri: | ||
| url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware" | ||
| method: POST | ||
| headers: | ||
| Authorization: "Bearer {{ lookup('file', 'files/snipeIT.key') }}" | ||
| Accept: "application/json" | ||
| Content-Type: "application/json" | ||
| body_format: json | ||
| body: | ||
| name: "{{ ansible_hostname }}" | ||
| asset_tag: "{{ serial_number }}" | ||
| serial: "{{ serial_number }}" | ||
| model_id: "{{ model_id }}" | ||
| status_id: 4 #Online status | ||
| _snipeit_bios_15: "{{ bios_version }}" | ||
| _snipeit_motherboard_16: "{{ motherboard }}" | ||
| _snipeit_os_8: "{{ ansible_distribution }} {{ ansible_distribution_version }}" | ||
| _snipeit_ram_10: "{{ ansible_memtotal_mb }} MB" | ||
| _snipeit_cpu_11: "{{ cpu }}" | ||
| _snipeit_gpu_9: "{{ gpu_model }}" | ||
| _snipeit_ip_address_12: "{{ ansible_all_ipv4_addresses | join(', ') }}" | ||
| _snipeit_mac_address_1: "{{ ansible_default_ipv4.macaddress }}" | ||
| _snipeit_location_nickname_3: "unknown" | ||
| _snipeit_primary_user_5: "unknown" | ||
| _snipeit_purchase_date_aprox_6: "unknown" | ||
| _snipeit_owner_4: "unknown" | ||
| _snipeit_service_status_13: "in service" | ||
| status_code: [200, 201] | ||
| delegate_to: localhost |
There was a problem hiding this comment.
Make the call idempotent (search → create or update) to avoid duplicate assets.
Current POST will fail on reruns if asset_tag/serial already exists (Snipe‑IT returns 4xx). Query first, then POST or PUT.
Here’s a minimal replacement for the POST task:
- - name: Create asset in Snipe-IT
- uri:
- url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware"
- method: POST
- headers:
- Authorization: "Bearer {{ snipeit_api_token }}"
- Accept: "application/json"
- Content-Type: "application/json"
- body_format: json
- body:
- name: "{{ ansible_hostname }}"
- asset_tag: "{{ serial_number }}"
- serial: "{{ serial_number }}"
- model_id: "{{ model_id }}"
- status_id: 4 #Online status
- _snipeit_bios_15: "{{ bios_version }}"
- _snipeit_motherboard_16: "{{ motherboard }}"
- _snipeit_os_8: "{{ ansible_distribution }} {{ ansible_distribution_version }}"
- _snipeit_ram_10: "{{ ansible_memtotal_mb }} MB"
- _snipeit_cpu_11: "{{ cpu }}"
- _snipeit_gpu_9: "{{ gpu_model }}"
- _snipeit_ip_address_12: "{{ ansible_all_ipv4_addresses | join(', ') }}"
- _snipeit_mac_address_1: "{{ ansible_default_ipv4.macaddress }}"
- _snipeit_location_nickname_3: "unknown"
- _snipeit_primary_user_5: "unknown"
- _snipeit_purchase_date_aprox_6: "unknown"
- _snipeit_owner_4: "unknown"
- _snipeit_service_status_13: "in service"
- status_code: [200, 201]
- delegate_to: localhost
+ - name: Lookup existing asset by serial/asset_tag
+ no_log: true
+ uri:
+ url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware?search={{ serial_number | urlencode }}"
+ method: GET
+ headers:
+ Authorization: "Bearer {{ snipeit_api_token }}"
+ Accept: "application/json"
+ return_content: true
+ status_code: 200
+ timeout: 30
+ register: snipeit_search
+ delegate_to: localhost
+
+ - name: Set existing asset id (if any)
+ set_fact:
+ snipeit_existing_id: "{{ (snipeit_search.json.rows | default([]) | selectattr('serial','equalto', serial_number) | list | first).id | default(omit) }}"
+
+ - name: Create asset in Snipe-IT (POST)
+ no_log: true
+ uri:
+ url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware"
+ method: POST
+ headers:
+ Authorization: "Bearer {{ snipeit_api_token }}"
+ Accept: "application/json"
+ Content-Type: "application/json"
+ body_format: json
+ body: "{{ snipeit_body }}"
+ status_code: [201]
+ timeout: 30
+ when: snipeit_existing_id is not defined
+ delegate_to: localhost
+
+ - name: Update asset in Snipe-IT (PUT)
+ no_log: true
+ uri:
+ url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware/{{ snipeit_existing_id }}"
+ method: PUT
+ headers:
+ Authorization: "Bearer {{ snipeit_api_token }}"
+ Accept: "application/json"
+ Content-Type: "application/json"
+ body_format: json
+ body: "{{ snipeit_body }}"
+ status_code: [200]
+ timeout: 30
+ when: snipeit_existing_id is defined
+ delegate_to: localhostAdd once above these tasks:
- name: Compose Snipe-IT body
set_fact:
snipeit_body:
name: "{{ ansible_hostname }}"
asset_tag: "{{ serial_number }}"
serial: "{{ serial_number }}"
model_id: "{{ model_id }}"
status_id: 4
_snipeit_bios_15: "{{ bios_info }}"
_snipeit_motherboard_16: "{{ motherboard }}"
_snipeit_os_8: "{{ ansible_distribution }} {{ ansible_distribution_version }}"
_snipeit_ram_10: "{{ ansible_memtotal_mb }} MB"
_snipeit_cpu_11: "{{ cpu }}"
_snipeit_gpu_9: "{{ gpu_model }}"
_snipeit_ip_address_12: "{{ ansible_all_ipv4_addresses | join(', ') | default('') }}"
_snipeit_mac_address_1: "{{ ansible_default_ipv4.macaddress | default('') }}"
_snipeit_location_nickname_3: "unknown"
_snipeit_primary_user_5: "unknown"
_snipeit_purchase_date_aprox_6: "unknown"
_snipeit_owner_4: "unknown"
_snipeit_service_status_13: "in service"| - name: Create asset in Snipe-IT | ||
| uri: | ||
| url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware" | ||
| method: POST | ||
| headers: | ||
| Authorization: "Bearer {{ lookup('file', 'files/snipeIT.key') }}" | ||
| Accept: "application/json" | ||
| Content-Type: "application/json" | ||
| body_format: json |
There was a problem hiding this comment.
Don't log the Snipe‑IT API token; use a vaulted var instead of file lookup.
- Headers may be printed in logs; the Bearer token would leak.
lookup('file', 'files/snipeIT.key')won’t auto‑decrypt an Ansible Vault file and will likely send ciphertext, breaking auth.
Apply this diff to hide secrets and use a vaulted variable (define snipeit_api_token in group_vars/all/vault.yml or the role’s defaults/vars as vaulted):
- - name: Create asset in Snipe-IT
+ - name: Create asset in Snipe-IT
+ no_log: true
uri:
url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware"
method: POST
headers:
- Authorization: "Bearer {{ lookup('file', 'files/snipeIT.key') }}"
+ Authorization: "Bearer {{ snipeit_api_token }}"
Accept: "application/json"
Content-Type: "application/json"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Create asset in Snipe-IT | |
| uri: | |
| url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware" | |
| method: POST | |
| headers: | |
| Authorization: "Bearer {{ lookup('file', 'files/snipeIT.key') }}" | |
| Accept: "application/json" | |
| Content-Type: "application/json" | |
| body_format: json | |
| - name: Create asset in Snipe-IT | |
| no_log: true | |
| uri: | |
| url: "https://inventory.douglasneuroinformatics.ca/api/v1/hardware" | |
| method: POST | |
| headers: | |
| Authorization: "Bearer {{ snipeit_api_token }}" | |
| Accept: "application/json" | |
| Content-Type: "application/json" | |
| body_format: json |
🤖 Prompt for AI Agents
In roles/common/tasks/snipeIT.yml around lines 78-86, replace the insecure
lookup('file', 'files/snipeIT.key') usage in the Authorization header with a
vaulted variable (e.g. Authorization: "Bearer {{ snipeit_api_token }}") and add
no_log: true to the task to prevent the headers/body from being printed in logs;
also ensure snipeit_api_token is defined in a Vault file (e.g.
group_vars/all/vault.yml or the role’s vaulted vars) and remove the file lookup
so ciphertext is not sent.
| - servers | ||
| - userservers | ||
| - storageservers | ||
| - vms |
There was a problem hiding this comment.
🧩 Analysis chain
Expanding this maintenance play to servers/storage/vms may be risky.
This play runs kernel/NVIDIA install, initramfs, grub updates, and reboots. That’s often undesirable on storage servers and VMs.
- Gate GPU/kernel tasks to workstations/computenodes:
- - name: Install kernel and nvidia and hold packages
- ansible.builtin.script: bin/install_kernel.sh
- when: numusers.stdout_lines | length < 2
+ - name: Install kernel and nvidia and hold packages
+ ansible.builtin.script: bin/install_kernel.sh
+ when:
+ - numusers.stdout_lines | length < 2
+ - "'workstations' in group_names or 'computenodes' in group_names"- Consider splitting this into separate plays per host class with tailored tasks.
🏁 Script executed:
#!/bin/bash
# Quick check: tasks that may be unsafe for servers/vms
rg -n "install_kernel\.sh|update-initramfs|update-grub|reboot" -n -C1 update.ymlLength of output: 448
Gate kernel/GRUB/initramfs/reboot tasks to compute/workstation hosts
update-initramfs and update-grub run unguarded; install_kernel.sh (lines 22–24) and reboot (48–51) are only conditioned on numusers — expanding this play to servers/storageservers/vms will run risky operations on storage servers and VMs.
- Findings (update.yml): install_kernel.sh — 22–24; update-initramfs — 35–36 (no guard); update-grub — 38–39 (no guard); reboot — 48–51.
- Fix: add host-group guards (e.g., workstations / computenodes) to the kernel install, initramfs, grub, and reboot tasks. Example (original diff) for the kernel install — apply the same pattern to initramfs, grub, and reboot where appropriate:
- - name: Install kernel and nvidia and hold packages
- ansible.builtin.script: bin/install_kernel.sh
- when: numusers.stdout_lines | length < 2
+ - name: Install kernel and nvidia and hold packages
+ ansible.builtin.script: bin/install_kernel.sh
+ when:
+ - numusers.stdout_lines | length < 2
+ - "'workstations' in group_names or 'computenodes' in group_names"- Also consider splitting this play into separate plays per host class with tailored tasks.
Committable suggestion skipped: line range outside the PR's diff.
| [servers] | ||
| cicvmhost01 ansible_user=localadmin ansible_python_interpreter=/usr/bin/python3.8 | ||
|
|
||
| [userservers] | ||
| cicus03 | ||
| dnpus01 | ||
|
|
||
| [storageservers] | ||
| cicss04 ansible_python_interpreter=/usr/bin/python3.8 | ||
| cicss05 | ||
| cicss06 | ||
|
|
||
| [vms] | ||
| cloud1.douglasneuroinformatics.ca | ||
| cloud2.douglasneuroinformatics.ca |
There was a problem hiding this comment.
the dangerous thing here is accidentally running an ad-hoc command on all and touching these machines.
Can you make a separate inventory file for these to keep them isolated?
| @@ -0,0 +1,107 @@ | |||
| #Fetch system facts and add them to our inventory | |||
There was a problem hiding this comment.
add this calling of dmidecode:
How much of this is available from
https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_vars_facts.html ?
regarding dmidecode in general, might be be more reliable to use jc?
https://kellyjonbrazil.github.io/jc/docs/parsers/dmidecode.html
Similarly, lshw can output json we can parse.
|
|
||
| - hosts: servers | ||
| become: yes | ||
| roles: | ||
| - bodsch.snapd | ||
| - common | ||
|
|
||
| - hosts: userservers | ||
| become: yes | ||
| roles: | ||
| - bodsch.snapd | ||
| - common | ||
|
|
||
| - hosts: storageservers | ||
| become: yes | ||
| roles: | ||
| - bodsch.snapd | ||
| # - common | ||
|
|
||
| - hosts: vms | ||
| become: yes | ||
| roles: | ||
| - bodsch.snapd | ||
| # - common | ||
|
|
There was a problem hiding this comment.
I'm not sure we should be running the nosnap stuff on the servers.
|
What happens if I run this a second time on the same machine? |
It re-adds it to snipeIT but nothing changes (silent fail on snipeIT's part, you don't get duplicate entries in the inventory for the same asset) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ansible.cfg (2)
19-20: Scope remote_tmp to the Snipe‑IT play instead of globally.A global override affects all plays/hosts and can have unintended side‑effects. Set it per‑play (or per‑group) via
ansible_remote_tmpfor the Snipe‑IT run only.Apply within this file:
-#set the remote tmp folder (used for snipeIT play) -remote_tmp = /tmp/.ansibleThen, in the Snipe‑IT play (example):
# site.yml (Snipe‑IT play block) - hosts: servers,userservers,storageservers,vms tags: [snipeit] vars: ansible_remote_tmp: "/tmp/.ansible-{{ ansible_user_id }}" roles: - common
19-20: Harden tmp dir usage and isolate by user.Use a per‑user path and strict perms to avoid cross‑user collisions and directory snooping on shared systems.
Pre‑create with safe perms (example pre_task in the Snipe‑IT play):
- name: Ensure per-user Ansible tmp dir become: false file: path: "/tmp/.ansible-{{ ansible_user_id }}" state: directory mode: '0700'
| #set the remote tmp folder (used for snipeIT play) | ||
| remote_tmp = /tmp/.ansible |
There was a problem hiding this comment.
🧩 Analysis chain
Note on /tmp policies and re-runs.
- Some fleets clean
/tmpaggressively; keep this in mind for long runs. If issues arise, prefer/var/tmp/.ansible-{{ ansible_user_id }}. - Re: “What happens if I run this a second time?” Ensure the Snipe‑IT tasks are idempotent (lookup by serial/asset tag, PATCH if exists, no-op otherwise).
Quick check script:
Expected: a GET (or search) by serial/asset_tag, conditional POST/PATCH, and changed_when: false on no-op paths.
🏁 Script executed:
#!/bin/bash
# Verify Snipe-IT task idempotency patterns
rg -n -C2 --type=yaml 'roles/common/tasks/snipeIT.yml|snipeIT\.yml' ; \
rg -n -C3 --type=yaml 'GET|PATCH|changed_when|check_mode|serial|asset_tag' roles/common/tasks/snipeIT.ymlLength of output: 1141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="roles/common/tasks/snipeIT.yml"
if [ ! -f "$f" ]; then
echo "MISSING:$f"
rg -n -S --type=yaml 'uri:|ansible.builtin.uri:|method:|changed_when:|check_mode:|register:|asset_tag:|serial|GET|PATCH|POST|PUT' roles/common || true
exit 0
fi
echo "=== grep matches ==="
rg -n -S --type=yaml 'uri:|ansible.builtin.uri:|method:|changed_when:|check_mode:|register:|asset_tag:|serial|GET|PATCH|POST|PUT' "$f" || true
echo
echo "=== full file ==="
nl -ba "$f"Length of output: 780
Use a persistent remote_tmp and make the Snipe‑IT role idempotent (GET → PATCH/POST, add changed_when/check_mode)
- ansible.cfg: avoid /tmp/.ansible for remote_tmp — use remote_tmp = /var/tmp/.ansible-{{ ansible_user_id }} (currently /tmp/.ansible at lines 19–20).
- roles/common/tasks/snipeIT.yml: the role issues an unconditional POST (uri method: POST; body contains asset_tag/serial around lines ~79–93). Replace with: GET/search by serial/asset_tag, PATCH when the asset exists, POST when it does not; add changed_when: false on true no‑ops, support check_mode, and register responses for decision logic.
🤖 Prompt for AI Agents
In ansible.cfg around lines 19–20 change remote_tmp from /tmp/.ansible to a
persistent per-user path such as /var/tmp/.ansible-{{ ansible_user_id }}; in
roles/common/tasks/snipeIT.yml around lines ~79–93 replace the unconditional
POST with logic that first issues a GET (search by serial/asset_tag) and
registers the response, then if the asset exists send a PATCH to update it and
if it does not exist send a POST to create it, add appropriate changed_when
conditions so no-op responses report changed_when: false, ensure tasks respect
check_mode (skip network mutations when check_mode is true and signal what would
change), and register all responses used for decision logic so subsequent tasks
can branch correctly.
2 participants
I've been using this for awhile - we should add it to the playbook
Summary by CodeRabbit
New Features
Chores
Configuration