Commit initial
This commit is contained in:
@@ -0,0 +1,356 @@
|
||||
# Phase 6: Wire Icon into .intunewin Export — Research
|
||||
|
||||
**Researched:** 2026-04-10
|
||||
**Domain:** Python file I/O, Peewee ORM query, .intunewin staging pipeline
|
||||
**Confidence:** HIGH
|
||||
|
||||
---
|
||||
|
||||
<phase_requirements>
|
||||
## Phase Requirements
|
||||
|
||||
| ID | Description | Research Support |
|
||||
|----|-------------|-----------------|
|
||||
| PKG-04 | User can upload a custom PNG icon for Intune app display (256x256, max 750KB) | Icon upload (icons.py + Icon model) is already complete. This phase wires the stored icon into the .intunewin build by copying it into the tmpdir before `build_intunewin()` is called. |
|
||||
</phase_requirements>
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
Phase 5 implemented icon upload and storage (`icons.py`, `Icon` model, SHA256-addressed
|
||||
file under `DATA_DIR/icons/`). The `get_intunewin_package()` endpoint in `packages.py`
|
||||
already builds a `.intunewin` file from a `TemporaryDirectory` staging area. The gap is
|
||||
that the endpoint never queries the `Icon` model, so the PNG never lands in the tmpdir and
|
||||
is never encrypted into the inner ZIP.
|
||||
|
||||
The work is minimal and mechanical: one `Icon.get_or_none()` lookup, one `shutil.copy2()`
|
||||
(or equivalent `open/write`) into `tmpdir`, and an integration test that verifies the icon
|
||||
is present inside the exported package. No schema changes, no new dependencies.
|
||||
|
||||
**Primary recommendation:** Query `Icon.get_or_none(Icon.printer == printer_id)` in
|
||||
`get_intunewin_package()`, and if an icon exists copy it into `tmpdir` as `icon.png`
|
||||
before calling `build_intunewin()`. Add one integration test that uploads an icon and
|
||||
verifies `icon.png` appears inside the `.intunewin` inner ZIP.
|
||||
|
||||
---
|
||||
|
||||
## Standard Stack
|
||||
|
||||
### Core (already in place — no new installs)
|
||||
|
||||
| Library | Version | Purpose | Why Standard |
|
||||
|---------|---------|---------|--------------|
|
||||
| peewee | 3.x | ORM query for Icon record | Already used throughout; `Icon.get_or_none()` follows established pattern |
|
||||
| Python shutil / pathlib | stdlib | Copy icon file from ICONS_DIR into tmpdir | Zero-dep, already used in codebase |
|
||||
| Pillow | current | Not needed here (already validated on upload) | Upload already guarantees PNG 256x256 |
|
||||
| zipfile | stdlib | Read inner ZIP of .intunewin to assert icon presence in tests | Already used in test_packages.py |
|
||||
|
||||
**Installation:** None required. All dependencies exist.
|
||||
|
||||
---
|
||||
|
||||
## Architecture Patterns
|
||||
|
||||
### Existing tmpdir staging pattern (packages.py lines 132-149)
|
||||
|
||||
```python
|
||||
with tempfile.TemporaryDirectory(prefix="imptune_") as tmpdir:
|
||||
# Write scripts
|
||||
with open(os.path.join(tmpdir, "install.ps1"), "w", encoding="utf-8") as f:
|
||||
f.write(install_script)
|
||||
# ... uninstall.ps1, detect.ps1 ...
|
||||
|
||||
# Extract driver ZIP into tmpdir/drivers/
|
||||
drivers_subdir = os.path.join(tmpdir, "drivers")
|
||||
os.makedirs(drivers_subdir, exist_ok=True)
|
||||
with zipfile.ZipFile(driver_zip_path, "r") as driver_zf:
|
||||
driver_zf.extractall(drivers_subdir)
|
||||
|
||||
# Build .intunewin
|
||||
build_intunewin(tmpdir, "install.ps1", output_path)
|
||||
```
|
||||
|
||||
The icon copy slots in immediately after driver extraction, before `build_intunewin()`.
|
||||
|
||||
### Pattern: icon lookup + conditional copy
|
||||
|
||||
```python
|
||||
import shutil
|
||||
from imptune.db.models import Icon
|
||||
|
||||
# Inside get_intunewin_package(), after driver validation:
|
||||
icon_record = Icon.get_or_none(Icon.printer == printer_id)
|
||||
if icon_record is not None:
|
||||
icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
|
||||
icon_dst = os.path.join(tmpdir, "icon.png")
|
||||
shutil.copy2(icon_src, icon_dst)
|
||||
```
|
||||
|
||||
**Key decision:** Copy is conditional — printers without an uploaded icon still export
|
||||
successfully. `shutil.copy2` preserves metadata and is the idiomatic stdlib copy call.
|
||||
|
||||
### Icon filename in tmpdir
|
||||
|
||||
Use `"icon.png"` as the fixed destination name regardless of `original_filename`. This is
|
||||
predictable for the test assertion and matches Intune's expectation for app icons (a
|
||||
well-known filename in the package root).
|
||||
|
||||
### Established ORM access pattern (from project decisions)
|
||||
|
||||
- Use `Model.get_or_none()` (not `Model.get()`) — avoids `DoesNotExist` exception
|
||||
- Test assertions use `list(Model.select().where(...))` not `Model.get()` — avoids Peewee
|
||||
cursor-caching issue with re-init'd DBs across tests (Phase 03 decision)
|
||||
|
||||
### Anti-Patterns to Avoid
|
||||
|
||||
- **Raising 422 when icon is absent:** Icon is optional. The export must succeed without one.
|
||||
- **Storing icon in a subdirectory:** `build_intunewin` walks `source_dir` recursively; any
|
||||
placement works, but root-level `icon.png` is simplest and most predictable.
|
||||
- **Re-validating the PNG in packages.py:** Validation already happened at upload time. Do
|
||||
not import Pillow into packages.py; just copy the bytes.
|
||||
|
||||
---
|
||||
|
||||
## Don't Hand-Roll
|
||||
|
||||
| Problem | Don't Build | Use Instead | Why |
|
||||
|---------|-------------|-------------|-----|
|
||||
| File copy into tmpdir | Custom open/read/write loop | `shutil.copy2()` | Handles edge cases, one line |
|
||||
| Icon existence check | Filesystem path probe | `Icon.get_or_none()` | Source of truth is the DB record, not filesystem |
|
||||
| Verify icon in .intunewin | Manual ZIP decryption | Read inner ZIP via `zipfile` after decoding (see test pattern below) | The test can open the outer ZIP and then the inner ZIP without decryption using stored bytes |
|
||||
|
||||
---
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Pitfall 1: ICONS_DIR not `cfg.ICONS_DIR`
|
||||
|
||||
**What goes wrong:** Hardcoding `DATA_DIR + "/icons"` instead of reading `cfg.ICONS_DIR`.
|
||||
**Why it happens:** `cfg.ICONS_DIR` was added in Phase 5 (config.py line 13); easy to miss.
|
||||
**How to avoid:** Always use `cfg.ICONS_DIR` — monkeypatched by `tmp_data_dir` fixture in tests.
|
||||
**Warning signs:** Integration test writes icon to `cfg.ICONS_DIR` but packages.py reads
|
||||
from a hardcoded path — icon silently missing in package.
|
||||
|
||||
### Pitfall 2: Icon copy placed after `build_intunewin()` call
|
||||
|
||||
**What goes wrong:** Icon is written to tmpdir but after `build_intunewin()` already ran;
|
||||
not included in the inner ZIP.
|
||||
**How to avoid:** Icon copy must come BEFORE `build_intunewin(tmpdir, ...)` call.
|
||||
|
||||
### Pitfall 3: Test verifying icon in outer ZIP instead of inner ZIP
|
||||
|
||||
**What goes wrong:** The `.intunewin` outer ZIP contains only `IntuneWinPackage/Contents/
|
||||
IntunePackage.intunewin` (encrypted blob) and `Detection.xml`. `icon.png` is NOT a member
|
||||
of the outer ZIP; it is packed into the inner ZIP (the plaintext content ZIP). A test that
|
||||
opens the outer ZIP and checks `namelist()` for `icon.png` will always fail.
|
||||
**How to avoid:** The integration test must decrypt (or use a pre-decryption approach — see
|
||||
Code Examples below) to verify icon presence. The simplest approach: use a test fixture
|
||||
that calls `build_intunewin` directly against a tmpdir so the inner ZIP can be inspected
|
||||
before encryption, OR read the Detection.xml and re-derive key material from a
|
||||
test-seeded build. Given current architecture uses random keys, the cleanest test strategy
|
||||
is:
|
||||
a) Make a real HTTP request through the test client to get the `.intunewin` bytes
|
||||
b) Use `build_intunewin`'s known structure: the inner ZIP is the AES-encrypted payload
|
||||
— cannot open it directly without the key.
|
||||
c) **Best approach:** Test that the icon file is present in `tmpdir` staging BEFORE
|
||||
`build_intunewin()` is called by exposing a helper, OR introduce a
|
||||
`build_intunewin` that accepts a pre-built inner ZIP for testability.
|
||||
d) **Pragmatic approach that avoids refactoring:** In the integration test, directly
|
||||
call the packages.py function with a monkeypatched `build_intunewin` that records
|
||||
what was in `tmpdir` instead of encrypting. This is the pattern to use.
|
||||
|
||||
**Recommended test strategy:** Monkeypatch `build_intunewin` to capture the staging
|
||||
directory contents, then assert `icon.png` is present among the staged files. This is
|
||||
simpler than decrypting the output and avoids coupling the test to the encryption
|
||||
implementation.
|
||||
|
||||
### Pitfall 4: `printer_id` vs ORM instance in Icon query
|
||||
|
||||
**What goes wrong:** `Icon.get_or_none(Icon.printer == printer)` (ORM instance) vs
|
||||
`Icon.get_or_none(Icon.printer == printer_id)` (integer). Both work in Peewee, but the
|
||||
integer form is more explicit and consistent with how icons.py deletes records
|
||||
(`Icon.delete().where(Icon.printer == printer_id)`).
|
||||
**How to avoid:** Use integer `printer_id` (the local variable from `_get_printer_and_driver`
|
||||
returns the printer ORM object; extract `.id` from it or use it directly — Peewee resolves
|
||||
FK equality either way, but be consistent with existing codebase style).
|
||||
|
||||
---
|
||||
|
||||
## Code Examples
|
||||
|
||||
### Icon lookup and conditional copy (insert inside `get_intunewin_package`)
|
||||
|
||||
```python
|
||||
# Source: derived from icons.py pattern + packages.py TemporaryDirectory pattern
|
||||
import shutil
|
||||
from imptune.db.models import Icon
|
||||
|
||||
# After extracting driver ZIP, before build_intunewin():
|
||||
icon_record = Icon.get_or_none(Icon.printer == printer.id)
|
||||
if icon_record is not None:
|
||||
icon_src = os.path.join(cfg.ICONS_DIR, icon_record.sha256)
|
||||
if os.path.isfile(icon_src):
|
||||
shutil.copy2(icon_src, os.path.join(tmpdir, "icon.png"))
|
||||
```
|
||||
|
||||
### Integration test — icon presence in staged files (monkeypatch approach)
|
||||
|
||||
```python
|
||||
# Source: pattern established by test_packages.py + test_icon_upload.py
|
||||
|
||||
def test_intunewin_includes_icon(client, setup_printer_with_driver, tmp_data_dir, monkeypatch):
|
||||
"""Icon PNG is included in .intunewin staging directory."""
|
||||
import io
|
||||
import os
|
||||
from PIL import Image
|
||||
from imptune.db.models import Icon
|
||||
import imptune.config as cfg
|
||||
import imptune.api.packages as pkg_module
|
||||
|
||||
printer, _ = setup_printer_with_driver
|
||||
|
||||
# Create and upload icon
|
||||
img = Image.new("RGBA", (256, 256), color="red")
|
||||
buf = io.BytesIO()
|
||||
img.save(buf, format="PNG")
|
||||
png_data = buf.getvalue()
|
||||
|
||||
resp = client.post(
|
||||
f"/printers/{printer.id}/icon",
|
||||
files={"file": ("icon.png", io.BytesIO(png_data), "image/png")},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
|
||||
# Capture staging contents via monkeypatched build_intunewin
|
||||
staged_files = []
|
||||
|
||||
def fake_build(source_dir, setup_file, output_path):
|
||||
staged_files.extend(os.listdir(source_dir))
|
||||
# Write a minimal valid file so endpoint can read it
|
||||
with open(output_path, "wb") as f:
|
||||
f.write(b"FAKE")
|
||||
|
||||
monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)
|
||||
|
||||
resp = client.get(f"/printers/{printer.id}/packages/intunewin")
|
||||
assert resp.status_code == 200
|
||||
assert "icon.png" in staged_files
|
||||
```
|
||||
|
||||
### Test: export succeeds with no icon (icon-free baseline)
|
||||
|
||||
```python
|
||||
def test_intunewin_without_icon_succeeds(client, setup_printer_with_driver, monkeypatch):
|
||||
"""Export succeeds even when no icon has been uploaded."""
|
||||
import imptune.api.packages as pkg_module
|
||||
import os
|
||||
|
||||
printer, _ = setup_printer_with_driver
|
||||
|
||||
def fake_build(source_dir, setup_file, output_path):
|
||||
with open(output_path, "wb") as f:
|
||||
f.write(b"FAKE")
|
||||
|
||||
monkeypatch.setattr(pkg_module, "build_intunewin", fake_build)
|
||||
|
||||
resp = client.get(f"/printers/{printer.id}/packages/intunewin")
|
||||
assert resp.status_code == 200
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## State of the Art
|
||||
|
||||
| Old Approach | Current Approach | When Changed | Impact |
|
||||
|--------------|------------------|--------------|--------|
|
||||
| icon stored as column in Printer | Separate `Icon` model with FK, SHA256-addressed on disk | Phase 5 | Clean separation; icon is optional; no schema migration needed here |
|
||||
| build_intunewin reads arbitrary files | build_intunewin walks entire tmpdir | Phase 5 | Any file placed in tmpdir automatically included in inner ZIP |
|
||||
|
||||
---
|
||||
|
||||
## Open Questions
|
||||
|
||||
1. **Should missing icon file on disk (orphaned DB record) be silently skipped or 422?**
|
||||
- What we know: `icon_record` can exist in DB but file might be absent (disk corruption,
|
||||
manual cleanup). Current `icons.py` does not handle this case either.
|
||||
- Recommendation: Silent skip (don't copy if `os.path.isfile(icon_src)` is False) —
|
||||
consistent with the optional nature of icons and avoids breaking exports for stale records.
|
||||
|
||||
2. **Is there any Intune-specific requirement for the icon filename or location inside .intunewin?**
|
||||
- What we know: The `.intunewin` package is a deployment container; Intune does NOT
|
||||
read the icon from within the .intunewin file. The icon is set separately in the
|
||||
Intune portal during app creation. The PKG-04 requirement says "included in the
|
||||
.intunewin package" as a convenience/archive — not as an Intune-interpreted artifact.
|
||||
- Confidence: MEDIUM — based on reverse-engineering of .intunewin format. The
|
||||
requirement text "Intune displays it as the app icon" describes the end-state in
|
||||
Intune, not a technical requirement for the .intunewin binary.
|
||||
- Recommendation: Include the icon in the package root as `icon.png` for user
|
||||
convenience (they can extract it when uploading manually to Intune portal).
|
||||
|
||||
---
|
||||
|
||||
## Validation Architecture
|
||||
|
||||
### Test Framework
|
||||
|
||||
| Property | Value |
|
||||
|----------|-------|
|
||||
| Framework | pytest (detected: pytest.ini or pyproject.toml implicit) |
|
||||
| Config file | none detected — runs via `python -m pytest` |
|
||||
| Quick run command | `python -m pytest tests/test_packages.py -x -q` |
|
||||
| Full suite command | `python -m pytest tests/ -q` |
|
||||
|
||||
### Phase Requirements -> Test Map
|
||||
|
||||
| Req ID | Behavior | Test Type | Automated Command | File Exists? |
|
||||
|--------|----------|-----------|-------------------|-------------|
|
||||
| PKG-04 | Icon PNG is copied into .intunewin staging dir when icon exists | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — test_packages.py exists but no icon-in-package test yet |
|
||||
| PKG-04 | Export succeeds when no icon uploaded | integration | `python -m pytest tests/test_packages.py::TestIntunewinDownload -x -q` | Partial — baseline export tests exist |
|
||||
|
||||
### Sampling Rate
|
||||
|
||||
- **Per task commit:** `python -m pytest tests/test_packages.py -x -q`
|
||||
- **Per wave merge:** `python -m pytest tests/ -q`
|
||||
- **Phase gate:** Full suite green before `/gsd:verify-work`
|
||||
|
||||
### Wave 0 Gaps
|
||||
|
||||
- [ ] `tests/test_packages.py` — add `test_intunewin_includes_icon` test (monkeypatch `build_intunewin`)
|
||||
- [ ] `tests/test_packages.py` — add `test_intunewin_without_icon_succeeds` test
|
||||
|
||||
*(Existing test infrastructure covers the rest — no new files or fixtures needed)*
|
||||
|
||||
---
|
||||
|
||||
## Sources
|
||||
|
||||
### Primary (HIGH confidence)
|
||||
|
||||
- Direct code reading: `imptune/api/packages.py` — full `get_intunewin_package()` implementation
|
||||
- Direct code reading: `imptune/api/icons.py` — Icon upload, SHA256 storage path, `cfg.ICONS_DIR`
|
||||
- Direct code reading: `imptune/db/models.py` — `Icon` model, FK to `Printer`, `sha256` field
|
||||
- Direct code reading: `imptune/config.py` — `ICONS_DIR = str(Path(DATA_DIR) / "icons")`
|
||||
- Direct code reading: `imptune/generators/intunewin_builder.py` — `build_intunewin` walks entire `source_dir`
|
||||
- Direct code reading: `tests/conftest.py` — `tmp_data_dir` monkeypatches `cfg.ICONS_DIR`
|
||||
- Direct code reading: `tests/test_packages.py` — existing test patterns (fixtures, monkeypatch style)
|
||||
- Direct code reading: `.planning/STATE.md` — Phase 5 decisions on ICONS_DIR, Icon model, shutil pattern
|
||||
|
||||
### Secondary (MEDIUM confidence)
|
||||
|
||||
- Intune .intunewin format: icon is NOT read from within the package by Intune — icons are
|
||||
set in the Intune portal. Including it in the package is for user convenience only.
|
||||
(Based on reverse-engineering notes in `intunewin_builder.py` comments + svrooij.io reference)
|
||||
|
||||
---
|
||||
|
||||
## Metadata
|
||||
|
||||
**Confidence breakdown:**
|
||||
- Standard stack: HIGH — all libraries already in codebase, no new dependencies
|
||||
- Architecture: HIGH — single code location change (packages.py), clear insertion point
|
||||
- Pitfalls: HIGH — inner-ZIP test trap verified by reading intunewin_builder.py structure
|
||||
- Test strategy: HIGH — monkeypatch pattern already established in existing test suite
|
||||
|
||||
**Research date:** 2026-04-10
|
||||
**Valid until:** Stable — no moving parts (stdlib + existing codebase only)
|
||||
Reference in New Issue
Block a user