Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/lfx-platform/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ apiVersion: v2
name: lfx-platform
description: LFX Platform v2 Helm chart
type: application
version: 0.2.5
version: 0.2.7
Comment thread
mauriciozanettisalomao marked this conversation as resolved.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 29, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Bump chart version to at least 0.3.0 (behavior change, not a patch).

The PR adds middleware and enforces JSON via Heimdall authorizers—this alters runtime behavior and can break existing clients. Under SemVer (even pre-1.0), features and behavior changes warrant a minor bump; breaking defaults may justify a major. Patch (0.2.7) is too low.

Apply:

-version: 0.2.7
+version: 0.3.0

Also:

  • Add/adjust CHANGELOG and upgrade notes (call out JSON Content-Type enforcement).
  • Confirm whether the new authorizer is opt-in; if default-on and breaking, consider 1.0.0.

🏁 Script executed:

#!/bin/bash
set -e
# Search for Heimdall references in the codebase
rg -n 'Heimdall' || echo "No Heimdall reference found"
# Search for authorizer usage
rg -n 'authorizer' || echo "No authorizer reference found"
# Search for JSON enforcement
rg -n 'JSON' || echo "No JSON reference found"
# Search for middleware directory or usages
rg -n 'middleware' || echo "No middleware reference found"

Length of output: 3257


#!/bin/bash
set -e
# Show the first 50 lines of Chart.yaml to verify version and metadata
sed -n '1,50p' charts/lfx-platform/Chart.yaml

🏁 Script executed:

#!/bin/bash
set -e
# Display Chart.yaml header to confirm current version
sed -n '1,50p' charts/lfx-platform/Chart.yaml
# Display values.yaml around authorizers to check defaults
sed -n '160,220p' charts/lfx-platform/values.yaml
sed -n '240,300p' charts/lfx-platform/values.yaml

Length of output: 5479


Bump version in Chart.yaml to 0.3.0

  • Update CHANGELOG and UPGRADE notes to document the new JSON Content-Type enforcement
  • If the JSON authorizer is default-on and breaks existing clients, consider a 1.0.0 release
🤖 Prompt for AI Agents
charts/lfx-platform/Chart.yaml lines 8-8: bump the chart version from 0.2.7 to
0.3.0 and update release notes; change the version field to 0.3.0, add entries
to CHANGELOG and UPGRADE notes documenting the new enforcement of JSON
Content-Type and migration steps for clients, and if the JSON authorizer will be
enabled by default and breaks existing clients, instead plan a major 1.0.0
release (or document how to opt-out) so consumers are not unexpectedly broken.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For now, I'll leave it as is.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@mauriciozanettisalomao Understood! I'll respect your decision to keep the version at 0.2.7. You know your project's versioning strategy best.


✏️ Learnings added
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.674Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

icon: https://github.com/linuxfoundation/lfx-v2-helm/raw/main/img/lfx-logo-color.svg
dependencies:
- name: traefik
Expand Down
20 changes: 20 additions & 0 deletions charts/lfx-platform/templates/heimdall/middleware.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,26 @@
---
{{ if and .Values.heimdall.enabled (or
.Values.gateway.enabled .Values.lfx.parentGateway.enabled) -}}
# Heimdall middleware with body forwarding capability
# This is the default middleware that should be used in most cases, particularly
# when parentRef requiring authentication is in the request body.
# Note: For routes handling very large payloads (like file uploads), consider using
# the lighter-weight middleware below to reduce overhead.
apiVersion: traefik.io/v1alpha1
Comment thread
mauriciozanettisalomao marked this conversation as resolved.
kind: Middleware
metadata:
name: heimdall-forward-body
namespace: {{ .Release.Namespace }}
spec:
forwardAuth:
address: "http://{{ include "heimdall.fullname" .Subcharts.heimdall }}.{{ .Release.Namespace }}:{{ .Values.heimdall.service.main.port }}"
authResponseHeaders:
- Authorization
forwardBody: true
---
Comment thread
mauriciozanettisalomao marked this conversation as resolved.
# Alternative Heimdall middleware without body forwarding
# Use this middleware only for routes where body inspection isn't required for authentication
# and when dealing with large payloads where forwarding the entire body would be inefficient.
apiVersion: traefik.io/v1alpha1
kind: Middleware
metadata:
Expand Down
7 changes: 7 additions & 0 deletions charts/lfx-platform/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ heimdall:
type: allow
- id: deny_all
type: deny
- id: json_content_type
type: cel
config:
expressions:
- expression: |
Request.Header("Content-Type") == "application/json"
message: "Content-Type must be application/json"
Comment thread
mauriciozanettisalomao marked this conversation as resolved.
- id: openfga_check
type: remote
config:
Expand Down