Take the firmware repository endpoint from the environment #77
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/firmware-repository-setting"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The release endpoint was hardcoded in
updater/firmware.py. It now comes fromFIRMWARE_REPOSITORY, read inSettings.from_environmentthe way the registrytoken already is, and wired through
compose.yaml. The old URL is the defaultwhen the variable is unset or empty, so an existing van needs nothing added.
Settings.describenow names the endpoint, since it decides whether a van eversees a release and the startup line only said whether a token was set. The
control API's on-demand check gets the same repository as the poll, so the
check-now button cannot look somewhere else.
FIRMWARE_CACHE,CONTROL_API_*andNODE_FILE_SERVER_PORTare untouched.Tested by extending the existing settings and polling tests: the environment is
read, an empty value falls back,
describenames the endpoint, and the poll andthe control API both ask the repository the settings name.
CCS-UHA-21.
FIRMWARE_REPOSITORY=http://…now sends the registry token in clear, and a trailing slash breaks the URL. Both want handling inSettings.from_environment.@ -234,1 +234,3 @@firmware registry wants one; it caches into `updater-data/firmware`. It holdsfirmware registry wants one, and `FIRMWARE_REPOSITORY` to ask somewhere otherthan the firmware repository's own API; it caches into`updater-data/firmware`. It holdsThe rewrap leaves a stub line ("
updater-data/firmware. It holds"); reflow the paragraph.@ -34,11 +40,16 @@ class Settings:return cls(firmware_cache=FIRMWARE_CACHE,registry_token=source.get("FIRMWARE_REGISTRY_TOKEN") or "",repository=source.get("FIRMWARE_REPOSITORY") or DEFAULT_FIRMWARE_REPOSITORY,The token guard in
ForgejoReleases.requestmatches scheme and host againstself.repository, so a repository set tohttp://…matches itself and the token goes out as a plaintext header — the thing that guard exists to stop. While the endpoint was a constant this could not happen. Either fall back to the default when the value is nothttps://, or make the token conditional on an https destination.A value with a trailing slash gives
https://host/api/v1/repos/rob/van//releases/latest..rstrip("/")here.All three findings are taken. One thing left:
d92a5f0as pushed fails mypy —tests/test_control_api.py:783: "ReleaseSource" has no attribute "repository". Theisinstance(releases, ForgejoReleases)narrowing that fixes it is sitting uncommitted in the worktree. Commit and push it.One gap left. Nothing asserts the token is sent to a repository other than the default: replacing
self.repositorywithDEFAULT_FIRMWARE_REPOSITORYinForgejoReleases.requestkeeps all 1120 tests green, so a van pointed at a private HTTPS mirror would silently go unauthenticated and the suite would not notice. Add the positive case next totest_the_token_stays_off_a_repository_set_to_plain_http:ForgejoReleases("secret", "https://mirror.invalid/api/v1/repos/rob/van").request(".../releases/latest").get_header("Authorization") == "token secret".@ -346,0 +351,4 @@assert releases.request(f"{over_plain_http}/releases/latest").headers == {}Only the negative half of the new behaviour is covered. Pinning the header check to
DEFAULT_FIRMWARE_REPOSITORYinstead ofself.repositorypasses the whole suite, so add the matching positive case: a custom HTTPS repository must still gettoken secret.The round-three gap is closed. Two small ones left, neither blocking.
@ -325,3 +327,3 @@token: str = ""repository: str = FIRMWARE_REPOSITORYrepository: str = DEFAULT_FIRMWARE_REPOSITORYBoth production call sites now pass a repository, so this default only serves the tests, and it silently points the next caller at the firmware repository instead of failing. Worth making required.
@ -339,2 +342,2 @@urllib.request.Request(self.repository)):registry = _destination(urllib.request.Request(self.repository))if self.token and registry[0] == "https" and _destination(request) == registry:Host matching is case-sensitive, and the repository is user-typed now:
FIRMWARE_REPOSITORY=https://Git.Robware.uk/api/v1/repos/rob/CampervanControlSystemsdrops the token, so a private mirror answers 401 and the only trace isNo firmware release found. Lower-case the host in_destination.Same line of defence in
Settings.from_environment:.strip()alongside the existingrstrip("/"). A value with a stray space fails every check withunknown url type.Round four taken in
e9e3928:_destinationlower-cases the host, andfrom_environmentstrips whitespace as well as a trailing slash. Both have a test.Leaving
ForgejoReleases.repositorywith its default, though. It is the same defaultSettings.repositorycarries, so it cannot point a caller anywhere the settings would not, and making it required means reordering the dataclass —token: str = ""cannot precede a field with no default — and rewriting every positionalForgejoReleases("secret")in the tests for no change in behaviour. Not worth the churn on this branch.Two minor points, neither blocking.
@ -234,3 +234,1 @@firmware registry wants one; it caches into `updater-data/firmware`. It holdsno CAN interface of its own: until the integration names one it has no bus,reports `connected: false`, and answers every install `no_link`.firmware registry wants one, and `FIRMWARE_REPOSITORY` to point it at an APIThe value has to be the Forgejo API base —
https://host/api/v1/repos/<owner>/<repo>— and pasting the repository's web URL instead fails silently asNo firmware release found. Worth showing the shape once, here or as a comment in compose.yaml.@ -394,0 +397,4 @@urllib lower-cases a scheme but not a host, and the repository is typed byhand now, so a capital in it would otherwise be a different destination."""return request.type, request.host.lower()request.hostisNonewhen a URL has a scheme but no//, sohttps:/git.invalid/api/v1/repos/rob/van(one slash, an easy typo now it is hand-typed) raisesAttributeErrorhere._checkcatches onlyOSErrorandValueError, so the poll logs it but check-now escapes the handler and the caller gets a closed connection instead ofregistry_reached: false.(request.host or "").lower()covers it.