From 3889a5a0f4a9ac8385d7ba278bb520f6db37f8a6 Mon Sep 17 00:00:00 2001
From: Thomas Hallgren <thomas@tada.se>
Date: Thu, 2 Jan 2025 15:32:23 +0100
Subject: [PATCH] Changes in response to code-review.

- Make k8sapi.CanI a variadic function (and use where applicable).
- Sort AgentEnv.Excluded before checking slices for equality.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
---
 cmd/traffic/cmd/manager/cluster/info.go  | 19 +++++-------
 cmd/traffic/cmd/manager/config/config.go | 10 +++++--
 pkg/k8sapi/cani.go                       | 37 +++++++++++++-----------
 3 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/cmd/traffic/cmd/manager/cluster/info.go b/cmd/traffic/cmd/manager/cluster/info.go
index dee716efbc..a53e7fb6ef 100644
--- a/cmd/traffic/cmd/manager/cluster/info.go
+++ b/cmd/traffic/cmd/manager/cluster/info.go
@@ -296,17 +296,14 @@ func clusterDomainFromResolvConf(confFile, namespace string) (string, error) {
 }
 
 func (oi *info) watchNodeSubnets(ctx context.Context, mustSucceed bool) bool {
-	ok, err := k8sapi.CanI(ctx, &auth.ResourceAttributes{
-		Verb:     "list",
-		Resource: "nodes",
-	})
-	if err != nil || !ok {
-		return false
-	}
-	ok, err = k8sapi.CanI(ctx, &auth.ResourceAttributes{
-		Verb:     "watch",
-		Resource: "nodes",
-	})
+	ok, err := k8sapi.CanI(ctx,
+		&auth.ResourceAttributes{
+			Verb:     "list",
+			Resource: "nodes",
+		}, &auth.ResourceAttributes{
+			Verb:     "watch",
+			Resource: "nodes",
+		})
 	if err != nil || !ok {
 		return false
 	}
diff --git a/cmd/traffic/cmd/manager/config/config.go b/cmd/traffic/cmd/manager/config/config.go
index f030e83ad4..450d62a1c6 100644
--- a/cmd/traffic/cmd/manager/config/config.go
+++ b/cmd/traffic/cmd/manager/config/config.go
@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"slices"
+	"sort"
 	"sync"
 
 	core "k8s.io/api/core/v1"
@@ -164,9 +165,12 @@ func (c *config) refreshFile(ctx context.Context, mapData map[string]string) {
 		}
 		if err != nil {
 			dlog.Errorf(ctx, "failed to unmarshal YAML from %s: %v", agentEnvConfigFileName, err)
-		} else if !ae.Equal(c.agentEnv) {
-			c.agentEnv = ae
-			dlog.Debugf(ctx, "Refreshed agent-env:\n%s", yml)
+		} else {
+			sort.Strings(ae.Excluded)
+			if !ae.Equal(c.agentEnv) {
+				c.agentEnv = ae
+				dlog.Debugf(ctx, "Refreshed agent-env:\n%s", yml)
+			}
 		}
 	} else if !c.agentEnv.Equal(ae) {
 		c.agentEnv = ae
diff --git a/pkg/k8sapi/cani.go b/pkg/k8sapi/cani.go
index a4f257fece..954c21ce26 100644
--- a/pkg/k8sapi/cani.go
+++ b/pkg/k8sapi/cani.go
@@ -10,26 +10,29 @@ import (
 	"github.com/datawire/dlib/dlog"
 )
 
-func CanI(ctx context.Context, ra *auth.ResourceAttributes) (bool, error) {
+func CanI(ctx context.Context, ras ...*auth.ResourceAttributes) (bool, error) {
 	authHandler := GetK8sInterface(ctx).AuthorizationV1().SelfSubjectAccessReviews()
-	review := auth.SelfSubjectAccessReview{Spec: auth.SelfSubjectAccessReviewSpec{ResourceAttributes: ra}}
-	ar, err := authHandler.Create(ctx, &review, meta.CreateOptions{})
-	if err == nil && ar.Status.Allowed {
-		return true, nil
-	}
-	where := ""
-	if ra.Namespace != "" {
-		where = " in namespace " + ra.Namespace
-	}
-	if err != nil {
-		err = fmt.Errorf(`unable to do "can-i %s %s%s": %v`, ra.Verb, ra.Resource, where, err)
-		if ctx.Err() == nil {
-			dlog.Error(ctx, err)
+	for _, ra := range ras {
+		review := auth.SelfSubjectAccessReview{Spec: auth.SelfSubjectAccessReviewSpec{ResourceAttributes: ra}}
+		ar, err := authHandler.Create(ctx, &review, meta.CreateOptions{})
+		if err == nil && ar.Status.Allowed {
+			continue
+		}
+		where := ""
+		if ra.Namespace != "" {
+			where = " in namespace " + ra.Namespace
+		}
+		if err != nil {
+			err = fmt.Errorf(`unable to do "can-i %s %s%s": %v`, ra.Verb, ra.Resource, where, err)
+			if ctx.Err() == nil {
+				dlog.Error(ctx, err)
+			}
+		} else {
+			dlog.Infof(ctx, `"can-i %s %s%s" is not allowed`, ra.Verb, ra.Resource, where)
 		}
-	} else {
-		dlog.Infof(ctx, `"can-i %s %s%s" is not allowed`, ra.Verb, ra.Resource, where)
+		return false, err
 	}
-	return false, err
+	return true, nil
 }
 
 func CanWatch(ctx context.Context, group, resource, name, ns string) bool {