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

feat(api): redact secrets in logs. Fixes #8685 #9859

Closed
49 changes: 49 additions & 0 deletions test/e2e/argo_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1975,3 +1975,52 @@ func (s *ArgoServerSuite) TestRateLimitHeader() {
func TestArgoServerSuite(t *testing.T) {
suite.Run(t, new(ArgoServerSuite))
}

func (s *ArgoServerSuite) TestWorkflowLogRedaction() {
nsName := fixtures.Namespace
// create secret if not present
secretName := "test-secret"
secretData := map[string][]byte{
"testpassword": []byte("S00perS3cretPa55word"),
}
secret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName}, Data: secretData}
ctx := context.Background()
s.Run("CreateSecret", func() {
_, e := s.KubeClient.CoreV1().Secrets(nsName).Create(ctx, secret, metav1.CreateOptions{})
assert.NoError(s.T(), e)
})
defer func() {
// Clean up created secret
_ = s.KubeClient.CoreV1().Secrets(nsName).Delete(ctx, secretName, metav1.DeleteOptions{})
}()

var name string
s.Given().
Workflow("@smoke/workflow-with-secrets.yaml").
When().
SubmitWorkflow().
WaitForWorkflow(fixtures.ToStart).
Then().
ExpectWorkflow(func(t *testing.T, metadata *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
name = metadata.Name
})

// lets check the logs
for _, tt := range []struct {
name string
path string
}{
{"PodLogs", "/" + name + "/log?logOptions.container=main"},
{"WorkflowLogs", "/log?podName=" + name + "&logOptions.container=main"},
} {
s.Run(tt.name, func() {
s.stream("/api/v1/workflows/argo/"+name+tt.path, func(t *testing.T, line string) (done bool) {
if strings.Contains(line, "data: ") {
assert.Contains(t, line, "secret from env: [*********]")
return true
}
return false
})
})
}
}
17 changes: 17 additions & 0 deletions test/e2e/smoke/workflow-with-secrets.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: secrets-
spec:
entrypoint: print-secret
templates:
- name: print-secret
container:
image: argoproj/argosay:v2
args: [echo, "secret from env: $MYSECRETPASSWORD"]
env:
- name: MYSECRETPASSWORD
valueFrom:
secretKeyRef:
name: test-secret
key: testpassword
17 changes: 17 additions & 0 deletions util/logs/workflow-logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ func WorkflowLogs(ctx context.Context, wfClient versioned.Interface, kubeClient

var podListOptions metav1.ListOptions

// get secrets for redaction
secrets, err := kubeClient.CoreV1().Secrets(req.GetNamespace()).List(ctx, metav1.ListOptions{})
Copy link
Member

@terrytangyuan terrytangyuan Oct 21, 2022

Choose a reason for hiding this comment

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

I don't think we want to list all the secrets. See #8534

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@terrytangyuan I can see the GetSecret method is available but it requires the service account name and we can only fetch the secrets associated with a service account using this method.

One approach is using Service account Lister to get all the service accounts and fetch the secrets of each SA using above mentioned GetSecret method. Although we might not have all the secrets with this approach.

Can you suggest any other alternate approaches?

Copy link
Contributor

Choose a reason for hiding this comment

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

@terrytangyuan @anilkumar-pcs
I'd like to resolve #8685 but I don't have any better idea then

  1. list secrets and keep then in cache or queue
  2. when command like "echo" or "export", check command if they include one of those secrets
  3. hide secrets

but I think those feature make large burden on argo-workflows....
any suggestion on this??

if err != nil {
logCtx.WithField("err", err).Debugln("error in listing secrets")
}

// we add selector if cli specify the pod selector when using logs
if req.GetSelector() != "" {
podListOptions = metav1.ListOptions{LabelSelector: common.LabelKeyWorkflow + "=" + req.GetName() + "," + req.GetSelector()}
Expand Down Expand Up @@ -165,6 +171,17 @@ func WorkflowLogs(ctx context.Context, wfClient versioned.Interface, kubeClient
}
if rx.MatchString(content) { // this means we filter the lines in the server, but will still incur the cost of retrieving them from Kubernetes
logCtx.WithFields(log.Fields{"timestamp": timestamp, "content": content}).Debug("Log line")

// log redaction for secrets
if secrets != nil {
for _, s := range secrets.Items {
for _, v := range s.Data {
if strings.Contains(content, string(v)) {
content = strings.Replace(content, string(v), "[*********]", -1)
}
}
}
}
unsortedEntries <- logEntry{podName: podName, content: content, timestamp: timestamp}
}
}
Expand Down