Skip to content

Conversation

@emsearcy
Copy link
Contributor

This client is not actually authenticated-to by v1-sync-helper, but that service, which impersonates authenticated users via its own Heimdall-JWT generation, may use this client ID as the authenticated principal in a bearer token if there is no relevant principal to impersonate for a given data update.

Also update Chart dependencies and bump chart version.

Copilot AI review requested due to automatic review settings November 19, 2025 20:46
@emsearcy emsearcy requested a review from a team as a code owner November 19, 2025 20:46
@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Walkthrough

Helm chart version bumped from 0.3.10 to 0.3.11. A new Authelia client configuration for v1_sync_helper (LFX Data Backfill Service) is added to values.yaml with client_credentials grant type, one_factor authorization policy, and client_secret_basic authentication, and registered in the client generation list.

Changes

Cohort / File(s) Summary
Helm chart version
charts/lfx-platform/Chart.yaml
Version updated from 0.3.10 to 0.3.11
Authelia client configuration
charts/lfx-platform/values.yaml
Added new Authelia client v1_sync_helper with configuration including client_secret path, non-public access, client_credentials grant type, one_factor policy, and client_secret_basic auth method; registered client in authelia_client_generation.clients list

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that the new Authelia client configuration follows the established pattern of existing clients (heimdall, m2m_test, lfx)
  • Confirm the client_secret path /secrets/authelia-clients-hashed/v1_sync_helper is correctly provisioned in the deployment environment
  • Validate that the audience URL and grant type align with the intended use case for the LFX Data Backfill Service

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add v1_sync_helper client' directly and concisely describes the main change - adding a new Authelia client entry.
Description check ✅ Passed The description explains the purpose of the v1_sync_helper client and mentions chart updates, which align with the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 168a3ba and 9b75fcd.

⛔ Files ignored due to path filters (1)
  • charts/lfx-platform/Chart.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • charts/lfx-platform/Chart.yaml (1 hunks)
  • charts/lfx-platform/values.yaml (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
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).
📚 Learning: 2025-08-29T16:53:12.710Z
Learnt from: mauriciozanettisalomao
Repo: linuxfoundation/lfx-v2-helm PR: 49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.710Z
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).

Applied to files:

  • charts/lfx-platform/Chart.yaml
🔇 Additional comments (3)
charts/lfx-platform/Chart.yaml (1)

8-8: Version bump is conservative and appropriate.

The patch-level increment (0.3.10 → 0.3.11) aligns with adding a new Authelia client configuration and maintains your preference for conservative version increments. Based on learnings

charts/lfx-platform/values.yaml (2)

502-514: New client configuration is consistent and complete.

The v1_sync_helper client follows the established M2M client pattern (mirroring m2m_test):

  • Secret path follows the convention used by other clients
  • Grant type and auth method are appropriate for service-to-service communication
  • Configuration aligns with the one_factor authorization policy for non-interactive flows

525-525: Client properly registered in generation list.

The v1_sync_helper client is correctly added to the authelia_client_generation.clients list, ensuring its secret will be generated alongside other configured clients during deployment.


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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new machine-to-machine Authelia client (v1_sync_helper) for the LFX Data Backfill Service, which will be used by the v1-sync-helper service as a fallback authentication principal when there's no relevant user to impersonate for data updates.

Key Changes:

  • Added new v1_sync_helper Authelia client with client_credentials grant type for service authentication
  • Included v1_sync_helper in the authelia_client_generation list for secret generation
  • Bumped chart version from 0.3.10 to 0.3.11 and updated Chart.lock dependencies

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
charts/lfx-platform/values.yaml Adds v1_sync_helper client configuration following M2M authentication patterns and includes it in client generation list
charts/lfx-platform/Chart.yaml Bumps chart version to 0.3.11 to reflect the new client addition
charts/lfx-platform/Chart.lock Updates dependency lock file with new digest and meeting-service version bump to 0.5.5

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This client is not actually authenticated-to by v1-sync-helper, but that
service, which impersonates authenticated users via its own Heimdall-JWT
generation, may use this client ID as the authenticated principal in a
bearer token if there is no relevant principal to impersonate for a
given data update.

Also update Chart dependencies and bump chart version.

Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
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