Merge pull request #164 from langchain-ai/brian/ls-2216-prevent-dupli…

…cate-env-vars feat: prevent duplicate env var keys
langchain-ai · Oct 28, 2024 · a4ef7ae · a4ef7ae
2 parents f894a51 + ef0030f
commit a4ef7ae
Show file tree

Hide file tree

Showing 14 changed files with 172 additions and 119 deletions.
diff --git a/charts/langsmith/Chart.yaml b/charts/langsmith/Chart.yaml
@@ -5,5 +5,5 @@ maintainers:
     email: [email protected]
 description: Helm chart to deploy the langsmith application and all services it depends on.
 type: application
-version: 0.7.10
+version: 0.7.11
 appVersion: "0.7.45"
diff --git a/charts/langsmith/README.md b/charts/langsmith/README.md
@@ -1,6 +1,6 @@
 # langsmith
 
-![Version: 0.7.10](https://img.shields.io/badge/Version-0.7.10-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.7.45](https://img.shields.io/badge/AppVersion-0.7.45-informational?style=flat-square)
+![Version: 0.7.11](https://img.shields.io/badge/Version-0.7.11-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 0.7.45](https://img.shields.io/badge/AppVersion-0.7.45-informational?style=flat-square)
 
 Helm chart to deploy the langsmith application and all services it depends on.
 

diff --git a/charts/langsmith/templates/_helpers.tpl b/charts/langsmith/templates/_helpers.tpl
@@ -116,6 +116,32 @@ the user or some other secret provisioning mechanism
 {{- end }}
 {{- end }}
 
+{{/* Include these env vars if they aren't defined in .Values.commonEnv */}}
+{{- define "langsmith.conditionalEnvVars" -}}
+- name: X_SERVICE_AUTH_JWT_SECRET
+  valueFrom:
+    secretKeyRef:
+      name: {{ include "langsmith.secretsName" . }}
+      key: api_key_salt
+{{- end }}
+{{- define "langsmith.conditionalEnvVarsResolved" -}}
+  {{- $commonEnvKeys := list -}}
+  {{- range $i, $commonEnvVar := .Values.commonEnv -}}
+    {{- $commonEnvKeys = append $commonEnvKeys $commonEnvVar.name -}}
+  {{- end -}}
+
+  {{- $resolvedEnvVars := list -}}
+  {{- range $i, $envVar := include "langsmith.conditionalEnvVars" . | fromYamlArray }}
+    {{- if not (has $envVar.name $commonEnvKeys) }}
+      {{- $resolvedEnvVars = append $resolvedEnvVars $envVar -}}
+    {{- end }}
+  {{- end }}
+
+  {{- if gt (len $resolvedEnvVars) 0 -}}
+    {{ $resolvedEnvVars | toYaml }}
+  {{- end -}}
+{{- end }}
+
 
 {{/*
 Template containing common environment variables that are used by several services.
@@ -227,11 +253,6 @@ Template containing common environment variables that are used by several servic
   value: http://{{- include "langsmith.fullname" . }}-{{.Values.platformBackend.name}}:{{ .Values.platformBackend.service.port }}
 - name: SMITH_BACKEND_ENDPOINT
   value: http://{{- include "langsmith.fullname" . }}-{{.Values.backend.name}}:{{ .Values.backend.service.port }}
-- name: X_SERVICE_AUTH_JWT_SECRET
-  valueFrom:
-    secretKeyRef:
-      name: {{ include "langsmith.secretsName" . }}
-      key: api_key_salt
 {{- if .Values.config.ttl.enabled }}
 - name: FF_TRACE_TIERS_ENABLED
   value: {{ .Values.config.ttl.enabled | quote }}
@@ -268,6 +289,7 @@ Template containing common environment variables that are used by several servic
 {{- end }}
 - name: FF_CH_SEARCH_ENABLED
   value: {{ .Values.config.blobStorage.chSearchEnabled | quote }}
+{{ include "langsmith.conditionalEnvVarsResolved" . }}
 {{- end }}
 
 {{- define "backend.serviceAccountName" -}}
@@ -333,3 +355,27 @@ Template containing common environment variables that are used by several servic
     {{ default "default" .Values.redis.serviceAccount.name }}
 {{- end -}}
 {{- end -}}
+
+{{/* Fail on duplicate keys in the inputted list of environment variables */}}
+{{- define "langsmith.detectDuplicates" -}}
+{{- $inputList := . -}}
+{{- $keyCounts := dict -}}
+{{- $duplicates := list -}}
+
+{{- range $i, $val := $inputList }}
+  {{- $key := $val.name -}}
+  {{- if hasKey $keyCounts $key }}
+    {{- $_ := set $keyCounts $key (add (get $keyCounts $key) 1) -}}
+  {{- else }}
+    {{- $_ := set $keyCounts $key 1 -}}
+  {{- end }}
+  {{- if gt (get $keyCounts $key) 1 }}
+    {{- $duplicates = append $duplicates $key -}}
+  {{- end }}
+{{- end }}
+
+{{- if gt (len $duplicates) 0 }}
+  {{ fail (printf "Duplicate keys detected: %v" $duplicates) }}
+{{- end }}
+{{- end -}}
+
diff --git a/charts/langsmith/templates/backend/auth-bootstrap.yaml b/charts/langsmith/templates/backend/auth-bootstrap.yaml
@@ -1,4 +1,15 @@
+{{- define "authBootstrapEnvVars" -}}
+- name: INITIAL_ORG_ADMIN_EMAIL
+  value: {{ .Values.config.basicAuth.initialOrgAdminEmail }}
+- name: INITIAL_ORG_ADMIN_PASSWORD
+  valueFrom:
+    secretKeyRef:
+      name: {{ include "langsmith.secretsName" . }}
+      key: initial_org_admin_password
+{{- end -}}
 {{- if .Values.config.basicAuth.enabled }}
+{{- $envVars := concat (include "langsmith.commonEnv" . | fromYamlArray) (include "authBootstrapEnvVars" . | fromYamlArray) .Values.backend.authBootstrap.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: batch/v1
 kind: Job
 metadata:
@@ -47,15 +58,7 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- include "langsmith.commonEnv" . | nindent 12 }}
-            - name: INITIAL_ORG_ADMIN_EMAIL
-              value: {{ .Values.config.basicAuth.initialOrgAdminEmail }}
-            - name: INITIAL_ORG_ADMIN_PASSWORD
-              valueFrom:
-                secretKeyRef:
-                  name: {{ include "langsmith.secretsName" . }}
-                  key: initial_org_admin_password
-            {{- with .Values.backend.authBootstrap.extraEnv }}
+            {{- with $envVars }}
               {{- toYaml . | nindent 12 }}
             {{- end }}
           envFrom:

diff --git a/charts/langsmith/templates/backend/clickhouse-migrations.yaml b/charts/langsmith/templates/backend/clickhouse-migrations.yaml
@@ -1,4 +1,6 @@
 {{- if .Values.backend.clickhouseMigrations.enabled }}
+{{- $envVars := concat (include "langsmith.commonEnv" . | fromYamlArray) .Values.backend.clickhouseMigrations.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: batch/v1
 kind: Job
 metadata:
@@ -47,8 +49,7 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- include "langsmith.commonEnv" . | nindent 12 }}
-            {{- with .Values.backend.clickhouseMigrations.extraEnv }}
+            {{- with $envVars }}
               {{- toYaml . | nindent 12 }}
             {{- end }}
           envFrom:

diff --git a/charts/langsmith/templates/backend/deployment.yaml b/charts/langsmith/templates/backend/deployment.yaml
@@ -1,3 +1,13 @@
+{{- define "backendEnvVars" -}}
+- name: PORT
+  value: {{ .Values.backend.containerPort | quote }}
+{{- with .Values.ingress.subdomain }}
+- name: DOCS_PREFIX
+  value: /{{ . }}/api
+{{- end }}
+{{- end -}}
+{{- $envVars := concat .Values.commonEnv (include "langsmith.commonEnv" . | fromYamlArray) (include "backendEnvVars" . | fromYamlArray) .Values.backend.deployment.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -50,19 +60,9 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- include "langsmith.commonEnv" . | nindent 12 }}
-            - name: PORT
-              value: {{ .Values.backend.containerPort | quote }}
-            {{- with .Values.ingress.subdomain }}
-            - name: DOCS_PREFIX
-              value: /{{ . }}/api
-            {{- end }}
-            {{- with .Values.backend.deployment.extraEnv }}
+            {{- with $envVars }}
               {{- toYaml . | nindent 12 }}
             {{- end }}
-            {{- with .Values.commonEnv }}
-              {{ toYaml . | nindent 12 }}
-            {{- end }}
           envFrom:
             - configMapRef:
                 name: {{ include "langsmith.fullname" . }}-config

diff --git a/charts/langsmith/templates/backend/postgres-migrations.yaml b/charts/langsmith/templates/backend/postgres-migrations.yaml
@@ -1,4 +1,6 @@
 {{- if .Values.backend.migrations.enabled }}
+{{- $envVars := concat (include "langsmith.commonEnv" . | fromYamlArray) .Values.backend.migrations.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: batch/v1
 kind: Job
 metadata:
@@ -47,8 +49,7 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- include "langsmith.commonEnv" . | nindent 12 }}
-            {{- with .Values.backend.migrations.extraEnv }}
+            {{- with $envVars }}
               {{- toYaml . | nindent 12 }}
             {{- end }}
           envFrom:

diff --git a/charts/langsmith/templates/clickhouse/stateful-set.yaml b/charts/langsmith/templates/clickhouse/stateful-set.yaml
@@ -1,4 +1,23 @@
+{{- define "clickhouseEnvVars" -}}
+- name: CLICKHOUSE_DB
+  valueFrom:
+    secretKeyRef:
+      name: {{ include "langsmith.clickhouseSecretsName" . }}
+      key: clickhouse_db
+- name: CLICKHOUSE_USER
+  valueFrom:
+    secretKeyRef:
+      name: {{ include "langsmith.clickhouseSecretsName" . }}
+      key: clickhouse_user
+- name: CLICKHOUSE_PASSWORD
+  valueFrom:
+    secretKeyRef:
+      name: {{ include "langsmith.clickhouseSecretsName" . }}
+      key: clickhouse_password
+{{- end -}}
 {{- if not .Values.clickhouse.external.enabled }}
+{{- $envVars := concat .Values.commonEnv (include "clickhouseEnvVars" . | fromYamlArray) .Values.clickhouse.statefulSet.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
@@ -49,26 +68,8 @@ spec:
              {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            - name: CLICKHOUSE_DB
-              valueFrom:
-                secretKeyRef:
-                  name: {{ include "langsmith.clickhouseSecretsName" . }}
-                  key: clickhouse_db
-            - name: CLICKHOUSE_USER
-              valueFrom:
-                secretKeyRef:
-                  name: {{ include "langsmith.clickhouseSecretsName" . }}
-                  key: clickhouse_user
-            - name: CLICKHOUSE_PASSWORD
-              valueFrom:
-                secretKeyRef:
-                  name: {{ include "langsmith.clickhouseSecretsName" . }}
-                  key: clickhouse_password
-            {{- with .Values.clickhouse.statefulSet.extraEnv }}
-               {{- toYaml . | nindent 12 }}
-             {{- end }}
-            {{- with .Values.commonEnv }}
-              {{ toYaml . | nindent 12 }}
+            {{- with $envVars }}
+              {{- toYaml . | nindent 12 }}
             {{- end }}
           ports:
             - name: ch

diff --git a/charts/langsmith/templates/frontend/deployment.yaml b/charts/langsmith/templates/frontend/deployment.yaml
@@ -1,3 +1,23 @@
+{{- define "frontendEnvVars" -}}
+{{- if .Values.config.oauth.enabled }}
+- name: VITE_OAUTH_CLIENT_ID
+  valueFrom:
+    secretKeyRef:
+      key: oauth_client_id
+      name: {{ include "langsmith.secretsName" .}}
+- name: VITE_OAUTH_ISSUER_URL
+  valueFrom:
+    secretKeyRef:
+      key: oauth_issuer_url
+      name: {{ include "langsmith.secretsName" .}}
+{{- end }}
+{{- if .Values.ingress.subdomain }}
+- name: VITE_SUBDOMAIN
+value: {{ .Values.ingress.subdomain }}
+{{- end }}
+{{- end -}}
+{{- $envVars := concat .Values.commonEnv (include "frontendEnvVars" . | fromYamlArray) .Values.frontend.deployment.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -50,26 +70,7 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- if .Values.config.oauth.enabled }}
-            - name: VITE_OAUTH_CLIENT_ID
-              valueFrom:
-                secretKeyRef:
-                  key: oauth_client_id
-                  name: {{ include "langsmith.secretsName" .}}
-            - name: VITE_OAUTH_ISSUER_URL
-              valueFrom:
-                secretKeyRef:
-                  key: oauth_issuer_url
-                  name: {{ include "langsmith.secretsName" .}}
-            {{- end }}
-            {{- with .Values.frontend.deployment.extraEnv }}
-              {{- toYaml . | nindent 12 }}
-            {{- end }}
-            {{- if .Values.ingress.subdomain }}
-            - name: VITE_SUBDOMAIN
-              value: {{ .Values.ingress.subdomain }}
-            {{- end }}
-            {{- with .Values.commonEnv }}
+            {{- with $envVars }}
               {{ toYaml . | nindent 12 }}
             {{- end }}
           envFrom:

diff --git a/charts/langsmith/templates/platform-backend/deployment.yaml b/charts/langsmith/templates/platform-backend/deployment.yaml
@@ -1,3 +1,13 @@
+{{- define "platformBackendEnvVars" -}}
+- name: PORT
+  value: {{ .Values.platformBackend.containerPort | quote }}
+{{- with .Values.ingress.subdomain }}
+- name: DOCS_PREFIX
+  value: /{{ . }}/api
+{{- end }}
+{{- end -}}
+{{- $envVars := concat .Values.commonEnv (include "langsmith.commonEnv" . | fromYamlArray) (include "platformBackendEnvVars" . | fromYamlArray) .Values.platformBackend.deployment.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -50,13 +60,7 @@ spec:
             {{- toYaml . | nindent 12 }}
           {{- end }}
           env:
-            {{- include "langsmith.commonEnv" . | nindent 12 }}
-            - name: PORT
-              value: {{ .Values.platformBackend.containerPort | quote }}
-            {{- with .Values.platformBackend.deployment.extraEnv }}
-              {{- toYaml . | nindent 12 }}
-            {{- end }}
-            {{- with .Values.commonEnv }}
+            {{- with $envVars }}
               {{ toYaml . | nindent 12 }}
             {{- end }}
           envFrom:

diff --git a/charts/langsmith/templates/playground/deployment.yaml b/charts/langsmith/templates/playground/deployment.yaml
@@ -1,3 +1,5 @@
+{{- $envVars := concat .Values.commonEnv .Values.playground.deployment.extraEnv -}}
+{{- include "langsmith.detectDuplicates" $envVars -}}
 apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -51,13 +53,7 @@ spec:
           {{- end }}
           {{- if or .Values.playground.deployment.extraEnv .Values.commonEnv  }}
           env:
-            {{- with .Values.playground.deployment.extraEnv}}
-             {{- toYaml . | nindent 12 }}
-            {{- end }}
-            {{- with .Values.commonEnv}}
-             {{- toYaml . | nindent 12 }}
-            {{- end }}
-           {{- end }}
+          {{- end }}
           image: "{{ .Values.images.playgroundImage.repository }}:{{ .Values.images.playgroundImage.tag | default .Chart.AppVersion }}"
           imagePullPolicy: {{ .Values.images.playgroundImage.pullPolicy }}
           ports: