Skip to content

chore: fix no-binding chart admission deployment - #3292

Open
AmberFryar wants to merge 6 commits into
mainfrom
fix-no-binding-chart-admission-deployment
Open

chore: fix no-binding chart admission deployment#3292
AmberFryar wants to merge 6 commits into
mainfrom
fix-no-binding-chart-admission-deployment

Conversation

@AmberFryar

@AmberFryar AmberFryar commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

The UDS Dependency Operator works when deployed from Pepr’s generated raw manifests, but fails when using pepr build -z chart.

The UDS Dependency Operator does not use standard Pepr bindings such as When(...).Watch(...), Mutate(...), or Validate(...). Instead, it starts lower-level Kubernetes watches from the Pepr runtime. The raw manifest generator handles this no-binding case by generating the admission controller deployment, but the Helm chart generator did not.

As a result, the generated chart set admission.enabled: true but did not write templates/admission-deployment.yaml, so no UDS Dependency Operator controller pods were created.

Changes

  • Align Helm chart admission deployment generation with the existing manifest generation behavior.
  • Generate admission-deployment.yaml when the module has admission bindings or has neither admission nor watcher bindings.
  • Keep webhook templates gated on actual mutating/validating webhook objects.
  • Add a regression test for no-binding modules using chart output....

End to End Test:
(See Pepr Excellent Examples)

Related Issue

Fixes #

Relates to #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@AmberFryar
AmberFryar requested a review from a team as a code owner August 26, 2026 23:35
@AmberFryar
AmberFryar marked this pull request as draft August 26, 2026 23:35
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR aligns Helm chart generation with raw-manifest generation for modules that have no standard bindings.

  • Uses one capability-derived predicate for both admission.enabled and admission deployment template generation.
  • Keeps mutation and validation webhook templates conditional on generated webhook objects.
  • Adds regression coverage for no-binding chart output and updates chart file-count expectations.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/lib/assets/assets.ts Aligns admission template emission with the same capability predicate used by Helm values and existing raw-manifest generation.
src/lib/assets/assets.test.ts Adds focused no-binding regression coverage and updates the expected chart-generation write count.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  C[Module capabilities] --> P{Admission bindings or neither admission nor watcher bindings?}
  P -->|Yes| V[Set admission.enabled]
  P -->|Yes| D[Write admission Deployment and ServiceMonitor templates]
  C --> W{Generated mutation or validation webhook?}
  W -->|Mutation| M[Write mutation webhook template]
  W -->|Validation| X[Write validation webhook template]
  W -->|Neither| N[Do not write webhook templates]
Loading

Reviews (2): Last reviewed commit: "Merge branch 'main' into fix-no-binding-..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 77.92%. Comparing base (eb70997) to head (d7baa04).

Files with missing lines Patch % Lines
src/lib/assets/assets.ts 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3292      +/-   ##
==========================================
+ Coverage   77.79%   77.92%   +0.12%     
==========================================
  Files          92       92              
  Lines        2545     2546       +1     
  Branches      544      545       +1     
==========================================
+ Hits         1980     1984       +4     
  Misses        439      439              
+ Partials      126      123       -3     
Files with missing lines Coverage Δ
src/lib/assets/assets.ts 93.33% <75.00%> (+3.44%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AmberFryar
AmberFryar marked this pull request as ready for review September 3, 2026 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant