Skip to content

Commit 458c82e

Browse files
committed
feat: Add comprehensive linting setup
Major improvements to code quality checks: 1. Created separate lint.yml workflow with parallel jobs: - ansible-lint (without || true so it actually fails) - yamllint for YAML files - Python linting (ruff, black, mypy) - shellcheck for all shell scripts - Security scanning (bandit, safety) 2. Added linter configurations: - .yamllint - YAML style rules - pyproject.toml - Python tool configs (ruff, black, mypy) - Updated .ansible-lint with better rules 3. Improved main.yml workflow: - Renamed 'lint' to 'syntax-check' for clarity - Removed redundant linting (moved to lint.yml) 4. Added documentation: - docs/linting.md explains all linters and how to use them Current linters are set to warn (|| true) to allow gradual adoption. As code improves, these can be changed to hard failures. Benefits: - Catches Python security issues - Enforces consistent code style - Validates all shell scripts (not just 2) - Checks YAML formatting - Separates linting from testing concerns
1 parent ee3b88b commit 458c82e

File tree

7 files changed

+296
-13
lines changed

7 files changed

+296
-13
lines changed

.ansible-lint

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
1+
# Ansible-lint configuration
2+
exclude_paths:
3+
- .cache/
4+
- .github/
5+
- tests/legacy-lxd/
6+
17
skip_list:
2-
- yaml
3-
- '204'
4-
verbosity: 1
8+
- '204' # Lines should be less than 160 characters
9+
- 'package-latest' # Package installs should not use latest
10+
- 'experimental' # Experimental rules
511

612
warn_list:
713
- no-changed-when
814
- no-handler
915
- fqcn-builtins
1016
- var-spacing
17+
- yaml[line-length]
18+
19+
# Enable additional rules
20+
enable_list:
21+
- no-log-password
22+
- no-same-owner
23+
- partial-become
24+
25+
verbosity: 1

.github/workflows/lint.yml

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
name: Lint
2+
3+
on: [push, pull_request]
4+
5+
permissions:
6+
contents: read
7+
8+
jobs:
9+
ansible-lint:
10+
name: Ansible linting
11+
runs-on: ubuntu-22.04
12+
steps:
13+
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
14+
with:
15+
persist-credentials: false
16+
- uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
17+
with:
18+
python-version: '3.11'
19+
cache: 'pip'
20+
21+
- name: Install ansible-lint
22+
run: |
23+
python -m pip install --upgrade pip
24+
pip install ansible-lint
25+
26+
- name: Run ansible-lint
27+
run: |
28+
# Run without || true so it actually fails on issues
29+
ansible-lint -v *.yml roles/{local,cloud-*}/*/*.yml
30+
31+
yaml-lint:
32+
name: YAML linting
33+
runs-on: ubuntu-22.04
34+
steps:
35+
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
36+
with:
37+
persist-credentials: false
38+
39+
- name: Run yamllint
40+
run: |
41+
pip install yamllint
42+
yamllint -c .yamllint . || true # Start with warnings only
43+
44+
python-lint:
45+
name: Python linting
46+
runs-on: ubuntu-22.04
47+
steps:
48+
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
49+
with:
50+
persist-credentials: false
51+
- uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
52+
with:
53+
python-version: '3.11'
54+
cache: 'pip'
55+
56+
- name: Install Python linters
57+
run: |
58+
python -m pip install --upgrade pip
59+
pip install ruff black mypy
60+
61+
- name: Run ruff
62+
run: |
63+
# Fast Python linter
64+
ruff check . || true # Start with warnings only
65+
66+
- name: Check Python formatting with black
67+
run: |
68+
# Check formatting (don't modify)
69+
black --check --diff . || true # Start with warnings only
70+
71+
- name: Run mypy
72+
run: |
73+
# Type checking for Python
74+
mypy library/ --ignore-missing-imports || true # Start with warnings only
75+
76+
shellcheck:
77+
name: Shell script linting
78+
runs-on: ubuntu-22.04
79+
steps:
80+
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
81+
with:
82+
persist-credentials: false
83+
84+
- name: Run shellcheck
85+
run: |
86+
sudo apt-get update && sudo apt-get install -y shellcheck
87+
# Check all shell scripts, not just algo and install.sh
88+
find . -type f -name "*.sh" -not -path "./.git/*" -exec shellcheck {} \;
89+
90+
security-checks:
91+
name: Security scanning
92+
runs-on: ubuntu-22.04
93+
steps:
94+
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
95+
with:
96+
persist-credentials: false
97+
- uses: actions/setup-python@f677139bbe7f9c59b41e40162b753c062f5d49a3 # v5.2.0
98+
with:
99+
python-version: '3.11'
100+
cache: 'pip'
101+
102+
- name: Install security tools
103+
run: |
104+
python -m pip install --upgrade pip
105+
pip install bandit safety
106+
107+
- name: Run bandit
108+
run: |
109+
# Security linter for Python
110+
bandit -r library/ || true # Start with warnings only
111+
112+
- name: Check dependencies with safety
113+
run: |
114+
# Check for known security issues in dependencies
115+
pip install -r requirements.txt
116+
safety check || true # Start with warnings only

.github/workflows/main.yml

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ permissions:
66
contents: read
77

88
jobs:
9-
lint:
10-
name: Lint code
9+
syntax-check:
10+
name: Ansible syntax check
1111
runs-on: ubuntu-22.04
1212
permissions:
1313
contents: read
@@ -24,15 +24,9 @@ jobs:
2424
run: |
2525
python -m pip install --upgrade pip
2626
pip install -r requirements.txt
27-
pip install ansible-lint
28-
# Install shellcheck from apt (faster than snap)
29-
sudo apt-get update && sudo apt-get install -y shellcheck
3027
31-
- name: Run linters
32-
run: |
33-
shellcheck algo install.sh
34-
ansible-playbook main.yml --syntax-check
35-
ansible-lint -x experimental,package-latest,unnamed-task -v *.yml roles/{local,cloud-*}/*/*.yml || true
28+
- name: Check Ansible playbook syntax
29+
run: ansible-playbook main.yml --syntax-check
3630

3731
basic-tests:
3832
name: Basic sanity tests

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ inventory_users
88
venvs/*
99
!venvs/.gitinit
1010
.vagrant
11+
.ansible/

.yamllint

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
extends: default
3+
4+
rules:
5+
line-length:
6+
max: 160
7+
level: warning
8+
comments:
9+
min-spaces-from-content: 1
10+
braces:
11+
max-spaces-inside: 1
12+
truthy:
13+
allowed-values: ['true', 'false', 'yes', 'no']

docs/linting.md

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
# Linting and Code Quality
2+
3+
This document describes the linting and code quality checks used in the Algo VPN project.
4+
5+
## Overview
6+
7+
The project uses multiple linters to ensure code quality across different file types:
8+
- **Ansible** playbooks and roles
9+
- **Python** library modules and tests
10+
- **Shell** scripts
11+
- **YAML** configuration files
12+
13+
## Linters in Use
14+
15+
### 1. Ansible Linting
16+
- **Tool**: `ansible-lint`
17+
- **Config**: `.ansible-lint`
18+
- **Checks**: Best practices, security issues, deprecated syntax
19+
- **Key Rules**:
20+
- `no-log-password`: Ensure passwords aren't logged
21+
- `no-same-owner`: File ownership should be explicit
22+
- `partial-become`: Avoid unnecessary privilege escalation
23+
24+
### 2. Python Linting
25+
- **Tools**:
26+
- `ruff` - Fast Python linter (replaces flake8, isort, etc.)
27+
- `black` - Code formatter
28+
- `mypy` - Type checker
29+
- `bandit` - Security linter
30+
- **Config**: `pyproject.toml`
31+
- **Style**: 120 character line length, Python 3.10+
32+
33+
### 3. Shell Script Linting
34+
- **Tool**: `shellcheck`
35+
- **Checks**: All `.sh` files in the repository
36+
- **Catches**: Common shell scripting errors and pitfalls
37+
38+
### 4. YAML Linting
39+
- **Tool**: `yamllint`
40+
- **Config**: `.yamllint`
41+
- **Rules**: Extended from default with custom line length
42+
43+
### 5. Security Scanning
44+
- **Tools**:
45+
- `bandit` - Python security issues
46+
- `safety` - Known vulnerabilities in dependencies
47+
- `zizmor` - GitHub Actions security (run separately)
48+
49+
## CI/CD Integration
50+
51+
### Main Workflow (`main.yml`)
52+
- **syntax-check**: Validates Ansible playbook syntax
53+
- **basic-tests**: Runs unit tests including validation tests
54+
55+
### Lint Workflow (`lint.yml`)
56+
Separate workflow with parallel jobs:
57+
- **ansible-lint**: Ansible best practices
58+
- **yaml-lint**: YAML formatting
59+
- **python-lint**: Python code quality
60+
- **shellcheck**: Shell script validation
61+
- **security-checks**: Security scanning
62+
63+
## Running Linters Locally
64+
65+
```bash
66+
# Ansible
67+
ansible-lint -v *.yml roles/{local,cloud-*}/*/*.yml
68+
69+
# Python
70+
ruff check .
71+
black --check .
72+
mypy library/
73+
74+
# Shell
75+
find . -name "*.sh" -exec shellcheck {} \;
76+
77+
# YAML
78+
yamllint .
79+
80+
# Security
81+
bandit -r library/
82+
safety check
83+
```
84+
85+
## Current Status
86+
87+
Most linters are configured to warn rather than fail (`|| true`) to allow gradual adoption. As code quality improves, these should be changed to hard failures.
88+
89+
### Known Issues to Address:
90+
1. Python library modules need formatting updates
91+
2. Some Ansible tasks missing `changed_when` conditions
92+
3. YAML files have inconsistent indentation
93+
4. Shell scripts could use more error handling
94+
95+
## Contributing
96+
97+
When adding new code:
98+
1. Run relevant linters before committing
99+
2. Fix any errors (not just warnings)
100+
3. Add linting exceptions only with good justification
101+
4. Update linter configs if adding new file types

pyproject.toml

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
[tool.ruff]
2+
# Ruff configuration
3+
target-version = "py310"
4+
line-length = 120
5+
select = [
6+
"E", # pycodestyle errors
7+
"W", # pycodestyle warnings
8+
"F", # pyflakes
9+
"I", # isort
10+
"B", # flake8-bugbear
11+
"C4", # flake8-comprehensions
12+
"UP", # pyupgrade
13+
]
14+
ignore = [
15+
"E501", # line too long (handled by formatter)
16+
]
17+
18+
[tool.black]
19+
line-length = 120
20+
target-version = ['py310']
21+
include = '\.pyi?$'
22+
extend-exclude = '''
23+
(
24+
/(
25+
\.eggs
26+
| \.git
27+
| \.mypy_cache
28+
| \.tox
29+
| \.venv
30+
| _build
31+
| buck-out
32+
| build
33+
| dist
34+
)/
35+
)
36+
'''
37+
38+
[tool.mypy]
39+
python_version = "3.10"
40+
warn_return_any = true
41+
warn_unused_configs = true
42+
disallow_untyped_defs = false
43+
ignore_missing_imports = true

0 commit comments

Comments
 (0)