Skip to content

Commit be2eb0c

Browse files
committed
feat: Support CLI flag to discard env vars with empty value
Instead of failing. This eliminates the need to wrap `lk` invocations with custom logic for manually filtering out such values. For example, this is a real script for deploying using `lk agent deploy`: ``` rm -f .env.lk sed '/^#/d' .env | grep -vE '^[A-Za-z_][A-Za-z0-9_]*=$' > .env.lk lk agent deploy --secrets-file .env.lk ```
1 parent 85482d4 commit be2eb0c

File tree

2 files changed

+334
-1
lines changed

2 files changed

+334
-1
lines changed

cmd/lk/agent.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ var (
7777
Required: false,
7878
}
7979

80+
ignoreEmptyFlag = &cli.BoolFlag{
81+
Name: "ignore-empty-vars",
82+
Usage: "If set, will skip environment variables with empty values from secrets files instead of failing",
83+
Required: false,
84+
Value: false,
85+
}
86+
8087
logTypeFlag = &cli.StringFlag{
8188
Name: "log-type",
8289
Usage: "Type of logs to retrieve. Valid values are 'deploy' and 'build'",
@@ -156,6 +163,7 @@ var (
156163
secretsFlag,
157164
secretsFileFlag,
158165
secretsMountFlag,
166+
ignoreEmptyFlag,
159167
silentFlag,
160168
regionFlag,
161169
skipSDKCheckFlag,
@@ -201,6 +209,7 @@ var (
201209
secretsFileFlag,
202210
secretsMountFlag,
203211
silentFlag,
212+
ignoreEmptyFlag,
204213
skipSDKCheckFlag,
205214
},
206215
// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
@@ -227,6 +236,7 @@ var (
227236
secretsFlag,
228237
secretsFileFlag,
229238
secretsMountFlag,
239+
ignoreEmptyFlag,
230240
},
231241
// NOTE: since secrets may contain commas, or indeed any special character we might want to treat as a flag separator,
232242
// we disable it entirely here and require multiple --secrets flags to be used.
@@ -321,6 +331,7 @@ var (
321331
secretsFlag,
322332
secretsFileFlag,
323333
secretsMountFlag,
334+
ignoreEmptyFlag,
324335
idFlag(false),
325336
&cli.BoolFlag{
326337
Name: "overwrite",
@@ -1317,13 +1328,20 @@ func requireSecrets(_ context.Context, cmd *cli.Command, required, lazy bool) ([
13171328
fmt.Printf("Using secrets file [%s]\n", util.Accented(file))
13181329
}
13191330

1331+
ignoreEmpty := cmd.Bool("ignore-empty-vars")
1332+
var skippedEmpty []string
1333+
13201334
for k, v := range env {
13211335
if _, exists := secrets[k]; exists {
13221336
continue
13231337
}
13241338

13251339
if v == "" {
1326-
return nil, fmt.Errorf("failed to parse secrets file: secret %s is empty, either remove it or provide a value", k)
1340+
if ignoreEmpty {
1341+
skippedEmpty = append(skippedEmpty, k)
1342+
continue
1343+
}
1344+
return nil, fmt.Errorf("failed to parse secrets file: secret %s is empty, either remove it or provide a value, or use --ignore-empty-vars to skip empty values", k)
13271345
}
13281346

13291347
secret := &lkproto.AgentSecret{
@@ -1333,6 +1351,12 @@ func requireSecrets(_ context.Context, cmd *cli.Command, required, lazy bool) ([
13331351
}
13341352
secrets[k] = secret
13351353
}
1354+
1355+
// Log skipped secrets if any (unless silent)
1356+
if len(skippedEmpty) > 0 && !silent {
1357+
skippedNames := strings.Join(skippedEmpty, ", ")
1358+
fmt.Printf("Skipped %d empty secret(s): %s\n", len(skippedEmpty), util.Dimmed(skippedNames))
1359+
}
13361360
}
13371361

13381362
var secretsSlice []*lkproto.AgentSecret

cmd/lk/agent_test.go

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
// Copyright 2025 LiveKit, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package main
16+
17+
import (
18+
"context"
19+
"os"
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
"github.com/stretchr/testify/require"
24+
"github.com/urfave/cli/v3"
25+
)
26+
27+
// buildTestCommand creates a *cli.Command with flags set for testing requireSecrets()
28+
func buildTestCommand(
29+
t *testing.T,
30+
ignoreEmpty bool,
31+
silent bool,
32+
secretsFile string,
33+
inlineSecrets []string,
34+
) *cli.Command {
35+
var capturedCmd *cli.Command
36+
37+
// Create a test app with the necessary flags
38+
app := &cli.Command{
39+
Name: "test",
40+
Flags: []cli.Flag{
41+
&cli.BoolFlag{
42+
Name: "ignore-empty-vars",
43+
},
44+
&cli.BoolFlag{
45+
Name: "silent",
46+
},
47+
&cli.StringFlag{
48+
Name: "secrets-file",
49+
},
50+
&cli.StringSliceFlag{
51+
Name: "secrets",
52+
},
53+
&cli.StringSliceFlag{
54+
Name: "secret-mount",
55+
},
56+
},
57+
Action: func(ctx context.Context, cmd *cli.Command) error {
58+
// Capture the command after flags are parsed
59+
capturedCmd = cmd
60+
return nil
61+
},
62+
}
63+
64+
// Build args array
65+
args := []string{"test"}
66+
if ignoreEmpty {
67+
args = append(args, "--ignore-empty-vars")
68+
}
69+
if silent {
70+
args = append(args, "--silent")
71+
}
72+
if secretsFile != "" {
73+
args = append(args, "--secrets-file", secretsFile)
74+
}
75+
for _, secret := range inlineSecrets {
76+
args = append(args, "--secrets", secret)
77+
}
78+
79+
// Run the app to parse flags
80+
err := app.Run(context.Background(), args)
81+
if err != nil {
82+
t.Fatalf("Failed to run test command: %v", err)
83+
}
84+
85+
if capturedCmd == nil {
86+
t.Fatal("Failed to capture command")
87+
}
88+
89+
return capturedCmd
90+
}
91+
92+
// TestRequireSecrets tests the requireSecrets() function with the --ignore-empty-vars flag
93+
func TestRequireSecrets(t *testing.T) {
94+
tests := []struct {
95+
name string
96+
ignoreEmpty bool
97+
silent bool
98+
envFileContent string // .env file content to create
99+
inlineSecrets []string // --secrets flag values
100+
required bool // required parameter
101+
lazy bool // lazy parameter
102+
expectedError bool
103+
expectedErrorMsg string // partial match
104+
expectedSecrets []string // expected secret names (must be present)
105+
notExpectedSecrets []string // secret names that must NOT be present
106+
}{
107+
// Core 2x2 Matrix
108+
{
109+
name: "Case 1: Empty secrets with ignore-empty-vars flag",
110+
ignoreEmpty: true,
111+
silent: false,
112+
envFileContent: "KEY1=value1\nEMPTY_KEY=\nKEY2=value2",
113+
required: false,
114+
lazy: false,
115+
expectedError: false,
116+
expectedSecrets: []string{"KEY1", "KEY2"},
117+
notExpectedSecrets: []string{"EMPTY_KEY"},
118+
},
119+
{
120+
name: "Case 2: Empty secrets without flag - should error",
121+
ignoreEmpty: false,
122+
silent: false,
123+
envFileContent: "KEY1=value1\nEMPTY_KEY=\nKEY2=value2",
124+
required: false,
125+
lazy: false,
126+
expectedError: true,
127+
expectedErrorMsg: "secret EMPTY_KEY is empty",
128+
},
129+
{
130+
name: "Case 3: No empty secrets with ignore-empty-vars flag",
131+
ignoreEmpty: true,
132+
silent: false,
133+
envFileContent: "KEY1=value1\nKEY2=value2",
134+
required: false,
135+
lazy: false,
136+
expectedError: false,
137+
expectedSecrets: []string{"KEY1", "KEY2"},
138+
notExpectedSecrets: []string{},
139+
},
140+
{
141+
name: "Case 4: No empty secrets without flag (baseline)",
142+
ignoreEmpty: false,
143+
silent: false,
144+
envFileContent: "KEY1=value1\nKEY2=value2",
145+
required: false,
146+
lazy: false,
147+
expectedError: false,
148+
expectedSecrets: []string{"KEY1", "KEY2"},
149+
notExpectedSecrets: []string{},
150+
},
151+
// Extended Cases
152+
{
153+
name: "Case 5: All empty with flag - should error no secrets",
154+
ignoreEmpty: true,
155+
silent: false,
156+
envFileContent: "EMPTY1=\nEMPTY2=",
157+
required: true,
158+
lazy: false,
159+
expectedError: true,
160+
expectedErrorMsg: "no secrets provided",
161+
},
162+
{
163+
name: "Case 6: Mixed empty/non-empty with flag",
164+
ignoreEmpty: true,
165+
silent: false,
166+
envFileContent: "EMPTY1=\nVALID=value\nEMPTY2=\nALSO_VALID=value2",
167+
required: false,
168+
lazy: false,
169+
expectedError: false,
170+
expectedSecrets: []string{"VALID", "ALSO_VALID"},
171+
notExpectedSecrets: []string{"EMPTY1", "EMPTY2"},
172+
},
173+
{
174+
name: "Case 7: Multiple empty secrets tracked",
175+
ignoreEmpty: true,
176+
silent: false,
177+
envFileContent: "E1=\nE2=\nE3=\nVALID=value",
178+
required: false,
179+
lazy: false,
180+
expectedError: false,
181+
expectedSecrets: []string{"VALID"},
182+
notExpectedSecrets: []string{"E1", "E2", "E3"},
183+
},
184+
{
185+
name: "Case 8: Inline secrets not affected by flag",
186+
ignoreEmpty: true,
187+
silent: false,
188+
envFileContent: "", // No env file
189+
inlineSecrets: []string{"EMPTY_INLINE=", "VALID_INLINE=value"},
190+
required: false,
191+
lazy: false,
192+
expectedError: false,
193+
expectedSecrets: []string{"EMPTY_INLINE", "VALID_INLINE"},
194+
notExpectedSecrets: []string{},
195+
},
196+
{
197+
name: "Case 9: Error message mentions --ignore-empty-vars flag",
198+
ignoreEmpty: false,
199+
silent: false,
200+
envFileContent: "EMPTY=",
201+
required: false,
202+
lazy: false,
203+
expectedError: true,
204+
expectedErrorMsg: "--ignore-empty-vars",
205+
},
206+
{
207+
name: "Case 10: Silent mode suppresses skip message",
208+
ignoreEmpty: true,
209+
silent: true,
210+
envFileContent: "EMPTY=\nVALID=value",
211+
required: false,
212+
lazy: false,
213+
expectedError: false,
214+
expectedSecrets: []string{"VALID"},
215+
notExpectedSecrets: []string{"EMPTY"},
216+
},
217+
}
218+
219+
for _, tt := range tests {
220+
t.Run(tt.name, func(t *testing.T) {
221+
// Setup temporary directory
222+
tempDir, err := os.MkdirTemp("", "agent-secrets-test")
223+
require.NoError(t, err)
224+
defer os.RemoveAll(tempDir)
225+
226+
// Change to temp directory
227+
oldWd, _ := os.Getwd()
228+
err = os.Chdir(tempDir)
229+
require.NoError(t, err)
230+
defer os.Chdir(oldWd)
231+
232+
// Create .env file if specified
233+
var secretsFile string
234+
if tt.envFileContent != "" {
235+
secretsFile = ".env"
236+
err := os.WriteFile(secretsFile, []byte(tt.envFileContent), 0644)
237+
require.NoError(t, err)
238+
}
239+
240+
// Build test command with proper flags
241+
cmd := buildTestCommand(t, tt.ignoreEmpty, tt.silent, secretsFile, tt.inlineSecrets)
242+
243+
// Call the REAL requireSecrets function
244+
secrets, err := requireSecrets(
245+
context.Background(),
246+
cmd,
247+
tt.required,
248+
tt.lazy,
249+
)
250+
251+
// Assertions
252+
if tt.expectedError {
253+
assert.Error(t, err)
254+
if tt.expectedErrorMsg != "" {
255+
assert.Contains(t, err.Error(), tt.expectedErrorMsg)
256+
}
257+
} else {
258+
assert.NoError(t, err)
259+
260+
// Verify expected secrets count
261+
assert.Equal(t, len(tt.expectedSecrets), len(secrets),
262+
"Expected %d secrets, got %d", len(tt.expectedSecrets), len(secrets))
263+
264+
// Collect secret names for assertions
265+
secretNames := make([]string, len(secrets))
266+
for i, s := range secrets {
267+
secretNames[i] = s.Name
268+
}
269+
270+
// Verify expected secret names are present
271+
for _, expected := range tt.expectedSecrets {
272+
assert.Contains(t, secretNames, expected,
273+
"Expected secret %s to be present", expected)
274+
}
275+
276+
// Verify that empty secrets are NOT present
277+
for _, notExpected := range tt.notExpectedSecrets {
278+
assert.NotContains(t, secretNames, notExpected,
279+
"Secret %s should NOT be present (should have been filtered out)", notExpected)
280+
}
281+
}
282+
})
283+
}
284+
}
285+
286+
// TestRequireSecrets_InlineOverridesFile tests that inline secrets override file secrets
287+
func TestRequireSecrets_InlineOverridesFile(t *testing.T) {
288+
tempDir, err := os.MkdirTemp("", "agent-secrets-test")
289+
require.NoError(t, err)
290+
defer os.RemoveAll(tempDir)
291+
292+
oldWd, _ := os.Getwd()
293+
err = os.Chdir(tempDir)
294+
require.NoError(t, err)
295+
defer os.Chdir(oldWd)
296+
297+
// Create .env with KEY=file_value
298+
err = os.WriteFile(".env", []byte("KEY=file_value"), 0644)
299+
require.NoError(t, err)
300+
301+
// Create command with inline secret KEY=inline_value
302+
cmd := buildTestCommand(t, true, false, ".env", []string{"KEY=inline_value"})
303+
304+
secrets, err := requireSecrets(context.Background(), cmd, false, false)
305+
require.NoError(t, err)
306+
require.Len(t, secrets, 1)
307+
assert.Equal(t, "KEY", secrets[0].Name)
308+
assert.Equal(t, "inline_value", string(secrets[0].Value))
309+
}

0 commit comments

Comments
 (0)