Skip to content

Clean up service account print and describe #117160

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −75
Open

Clean up service account print and describe #117160

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
3 changes: 1 addition & 2 deletions pkg/printers/internalversion/printers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ func AddHandlers(h printers.PrintHandler) {

serviceAccountColumnDefinitions := []metav1.TableColumnDefinition{
{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
{Name: "Secrets", Type: "string", Description: apiv1.ServiceAccount{}.SwaggerDoc()["secrets"]},
{Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]},
}
_ = h.TableHandler(serviceAccountColumnDefinitions, printServiceAccount)
Expand Down Expand Up @@ -1915,7 +1914,7 @@ func printServiceAccount(obj *api.ServiceAccount, options printers.GenerateOptio
row := metav1.TableRow{
Object: runtime.RawExtension{Object: obj},
}
row.Cells = append(row.Cells, obj.Name, int64(len(obj.Secrets)), translateTimestampSince(obj.CreationTimestamp))
row.Cells = append(row.Cells, obj.Name, translateTimestampSince(obj.CreationTimestamp))
return []metav1.TableRow{row}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/printers/internalversion/printers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func TestPrintServiceAccount(t *testing.T) {
Secrets: []api.ObjectReference{},
},
// Columns: Name, (Num) Secrets, Age
expected: []metav1.TableRow{{Cells: []interface{}{"sa1", int64(0), "0s"}}},
expected: []metav1.TableRow{{Cells: []interface{}{"sa1", "0s"}}},
},
// Basic service account with two secrets.
{
Expand All @@ -483,7 +483,7 @@ func TestPrintServiceAccount(t *testing.T) {
},
},
// Columns: Name, (Num) Secrets, Age
expected: []metav1.TableRow{{Cells: []interface{}{"sa1", int64(2), "0s"}}},
expected: []metav1.TableRow{{Cells: []interface{}{"sa1", "0s"}}},
},
}

Expand Down
72 changes: 5 additions & 67 deletions staging/src/k8s.io/kubectl/pkg/describe/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3445,61 +3445,15 @@ func (d *ServiceAccountDescriber) Describe(namespace, name string, describerSett
return "", err
}

tokens := []corev1.Secret{}

// missingSecrets is the set of all secrets present in the
// serviceAccount but not present in the set of existing secrets.
missingSecrets := sets.New[string]()
secrets := corev1.SecretList{}
err = runtimeresource.FollowContinue(&metav1.ListOptions{Limit: describerSettings.ChunkSize},
func(options metav1.ListOptions) (runtime.Object, error) {
newList, err := d.CoreV1().Secrets(namespace).List(context.TODO(), options)
if err != nil {
return nil, runtimeresource.EnhanceListError(err, options, corev1.ResourceSecrets.String())
}
secrets.Items = append(secrets.Items, newList.Items...)
return newList, nil
})

// errors are tolerated here in order to describe the serviceAccount with all
// of the secrets that it references, even if those secrets cannot be fetched.
if err == nil {
// existingSecrets is the set of all secrets remaining on a
// service account that are not present in the "tokens" slice.
existingSecrets := sets.New[string]()

for _, s := range secrets.Items {
if s.Type == corev1.SecretTypeServiceAccountToken {
name := s.Annotations[corev1.ServiceAccountNameKey]
uid := s.Annotations[corev1.ServiceAccountUIDKey]
if name == serviceAccount.Name && uid == string(serviceAccount.UID) {
tokens = append(tokens, s)
}
}
existingSecrets.Insert(s.Name)
}

for _, s := range serviceAccount.Secrets {
if !existingSecrets.Has(s.Name) {
missingSecrets.Insert(s.Name)
}
}
for _, s := range serviceAccount.ImagePullSecrets {
if !existingSecrets.Has(s.Name) {
missingSecrets.Insert(s.Name)
}
}
}

var events *corev1.EventList
if describerSettings.ShowEvents {
events, _ = searchEvents(d.CoreV1(), serviceAccount, describerSettings.ChunkSize)
}

return describeServiceAccount(serviceAccount, tokens, missingSecrets, events)
return describeServiceAccount(serviceAccount, events)
}

func describeServiceAccount(serviceAccount *corev1.ServiceAccount, tokens []corev1.Secret, missingSecrets sets.Set[string], events *corev1.EventList) (string, error) {
func describeServiceAccount(serviceAccount *corev1.ServiceAccount, events *corev1.EventList) (string, error) {
return tabbedString(func(out io.Writer) error {
w := NewPrefixWriter(out)
w.Write(LEVEL_0, "Name:\t%s\n", serviceAccount.Name)
Expand All @@ -3510,28 +3464,16 @@ func describeServiceAccount(serviceAccount *corev1.ServiceAccount, tokens []core
var (
emptyHeader = " "
pullHeader = "Image pull secrets:"
mountHeader = "Mountable secrets: "
tokenHeader = "Tokens: "

pullSecretNames = []string{}
mountSecretNames = []string{}
tokenSecretNames = []string{}
pullSecretNames = []string{}
)

for _, s := range serviceAccount.ImagePullSecrets {
pullSecretNames = append(pullSecretNames, s.Name)
}
for _, s := range serviceAccount.Secrets {
mountSecretNames = append(mountSecretNames, s.Name)
}
for _, s := range tokens {
tokenSecretNames = append(tokenSecretNames, s.Name)
}

types := map[string][]string{
pullHeader: pullSecretNames,
mountHeader: mountSecretNames,
tokenHeader: tokenSecretNames,
pullHeader: pullSecretNames,
}
for _, header := range sets.List(sets.KeySet(types)) {
names := types[header]
Expand All @@ -3540,11 +3482,7 @@ func describeServiceAccount(serviceAccount *corev1.ServiceAccount, tokens []core
} else {
prefix := header
for _, name := range names {
if missingSecrets.Has(name) {
w.Write(LEVEL_0, "%s\t%s (not found)\n", prefix, name)
} else {
w.Write(LEVEL_0, "%s\t%s\n", prefix, name)
}
w.Write(LEVEL_0, "%s\t%s\n", prefix, name)
prefix = emptyHeader
}
}
Expand Down
4 changes: 1 addition & 3 deletions staging/src/k8s.io/kubectl/pkg/describe/describe_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6238,9 +6238,7 @@ func TestDescribeServiceAccount(t *testing.T) {
Namespace: foo
Labels: <none>
Annotations: <none>
Image pull secrets: test-local-ref (not found)
Mountable secrets: test-objectref (not found)
Tokens: <none>
Image pull secrets: test-local-ref
Events: <none>` + "\n"
if out != expectedOut {
t.Errorf("expected : %q\n but got output:\n %q", expectedOut, out)
Expand Down
2 changes: 1 addition & 1 deletion test/cmd/core.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ run_service_accounts_tests() {
# Post-condition: secret exists and has expected values
kube::test::get_object_assert 'serviceaccount/test-service-account --namespace=test-service-accounts' "{{$id_field}}" 'test-service-account'
# Describe command should respect the chunk size parameter
kube::test::describe_resource_chunk_size_assert serviceaccounts secrets,events "--namespace=test-service-accounts"
kube::test::describe_resource_chunk_size_assert serviceaccounts events "--namespace=test-service-accounts"
# Clean-up
kubectl delete serviceaccount test-service-account --namespace=test-service-accounts
# Clean up
Expand Down