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.

Sign up for GitHub

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

Jump to bottom

Filter sensitive fields in debug level logs #62

Merged
merged 1 commit into from
Aug 10, 2022
Merged

Filter sensitive fields in debug level logs #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 12 additions & 1 deletion pkg/terraform/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"sync"

"github.com/crossplane/crossplane-runtime/pkg/logging"
Expand Down Expand Up @@ -129,7 +130,7 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient
ws.mu.Lock()
w, ok := ws.store[tr.GetUID()]
if !ok {
ws.store[tr.GetUID()] = NewWorkspace(dir, WithLogger(l), WithExecutor(ws.executor))
ws.store[tr.GetUID()] = NewWorkspace(dir, WithLogger(l), WithExecutor(ws.executor), WithFilterFn(ts.filterSensitiveInformation))
w = ws.store[tr.GetUID()]
}
ws.mu.Unlock()
Expand All @@ -139,6 +140,7 @@ func (ws *WorkspaceStore) Workspace(ctx context.Context, c resource.SecretClient
}
w.env = ts.Env
w.env = append(w.env, fmt.Sprintf(fmtEnv, envReattachConfig, attachmentConfig))

// We need to initialize only if the workspace hasn't been initialized yet.
if !os.IsNotExist(err) {
return w, nil
Expand All @@ -165,3 +167,12 @@ func (ws *WorkspaceStore) Remove(obj xpresource.Object) error {
delete(ws.store, obj.GetUID())
return nil
}

func (ts Setup) filterSensitiveInformation(s string) string {
for _, v := range ts.Configuration {
if str, ok := v.(string); ok && str != "" {
s = strings.ReplaceAll(s, str, "REDACTED")
}
}
return s
}
20 changes: 14 additions & 6 deletions pkg/terraform/workspace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ func WithAferoFs(fs afero.Fs) WorkspaceOption {
}
}

func WithFilterFn(filterFn func(string) string) WorkspaceOption {
Copy link
Member

Choose a reason for hiding this comment

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

If you'd like to enable providers to give this, then I think a similar option for WorkspaceStore will be needed, too, because that's what they initialize in their main function. An alternative could be to create an option for WorkspaceStore that accepts a list of WorkspaceOptions to apply.

return func(w *Workspace) {
w.filterFn = filterFn
}
}

// NewWorkspace returns a new Workspace object that operates in the given
// directory.
func NewWorkspace(dir string, opts ...WorkspaceOption) *Workspace {
Expand Down Expand Up @@ -87,6 +93,8 @@ type Workspace struct {
logger logging.Logger
executor k8sExec.Interface
fs afero.Afero

filterFn func(string) string
}

// ApplyAsync makes a terraform apply call without blocking and calls the given
Expand All @@ -104,7 +112,7 @@ func (w *Workspace) ApplyAsync(callback CallbackFn) error {
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.LastOperation.MarkEnd()
w.logger.Debug("apply async ended", "out", string(out))
w.logger.Debug("apply async ended", "out", w.filterFn(string(out)))
defer func() {
if cErr := callback(err, ctx); cErr != nil {
w.logger.Info("callback failed", "error", cErr.Error())
Expand All @@ -131,7 +139,7 @@ func (w *Workspace) Apply(ctx context.Context) (ApplyResult, error) {
cmd.SetEnv(append(os.Environ(), w.env...))
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.logger.Debug("apply ended", "out", string(out))
w.logger.Debug("apply ended", "out", w.filterFn(string(out)))
if err != nil {
return ApplyResult{}, tferrors.NewApplyFailed(out)
}
Expand Down Expand Up @@ -169,7 +177,7 @@ func (w *Workspace) DestroyAsync(callback CallbackFn) error {
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.LastOperation.MarkEnd()
w.logger.Debug("destroy async ended", "out", string(out))
w.logger.Debug("destroy async ended", "out", w.filterFn(string(out)))
defer func() {
if cErr := callback(err, ctx); cErr != nil {
w.logger.Info("callback failed", "error", cErr.Error())
Expand All @@ -191,7 +199,7 @@ func (w *Workspace) Destroy(ctx context.Context) error {
cmd.SetEnv(append(os.Environ(), w.env...))
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.logger.Debug("destroy ended", "out", string(out))
w.logger.Debug("destroy ended", "out", w.filterFn(string(out)))
if err != nil {
return tferrors.NewDestroyFailed(out)
}
Expand Down Expand Up @@ -222,7 +230,7 @@ func (w *Workspace) Refresh(ctx context.Context) (RefreshResult, error) {
cmd.SetEnv(append(os.Environ(), w.env...))
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.logger.Debug("refresh ended", "out", string(out))
w.logger.Debug("refresh ended", "out", w.filterFn(string(out)))
if err != nil {
return RefreshResult{}, tferrors.NewRefreshFailed(out)
}
Expand Down Expand Up @@ -257,7 +265,7 @@ func (w *Workspace) Plan(ctx context.Context) (PlanResult, error) {
cmd.SetEnv(append(os.Environ(), w.env...))
cmd.SetDir(w.dir)
out, err := cmd.CombinedOutput()
w.logger.Debug("plan ended", "out", string(out))
w.logger.Debug("plan ended", "out", w.filterFn(string(out)))
if err != nil {
return PlanResult{}, tferrors.NewPlanFailed(out)
}
Expand Down
84 changes: 66 additions & 18 deletions pkg/terraform/workspace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var (
changeSummaryAdd = `{"@level":"info","@message":"Plan: 1 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","@timestamp":"0000-00-00T00:00:00.000000+03:00","changes":{"add":1,"change":0,"remove":0,"operation":"plan"},"type":"change_summary"}`
changeSummaryUpdate = `{"@level":"info","@message":"Plan: 0 to add, 1 to change, 0 to destroy.","@module":"terraform.ui","@timestamp":"0000-00-00T00:00:00.000000+03:00","changes":{"add":0,"change":1,"remove":0,"operation":"plan"},"type":"change_summary"}`
changeSummaryNoAction = `{"@level":"info","@message":"Plan: 0 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","@timestamp":"0000-00-00T00:00:00.000000+03:00","changes":{"add":0,"change":0,"remove":0,"operation":"plan"},"type":"change_summary"}`
filter = `{"@level":"info","@message":"Terraform 1.2.1","@module":"terraform.ui","@timestamp":"2022-08-08T14:42:59.377073+03:00","terraform":"1.2.1","type":"version","ui":"1.0"}
{"@level":"error","@message":"Error: error configuring Terraform AWS Provider: error validating provider credentials: error calling sts:GetCallerIdentity: operation error STS: GetCallerIdentity, https response error StatusCode: 403, RequestID: *****, api error InvalidClientTokenId: The security token included in the request is invalid.","@module":"terraform.ui","@timestamp":"2022-08-08T14:43:00.808602+03:00","diagnostic":{"severity":"error","summary":"error configuring Terraform AWS Provider: error validating provider credentials: error calling sts:GetCallerIdentity: operation error STS: GetCallerIdentity, https response error StatusCode: 403, RequestID: *****, api error InvalidClientTokenId: The security token included in the request is invalid.","detail":"","address":"provider[\"registry.terraform.io/hashicorp/aws\"]","range":{"filename":"main.tf.json","start":{"line":1,"column":173,"byte":172},"end":{"line":1,"column":174,"byte":173}},"snippet":{"context":"provider.aws","code":"{\"provider\":{\"aws\":{\"access_key\":\"*****\",\"region\":\"us-east-1\",\"secret_key\":\"/*****\",\"skip_region_validation\":true,\"token\":\"\"}},\"resource\":{\"aws_iam_user\":{\"sample-user\":{\"lifecycle\":{\"prevent_destroy\":true},\"name\":\"sample-user\",\"tags\":{\"crossplane-kind\":\"user.iam.aws.upbound.io\",\"crossplane-name\":\"sample-user\",\"crossplane-providerconfig\":\"default\"}}}},\"terraform\":{\"required_providers\":{\"aws\":{\"source\":\"hashicorp/aws\",\"version\":\"4.15.1\"}}}}","start_line":1,"highlight_start_offset":172,"highlight_end_offset":173,"values":[]}},"type":"diagnostic"}`

state = &json.StateV4{
Version: uint64(version),
Expand All @@ -49,6 +51,10 @@ var (
}

tfstate = `{"version": 1,"terraform_version": "1.0.10","serial": 3,"lineage": "very-cool-lineage","outputs": {},"resources": []}`

filterFn = func(s string) string {
return ""
}
)

func newFakeExec(stdOut string, err error) *testingexec.FakeExec {
Expand Down Expand Up @@ -83,15 +89,16 @@ func TestWorkspaceApply(t *testing.T) {
"Running": {
args: args{
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil}),
WithAferoFs(fs)),
WithAferoFs(fs), WithFilterFn(filterFn)),
},
want: want{
err: errors.Errorf("%s operation that started at %s is still running", testType, now.String()),
},
},
"Success": {
args: args{
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithAferoFs(fs)),
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
r: ApplyResult{
Expand All @@ -101,12 +108,22 @@ func TestWorkspaceApply(t *testing.T) {
},
"Failure": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithAferoFs(fs)),
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewApplyFailed([]byte(errBoom.Error())),
},
},
"Filter": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(filter, errors.New(filter))), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewApplyFailed([]byte(filter)),
},
},
}

for name, tc := range cases {
Expand Down Expand Up @@ -139,7 +156,8 @@ func TestWorkspaceDestroy(t *testing.T) {
}{
"Running": {
args: args{
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil})),
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil}),
WithFilterFn(filterFn)),
},
want: want{
err: errors.Errorf("%s operation that started at %s is still running", testType, now.String()),
Expand All @@ -148,18 +166,27 @@ func TestWorkspaceDestroy(t *testing.T) {
"Success": {
args: args{
w: NewWorkspace(
directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true})),
directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithFilterFn(filterFn)),
},
want: want{},
},
"Failure": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom))),
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewDestroyFailed([]byte(errBoom.Error())),
},
},
"Filter": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(filter, errors.New(filter))), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewDestroyFailed([]byte(filter)),
},
},
}

for name, tc := range cases {
Expand Down Expand Up @@ -188,7 +215,7 @@ func TestWorkspaceRefresh(t *testing.T) {
"Running": {
args: args{
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: applyType, startTime: &now, endTime: nil}),
WithAferoFs(fs)),
WithAferoFs(fs), WithFilterFn(filterFn)),
},
want: want{
r: RefreshResult{
Expand All @@ -199,7 +226,8 @@ func TestWorkspaceRefresh(t *testing.T) {
"Success": {
args: args{
w: NewWorkspace(
directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithAferoFs(fs)),
directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
r: RefreshResult{
Expand All @@ -209,12 +237,22 @@ func TestWorkspaceRefresh(t *testing.T) {
},
"Failure": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithAferoFs(fs)),
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewRefreshFailed([]byte(errBoom.Error())),
},
},
"Filter": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(filter, errors.New(filter))), WithAferoFs(fs),
WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewRefreshFailed([]byte(filter)),
},
},
}

for name, tc := range cases {
Expand Down Expand Up @@ -256,15 +294,15 @@ func TestWorkspacePlan(t *testing.T) {
},
"NoChangeSummary": {
args: args{
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true})),
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithFilterFn(filterFn)),
},
want: want{
err: errors.Errorf("cannot find the change summary line in plan log: "),
},
},
"ChangeSummaryAdd": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryAdd, nil))),
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryAdd, nil)), WithFilterFn(filterFn)),
},
want: want{
r: PlanResult{
Expand All @@ -275,7 +313,7 @@ func TestWorkspacePlan(t *testing.T) {
},
"ChangeSummaryUpdate": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryUpdate, nil))),
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryUpdate, nil)), WithFilterFn(filterFn)),
},
want: want{
r: PlanResult{
Expand All @@ -286,7 +324,7 @@ func TestWorkspacePlan(t *testing.T) {
},
"ChangeSummaryNoAction": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryNoAction, nil))),
w: NewWorkspace(directory, WithExecutor(newFakeExec(changeSummaryNoAction, nil)), WithFilterFn(filterFn)),
},
want: want{
r: PlanResult{
Expand All @@ -297,12 +335,20 @@ func TestWorkspacePlan(t *testing.T) {
},
"Failure": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom))),
w: NewWorkspace(directory, WithExecutor(newFakeExec(errBoom.Error(), errBoom)), WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewPlanFailed([]byte(errBoom.Error())),
},
},
"Filter": {
args: args{
w: NewWorkspace(directory, WithExecutor(newFakeExec(filter, errors.New(filter))), WithAferoFs(fs), WithFilterFn(filterFn)),
},
want: want{
err: tferrors.NewPlanFailed([]byte(filter)),
},
},
}

for name, tc := range cases {
Expand Down Expand Up @@ -336,15 +382,16 @@ func TestWorkspaceApplyAsync(t *testing.T) {
}{
"Running": {
args: args{
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil})),
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil}),
WithFilterFn(filterFn)),
},
want: want{
err: errors.Errorf("%s operation that started at %s is still running", testType, now.String()),
},
},
"Callback": {
args: args{
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true})),
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithFilterFn(filterFn)),
c: func(err error, ctx context.Context) error {
calls <- true
return nil
Expand Down Expand Up @@ -391,15 +438,16 @@ func TestWorkspaceDestroyAsync(t *testing.T) {
}{
"Running": {
args: args{
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil})),
w: NewWorkspace(directory, WithLastOperation(&Operation{Type: testType, startTime: &now, endTime: nil}),
WithFilterFn(filterFn)),
},
want: want{
err: errors.Errorf("%s operation that started at %s is still running", testType, now.String()),
},
},
"Callback": {
args: args{
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true})),
w: NewWorkspace(directory, WithExecutor(&testingexec.FakeExec{DisableScripts: true}), WithFilterFn(filterFn)),
c: func(err error, ctx context.Context) error {
calls <- true
return nil
Expand Down