Skip to content

v2026.4.20#37

Merged
josecsotomorales merged 2 commits intomainfrom
v2026.4.20
Apr 20, 2026
Merged

v2026.4.20#37
josecsotomorales merged 2 commits intomainfrom
v2026.4.20

Conversation

@josecsotomorales
Copy link
Copy Markdown
Member

No description provided.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 20, 2026

Greptile Summary

This release migrates TLS management from cert-manager (Let's Encrypt) to a Bring-Your-Own certificate model, bumps spark-operator to 2.5.0 and ingress-nginx to 4.15.1, adds a spark-operator webhook readiness pre-flight hook, and introduces configurable frontend rate limiting.

  • P1 — ModSecurity rule 14855 is a no-op: t:length is placed on the chained @unconditionalMatch rule rather than the rule that runs @gt 4096. Without the transformation on rule 1, REQUEST_URI_RAW evaluates to 0 numerically, so the URI-length DDoS block never fires.

Confidence Score: 4/5

Safe to merge after fixing the ModSecurity chain rule; all other changes are well-structured and well-tested.

One P1 finding: ModSecurity rule 14855 has t:length on the wrong rule in the chain, making the intended URI-length DDoS protection a no-op. The rest of the PR (BYO TLS refactor, spark webhook wait hook, dependency bumps, rate-limit configurability) is clean and backed by Helm unit tests.

charts/qualytics/templates/ingress.yaml — the common.modsecurity.snippet template's new rule 14855 needs the t:length transformation moved to the first rule.

Important Files Changed

Filename Overview
charts/qualytics/templates/ingress.yaml Adds BYO TLS secret resolution (three-tier precedence via coalesce), configurable frontend rate limits, and a new ModSecurity URI-length rule (id:14855) — but the chain rule has t:length in the wrong position, making the length check a no-op.
charts/qualytics/Chart.yaml Bumped chart version to 2026.4.20, appVersion to 2026.4.8; spark-operator updated from 2.3.0 to 2.5.0, ingress-nginx from 4.12.4 to 4.15.1; cert-manager dependency removed entirely.
charts/qualytics/templates/spark.yaml Adds a pre-flight wait hook (ServiceAccount, Role, RoleBinding, Job) that blocks the SparkApplication post-install/post-upgrade hook until the spark-operator webhook deployment is ready, preventing webhook endpoint errors on fresh installs.
charts/qualytics/values.yaml Adds ingress.tls.secretName / apiSecretName / frontendSecretName fields for BYO TLS; adds frontendMaxRequestsPerSecondPerIP, frontendMaxConnectionsPerIP, burstMultiplier rate-limit values; removes certmanager block.
CLAUDE.md Updated dependency and TLS documentation but the chart version reference (2025.10.17) was not updated to match Chart.yaml (2026.4.20).
docs/ingress-tls.md New documentation covering BYO TLS patterns: shared secret, per-ingress secrets, and upstream TLS termination, with rotation and troubleshooting guidance.
charts/qualytics/tests/ingress_test.yaml Comprehensive Helm unit tests added for TLS secret precedence (shared vs. per-ingress), rate limiting defaults, and removal of cert-manager annotation.
charts/qualytics/tests/spark_test.yaml Tests extended to cover the new pre-flight wait hook resources (5 documents expected), verifying hook weights, RBAC resources, and Job configuration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[helm upgrade --install] --> B{ingress.tls.secretName set?}
    B -- Yes --> C[Both API + Frontend Ingresses\nuse secretName]
    B -- No --> D{ingress.tls.apiSecretName set?}
    D -- Yes --> E[API Ingresses use apiSecretName\nFrontend uses frontendSecretName]
    D -- No --> F[API Ingresses use 'api-tls-cert'\nFrontend uses 'frontend-tls-cert']
    C --> G[nginx reads k8s TLS Secret\ncreated by operator]
    E --> G
    F --> G
    G --> H[HTTPS served to browser]

    subgraph "Spark Readiness (new in this PR)"
      S1[helm post-install hook] --> S2[spark-webhook-wait Job\nhook-weight -10]
      S2 --> S3{webhook deployment\nrollout ready?}
      S3 -- No, retry 5s --> S3
      S3 -- Yes --> S4[SparkApplication\nhook-weight 0]
    end
Loading

Reviews (1): Last reviewed commit: "Update v2026.4.20" | Re-trigger Greptile

Comment thread charts/qualytics/templates/ingress.yaml Outdated
Comment on lines +49 to +50
SecRule REQUEST_URI_RAW "@gt 4096" "id:14855,phase:1,deny,status:414,chain"
SecRule REQUEST_URI_RAW "@unconditionalMatch" "t:length"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 ModSecurity rule 14855 — t:length in wrong chain position, URI length check never fires

@gt is a numeric operator: it converts REQUEST_URI_RAW to an integer before comparing. Without t:length on rule 1, a typical URI string (e.g. /api/foo?...) converts to 0, so 0 > 4096 is always false, the chain never continues, and the intended DDoS block is a no-op.

The t:length must be placed on the same rule as the @gt 4096 comparison. The simplest fix:

Suggested change
SecRule REQUEST_URI_RAW "@gt 4096" "id:14855,phase:1,deny,status:414,chain"
SecRule REQUEST_URI_RAW "@unconditionalMatch" "t:length"
SecRule REQUEST_URI_RAW "@gt 4096" "id:14855,phase:1,deny,status:414,t:length"

Alternatively, preserve the chain pattern with the operator order reversed (unconditional match first, length check in chain rule):

SecRule REQUEST_URI_RAW "@unconditionalMatch" "id:14855,phase:1,chain"
SecRule REQUEST_URI_RAW "@gt 4096" "t:length,deny,status:414"

Comment thread CLAUDE.md Outdated
- **Infrastructure**: Ingress with ModSecurity WAF, BYO TLS certificates (customer-provided Secret), platform-specific storage classes (AWS/GCP/Azure)
- **Dependencies**: Spark Operator 2.5.0, nginx-ingress 4.15.1 (TLS is BYO — customer-provided `kubernetes.io/tls` Secret, see [docs/ingress-tls.md](docs/ingress-tls.md))

Chart version: **2025.10.17** (application type)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale chart version in CLAUDE.md

The chart version here still reads 2025.10.17 but Chart.yaml was updated to 2026.4.20 in this PR.

Suggested change
Chart version: **2025.10.17** (application type)
Chart version: **2026.4.20** (application type)

- ingress.yaml: place t:length on the same rule as @gt 4096 so the
  URI-length DDoS block actually fires. The previous chain evaluated
  @gt against the raw string (coerced to 0), making the rule a no-op.
- CLAUDE.md: bump chart version reference from 2025.10.17 to 2026.4.20
  to match Chart.yaml.

Both flagged by Greptile review on #37.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@josecsotomorales josecsotomorales merged commit 02efa1b into main Apr 20, 2026
2 checks passed
@josecsotomorales josecsotomorales deleted the v2026.4.20 branch April 20, 2026 21:00
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