From ea96a586785fa431a5dc27a7c212d1226ac7d519 Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Mon, 22 Dec 2025 21:34:40 +0100
Subject: [PATCH] fix(helm): address PR #447 reviewer feedback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Critical fix:
- deployment.yaml: Only reference OAuth credentials when clientId is set
- Fixes pod failure when using existingSecret without static OAuth creds
- Aligns deployment behavior with secret template logic

Previously, the deployment referenced OAuth credentials when either
clientId OR existingSecret was set. However, the secret template only
includes OAuth credentials when clientId is explicitly provided. This
caused pod failures when users provided an existingSecret for offline
access without static OAuth credentials (intending to use DCR).

The fix ensures OAuth env vars are only referenced when clientId is set,
matching the OAuth mode pattern and allowing DCR to work correctly with
existingSecret configurations.

Minor improvements:
- values.yaml: Clarify OAuth credentials are optional (uses DCR if not provided)

Testing verified all scenarios:
✅ Pass-through only (no offline access): No secrets/PVCs/OAuth vars
✅ Offline + DCR (no clientId): Secret with encryption key only, no OAuth vars
✅ Offline + static OAuth: Secret with all keys, OAuth vars present
✅ existingSecret without clientId: No auto secret, no OAuth vars (FIXED)

Resolves reviewer feedback from PR #447
---
 charts/nextcloud-mcp-server/templates/deployment.yaml | 2 +-
 charts/nextcloud-mcp-server/values.yaml               | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/charts/nextcloud-mcp-server/templates/deployment.yaml b/charts/nextcloud-mcp-server/templates/deployment.yaml
index 0d9cb03..705b7f1 100644
--- a/charts/nextcloud-mcp-server/templates/deployment.yaml
+++ b/charts/nextcloud-mcp-server/templates/deployment.yaml
@@ -100,7 +100,7 @@ spec:
                   key: {{ .Values.auth.multiUserBasic.tokenEncryptionKeyKey }}
             - name: NEXTCLOUD_OIDC_SCOPES
               value: {{ .Values.auth.multiUserBasic.scopes | quote }}
-            {{- if or .Values.auth.multiUserBasic.clientId .Values.auth.multiUserBasic.existingSecret }}
+            {{- if .Values.auth.multiUserBasic.clientId }}
             # Static OAuth credentials (optional - uses DCR if not provided)
             - name: NEXTCLOUD_OIDC_CLIENT_ID
               valueFrom:
diff --git a/charts/nextcloud-mcp-server/values.yaml b/charts/nextcloud-mcp-server/values.yaml
index b267507..3911c93 100644
--- a/charts/nextcloud-mcp-server/values.yaml
+++ b/charts/nextcloud-mcp-server/values.yaml
@@ -64,7 +64,7 @@ auth:
   # Server optionally stores app passwords for background operations
   multiUserBasic:
     # Enable offline access (background operations using app passwords via Astrolabe)
-    # When enabled, requires token encryption key and OAuth client credentials
+    # When enabled, requires token encryption key. OAuth client credentials are optional (uses DCR if not provided)
     enableOfflineAccess: false
     # Token encryption key (required if enableOfflineAccess: true, ignored if existingSecret is set)
     # Generate with: python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"