# Ansible-SDP Changelist 32492 — Detailed Change Log

**Date:** 2026-03-31
**Perforce Server:** public.perforce.com:1666
**Changelist:** 32492
**Files Modified:** 13 edits, 7 deletes (20 total)

---

## Table of Contents

1. [Bug Fixes](#1-bug-fixes)
2. [Security Improvements](#2-security-improvements)
3. [Orphaned File Removal](#3-orphaned-file-removal)
4. [Best Practice Improvements](#4-best-practice-improvements)
5. [File-by-File Summary](#5-file-by-file-summary)

---

## 1. Bug Fixes

### 1.1 Broken Jinja2 Filter Syntax in main-playbook.yml

**File:** `main-playbook.yml`
**Severity:** High — playbook would fail to parse

All seven conditional expressions used invalid Jinja2 filter chaining. The pattern `install_perforce is defined and install_perforce |bool| default(false)` has two problems: `|bool|` is malformed syntax (missing spaces/pipe), and the filter ordering is wrong — `default()` must come before `bool` so that undefined variables get a default value before boolean coercion.

**Before:**
```yaml
- { role: 'perforce-sdp-install', when: install_perforce is defined and install_perforce |bool| default(false) }
```

**After:**
```yaml
- { role: 'perforce-sdp-install', when: install_perforce | default(false) | bool }
```

This fix was applied to all seven `when:` conditions in the file (2 in `roles:`, 5 in `tasks:`). The commented-out filebeat role reference was also removed.

---

### 1.2 Broken target_server Variable Logic in host_vars/perforce-commit.yml

**File:** `inventories/host_vars/perforce-commit.yml`
**Severity:** High — edge replica servers would always connect to the wrong target

The `target_server` variable had two bugs:

1. **String literal comparison instead of variable evaluation:** `'perforce_server_type'=='p4d_edgerep'` compares the string `"perforce_server_type"` to `"p4d_edgerep"` — two string literals. This always evaluates to `False`, so the `else` branch was always taken regardless of server type.

2. **Undefined variable reference:** `lookup('vars', 'perforce_dns')` references a variable `perforce_dns` that is never defined anywhere in the project. If the condition were ever `True`, this would fail with an undefined variable error.

**Before:**
```yaml
target_server: "{{ lookup('vars', 'perforce_dns') if 'perforce_server_type'=='p4d_edgerep' else lookup('vars', 'commit_dns') }}"
```

**After:**
```yaml
target_server: "{{ perforce_dnsname if perforce_server_type == 'p4d_edgerep' else commit_dns }}"
```

Now `perforce_server_type` is evaluated as a variable (not a string literal), and `perforce_dnsname` (which is defined in this same file) is used instead of the nonexistent `perforce_dns`.

---

### 1.3 Package Installation Failure on RedHat Hosts

**File:** `roles/perforce-sdp-install/tasks/dependencies.yml`, `roles/perforce-sdp-install/defaults/main.yml`
**Severity:** High — entire dependency installation would fail on RedHat

The `list_of_packages` variable contained Debian-specific packages (e.g., `php-sqlite3`, `libjson-perl`, `openssh-client`, `build-essential`, `python3-venv`) and the task installing them used `force_apt_get: yes` — an apt-only parameter — without any `when: ansible_os_family=="Debian"` guard. On RedHat hosts, both the package names and the `force_apt_get` parameter would cause failures.

**Changes to `defaults/main.yml`:**
- Split the single `list_of_packages` into two lists:
  - `list_of_packages` — OS-agnostic packages only (atop, python3, screen, curl, vim, git, etc.)
  - `list_of_debian_packages` — Debian-specific packages (build-essential, libssl-dev, php-sqlite3, libjson-perl, openssh-client, etc.), now also includes `iputils-ping` and `mailutils` from the previously unused `list_of_debian_package` variable
- Removed duplicate package entries: `libssl-dev` (appeared twice), `curl` (twice), `liblzma-dev` (twice), `libffi-dev` (twice)
- Renamed `list_of_debian_package` (singular, never referenced) to `list_of_debian_packages` and merged its contents into the new list

**Changes to `dependencies.yml`:**
- Common packages task now uses `package` module without `force_apt_get`
- New Debian packages task uses `apt` module with `list_of_debian_packages` and `when: ansible_os_family=="Debian"` guard
- Removed the redundant second Debian packages task that was duplicating the common packages install

---

### 1.4 Hardcoded Email Addresses in Cron Jobs

**File:** `roles/perforce-sdp-install/tasks/cron.yml`
**Severity:** Medium — cron notifications would go to wrong addresses in other deployments

The `MAILTO` and `MAILFROM` cron environment variables were hardcoded to `rusty.jackson@example.com` and `perforce-admin@example.com` respectively, despite the project having properly defined variables for these values in `group_vars/central/main.yml`.

**Before:**
```yaml
- name: "Mailto environment variable"
  cronvar:
    name: MAILTO
    value: rusty.jackson@example.com

- name: "Mailfrom environment variable"
  cronvar:
    name: MAILFROM
    value: perforce-admin@example.com
```

**After:**
```yaml
- name: "Mailto environment variable"
  cronvar:
    name: MAILTO
    value: "{{ perforce_mailto_email }}"

- name: "Mailfrom environment variable"
  cronvar:
    name: MAILFROM
    value: "{{ perforce_mailfrom_email }}"
```

---

### 1.5 Inconsistent Journal Rotation Commands

**File:** `roles/perforce-sdp-install/tasks/cron.yml`
**Severity:** Medium — 06:00 and 08:00 rotations skipped proper journal management

Ten of the twelve journal rotation cron jobs used `/p4/common/bin/rotate_journal.sh`, which handles journal rotation along with cleanup and management of old journal files. However, the 06:00 and 08:00 entries used bare `p4 admin journal` which only rotates the journal without any of the additional housekeeping.

**Before:**
```yaml
job: "/p4/common/bin/run_if_master.sh ${INSTANCE} p4 admin journal"
```

**After:**
```yaml
job: "/p4/common/bin/run_if_master.sh ${INSTANCE} /p4/common/bin/rotate_journal.sh ${INSTANCE}"
```

---

### 1.6 ansibleuser Sudo Group Fails on RedHat

**File:** `roles/perforce-sdp-install/tasks/dependencies.yml`
**Severity:** Medium — ansibleuser would not get sudo access on RedHat

The task `add ansibleuser to group sudo` hardcoded the group name `sudo`, which only exists on Debian-based systems. RedHat-based systems use the `wheel` group for sudo access.

**Before:**
```yaml
- name: "add ansibleuser to group sudo"
  user:
    name: ansibleuser
    groups: sudo
    append: yes
```

**After:**
```yaml
- name: "add ansibleuser to appropriate sudo group"
  user:
    name: ansibleuser
    groups: "{{ 'sudo' if ansible_os_family == 'Debian' else 'wheel' }}"
    append: yes
```

---

### 1.7 Malformed ansible.cfg Configuration

**File:** `ansible.cfg`
**Severity:** Medium — settings were silently ignored

The `ansible.cfg` file had a `vars:` block with indented settings, but `ansible.cfg` uses INI format and does not support a `vars:` section. The `allow_world_readable_tmpfiles` and `sshd_permit_root_login` settings were nested under `vars:` and therefore ignored. Additionally, `sshd_permit_root_login` is not a valid Ansible configuration setting at all.

**Before:**
```ini
[defaults]
...
timeout = 180
vars:
  allow_world_readable_tmpfiles: true
    sshd_permit_root_login: true
```

**After:**
```ini
[defaults]
...
timeout = 180
allow_world_readable_tmpfiles = true
```

---

### 1.8 Redundant Broker Service Operations in install.yml

**File:** `roles/perforce-sdp-install/tasks/install.yml`
**Severity:** Low — unnecessary service restarts during install

The broker installation section had three consecutive service operations:
1. `systemd: state: "started"` (enable and start via systemd)
2. `service: state: "started"` (start again — no-op since already started)
3. `service: state: "restarted"` (immediately restart what was just started)

Consolidated to a single `systemd: state: "started"` with `enabled: true` and `daemon_reload: true`.

---

### 1.9 Empty authorized_keys Line

**File:** `roles/perforce-sdp-install/tasks/main.yml`
**Severity:** Low — no-op placeholder adding empty line to file

The task "Add Perforce Public key to authorized_keys" used `lineinfile` with `line: ""`, which adds an empty line to the file — it does not actually add any SSH public key. Changed to a `file: state: touch` to ensure the file exists with correct permissions without inserting meaningless content.

---

### 1.10 Duplicate Handler Definition in Monitoring Role

**File:** `roles/perforce-sdp-monitoring/handlers/main.yml`
**Severity:** Low — ambiguous handler behavior

The `restart-node-exporter` handler was defined twice. The second definition referenced `{{ node_exporter_service }}`, a variable that is never defined anywhere in the project. Removed the duplicate and kept the first definition which correctly hardcodes `node_exporter` as the service name. Also replaced the `command: systemctl daemon-reload` handler with the proper `ansible.builtin.systemd: daemon_reload: yes` module.

---

## 2. Security Improvements

### 2.1 HTTP to HTTPS for Binary Downloads

**Files:** `roles/perforce-sdp-install/tasks/install.yml`, `roles/perforce-sdp-install/tasks/update.yml`, `roles/perforce-sdp-install/tasks/install_broker.yml`

All Perforce binary downloads (p4, p4d, p4broker) were fetched over plain HTTP from `http://ftp.perforce.com`. This allows man-in-the-middle attacks to substitute malicious binaries during download. All URLs changed to `https://ftp.perforce.com`.

**Affected URLs (all three files):**
- `http://ftp.perforce.com/perforce/r{{ perforce_version }}/bin.linux26x86_64/p4`
- `http://ftp.perforce.com/perforce/r{{ perforce_version }}/bin.linux26x86_64/p4d`
- `http://ftp.perforce.com/perforce/r{{ perforce_broker_version }}/bin.linux26x86_64/p4broker`

---

### 2.2 Hardcoded Password Hash Moved to Variable

**Files:** `roles/perforce-sdp-install/tasks/dependencies.yml`, `inventories/group_vars/central/main.yml`

The perforce user's password hash was hardcoded directly in the task file. Moved to a `perforce_user_password` variable in `group_vars/central/main.yml` with a comment indicating it should be overridden with Ansible Vault in production.

---

### 2.3 Hardcoded Personal Sudoers Entry Parameterized

**Files:** `roles/perforce-sdp-install/tasks/dependencies.yml`, `inventories/group_vars/central/main.yml`

A personal sudoers entry for `russelljackson` was hardcoded in the role, making the role non-reusable. Replaced with a parameterized loop over an `admin_users` list variable defined in `group_vars/central/main.yml`. New deployments can customize the list without modifying role code.

**Before (in dependencies.yml):**
```yaml
- name: "Setup russelljackson sudo file"
  copy:
    content: "russelljackson ALL=(ALL:ALL) NOPASSWD:ALL\n"
    dest: "/etc/sudoers.d/russelljackson"
```

**After:**
```yaml
- name: "Setup additional admin sudo files"
  copy:
    content: "{{ item }} ALL=(ALL:ALL) NOPASSWD:ALL\n"
    dest: "/etc/sudoers.d/{{ item }}"
    mode: "0440"
    owner: "root"
    group: "root"
  loop: "{{ admin_users | default([]) }}"
```

**In group_vars/central/main.yml:**
```yaml
admin_users:
  - russelljackson
```

---

### 2.4 Pyenv Install Hardened

**File:** `roles/perforce-sdp-install/tasks/dependencies.yml`

Added `-fsSL` flags to the curl command for pyenv installation (`curl -fsSL https://pyenv.run | bash`) so that HTTP errors cause failures rather than silently piping error pages to bash. Also added `creates: "/p4/.pyenv/bin/pyenv"` for idempotency — the task is skipped if pyenv is already installed.

---

## 3. Orphaned File Removal

The following 7 files were deleted because they are not referenced by any task, playbook, or template in the project. They appear to be remnants of removed functionality.

| File | Purpose |
|------|---------|
| `roles/perforce-sdp-monitoring/templates/p4prometheus_config.j2` | P4Prometheus config template — replaced by `install_p4prom.sh` script approach |
| `roles/perforce-sdp-monitoring/templates/p4prometheus_systemd.j2` | P4Prometheus systemd service template — same as above |
| `roles/perforce-sdp-monitoring/templates/nexpose_sudoer.j2` | Nexpose vulnerability scanner sudoers template — feature removed |
| `roles/perforce-sdp-monitoring/templates/audit.rules.j2` | Auditd rules template (NIST 800-53 compliance) — feature removed |
| `roles/perforce-sdp-monitoring/files/smartmon.sh` | SMART disk monitoring script — never referenced |
| `roles/perforce-sdp-monitoring/files/auditd_post_start` | Auditd post-start hook — never referenced |
| `roles/perforce-sdp-monitoring/files/otelcol-contrib.conf` | OpenTelemetry Collector config — never referenced |

---

## 4. Best Practice Improvements

### 4.1 Shell Commands Replaced with Ansible Modules

**File:** `roles/perforce-sdp-install/tasks/dependencies.yml`

| Task | Before | After |
|------|--------|-------|
| Restart systemd-networkd | `command` with `with_items` running three `systemctl` commands | `ansible.builtin.systemd` module with `loop` |
| Apply sysctl settings | Inline `command: "sysctl -p"` | `notify: "restart_sysctl"` handler |
| Enable THP disable service | Inline `command` with `with_items` | `notify: "start_thp"` handler |
| Change crontab ownership | `shell: "chown perforce: /p4/p4.crontab"` | `file` module with `owner`/`group` |

### 4.2 sed Replaced with Ansible replace Module

**File:** `roles/perforce-sdp-monitoring/tasks/main.yml`

The monitoring role used `shell: sed -i` to modify the downloaded p4prometheus install script. This is fragile if the upstream script changes format. Replaced with `ansible.builtin.replace` module which is idempotent and provides proper change detection.

**Before:**
```yaml
- name: Modify defaults in install script
  become: yes
  shell: 'sed -i "s/case_sensitive_server: true/case_sensitive_server: false/g" /p4/install_p4prom.sh'
```

**After:**
```yaml
- name: "Set case_sensitive_server to false in install script"
  ansible.builtin.replace:
    path: /p4/install_p4prom.sh
    regexp: 'case_sensitive_server: true'
    replace: 'case_sensitive_server: false'
```

### 4.3 Removed Excessive ignore_errors Usage

**File:** `roles/perforce-sdp-install/tasks/dependencies.yml`

Removed `ignore_errors: yes` from package installation, user/group creation, and pyenv install tasks. These were masking real failures. The pyenv install now uses `creates:` for idempotency instead of `ignore_errors` to handle the already-installed case.

---

## 5. File-by-File Summary

| # | File | Action | Changes |
|---|------|--------|---------|
| 1 | `ansible.cfg` | Edit | Fixed malformed vars block, removed invalid setting |
| 2 | `main-playbook.yml` | Edit | Fixed 7 Jinja2 filter expressions, removed dead comment |
| 3 | `inventories/group_vars/central/main.yml` | Edit | Added `perforce_user_password` and `admin_users` variables |
| 4 | `inventories/host_vars/perforce-commit.yml` | Edit | Fixed `target_server` string comparison and undefined variable |
| 5 | `roles/perforce-sdp-install/defaults/main.yml` | Edit | Split packages by OS, deduplicated, fixed variable name |
| 6 | `roles/perforce-sdp-install/tasks/main.yml` | Edit | Fixed empty authorized_keys to file touch |
| 7 | `roles/perforce-sdp-install/tasks/dependencies.yml` | Edit | OS family guards, parameterized users/passwords, handlers, pyenv hardening |
| 8 | `roles/perforce-sdp-install/tasks/install.yml` | Edit | HTTPS URLs, removed redundant broker starts, file module for chown |
| 9 | `roles/perforce-sdp-install/tasks/install_broker.yml` | Edit | HTTP to HTTPS for binary download |
| 10 | `roles/perforce-sdp-install/tasks/update.yml` | Edit | HTTP to HTTPS for binary downloads |
| 11 | `roles/perforce-sdp-install/tasks/cron.yml` | Edit | Template variables for email, consistent journal rotation |
| 12 | `roles/perforce-sdp-monitoring/tasks/main.yml` | Edit | Replaced sed with replace module |
| 13 | `roles/perforce-sdp-monitoring/handlers/main.yml` | Edit | Removed duplicate handler, fixed daemon-reload |
| 14 | `roles/perforce-sdp-monitoring/templates/p4prometheus_config.j2` | Delete | Orphaned — unused |
| 15 | `roles/perforce-sdp-monitoring/templates/p4prometheus_systemd.j2` | Delete | Orphaned — unused |
| 16 | `roles/perforce-sdp-monitoring/templates/nexpose_sudoer.j2` | Delete | Orphaned — unused |
| 17 | `roles/perforce-sdp-monitoring/templates/audit.rules.j2` | Delete | Orphaned — unused |
| 18 | `roles/perforce-sdp-monitoring/files/smartmon.sh` | Delete | Orphaned — unused |
| 19 | `roles/perforce-sdp-monitoring/files/auditd_post_start` | Delete | Orphaned — unused |
| 20 | `roles/perforce-sdp-monitoring/files/otelcol-contrib.conf` | Delete | Orphaned — unused |

---

## Known Remaining Items

The following files are referenced by `copy:` tasks but do not exist in the `roles/perforce-sdp-install/files/` directory. These are likely sensitive files managed outside of version control (Ansible Vault, secure file server, etc.):

- `id_rsa` — SSH private key for perforce user
- `certificate.txt` — SSL certificate for p4broker
- `privatekey.txt` — SSL private key for p4broker
- `perforce-license` — Perforce server license file

These should be documented in the project README or provided via Ansible Vault.