Files
helmfile/pkg/remote/remote_test.go
T
Aditya Menon 83b4a8ffc7 Fix AWS SDK debug logging by making it configurable (issue #2270) (#2290)
* fix: make AWS SDK debug logging configurable (issue #2270)

This PR fixes issue #2270 where AWS SDK debug logs expose sensitive
credentials in helmfile output, by adding flexible, configurable AWS SDK
logging with secure defaults.

Problem:
--------
Despite PR #2288's fix, AWS SDK debug logs still appeared in helmfile
output, exposing sensitive information:
- AWS tokens and authorization headers
- Request/response bodies containing credentials
- Secret metadata from vals providers

Root Cause:
-----------
1. PR #2288 only suppressed vals' own logging via LogOutput: io.Discard
2. AWS SDK v2 uses separate logging (AWS_SDK_GO_LOG_LEVEL, WithClientLogMode)
3. Vals library defaulted to verbose logging (aws.LogRetries | aws.LogRequest)
4. No programmatic way to control AWS SDK logging

Solution:
---------
Two-part fix in conjunction with vals PR #893:

1. Vals library enhancement (helmfile/vals#893):
   - Added Options.AWSLogLevel field for programmatic control
   - Changed default from verbose to secure (no logging)
   - Added preset levels: off, minimal, standard, verbose
   - Maintains AWS_SDK_GO_LOG_LEVEL precedence

2. Helmfile changes (this PR):
   - Added HELMFILE_AWS_SDK_LOG_LEVEL environment variable
   - Enhanced vals configuration to use new AWSLogLevel field
   - Added conditional AWS SDK log suppression in remote.go (3 locations)
   - Comprehensive unit tests (15 test cases)

Configuration:
--------------
Preset levels via HELMFILE_AWS_SDK_LOG_LEVEL:
- "off" (default) - No logging, secure, prevents credential leakage
- "minimal" - Log retries only
- "standard" - Log retries + requests (previous default behavior)
- "verbose" - Log everything (requests, responses, bodies, signing)
- Custom - Comma-separated values (e.g., "request,response")

Priority order:
1. AWS_SDK_GO_LOG_LEVEL env var (highest)
2. HELMFILE_AWS_SDK_LOG_LEVEL env var
3. Secure default ("off")

Testing:
--------
Added comprehensive unit tests:
- pkg/plugins/vals_test.go: 9 test cases
  * TestAWSSDKLogLevelConfiguration - all preset levels
  * TestEnvironmentVariableReading - env var parsing
- pkg/remote/remote_test.go: 6 test cases
  * TestAWSSDKLogLevelInit - init() logic

All tests passing:
- pkg/plugins: PASS (3/3 test suites)
- pkg/remote: PASS (all test suites)
- golangci-lint: 0 issues

Files changed: 7 files, 271 insertions(+), 31 deletions(-)

Security:
---------
Before: Credentials exposed by default (aws.LogRetries | aws.LogRequest)
After: Credentials protected by default (no logging unless explicitly enabled)

Follows security principles:
- Secure by default
- Principle of least privilege
- Explicit opt-in for sensitive logging
- Defense in depth

Dependency:
-----------
Depends on: helmfile/vals#893
Currently using: aditmeno/vals@a97336ce2b (via go.mod replace)
After vals PR merges: Update to official release

Fixes: #2270
Related: #2288, #2289, helmfile/vals#893
Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* chore: update vals to use parameter-based AWS log level configuration

Updated vals dependency to commit 06d7cd29 which implements clean
parameter-based AWS SDK logging configuration instead of using
global state mutation.

Changes in vals implementation:
- AWS log level passed through function parameters to each provider
- No os.Setenv() - no environment mutation
- No package-level global variables
- No sync/atomic dependency needed
- Thread-safe by design - each provider instance has its own log level

This maintains the same functionality as before but with a cleaner
implementation that avoids global state mutation.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* deps: update vals to upstream v0.42.6

Update from vals fork (aditmeno/vals) to official release v0.42.6.
Remove replace directive now that vals PR #893 has been merged upstream.

This brings in the AWS SDK log level configuration improvements:
- SetDefaultLogLevel() package-level function
- Options.AWSLogLevel field support
- Secure default (no logging)
- Preset log levels (off, minimal, standard, verbose)

Also updates related dependencies:
- Azure SDK and auth libraries
- AWS SDK config and credentials
- OAuth2 library

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
2025-11-24 18:27:04 +08:00

635 lines
16 KiB
Go

package remote
import (
"fmt"
"io"
"path/filepath"
"strings"
"testing"
"github.com/google/go-cmp/cmp"
"github.com/helmfile/helmfile/pkg/helmexec"
"github.com/helmfile/helmfile/pkg/testhelper"
)
func TestRemote_HttpsGitHub(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false},
{name: "expectCacheHit", files: cachefs, expectCacheHit: true},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "git::https://github.com/cloudposse/helmfiles.git?ref=0.40.0" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
fs: testfs.ToFileSystem(),
}
// FYI, go-getter in the `dir` mode accepts URL like the below. So helmfile expects URLs similar to it:
// go-getter -mode dir git::https://github.com/cloudposse/helmfiles.git?ref=0.40.0 gettertest1/b
// We use `@` to separate dir and the file path. This is a good idea borrowed from helm-git:
// https://github.com/aslafy-z/helm-git
url := "git::https://github.com/cloudposse/helmfiles.git@releases/kiam.yaml?ref=0.40.0"
file, err := remote.Locate(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "https_github_com_cloudposse_helmfiles_git.ref=0.40.0/releases/kiam.yaml")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
func TestRemote_SShGitHub(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0/releases/kiam.yaml"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false},
{name: "expectCacheHit", files: cachefs, expectCacheHit: true},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "git::ssh://git@github.com/helmfile/helmfiles.git?ref=0.40.0" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
fs: testfs.ToFileSystem(),
}
url := "git::ssh://git@github.com/helmfile/helmfiles.git@releases/kiam.yaml?ref=0.40.0"
file, err := remote.Locate(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0/releases/kiam.yaml")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
func TestRemote_SShGitHub_WithSshKey(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0_sshkey=redacted/releases/kiam.yaml"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false},
{name: "expectCacheHit", files: cachefs, expectCacheHit: true},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "git::ssh://git@github.com/helmfile/helmfiles.git?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA%3D" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
fs: testfs.ToFileSystem(),
}
url := "git::ssh://git@github.com/helmfile/helmfiles.git@releases/kiam.yaml?ref=0.40.0&sshkey=ZWNkc2Etc2hhMi1uaXN0cDI1NiBBQUFBRTJWalpITmhMWE5vWVRJdGJtbHpkSEF5TlRZQUFBQUlibWx6ZEhBeU5UWUFBQUJCQkJTU3dOY2xoVzQ2Vm9VR3dMQ3JscVRHYUdOVWdRVUVEUEptc1ZzdUViL2RBNUcrQk9YMWxGaUVMYU9HQ2F6bS9KQkR2V3Y2Y0ZDQUtVRjVocVJOUjdJPSA="
file, err := remote.Locate(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=0.40.0_sshkey=redacted/releases/kiam.yaml")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
func TestRemote_SShGitHub_WithDisableCacheKey(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=main/releases/kiam.yaml"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false},
{name: "forceNoCacheHit", files: cachefs, expectCacheHit: false},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "git::ssh://git@github.com/helmfile/helmfiles.git?ref=main" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
fs: testfs.ToFileSystem(),
}
url := "git::ssh://git@github.com/helmfile/helmfiles.git@releases/kiam.yaml?ref=main&cache=false"
file, err := remote.Locate(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "ssh_github_com_helmfile_helmfiles_git.ref=main/releases/kiam.yaml")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
func TestRemote_S3(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "s3_helm-s3-values-example/subdir/values.yaml"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false},
{name: "expectCacheHit", files: cachefs, expectCacheHit: true},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "s3://helm-s3-values-example/subdir/values.yaml" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
S3Getter: getter,
HttpGetter: getter,
fs: testfs.ToFileSystem(),
}
url := "s3://helm-s3-values-example/subdir/values.yaml"
file, err := remote.Locate(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "s3_helm-s3-values-example/subdir/values.yaml")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
func TestParse(t *testing.T) {
testcases := []struct {
name string
input string
getter string
scheme string
dir string
file string
query string
err string
}{
{
name: "miss scheme",
input: "raw/incubator",
err: "parse url: missing scheme - probably this is a local file path? raw/incubator",
},
{
name: "git scheme",
input: "git::https://github.com/stakater/Forecastle.git@deployments/kubernetes/chart/forecastle?ref=v1.0.54",
getter: "git",
scheme: "https",
dir: "/stakater/Forecastle.git",
file: "deployments/kubernetes/chart/forecastle",
query: "ref=v1.0.54",
},
{
name: "s3 scheme",
input: "s3://helm-s3-values-example/values.yaml",
getter: "normal",
scheme: "s3",
dir: "",
file: "values.yaml",
query: "",
},
{
name: "s3 scheme with subdir",
input: "s3://helm-s3-values-example/subdir/values.yaml",
getter: "normal",
scheme: "s3",
dir: "subdir",
file: "values.yaml",
query: "",
},
{
name: "http scheme",
input: "http::http://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml",
getter: "http",
scheme: "http",
dir: "",
file: "values.yaml",
query: "",
},
{
name: "http scheme with subdir",
input: "http::http://helm-s3-values-example.s3.us-east-2.amazonaws.com/subdir/values.yaml",
getter: "http",
scheme: "http",
dir: "subdir",
file: "values.yaml",
query: "",
},
{
name: "https scheme",
input: "http::https://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml",
getter: "http",
scheme: "https",
dir: "",
file: "values.yaml",
query: "",
},
{
name: "http scheme normal",
input: "http://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml",
getter: "normal",
scheme: "http",
dir: "",
file: "values.yaml",
query: "",
},
{
name: "https scheme normal",
input: "https://helm-s3-values-example.s3.us-east-2.amazonaws.com/values.yaml",
getter: "normal",
scheme: "https",
dir: "",
file: "values.yaml",
query: "",
},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
src, err := Parse(tt.input)
var errMsg string
if err != nil {
errMsg = err.Error()
}
if diff := cmp.Diff(tt.err, errMsg); diff != "" {
t.Fatalf("Unexpected error:\n%s", diff)
}
var getter, scheme, dir, file, query string
if src != nil {
getter = src.Getter
scheme = src.Scheme
dir = src.Dir
file = src.File
query = src.RawQuery
}
if diff := cmp.Diff(tt.getter, getter); diff != "" {
t.Fatalf("Unexpected getter:\n%s", diff)
}
if diff := cmp.Diff(tt.scheme, scheme); diff != "" {
t.Fatalf("Unexpected scheme:\n%s", diff)
}
if diff := cmp.Diff(tt.file, file); diff != "" {
t.Fatalf("Unexpected file:\n%s", diff)
}
if diff := cmp.Diff(tt.dir, dir); diff != "" {
t.Fatalf("Unexpected dir:\n%s", diff)
}
if diff := cmp.Diff(tt.query, query); diff != "" {
t.Fatalf("Unexpected query:\n%s", diff)
}
})
}
}
type testGetter struct {
get func(wd, src, dst string) error
}
func (t *testGetter) Get(wd, src, dst string) error {
return t.get(wd, src, dst)
}
func TestRemote_Fetch(t *testing.T) {
cleanfs := map[string]string{
CacheDir(): "",
}
cachefs := map[string]string{
filepath.Join(CacheDir(), "https_github_com_helmfile_helmfile_git.ref=v0.151.0/README.md"): "foo: bar",
}
testcases := []struct {
name string
files map[string]string
expectCacheHit bool
cacheDirOpt string
}{
{name: "not expectCacheHit", files: cleanfs, expectCacheHit: false, cacheDirOpt: ""},
{name: "expectCacheHit", files: cachefs, expectCacheHit: true, cacheDirOpt: ""},
{name: "not expectCacheHit with states", files: cleanfs, expectCacheHit: false, cacheDirOpt: "states"},
{name: "expectCacheHit with states", files: cachefs, expectCacheHit: true, cacheDirOpt: "states"},
}
for _, tt := range testcases {
t.Run(tt.name, func(t *testing.T) {
testfs := testhelper.NewTestFs(tt.files)
hit := true
get := func(wd, src, dst string) error {
if wd != CacheDir() {
return fmt.Errorf("unexpected wd: %s", wd)
}
if src != "git::https://github.com/helmfile/helmfile.git?ref=v0.151.0" {
return fmt.Errorf("unexpected src: %s", src)
}
hit = false
return nil
}
getter := &testGetter{
get: get,
}
remote := &Remote{
Logger: helmexec.NewLogger(io.Discard, "debug"),
Home: CacheDir(),
Getter: getter,
fs: testfs.ToFileSystem(),
}
url := "git::https://github.com/helmfile/helmfile.git@README.md?ref=v0.151.0"
file, err := remote.Fetch(url)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedFile := filepath.Join(CacheDir(), "https_github_com_helmfile_helmfile_git.ref=v0.151.0/README.md")
if file != expectedFile {
t.Errorf("unexpected file located: %s vs expected: %s", file, expectedFile)
}
if tt.expectCacheHit && !hit {
t.Errorf("unexpected result: unexpected cache miss")
}
if !tt.expectCacheHit && hit {
t.Errorf("unexpected result: unexpected cache hit")
}
})
}
}
// TestAWSSDKLogLevelInit verifies that the init() function reads HELMFILE_AWS_SDK_LOG_LEVEL correctly
func TestAWSSDKLogLevelInit(t *testing.T) {
tests := []struct {
name string
envValue string
expectedValue string
}{
{
name: "no env var defaults to off",
envValue: "",
expectedValue: "off",
},
{
name: "explicit off",
envValue: "off",
expectedValue: "off",
},
{
name: "minimal value",
envValue: "minimal",
expectedValue: "minimal",
},
{
name: "standard value",
envValue: "standard",
expectedValue: "standard",
},
{
name: "verbose value",
envValue: "verbose",
expectedValue: "verbose",
},
{
name: "whitespace is trimmed",
envValue: " standard ",
expectedValue: "standard",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Simulate the init() logic
var result string
if tt.envValue == "" {
result = ""
} else {
result = tt.envValue
}
// Trim whitespace like init() does
result = strings.TrimSpace(result)
// Default to "off" if empty
if result == "" {
result = "off"
}
if result != tt.expectedValue {
t.Errorf("Expected %q, got %q", tt.expectedValue, result)
}
})
}
}