Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buildkite-agent env has --from-env-file and --print flags #1803

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
81 changes: 72 additions & 9 deletions clicommand/env.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package clicommand

import (
"bufio"
"encoding/json"
"errors"
"fmt"
"os"
"strconv"
"strings"

"github.com/urfave/cli"
Expand Down Expand Up @@ -32,20 +35,41 @@ var EnvCommand = cli.Command{
Usage: "Pretty print the JSON output",
EnvVar: "BUILDKITE_AGENT_ENV_PRETTY",
},
cli.BoolFlag{
Name: "from-env-file",
Usage: "Source environment from file described by $BUILDKITE_ENV_FILE",
},
cli.StringFlag{
Name: "print",
Usage: "Print a single environment variable by `NAME` as raw text followed by a newline",
},
},
Action: func(c *cli.Context) error {
env := os.Environ()
envMap := make(map[string]string, len(env))
var envMap map[string]string
var err error

for _, e := range env {
k, v, _ := strings.Cut(e, "=")
envMap[k] = v
if c.Bool("from-env-file") {
envMap, err = loadEnvFile(os.Getenv("BUILDKITE_ENV_FILE"))
Comment on lines +51 to +52
Copy link
Member

Choose a reason for hiding this comment

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

This feels weird to me. I expect --from-env-file to take a value, but default to an environment variable, like our other options. But if we defaulted to the env file then it would break dumping of the current env as json.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. I like your idea of moving the current “dump entire process env as JSON” use-cases into a subcommand/flag(s), which then frees up the buildkite-agent env namespace to support some other environment-related use-cases.

if err != nil {
fmt.Fprintf(os.Stderr, "Error loading BUILDKITE_ENV_FILE: %v\n", err)
os.Exit(1)
}
} else {
env := os.Environ()
envMap = make(map[string]string, len(env))

for _, e := range env {
k, v, _ := strings.Cut(e, "=")
envMap[k] = v
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect @moskyb's efforts to also use this command in windows will cause a merge conflict here.

}

var (
envJSON []byte
err error
)
if name := c.String("print"); name != "" {
fmt.Println(envMap[name])
return nil
}

var envJSON []byte

if c.Bool("pretty") {
envJSON, err = json.MarshalIndent(envMap, "", " ")
Expand All @@ -69,3 +93,42 @@ var EnvCommand = cli.Command{
return nil
},
}

func loadEnvFile(path string) (map[string]string, error) {
envMap := make(map[string]string)

if path == "" {
return nil, errors.New("no path specified")
}

f, err := os.Open(path)
if err != nil {
return nil, err
}

scanner := bufio.NewScanner(f)

lineNo := 0
for scanner.Scan() {
lineNo++

line := scanner.Text()
if line == "" {
continue
}

name, quotedValue, ok := strings.Cut(line, "=")
if !ok {
return nil, fmt.Errorf("Unexpected format in %s:%d", path, lineNo)
}

value, err := strconv.Unquote(quotedValue)
if err != nil {
return nil, fmt.Errorf("unquoting value in %s:%d: %w", path, lineNo, err)
}

envMap[name] = value
}

return envMap, nil
}
42 changes: 42 additions & 0 deletions clicommand/env_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package clicommand

import (
"fmt"
"os"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestLoadEnvFile(t *testing.T) {
f, err := os.CreateTemp("", t.Name())
if err != nil {
t.Error(err)
}
data := map[string]string{
"HELLO": "world",
"FOO": "bar\n\"bar\"\n`black hat`\r\n$(have you any root)",
}
for name, value := range data {
fmt.Fprintf(f, "%s=%q\n", name, value)
}

result, err := loadEnvFile(f.Name())
require.NoError(t, err)

assert.Equal(t, data, result, "data should round-trip via env file")
}

func TestLoadEnvFileQuotingError(t *testing.T) {
f, err := os.CreateTemp("", t.Name())
require.NoError(t, err)

fmt.Fprintf(f, "%s=%q\n", "ONE", "ok")
fmt.Fprintln(f, "TWO=missing quotes")

result, err := loadEnvFile(f.Name())
assert.Nil(t, result)

assert.Equal(t, `unquoting value in `+f.Name()+`:2: invalid syntax`, err.Error())
}