allegro / allegro/ralph

Hostname allocation race can return duplicate hostnames

Open
#3,950 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
2.5k
Forks
593
Avg merge
2d 9h
Merged PRs (30d)
6

Description

### Steps to reproduce

1. Configure a hostname prefix so that `AssetLastHostname.increment_hostname(prefix)` is used when assets are created.
2. Start with an existing `AssetLastHostname` row where `counter = N`.
3. Trigger two concurrent asset creations, or call `increment_hostname(prefix)` concurrently from two workers.
4. Let worker A perform the `F("counter") + 1` update first, then let worker B perform its update before worker A performs the follow-up `get(pk=obj.pk)` read.

### Expected behavior

Each concurrent allocation should receive a distinct hostname, for example `prefix-N+1` and `prefix-N+2`.

### Actual behavior

`increment_hostname()` performs an atomic increment and then a separate unlocked read of the row. If one worker reads after another worker has already committed a later increment, both workers can format and return the same final counter value.

Relevant code:

- `src/ralph/assets/models/assets.py:280-291` updates `counter` and then reads the row again.
- The method contains a `TODO` mentioning `select_for_update`, which is the missing serialization primitive here.

Minimal interleaving:

```text
Initial counter = N

Worker A: UPDATE counter = N + 1
Worker B: UPDATE counter = N + 2
Worker A: SELECT counter -> N + 2, returns hostname prefix-(N+2)
Worker B: SELECT counter -> N + 2, returns hostname prefix-(N+2)
```

### Environment

* Ralph version: current `main` at `bcf65b994ef29fb3fc2e10b660e6288723d5209e`
* Operating system: not OS-specific
* Method of installation: not installation-specific

### Impact

Concurrent asset creation can assign duplicate hostnames. If hostnames are used for inventory identity, DNS, automation, or access mapping, duplicate allocation can cause incorrect asset attribution or automation against the wrong host.

### Suggested fix

Serialize hostname allocation by locking the `AssetLastHostname` row inside a transaction, for example with `select_for_update()`, or by using a single database operation that atomically increments and returns the allocated value. A regression test should exercise concurrent allocations and assert that every returned hostname is unique.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.