Skip to content

feat(k8s): accept image_pull_policy in the create payload - #122

Merged
oleksandr-nc merged 1 commit into
mainfrom
feat/k8s-image-pull-policy
Sep 25, 2026
Merged

oleksandr-nc merged 1 commit into
mainfrom
feat/k8s-image-pull-policy

Conversation

@oleksandr-nc

Copy link
Copy Markdown
Contributor

AppAPI deploy daemons can map a registry to the special target local, which on Docker keeps the image name and skips the pull. On Kubernetes HaRP always created the ExApp Deployment with imagePullPolicy: IfNotPresent, so there was no way to express "use only the image that is already on the node".

Changes

  • CreateExAppPayload accepts an optional image_pull_policy (IfNotPresent, Never or Always, default IfNotPresent), and the Kubernetes Deployment manifest uses it. The Docker backend ignores the field.
  • The pod wait loop also fails fast on ErrImageNeverPull, so a Never policy without the image on the node is reported within seconds instead of after the startup timeout.

Related: nextcloud/app_api#1046 sends image_pull_policy: Never for a local registry mapping. Older HaRP versions ignore the field and keep the current behaviour.

Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 35100370-0555-4f5e-b898-fd2b1452a8e3

📥 Commits

Reviewing files that changed from the base of the PR and between 618751a and e1102ba.

📒 Files selected for processing (1)
  • haproxy_agent.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

CreateExAppPayload now accepts IfNotPresent, Never, or Always as the image pull policy, defaulting to IfNotPresent. Kubernetes Deployment manifests use the selected policy. Startup polling also recognizes ErrImageNeverPull as an image-pull error.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e1102

The change applies the selected Kubernetes image pull policy and reports missing images under Never without waiting for the startup timeout. No concrete merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to e1102

Existing requests retain their prior behavior, but callers can now select image-loading behavior that affects which image runs and what remains deployed after a startup failure. The impact depends on image identity rules and cleanup behavior that are not established here.

Retained concerns

  • Medium · security · inferred: If callers use mutable image tags, the new Always option can run different registry content on a later pod start without changing the requested image reference. Whether callers pin images or cluster policy constrains this path is unknown.
  • Low · reliability · inferred: A Never image-missing failure is reported without changing the Deployment's desired replica count. Unless an external caller performs cleanup, a failed rollout remains desired and may later run if the image becomes available; the same lack of automatic cleanup predates this PR for other pull errors.
Security review details

Security Blast Radius

  • inferred — The changed choice affects ExApp containers created through the configured Kubernetes namespace. It does not make namespace selection or Kubernetes credentials request-controlled; effective exposure depends on which callers can create deployments and on cluster policy.

Security Findings and Attack Paths

  • inferred — If an authorized caller supplies a mutable tag with Always, registry content changed before a later pod start can change the code executed under the same image reference. This is conditional, not a verified exploit: tag usage and admission controls are unknown, and the prior policy could also pull when no matching image was cached.

Trust Boundaries and Controls

  • observed — The AppAPI forwarding path checks harp-shared-key, payload validation restricts policy values, and Kubernetes requests use the configured bearer token and namespace. The change does not modify those controls.

Resilience and Maintainability Implications

  • inferred — Fail-fast reporting for Never improves detection, but does not establish a terminal stopped state. Security-sensitive rollback depends on the external caller's response to failure, which was not available to verify.

Hardening Proposals

  • proposed — For deployments requiring stable image identity, establish digest pinning or an equivalent cluster-enforced provenance policy across all three pull modes.
  • proposed — Define and verify the caller's stop or removal behavior after a failed Never rollout, including interrupted responses and retries.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the new image_pull_policy field, Kubernetes behavior, Docker backend behavior, and ErrImageNeverPull handling.
Title check ✅ Passed The title clearly and concisely identifies the main change: accepting image_pull_policy in the Kubernetes create payload.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@oleksandr-nc
oleksandr-nc merged commit 6fb0e70 into main Sep 25, 2026
17 checks passed
@oleksandr-nc
oleksandr-nc deleted the feat/k8s-image-pull-policy branch September 25, 2026 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant