Repository: kubernetes-sigs/aws-alb-ingress-controller
Pull request #1228 (2020-04-18 02:50:25)

redact oidc
Fix https://github.com/kubernetes-sigs/aws-alb-ingress-controller/issues/1227
redact oidc clientID and secret in logs.

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-alb-ingress-controller/pull/1228#" title="Author self-approved">M00nF1sh</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Faws-alb-ingress-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/aws-alb-ingress-controller/blob/master/OWNERS)~~ [M00nF1sh]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
k8s-ci-robot: @M00nF1sh: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-aws-alb-ingress-controller-lint | d7a2f1ab3d4e895562944b76fb1490d448297426 | [link](https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_aws-alb-ingress-controller/1228/pull-aws-alb-ingress-controller-lint/1251342367319920641) | `/test pull-aws-alb-ingress-controller-lint`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=aws-alb-ingress-controller&pr=1228). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3M00nF1sh). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/aws-alb-ingress-controller/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/internal/alb/ls/listener.go b/internal/alb/ls/listener.go
index 082f7c293..32410c443 100644
--- a/internal/alb/ls/listener.go
+++ b/internal/alb/ls/listener.go
@@ -160,7 +160,9 @@ func (controller *defaultController) LSInstanceNeedsModification(ctx context.Con
 		needModification = true
 	}
 	if !actionsMatches(instance.DefaultActions, config.DefaultActions) {
-		albctx.GetLogger(ctx).DebugLevelf(1, "listener defaultActions needs modification: %v => %v", awsutil.Prettify(instance.DefaultActions), awsutil.Prettify(config.DefaultActions))
+		albctx.GetLogger(ctx).DebugLevelf(1, "listener defaultActions needs modification",
+			awsutil.Prettify(redactActions(instance.DefaultActions)),
+			awsutil.Prettify(redactActions(config.DefaultActions)))
 		needModification = true
 	}
 	return needModification
diff --git a/internal/alb/ls/rules.go b/internal/alb/ls/rules.go
index a66c1b026..69ba579ff 100644
--- a/internal/alb/ls/rules.go
+++ b/internal/alb/ls/rules.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"strconv"
 
+	"github.com/kubernetes-sigs/aws-alb-ingress-controller/pkg/util"
+
 	"github.com/kubernetes-sigs/aws-alb-ingress-controller/internal/ingress/annotations/conditions"
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -576,3 +578,18 @@ func isUnconditionalRedirect(listener *elbv2.Listener, r elbv2.Rule) bool {
 	}
 	return false
 }
+
+// redactActions will redact sensitive information from actions, so it's safe for logging.
+func redactActions(actions []*elbv2.Action) []*elbv2.Action {
+	actionsClone := make([]*elbv2.Action, len(actions))
+	for index, action := range actions {
+		actionClone := &elbv2.Action{}
+		util.DeepCopyInto(actionClone, action)
+		if actionClone.AuthenticateOidcConfig != nil {
+			actionClone.AuthenticateOidcConfig.ClientId = aws.String("<redacted>")
+			actionClone.AuthenticateOidcConfig.ClientSecret = aws.String("<redacted>")
+		}
+		actionsClone[index] = actionClone
+	}
+	return actionsClone
+}
diff --git a/pkg/util/deepcopy.go b/pkg/util/deepcopy.go
new file mode 100644
index 000000000..6a96f361a
--- /dev/null
+++ b/pkg/util/deepcopy.go
@@ -0,0 +1,14 @@
+package util
+
+import (
+	"bytes"
+	"encoding/gob"
+)
+
+func DeepCopyInto(to interface{}, from interface{}) {
+	buff := new(bytes.Buffer)
+	enc := gob.NewEncoder(buff)
+	dec := gob.NewDecoder(buff)
+	_ = enc.Encode(from)
+	_ = dec.Decode(to)
+}


Test patch:
diff --git a/internal/alb/ls/rules_test.go b/internal/alb/ls/rules_test.go
index b2f564793..80fb4f81b 100644
--- a/internal/alb/ls/rules_test.go
+++ b/internal/alb/ls/rules_test.go
@@ -3101,3 +3101,72 @@ func redirectActionConfig(override *elbv2.RedirectActionConfig) *elbv2.RedirectA
 	}
 	return r
 }
+
+func Test_redactActions(t *testing.T) {
+	type args struct {
+		actions []*elbv2.Action
+	}
+	tests := []struct {
+		name string
+		args args
+		want []*elbv2.Action
+	}{
+		{
+			name: "actions needs redact",
+			args: args{
+				actions: []*elbv2.Action{
+					{
+						AuthenticateOidcConfig: &elbv2.AuthenticateOidcActionConfig{
+							ClientId:      aws.String("my-client-id"),
+							ClientSecret:  aws.String("my-secret"),
+							TokenEndpoint: aws.String("endpoint-1"),
+						},
+					},
+					{
+						AuthenticateOidcConfig: &elbv2.AuthenticateOidcActionConfig{
+							ClientId:      aws.String("my-client-id"),
+							ClientSecret:  aws.String("my-secret"),
+							TokenEndpoint: aws.String("endpoint-2"),
+						},
+					},
+				},
+			},
+			want: []*elbv2.Action{
+				{
+					AuthenticateOidcConfig: &elbv2.AuthenticateOidcActionConfig{
+						ClientId:      aws.String("<redacted>"),
+						ClientSecret:  aws.String("<redacted>"),
+						TokenEndpoint: aws.String("endpoint-1"),
+					},
+				},
+				{
+					AuthenticateOidcConfig: &elbv2.AuthenticateOidcActionConfig{
+						ClientId:      aws.String("<redacted>"),
+						ClientSecret:  aws.String("<redacted>"),
+						TokenEndpoint: aws.String("endpoint-2"),
+					},
+				},
+			},
+		},
+		{
+			name: "empty actions",
+			args: args{
+				actions: []*elbv2.Action{},
+			},
+			want: []*elbv2.Action{},
+		},
+		{
+			name: "nil actions",
+			args: args{
+				actions: nil,
+			},
+			want: []*elbv2.Action{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := redactActions(tt.args.actions)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/pkg/util/deepcopy_test.go b/pkg/util/deepcopy_test.go
new file mode 100644
index 000000000..4f9fa66ba
--- /dev/null
+++ b/pkg/util/deepcopy_test.go
@@ -0,0 +1,41 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type structA struct {
+	Name string
+}
+
+type structB struct {
+	Name string
+	A    *structA
+}
+
+func TestDeepCopyInto(t *testing.T) {
+	obj := structB{
+		Name: "parent",
+		A: &structA{
+			Name: "child-1",
+		},
+	}
+	objClone := structB{}
+	DeepCopyInto(&objClone, obj)
+	obj.A.Name = "child-2"
+
+	assert.Equal(t, structB{
+		Name: "parent",
+		A: &structA{
+			Name: "child-2",
+		},
+	}, obj)
+	assert.Equal(t, structB{
+		Name: "parent",
+		A: &structA{
+			Name: "child-1",
+		},
+	}, objClone)
+}