cmd/{containerboot,k8s-operator}: don't return pointers to maps (#19593)

This commit modifies the usage of the `egressservices.Configs` type
within containerboot and the k8s operator.

Originally it was being thrown around as a pointer which is not required
as maps are already pointers under the hood.

Signed-off-by: David Bond <davidsbond93@gmail.com>

cmd/containerboot/egressservices.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/fsnotify/fsnotify"
+
 	"tailscale.com/client/local"
 	"tailscale.com/ipn"
 	"tailscale.com/kube/egressservices"
@@ -194,7 +195,7 @@ func (ep *egressProxy) addrsHaveChanged(n ipn.Notify) bool {
 // syncEgressConfigs adds and deletes firewall rules to match the desired
 // configuration. It uses the provided status to determine what is currently
 // applied and updates the status after a successful sync.
-func (ep *egressProxy) syncEgressConfigs(cfgs *egressservices.Configs, status *egressservices.Status, n ipn.Notify) (*egressservices.Status, error) {
+func (ep *egressProxy) syncEgressConfigs(cfgs egressservices.Configs, status *egressservices.Status, n ipn.Notify) (*egressservices.Status, error) {
 	if !(wantsServicesConfigured(cfgs) || hasServicesConfigured(status)) {
 		return nil, nil
 	}
@@ -212,7 +213,7 @@ func (ep *egressProxy) syncEgressConfigs(cfgs *egressservices.Configs, status *e
 	// Add new services, update rules for any that have changed.
 	rulesPerSvcToAdd := make(map[string][]rule, 0)
 	rulesPerSvcToDelete := make(map[string][]rule, 0)
-	for svcName, cfg := range *cfgs {
+	for svcName, cfg := range cfgs {
 		tailnetTargetIPs, err := ep.tailnetTargetIPsForSvc(cfg, n)
 		if err != nil {
 			return nil, fmt.Errorf("error determining tailnet target IPs: %w", err)
@@ -352,7 +353,7 @@ func updatesForCfg(svcName string, cfg egressservices.Config, status *egressserv
 
 // deleteUnneccessaryServices ensure that any services found on status, but not
 // present in config are deleted.
-func (ep *egressProxy) deleteUnnecessaryServices(cfgs *egressservices.Configs, status *egressservices.Status) error {
+func (ep *egressProxy) deleteUnnecessaryServices(cfgs egressservices.Configs, status *egressservices.Status) error {
 	if !hasServicesConfigured(status) {
 		return nil
 	}
@@ -367,7 +368,7 @@ func (ep *egressProxy) deleteUnnecessaryServices(cfgs *egressservices.Configs, s
 	}
 
 	for svcName, svc := range status.Services {
-		if _, ok := (*cfgs)[svcName]; !ok {
+		if _, ok := cfgs[svcName]; !ok {
 			log.Printf("service %s is no longer required, deleting", svcName)
 			if err := ensureServiceDeleted(svcName, svc, ep.nfr); err != nil {
 				return fmt.Errorf("error deleting service %s: %w", svcName, err)
@@ -379,7 +380,7 @@ func (ep *egressProxy) deleteUnnecessaryServices(cfgs *egressservices.Configs, s
 }
 
 // getConfigs gets the mounted egress service configuration.
-func (ep *egressProxy) getConfigs() (*egressservices.Configs, error) {
+func (ep *egressProxy) getConfigs() (egressservices.Configs, error) {
 	svcsCfg := filepath.Join(ep.cfgPath, egressservices.KeyEgressServices)
 	j, err := os.ReadFile(svcsCfg)
 	if os.IsNotExist(err) {
@@ -391,7 +392,7 @@ func (ep *egressProxy) getConfigs() (*egressservices.Configs, error) {
 	if len(j) == 0 || string(j) == "" {
 		return nil, nil
 	}
-	cfg := &egressservices.Configs{}
+	cfg := egressservices.Configs{}
 	if err := json.Unmarshal(j, &cfg); err != nil {
 		return nil, err
 	}
@@ -602,8 +603,8 @@ type rule struct {
 	protocol      string
 }
 
-func wantsServicesConfigured(cfgs *egressservices.Configs) bool {
+func wantsServicesConfigured(cfgs egressservices.Configs) bool {
-	return cfgs != nil && len(*cfgs) != 0
+	return cfgs != nil && len(cfgs) != 0
 }
 
 func hasServicesConfigured(status *egressservices.Status) bool {
@@ -657,13 +658,13 @@ func (ep *egressProxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 // would normally be this Pod. When this Pod is being deleted, the operator should have removed it from the Service
 // backends and eventually kube proxy routing rules should be updated to no longer route traffic for the Service to this
 // Pod.
-func (ep *egressProxy) waitTillSafeToShutdown(ctx context.Context, cfgs *egressservices.Configs, hp int) {
+func (ep *egressProxy) waitTillSafeToShutdown(ctx context.Context, cfgs egressservices.Configs, hp int) {
-	if cfgs == nil || len(*cfgs) == 0 { // avoid sleeping if no services are configured
+	if cfgs == nil || len(cfgs) == 0 { // avoid sleeping if no services are configured
 		return
 	}
 	log.Printf("Ensuring that cluster traffic for egress targets is no longer routed via this Pod...")
 	var wg sync.WaitGroup
-	for s, cfg := range *cfgs {
+	for s, cfg := range cfgs {
 		hep := cfg.HealthCheckEndpoint
 		if hep == "" {
 			log.Printf("Tailnet target %q does not have a cluster healthcheck specified, unable to verify if cluster traffic for the target is still routed via this Pod", s)

cmd/containerboot/egressservices_test.go

@@ -255,13 +255,13 @@ func TestWaitTillSafeToShutdown(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cfgs := &egressservices.Configs{}
+			cfgs := egressservices.Configs{}
 			switches := make(map[string]int)
 
 			for svc, callsToSwitch := range tt.services {
 				endpoint := fmt.Sprintf("http://%s.local", svc)
 				if tt.healthCheckSet {
-					(*cfgs)[svc] = egressservices.Config{
+					cfgs[svc] = egressservices.Config{
 						HealthCheckEndpoint: endpoint,
 					}
 				}

cmd/k8s-operator/egress-eps.go

@@ -20,6 +20,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
 	"tailscale.com/kube/egressservices"
 )
 
@@ -90,7 +91,7 @@ func (er *egressEpsReconciler) Reconcile(ctx context.Context, req reconcile.Requ
 		lg.Debugf("No egress config found, likely because ProxyGroup has not been created")
 		return res, nil
 	}
-	cfg, ok := (*cfgs)[tailnetSvc]
+	cfg, ok := cfgs[tailnetSvc]
 	if !ok {
 		lg.Infof("[unexpected] configuration for tailnet service %s not found", tailnetSvc)
 		return res, nil

cmd/k8s-operator/egress-services.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
 	tsoperator "tailscale.com/k8s-operator"
 	tsapi "tailscale.com/k8s-operator/apis/v1alpha1"
 	"tailscale.com/kube/egressservices"
@@ -347,11 +348,11 @@ func (esr *egressSvcsReconciler) provision(ctx context.Context, proxyGroupName s
 		return nil, false, nil
 	}
 	tailnetSvc := tailnetSvcName(svc)
-	gotCfg := (*cfgs)[tailnetSvc]
+	gotCfg := cfgs[tailnetSvc]
 	wantsCfg := egressSvcCfg(svc, clusterIPSvc, esr.tsNamespace, lg)
 	if !reflect.DeepEqual(gotCfg, wantsCfg) {
 		lg.Debugf("updating egress services ConfigMap %s", cm.Name)
-		mak.Set(cfgs, tailnetSvc, wantsCfg)
+		mak.Set(&cfgs, tailnetSvc, wantsCfg)
 		bs, err := json.Marshal(cfgs)
 		if err != nil {
 			return nil, false, fmt.Errorf("error marshalling egress services configs: %w", err)
@@ -485,19 +486,19 @@ func (esr *egressSvcsReconciler) ensureEgressSvcCfgDeleted(ctx context.Context,
 		lggr.Debugf("ConfigMap does not contain egress service configs")
 		return nil
 	}
-	cfgs := &egressservices.Configs{}
+	cfgs := egressservices.Configs{}
-	if err := json.Unmarshal(bs, cfgs); err != nil {
+	if err := json.Unmarshal(bs, &cfgs); err != nil {
 		return fmt.Errorf("error unmarshalling egress services configs")
 	}
 	tailnetSvc := tailnetSvcName(svc)
-	_, ok := (*cfgs)[tailnetSvc]
+	_, ok := cfgs[tailnetSvc]
 	if !ok {
 		lggr.Debugf("ConfigMap does not contain egress service config, likely because it was already deleted")
 		return nil
 	}
-	lggr.Infof("before deleting config %+#v", *cfgs)
+	lggr.Infof("before deleting config %+#v", cfgs)
-	delete(*cfgs, tailnetSvc)
+	delete(cfgs, tailnetSvc)
-	lggr.Infof("after deleting config %+#v", *cfgs)
+	lggr.Infof("after deleting config %+#v", cfgs)
 	bs, err := json.Marshal(cfgs)
 	if err != nil {
 		return fmt.Errorf("error marshalling egress services configs: %w", err)
@@ -649,7 +650,7 @@ func isEgressSvcForProxyGroup(obj client.Object) bool {
 
 // egressSvcConfig returns a ConfigMap that contains egress services configuration for the provided ProxyGroup as well
 // as unmarshalled configuration from the ConfigMap.
-func egressSvcsConfigs(ctx context.Context, cl client.Client, proxyGroupName, tsNamespace string) (cm *corev1.ConfigMap, cfgs *egressservices.Configs, err error) {
+func egressSvcsConfigs(ctx context.Context, cl client.Client, proxyGroupName, tsNamespace string) (cm *corev1.ConfigMap, cfgs egressservices.Configs, err error) {
 	name := pgEgressCMName(proxyGroupName)
 	cm = &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
@@ -664,9 +665,9 @@ func egressSvcsConfigs(ctx context.Context, cl client.Client, proxyGroupName, ts
 	if err != nil {
 		return nil, nil, fmt.Errorf("error retrieving egress services ConfigMap %s: %v", name, err)
 	}
-	cfgs = &egressservices.Configs{}
+	cfgs = egressservices.Configs{}
 	if len(cm.BinaryData[egressservices.KeyEgressServices]) != 0 {
-		if err := json.Unmarshal(cm.BinaryData[egressservices.KeyEgressServices], cfgs); err != nil {
+		if err := json.Unmarshal(cm.BinaryData[egressservices.KeyEgressServices], &cfgs); err != nil {
 			return nil, nil, fmt.Errorf("error unmarshaling egress services config %v: %w", cm.BinaryData[egressservices.KeyEgressServices], err)
 		}
 	}

cmd/k8s-operator/egress-services_test.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
 	tsapi "tailscale.com/k8s-operator/apis/v1alpha1"
 	"tailscale.com/kube/egressservices"
 	"tailscale.com/tstest"
@@ -284,11 +285,11 @@ func configFromCM(t *testing.T, cm *corev1.ConfigMap, svcName string) *egressser
 	if !ok {
 		return nil
 	}
-	cfgs := &egressservices.Configs{}
+	cfgs := egressservices.Configs{}
-	if err := json.Unmarshal(cfgBs, cfgs); err != nil {
+	if err := json.Unmarshal(cfgBs, &cfgs); err != nil {
 		t.Fatalf("error unmarshalling config: %v", err)
 	}
-	cfg, ok := (*cfgs)[svcName]
+	cfg, ok := cfgs[svcName]
 	if ok {
 		return &cfg
 	}