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

Removed the legacy env var: LOGXI_FORMAT #17822

Merged
merged 4 commits into from
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/17822.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:change
logging: Removed legacy environment variable for log format, should use 'VAULT_LOG_FORMAT' instead
peteski22 marked this conversation as resolved.
Show resolved Hide resolved
```
10 changes: 4 additions & 6 deletions command/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,10 @@ func (c *ServerCommand) Flags() *FlagSets {
})

f.StringVar(&StringVar{
Name: "log-format",
Target: &c.flagLogFormat,
Default: notSetValue,
// EnvVar can't be just "VAULT_LOG_FORMAT", because more than one env var name is supported
// for backwards compatibility reasons.
// See github.com/hashicorp/vault/sdk/helper/logging.ParseEnvLogFormat()
Name: "log-format",
Target: &c.flagLogFormat,
Default: notSetValue,
EnvVar: "VAULT_LOG_FORMAT",
Completion: complete.PredictSet("standard", "json"),
Usage: `Log format. Supported values are "standard" and "json".`,
})
Expand Down
5 changes: 1 addition & 4 deletions sdk/helper/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,13 @@ func ParseLogFormat(format string) (LogFormat, error) {
case "json":
return JSONFormat, nil
default:
return UnspecifiedFormat, fmt.Errorf("Unknown log format: %s", format)
return UnspecifiedFormat, fmt.Errorf("unknown log format: %s", format)
}
}

// ParseEnvLogFormat parses the log format from an environment variable.
func ParseEnvLogFormat() LogFormat {
Copy link
Contributor

Choose a reason for hiding this comment

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

The one thing that's not immediately clear to me, and probably doesn't need to be answered in this PR, is why is this function necessary?

If we use EnvVar in the StringVar in the command side, do we need to call this as well/alternatively? Or can we just rely on the option variable handling this instead?

logFormat := os.Getenv("VAULT_LOG_FORMAT")
if logFormat == "" {
logFormat = os.Getenv("LOGXI_FORMAT")
}
switch strings.ToLower(logFormat) {
case "json", "vault_json", "vault-json", "vaultjson":
return JSONFormat
Expand Down
13 changes: 1 addition & 12 deletions sdk/helper/logging/logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func Test_ParseLogFormat(t *testing.T) {
{format: "STANDARD", expected: StandardFormat, expectedErr: nil},
{format: "json", expected: JSONFormat, expectedErr: nil},
{format: " json ", expected: JSONFormat, expectedErr: nil},
{format: "bogus", expected: UnspecifiedFormat, expectedErr: errors.New("Unknown log format: bogus")},
{format: "bogus", expected: UnspecifiedFormat, expectedErr: errors.New("unknown log format: bogus")},
}

for _, test := range tests {
Expand All @@ -42,17 +42,6 @@ func Test_ParseEnv_VAULT_LOG_FORMAT(t *testing.T) {
testParseEnvLogFormat(t, "VAULT_LOG_FORMAT")
}

func Test_ParseEnv_LOGXI_FORMAT(t *testing.T) {
oldVLF := os.Getenv("VAULT_LOG_FORMAT")
defer os.Setenv("VAULT_LOG_FORMAT", oldVLF)

oldLogxi := os.Getenv("LOGXI_FORMAT")
defer os.Setenv("LOGXI_FORMAT", oldLogxi)

os.Setenv("VAULT_LOG_FORMAT", "")
testParseEnvLogFormat(t, "LOGXI_FORMAT")
}

func testParseEnvLogFormat(t *testing.T, name string) {
env := []string{
"json", "vauLT_Json", "VAULT-JSON", "vaulTJSon",
Expand Down