{,cmd/}k8s-operator: review comments and self-review

* rename helm chart config to allowImpersonation
* rename CRD AuthMode to Mode and ensure default is auth
* rename spec.kubeAPIServerConfig to spec.kubeAPIServer for consistency
* use ProxyGroupAvailable reason for ProxyGroupAvailable condition
* react to changes to the static ServiceAccount used for auth mode
* more helpful error message when pg invalid

Change-Id: Ia03bff2622c5e0ae890c5e97b71c9715332e4739
Signed-off-by: Tom Proctor <tomhjp@users.noreply.github.com>

cmd/k8s-operator/deploy/chart/templates/apiserverproxy-rbac.yaml

@@ -1,10 +1,10 @@
 # Copyright (c) Tailscale Inc & AUTHORS
 # SPDX-License-Identifier: BSD-3-Clause
 
-# If deprecated setting used, enable both legacy and new workflows.
+# If old setting used, enable both old (operator) and new (ProxyGroup) workflows.
 # If new setting used, enable only new workflow.
 {{ if or (eq .Values.apiServerProxyConfig.mode "true")
-  (eq .Values.apiServerProxyConfig.authEnabled "true") }}
+  (eq .Values.apiServerProxyConfig.allowImpersonation "true") }}
 apiVersion: v1
 kind: ServiceAccount
 metadata:

cmd/k8s-operator/deploy/chart/values.yaml

@@ -111,13 +111,15 @@ proxyConfig:
 # Kubernetes API server.
 # https://tailscale.com/kb/1437/kubernetes-operator-api-server-proxy
 apiServerProxyConfig:
-  # Set to "true" to enable the ClusterRole permissions required for the API
+  # Set to "true" to create the ClusterRole permissions required for the API
-  # server proxy to impersonate groups and users based on tailnet ACL grants.
+  # server proxy's auth mode. In auth mode, the API server proxy impersonates
-  # Required to deploy ProxyGroups of type "kube-apiserver" in auth mode.
+  # groups and users based on tailnet ACL grants. Required for ProxyGroups of
-  authEnabled: "false" # "true", "false"
+  # type "kube-apiserver" running in auth mode.
+  allowImpersonation: "false" # "true", "false"
 
   # If true or noauth, the operator will run an in-process API server proxy.
-  # Deprecated: use apiServerProxyConfig.authEnabled instead.
+  # You can deploy a ProxyGroup of type "kube-apiserver" to run a high
+  # availability set of API server proxies instead.
   mode: "false" # "true", "false", "noauth"
 
 imagePullSecrets: []

cmd/k8s-operator/deploy/crds/tailscale.com_proxygroups.yaml

@@ -77,18 +77,22 @@ spec:
                     must not start with a dash and must be between 1 and 62 characters long.
                   type: string
                   pattern: ^[a-z0-9][a-z0-9-]{0,61}$
-                kubeAPIServerConfig:
+                kubeAPIServer:
                   description: |-
-                    KubeAPIServerConfig contains configuration specific to the kube-apiserver
+                    KubeAPIServer contains configuration specific to the kube-apiserver
                     ProxyGroup type. This field is only used when Type is set to "kube-apiserver".
                   type: object
                   properties:
-                    authMode:
+                    mode:
                       description: |-
-                        AuthMode enables auth mode for the API Server proxy. In auth mode,
+                        Mode to run the API server proxy in. Supported modes are auth and noauth.
-                        requests from the tailnet proxied over to the Kubernetes API server
+                        In auth mode, requests from the tailnet proxied over to the Kubernetes
-                        are additionally impersonated using the sender's tailnet identity.
+                        API server are additionally impersonated using the sender's tailnet identity.
-                      type: boolean
+                        If not specified, defaults to auth mode.
+                      type: string
+                      enum:
+                        - auth
+                        - noauth
                 proxyClass:
                   description: |-
                     ProxyClass is the name of the ProxyClass custom resource that contains

cmd/k8s-operator/deploy/manifests/operator.yaml

@@ -2904,17 +2904,21 @@ spec:
                                     must not start with a dash and must be between 1 and 62 characters long.
                                 pattern: ^[a-z0-9][a-z0-9-]{0,61}$
                                 type: string
-                            kubeAPIServerConfig:
+                            kubeAPIServer:
                                 description: |-
-                                    KubeAPIServerConfig contains configuration specific to the kube-apiserver
+                                    KubeAPIServer contains configuration specific to the kube-apiserver
                                     ProxyGroup type. This field is only used when Type is set to "kube-apiserver".
                                 properties:
-                                    authMode:
+                                    mode:
                                         description: |-
-                                            AuthMode enables auth mode for the API Server proxy. In auth mode,
+                                            Mode to run the API server proxy in. Supported modes are auth and noauth.
-                                            requests from the tailnet proxied over to the Kubernetes API server
+                                            In auth mode, requests from the tailnet proxied over to the Kubernetes
-                                            are additionally impersonated using the sender's tailnet identity.
+                                            API server are additionally impersonated using the sender's tailnet identity.
-                                        type: boolean
+                                            If not specified, defaults to auth mode.
+                                        enum:
+                                            - auth
+                                            - noauth
+                                        type: string
                                 type: object
                             proxyClass:
                                 description: |-

cmd/k8s-operator/operator.go

@@ -26,6 +26,7 @@ import (
 	networkingv1 "k8s.io/api/networking/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	klabels "k8s.io/apimachinery/pkg/labels"
@@ -632,13 +633,14 @@ func runReconcilers(opts reconcilerOpts) {
 	ownedByProxyGroupFilter := handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &tsapi.ProxyGroup{})
 	proxyClassFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(proxyClassHandlerForProxyGroup(mgr.GetClient(), startlog))
 	nodeFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(nodeHandlerForProxyGroup(mgr.GetClient(), opts.defaultProxyClass, startlog))
+	saFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(serviceAccountHandlerForProxyGroup(mgr.GetClient(), startlog))
 	err = builder.ControllerManagedBy(mgr).
 		For(&tsapi.ProxyGroup{}).
 		Named("proxygroup-reconciler").
 		Watches(&corev1.Service{}, ownedByProxyGroupFilter).
 		Watches(&appsv1.StatefulSet{}, ownedByProxyGroupFilter).
 		Watches(&corev1.ConfigMap{}, ownedByProxyGroupFilter).
-		Watches(&corev1.ServiceAccount{}, ownedByProxyGroupFilter).
+		Watches(&corev1.ServiceAccount{}, saFilterForProxyGroup).
 		Watches(&corev1.Secret{}, ownedByProxyGroupFilter).
 		Watches(&rbacv1.Role{}, ownedByProxyGroupFilter).
 		Watches(&rbacv1.RoleBinding{}, ownedByProxyGroupFilter).
@@ -1002,8 +1004,8 @@ func nodeHandlerForProxyGroup(cl client.Client, defaultProxyClass string, logger
 }
 
 // proxyClassHandlerForProxyGroup returns a handler that, for a given ProxyClass,
-// returns a list of reconcile requests for all Connectors that have
+// returns a list of reconcile requests for all ProxyGroups that have
-// .spec.proxyClass set.
+// .spec.proxyClass set to that ProxyClass.
 func proxyClassHandlerForProxyGroup(cl client.Client, logger *zap.SugaredLogger) handler.MapFunc {
 	return func(ctx context.Context, o client.Object) []reconcile.Request {
 		pgList := new(tsapi.ProxyGroupList)
@@ -1022,6 +1024,37 @@ func proxyClassHandlerForProxyGroup(cl client.Client, logger *zap.SugaredLogger)
 	}
 }
 
+// serviceAccountHandlerForProxyGroup returns a handler that, for a given ServiceAccount,
+// returns a list of reconcile requests for all ProxyGroups that use that ServiceAccount.
+// For most ProxyGroups, this will be a dedicated ServiceAccount owned by a specific
+// ProxyGroup. But for kube-apiserver ProxyGroups running in auth mode, they use a shared
+// static ServiceAccount named "kube-apiserver-auth-proxy".
+func serviceAccountHandlerForProxyGroup(cl client.Client, logger *zap.SugaredLogger) handler.MapFunc {
+	return func(ctx context.Context, o client.Object) []reconcile.Request {
+		pgList := new(tsapi.ProxyGroupList)
+		if err := cl.List(ctx, pgList); err != nil {
+			logger.Debugf("error listing ProxyGroups for ServiceAccount: %v", err)
+			return nil
+		}
+		reqs := make([]reconcile.Request, 0)
+		saName := o.GetName()
+		for _, pg := range pgList.Items {
+			if saName == authAPIServerProxySAName && isAuthAPIServerProxy(&pg) {
+				reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&pg)})
+			}
+			expectedOwner := pgOwnerReference(&pg)[0]
+			saOwnerRefs := o.GetOwnerReferences()
+			for _, ref := range saOwnerRefs {
+				if apiequality.Semantic.DeepEqual(ref, expectedOwner) {
+					reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&pg)})
+					break
+				}
+			}
+		}
+		return reqs
+	}
+}
+
 // serviceHandlerForIngress returns a handler for Service events for ingress
 // reconciler that ensures that if the Service associated with an event is of
 // interest to the reconciler, the associated Ingress(es) gets be reconciled.

cmd/k8s-operator/proxygroup.go

@@ -50,6 +50,7 @@ import (
 const (
 	reasonProxyGroupCreationFailed = "ProxyGroupCreationFailed"
 	reasonProxyGroupReady          = "ProxyGroupReady"
+	reasonProxyGroupAvailable      = "ProxyGroupAvailable"
 	reasonProxyGroupCreating       = "ProxyGroupCreating"
 	reasonProxyGroupInvalid        = "ProxyGroupInvalid"
 
@@ -209,7 +210,9 @@ func (r *ProxyGroupReconciler) validate(ctx context.Context, pg *tsapi.ProxyGrou
 		sa := &corev1.ServiceAccount{}
 		if err := r.Get(ctx, types.NamespacedName{Namespace: r.tsNamespace, Name: authAPIServerProxySAName}, sa); err != nil {
 			if apierrors.IsNotFound(err) {
-				return fmt.Errorf("the ServiceAccount %q used for the API server proxy in auth mode does not exist but should have been created during operator installation", authAPIServerProxySAName)
+				return fmt.Errorf("the ServiceAccount %q used for the API server proxy in auth mode does not exist but "+
+					"should have been created during operator installation; use apiServerProxyConfig.allowImpersonation=true "+
+					"in the helm chart, or authproxy-rbac.yaml from the static manifests", authAPIServerProxySAName)
 			}
 
 			return fmt.Errorf("error validating that ServiceAccount %q exists: %w", authAPIServerProxySAName, err)
@@ -423,7 +426,7 @@ func (r *ProxyGroupReconciler) maybeUpdateStatus(ctx context.Context, logger *za
 	if len(devices) > 0 {
 		status = metav1.ConditionTrue
 		if len(devices) == desiredReplicas {
-			reason = reasonProxyGroupReady
+			reason = reasonProxyGroupAvailable
 		}
 	}
 	tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupAvailable, status, reason, message, 0, r.clock, logger)

cmd/k8s-operator/proxygroup_specs.go

@@ -481,10 +481,14 @@ func pgServiceAccountName(pg *tsapi.ProxyGroup) string {
 }
 
 func isAuthAPIServerProxy(pg *tsapi.ProxyGroup) bool {
-	return pg.Spec.Type == tsapi.ProxyGroupTypeKubernetesAPIServer &&
+	if pg.Spec.Type != tsapi.ProxyGroupTypeKubernetesAPIServer {
-		pg.Spec.KubeAPIServerConfig != nil &&
+		return false
-		pg.Spec.KubeAPIServerConfig.AuthMode != nil &&
+	}
-		*pg.Spec.KubeAPIServerConfig.AuthMode
+
+	// The default is auth mode.
+	return pg.Spec.KubeAPIServer == nil ||
+		pg.Spec.KubeAPIServer.Mode == nil ||
+		*pg.Spec.KubeAPIServer.Mode == tsapi.APIServerProxyModeAuth
 }
 
 func pgStateSecrets(pg *tsapi.ProxyGroup, namespace string) (secrets []*corev1.Secret) {

k8s-operator/api.md

@@ -21,6 +21,21 @@
 
 
 
+#### APIServerProxyMode
+
+_Underlying type:_ _string_
+
+
+
+_Validation:_
+- Enum: [auth noauth]
+- Type: string
+
+_Appears in:_
+- [KubeAPIServerConfig](#kubeapiserverconfig)
+
+
+
 #### AppConnector
 
 
@@ -326,7 +341,7 @@ _Appears in:_
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
-| `authMode` _boolean_ | AuthMode enables auth mode for the API Server proxy. In auth mode,<br />requests from the tailnet proxied over to the Kubernetes API server<br />are additionally impersonated using the sender's tailnet identity. |  |  |
+| `mode` _[APIServerProxyMode](#apiserverproxymode)_ | Mode to run the API server proxy in. Supported modes are auth and noauth.<br />In auth mode, requests from the tailnet proxied over to the Kubernetes<br />API server are additionally impersonated using the sender's tailnet identity.<br />If not specified, defaults to auth mode. |  | Enum: [auth noauth] <br />Type: string <br /> |
 
 
 #### LabelValue
@@ -659,7 +674,7 @@ _Appears in:_
 | `replicas` _integer_ | Replicas specifies how many replicas to create the StatefulSet with.<br />Defaults to 2. |  | Minimum: 0 <br /> |
 | `hostnamePrefix` _[HostnamePrefix](#hostnameprefix)_ | HostnamePrefix is the hostname prefix to use for tailnet devices created<br />by the ProxyGroup. Each device will have the integer number from its<br />StatefulSet pod appended to this prefix to form the full hostname.<br />HostnamePrefix can contain lower case letters, numbers and dashes, it<br />must not start with a dash and must be between 1 and 62 characters long. |  | Pattern: `^[a-z0-9][a-z0-9-]{0,61}$` <br />Type: string <br /> |
 | `proxyClass` _string_ | ProxyClass is the name of the ProxyClass custom resource that contains<br />configuration options that should be applied to the resources created<br />for this ProxyGroup. If unset, and there is no default ProxyClass<br />configured, the operator will create resources with the default<br />configuration. |  |  |
-| `kubeAPIServerConfig` _[KubeAPIServerConfig](#kubeapiserverconfig)_ | KubeAPIServerConfig contains configuration specific to the kube-apiserver<br />ProxyGroup type. This field is only used when Type is set to "kube-apiserver". |  |  |
+| `kubeAPIServer` _[KubeAPIServerConfig](#kubeapiserverconfig)_ | KubeAPIServer contains configuration specific to the kube-apiserver<br />ProxyGroup type. This field is only used when Type is set to "kube-apiserver". |  |  |
 
 
 #### ProxyGroupStatus

k8s-operator/apis/v1alpha1/types_proxygroup.go

@@ -85,10 +85,10 @@ type ProxyGroupSpec struct {
 	// +optional
 	ProxyClass string `json:"proxyClass,omitempty"`
 
-	// KubeAPIServerConfig contains configuration specific to the kube-apiserver
+	// KubeAPIServer contains configuration specific to the kube-apiserver
 	// ProxyGroup type. This field is only used when Type is set to "kube-apiserver".
 	// +optional
-	KubeAPIServerConfig *KubeAPIServerConfig `json:"kubeAPIServerConfig,omitempty"`
+	KubeAPIServer *KubeAPIServerConfig `json:"kubeAPIServer,omitempty"`
 }
 
 type ProxyGroupStatus struct {
@@ -136,15 +136,25 @@ const (
 	ProxyGroupTypeKubernetesAPIServer ProxyGroupType = "kube-apiserver"
 )
 
+// +kubebuilder:validation:Type=string
+// +kubebuilder:validation:Enum=auth;noauth
+type APIServerProxyMode string
+
+const (
+	APIServerProxyModeAuth   APIServerProxyMode = "auth"
+	APIServerProxyModeNoAuth APIServerProxyMode = "noauth"
+)
+
 // +kubebuilder:validation:Type=string
 // +kubebuilder:validation:Pattern=`^[a-z0-9][a-z0-9-]{0,61}$`
 type HostnamePrefix string
 
 // KubeAPIServerConfig contains configuration specific to the kube-apiserver ProxyGroup type.
 type KubeAPIServerConfig struct {
-	// AuthMode enables auth mode for the API Server proxy. In auth mode,
+	// Mode to run the API server proxy in. Supported modes are auth and noauth.
-	// requests from the tailnet proxied over to the Kubernetes API server
+	// In auth mode, requests from the tailnet proxied over to the Kubernetes
-	// are additionally impersonated using the sender's tailnet identity.
+	// API server are additionally impersonated using the sender's tailnet identity.
+	// If not specified, defaults to auth mode.
 	// +optional
-	AuthMode *bool `json:"authMode,omitempty"`
+	Mode *APIServerProxyMode `json:"mode,omitempty"`
 }

k8s-operator/apis/v1alpha1/zz_generated.deepcopy.go

@@ -319,9 +319,9 @@ func (in *Env) DeepCopy() *Env {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *KubeAPIServerConfig) DeepCopyInto(out *KubeAPIServerConfig) {
 	*out = *in
-	if in.AuthMode != nil {
+	if in.Mode != nil {
-		in, out := &in.AuthMode, &out.AuthMode
+		in, out := &in.Mode, &out.Mode
-		*out = new(bool)
+		*out = new(APIServerProxyMode)
 		**out = **in
 	}
 }
@@ -751,8 +751,8 @@ func (in *ProxyGroupSpec) DeepCopyInto(out *ProxyGroupSpec) {
 		*out = new(int32)
 		**out = **in
 	}
-	if in.KubeAPIServerConfig != nil {
+	if in.KubeAPIServer != nil {
-		in, out := &in.KubeAPIServerConfig, &out.KubeAPIServerConfig
+		in, out := &in.KubeAPIServer, &out.KubeAPIServer
 		*out = new(KubeAPIServerConfig)
 		(*in).DeepCopyInto(*out)
 	}