From b80cbb8cc55edc41a5d427f88a65144d25535d1e Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Mon, 3 May 2021 16:20:13 -0500 Subject: [PATCH] Run kube-cert-agent pod as Concierge ServiceAccount. Since 0dfb3e95c55b0f38d899b5efc40cd0643e1e0f86, we no longer directly create the kube-cert-agent Pod, so our "use" permission on PodSecurityPolicies no longer has the intended effect. Since the deployments controller is now the one creating pods for us, we need to get the permission on the PodSpec of the target pod instead, which we do somewhat simply by using the same service account as the main Concierge pods. We still set `automountServiceAccountToken: false`, so this should not actually give any useful permissions to the agent pod when running. Signed-off-by: Matt Moyer --- deploy/concierge/deployment.yaml | 1 + internal/config/concierge/config.go | 3 +++ internal/config/concierge/config_test.go | 15 ++++++++++++++- internal/config/concierge/types.go | 1 + .../controller/kubecertagent/kubecertagent.go | 4 ++++ .../kubecertagent/kubecertagent_test.go | 2 ++ internal/controllermanager/prepare_controllers.go | 1 + 7 files changed, 26 insertions(+), 1 deletion(-) diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index 223b7fa80..ad12e915a 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -47,6 +47,7 @@ data: impersonationTLSCertificateSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-tls-serving-certificate") @) impersonationCACertificateSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-ca-certificate") @) impersonationSignerSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-signer-ca-certificate") @) + serviceAccount: (@= defaultResourceName() @) labels: (@= json.encode(labels()).rstrip() @) kubeCertAgent: namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @) diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index b4d4c9a5e..4c080a480 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -122,6 +122,9 @@ func validateNames(names *NamesConfigSpec) error { if names.ImpersonationSignerSecret == "" { missingNames = append(missingNames, "impersonationSignerSecret") } + if names.ServiceAccount == "" { + missingNames = append(missingNames, "serviceAccount") + } if len(missingNames) > 0 { return constable.Error("missing required names: " + strings.Join(missingNames, ", ")) } diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index 0034d06b4..da1172e78 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -43,6 +43,7 @@ func TestFromPath(t *testing.T) { impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value labels: myLabelKey1: myLabelValue1 myLabelKey2: myLabelValue2 @@ -72,6 +73,7 @@ func TestFromPath(t *testing.T) { ImpersonationTLSCertificateSecret: "impersonationTLSCertificateSecret-value", ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value", ImpersonationSignerSecret: "impersonationSignerSecret-value", + ServiceAccount: "serviceAccount-value", }, Labels: map[string]string{ "myLabelKey1": "myLabelValue1", @@ -98,6 +100,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantConfig: &Config{ DiscoveryInfo: DiscoveryInfoSpec{ @@ -119,6 +122,7 @@ func TestFromPath(t *testing.T) { ImpersonationTLSCertificateSecret: "impersonationTLSCertificateSecret-value", ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value", ImpersonationSignerSecret: "impersonationSignerSecret-value", + ServiceAccount: "serviceAccount-value", }, Labels: map[string]string{}, KubeCertAgentConfig: KubeCertAgentSpec{ @@ -133,7 +137,7 @@ func TestFromPath(t *testing.T) { wantError: "validate names: missing required names: servingCertificateSecret, credentialIssuer, " + "apiService, impersonationConfigMap, impersonationLoadBalancerService, " + "impersonationTLSCertificateSecret, impersonationCACertificateSecret, " + - "impersonationSignerSecret", + "impersonationSignerSecret, serviceAccount", }, { name: "Missing apiService name", @@ -147,6 +151,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: apiService", }, @@ -162,6 +167,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: credentialIssuer", }, @@ -177,6 +183,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: servingCertificateSecret", }, @@ -192,6 +199,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationConfigMap", }, @@ -207,6 +215,7 @@ func TestFromPath(t *testing.T) { impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationLoadBalancerService", }, @@ -222,6 +231,7 @@ func TestFromPath(t *testing.T) { impersonationLoadBalancerService: impersonationLoadBalancerService-value impersonationCACertificateSecret: impersonationCACertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationTLSCertificateSecret", }, @@ -237,6 +247,7 @@ func TestFromPath(t *testing.T) { impersonationLoadBalancerService: impersonationLoadBalancerService-value impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationCACertificateSecret", }, @@ -252,6 +263,7 @@ func TestFromPath(t *testing.T) { impersonationLoadBalancerService: impersonationLoadBalancerService-value impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value impersonationCACertificateSecret: impersonationCACertificateSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationSignerSecret", }, @@ -265,6 +277,7 @@ func TestFromPath(t *testing.T) { apiService: pinniped-api impersonationLoadBalancerService: impersonationLoadBalancerService-value impersonationSignerSecret: impersonationSignerSecret-value + serviceAccount: serviceAccount-value `), wantError: "validate names: missing required names: impersonationConfigMap, " + "impersonationTLSCertificateSecret, impersonationCACertificateSecret", diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index cfec621ef..b2fed37bc 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -41,6 +41,7 @@ type NamesConfigSpec struct { ImpersonationTLSCertificateSecret string `json:"impersonationTLSCertificateSecret"` ImpersonationCACertificateSecret string `json:"impersonationCACertificateSecret"` ImpersonationSignerSecret string `json:"impersonationSignerSecret"` + ServiceAccount string `json:"serviceAccount"` } // ServingCertificateConfigSpec contains the configuration knobs for the API's diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go index 478d27fe0..e45bf8507 100644 --- a/internal/controller/kubecertagent/kubecertagent.go +++ b/internal/controller/kubecertagent/kubecertagent.go @@ -64,6 +64,9 @@ type AgentConfig struct { // NamePrefix will be prefixed to all agent pod names. NamePrefix string + // ServiceAccountName is the service account under which to run the agent pods. + ServiceAccountName string + // ContainerImagePullSecrets is a list of names of Kubernetes Secret objects that will be used as // ImagePullSecrets on the kube-cert-agent pods. ContainerImagePullSecrets []string @@ -472,6 +475,7 @@ func (c *agentController) newAgentDeployment(controllerManagerPod *corev1.Pod) * RestartPolicy: corev1.RestartPolicyAlways, NodeSelector: controllerManagerPod.Spec.NodeSelector, AutomountServiceAccountToken: pointer.BoolPtr(false), + ServiceAccountName: c.cfg.ServiceAccountName, NodeName: controllerManagerPod.Spec.NodeName, Tolerations: controllerManagerPod.Spec.Tolerations, // We need to run the agent pod as root since the file permissions diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 4a1a17f91..73d0a749d 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -123,6 +123,7 @@ func TestAgentController(t *testing.T) { }}, RestartPolicy: corev1.RestartPolicyAlways, TerminationGracePeriodSeconds: pointer.Int64Ptr(0), + ServiceAccountName: "test-service-account-name", AutomountServiceAccountToken: pointer.BoolPtr(false), SecurityContext: &corev1.PodSecurityContext{ RunAsUser: pointer.Int64Ptr(0), @@ -672,6 +673,7 @@ func TestAgentController(t *testing.T) { AgentConfig{ Namespace: "concierge", ContainerImage: "pinniped-server-image", + ServiceAccountName: "test-service-account-name", NamePrefix: "pinniped-concierge-kube-cert-agent-", ContainerImagePullSecrets: []string{"pinniped-image-pull-secret"}, CredentialIssuerName: "pinniped-concierge-config", diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 8990a4ecf..ed74c4acb 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -121,6 +121,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) { agentConfig := kubecertagent.AgentConfig{ Namespace: c.ServerInstallationInfo.Namespace, + ServiceAccountName: c.NamesConfig.ServiceAccount, ContainerImage: *c.KubeCertAgentConfig.Image, NamePrefix: *c.KubeCertAgentConfig.NamePrefix, ContainerImagePullSecrets: c.KubeCertAgentConfig.ImagePullSecrets,