diff --git a/Dockerfile b/Dockerfile index c457ba24..a38c0e79 100644 --- a/Dockerfile +++ b/Dockerfile @@ -29,8 +29,8 @@ FROM gcr.io/distroless/static:nonroot@sha256:bca3c203cdb36f5914ab8568e4c25165643 # Copy the server binary from the build-env stage. COPY --from=build-env /usr/local/bin /usr/local/bin -# Document the ports -EXPOSE 8080 8443 +# Document the default server ports for the various server apps +EXPOSE 8080 8443 8444 10250 # Run as non-root for security posture # Use the same non-root user as https://github.com/GoogleContainerTools/distroless/blob/fc3c4eaceb0518900f886aae90407c43be0a42d9/base/base.bzl#L9 diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index 6cfbf09e..9292168f 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -58,6 +58,8 @@ data: durationSeconds: (@= str(data.values.api_serving_certificate_duration_seconds) @) renewBeforeSeconds: (@= str(data.values.api_serving_certificate_renew_before_seconds) @) apiGroupSuffix: (@= data.values.api_group_suffix @) + # aggregatedAPIServerPort may be set here, although other YAML references to the default port (10250) may also need to be updated + # impersonationProxyServerPort may be set here, although other YAML references to the default port (8444) may also need to be updated names: servingCertificateSecret: (@= defaultResourceNameWithSuffix("api-tls-serving-certificate") @) credentialIssuer: (@= defaultResourceNameWithSuffix("config") @) @@ -175,7 +177,7 @@ spec: livenessProbe: httpGet: path: /healthz - port: 8443 + port: 10250 scheme: HTTPS initialDelaySeconds: 2 timeoutSeconds: 15 @@ -184,7 +186,7 @@ spec: readinessProbe: httpGet: path: /healthz - port: 8443 + port: 10250 scheme: HTTPS initialDelaySeconds: 2 timeoutSeconds: 3 @@ -251,7 +253,7 @@ spec: ports: - protocol: TCP port: 443 - targetPort: 8443 + targetPort: 10250 --- apiVersion: v1 kind: Service diff --git a/deploy/concierge/values.yaml b/deploy/concierge/values.yaml index 76d77a64..d902f89f 100644 --- a/deploy/concierge/values.yaml +++ b/deploy/concierge/values.yaml @@ -41,7 +41,7 @@ kube_cert_agent_image: image_pull_dockerconfigjson: #! e.g. {"auths":{"https://registry.example.com":{"username":"USERNAME","password":"PASSWORD","auth":"BASE64_ENCODED_USERNAME_COLON_PASSWORD"}}} #! Pinniped will try to guess the right K8s API URL for sharing that information with potential clients. -#! This settings allows the guess to be overridden. +#! This setting allows the guess to be overridden. #! Optional. discovery_url: #! e.g., https://example.com diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index f9e9210b..84247d9b 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -152,6 +152,8 @@ func (a *App) runServer(ctx context.Context) error { ServingCertDuration: time.Duration(*cfg.APIConfig.ServingCertificateConfig.DurationSeconds) * time.Second, ServingCertRenewBefore: time.Duration(*cfg.APIConfig.ServingCertificateConfig.RenewBeforeSeconds) * time.Second, AuthenticatorCache: authenticators, + // This port should be safe to cast because the config reader already validated it. + ImpersonationProxyServerPort: int(*cfg.ImpersonationProxyServerPort), }, ) if err != nil { @@ -170,6 +172,7 @@ func (a *App) runServer(ctx context.Context) error { certIssuer, buildControllers, *cfg.APIGroupSuffix, + *cfg.AggregatedAPIServerPort, scheme, loginGV, identityGV, @@ -195,6 +198,7 @@ func getAggregatedAPIServerConfig( issuer issuer.ClientCertIssuer, buildControllers controllerinit.RunnerBuilder, apiGroupSuffix string, + aggregatedAPIServerPort int64, scheme *runtime.Scheme, loginConciergeGroupVersion, identityConciergeGroupVersion schema.GroupVersion, ) (*apiserver.Config, error) { @@ -209,7 +213,9 @@ func getAggregatedAPIServerConfig( ) recommendedOptions.Etcd = nil // turn off etcd storage because we don't need it yet recommendedOptions.SecureServing.ServerCert.GeneratedCert = dynamicCertProvider - recommendedOptions.SecureServing.BindPort = 8443 // Don't run on default 443 because that requires root + + // This port is configurable. It should be safe to cast because the config reader already validated it. + recommendedOptions.SecureServing.BindPort = int(aggregatedAPIServerPort) // secure TLS for connections coming from and going to the Kube API server // this is best effort because not all options provide the right hooks to override TLS config diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index cbe9d7f9..4749c7ca 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -21,6 +21,17 @@ import ( const ( aboutAYear = 60 * 60 * 24 * 365 about9Months = 60 * 60 * 24 * 30 * 9 + + // Use 10250 because it happens to be the same port on which the Kubelet listens, so some cluster types + // are more permissive with servers that run on this port. For example, GKE private clusters do not + // allow traffic from the control plane to most ports, but do allow traffic to port 10250. This allows + // the Concierge to work without additional configuration on these types of clusters. + aggregatedAPIServerPortDefault = 10250 + + // Use port 8444 because that is the port that was selected for the first released version of the + // impersonation proxy, and has been the value since. It was originally selected because the + // aggregated API server used to run on 8443 (has since changed), so 8444 was the next available port. + impersonationProxyPortDefault = 8444 ) // FromPath loads an Config from a provided local file path, inserts any @@ -42,6 +53,8 @@ func FromPath(path string) (*Config, error) { } maybeSetAPIDefaults(&config.APIConfig) + maybeSetAggregatedAPIServerPortDefaults(&config.AggregatedAPIServerPort) + maybeSetImpersonationProxyServerPortDefaults(&config.ImpersonationProxyServerPort) maybeSetAPIGroupSuffixDefault(&config.APIGroupSuffix) maybeSetKubeCertAgentDefaults(&config.KubeCertAgentConfig) @@ -53,6 +66,14 @@ func FromPath(path string) (*Config, error) { return nil, fmt.Errorf("validate apiGroupSuffix: %w", err) } + if err := validateServerPort(config.AggregatedAPIServerPort); err != nil { + return nil, fmt.Errorf("validate aggregatedAPIServerPort: %w", err) + } + + if err := validateServerPort(config.ImpersonationProxyServerPort); err != nil { + return nil, fmt.Errorf("validate impersonationProxyServerPort: %w", err) + } + if err := validateNames(&config.NamesConfig); err != nil { return nil, fmt.Errorf("validate names: %w", err) } @@ -84,6 +105,18 @@ func maybeSetAPIGroupSuffixDefault(apiGroupSuffix **string) { } } +func maybeSetAggregatedAPIServerPortDefaults(port **int64) { + if *port == nil { + *port = pointer.Int64Ptr(aggregatedAPIServerPortDefault) + } +} + +func maybeSetImpersonationProxyServerPortDefaults(port **int64) { + if *port == nil { + *port = pointer.Int64Ptr(impersonationProxyPortDefault) + } +} + func maybeSetKubeCertAgentDefaults(cfg *KubeCertAgentSpec) { if cfg.NamePrefix == nil { cfg.NamePrefix = pointer.StringPtr("pinniped-kube-cert-agent-") @@ -147,3 +180,11 @@ func validateAPI(apiConfig *APIConfigSpec) error { func validateAPIGroupSuffix(apiGroupSuffix string) error { return groupsuffix.Validate(apiGroupSuffix) } + +func validateServerPort(port *int64) error { + // It cannot be below 1024 because the container is not running as root. + if *port < 1024 || *port > 65535 { + return constable.Error("must be within range 1024 to 65535") + } + return nil +} diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index c29fb5f6..86139182 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -33,6 +33,8 @@ func TestFromPath(t *testing.T) { durationSeconds: 3600 renewBeforeSeconds: 2400 apiGroupSuffix: some.suffix.com + aggregatedAPIServerPort: 12345 + impersonationProxyServerPort: 4242 names: servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate credentialIssuer: pinniped-config @@ -65,7 +67,9 @@ func TestFromPath(t *testing.T) { RenewBeforeSeconds: pointer.Int64Ptr(2400), }, }, - APIGroupSuffix: pointer.StringPtr("some.suffix.com"), + APIGroupSuffix: pointer.StringPtr("some.suffix.com"), + AggregatedAPIServerPort: pointer.Int64Ptr(12345), + ImpersonationProxyServerPort: pointer.Int64Ptr(4242), NamesConfig: NamesConfigSpec{ ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate", CredentialIssuer: "pinniped-config", @@ -108,7 +112,9 @@ func TestFromPath(t *testing.T) { DiscoveryInfo: DiscoveryInfoSpec{ URL: nil, }, - APIGroupSuffix: pointer.StringPtr("pinniped.dev"), + APIGroupSuffix: pointer.StringPtr("pinniped.dev"), + AggregatedAPIServerPort: pointer.Int64Ptr(10250), + ImpersonationProxyServerPort: pointer.Int64Ptr(8444), APIConfig: APIConfigSpec{ ServingCertificateConfig: ServingCertificateConfigSpec{ DurationSeconds: pointer.Int64Ptr(60 * 60 * 24 * 365), // about a year @@ -323,6 +329,38 @@ func TestFromPath(t *testing.T) { `), wantError: "validate api: renewBefore must be positive", }, + { + name: "AggregatedAPIServerPortDefault too small", + yaml: here.Doc(` + --- + aggregatedAPIServerPort: 1023 + `), + wantError: "validate aggregatedAPIServerPort: must be within range 1024 to 65535", + }, + { + name: "AggregatedAPIServerPortDefault too large", + yaml: here.Doc(` + --- + aggregatedAPIServerPort: 65536 + `), + wantError: "validate aggregatedAPIServerPort: must be within range 1024 to 65535", + }, + { + name: "ImpersonationProxyServerPort too small", + yaml: here.Doc(` + --- + impersonationProxyServerPort: 1023 + `), + wantError: "validate impersonationProxyServerPort: must be within range 1024 to 65535", + }, + { + name: "ImpersonationProxyServerPort too large", + yaml: here.Doc(` + --- + impersonationProxyServerPort: 65536 + `), + wantError: "validate impersonationProxyServerPort: must be within range 1024 to 65535", + }, { name: "ZeroRenewBefore", yaml: here.Doc(` diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index 6aa6733a..8f2c15c9 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -7,13 +7,15 @@ import "go.pinniped.dev/internal/plog" // Config contains knobs to setup an instance of the Pinniped Concierge. type Config struct { - DiscoveryInfo DiscoveryInfoSpec `json:"discovery"` - APIConfig APIConfigSpec `json:"api"` - APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` - NamesConfig NamesConfigSpec `json:"names"` - KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` - Labels map[string]string `json:"labels"` - LogLevel plog.LogLevel `json:"logLevel"` + DiscoveryInfo DiscoveryInfoSpec `json:"discovery"` + APIConfig APIConfigSpec `json:"api"` + APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` + AggregatedAPIServerPort *int64 `json:"aggregatedAPIServerPort"` + ImpersonationProxyServerPort *int64 `json:"impersonationProxyServerPort"` + NamesConfig NamesConfigSpec `json:"names"` + KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` + Labels map[string]string `json:"labels"` + LogLevel plog.LogLevel `json:"logLevel"` } // DiscoveryInfoSpec contains configuration knobs specific to diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index e7e25e4f..76d5334f 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -48,7 +48,6 @@ import ( ) const ( - impersonationProxyPort = 8444 defaultHTTPSPort = 443 approximatelyOneHundredYears = 100 * 365 * 24 * time.Hour caCommonName = "Pinniped Impersonation Proxy Serving CA" @@ -61,6 +60,7 @@ const ( type impersonatorConfigController struct { namespace string credentialIssuerResourceName string + impersonationProxyPort int generatedLoadBalancerServiceName string generatedClusterIPServiceName string tlsSecretName string @@ -96,6 +96,7 @@ func NewImpersonatorConfigController( servicesInformer corev1informers.ServiceInformer, secretsInformer corev1informers.SecretInformer, withInformer pinnipedcontroller.WithInformerOptionFunc, + impersonationProxyPort int, generatedLoadBalancerServiceName string, generatedClusterIPServiceName string, tlsSecretName string, @@ -115,6 +116,7 @@ func NewImpersonatorConfigController( Syncer: &impersonatorConfigController{ namespace: namespace, credentialIssuerResourceName: credentialIssuerResourceName, + impersonationProxyPort: impersonationProxyPort, generatedLoadBalancerServiceName: generatedLoadBalancerServiceName, generatedClusterIPServiceName: generatedClusterIPServiceName, tlsSecretName: tlsSecretName, @@ -401,9 +403,9 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted(syncCtx contr } } - c.infoLog.Info("starting impersonation proxy", "port", impersonationProxyPort) + c.infoLog.Info("starting impersonation proxy", "port", c.impersonationProxyPort) startImpersonatorFunc, err := c.impersonatorFunc( - impersonationProxyPort, + c.impersonationProxyPort, c.tlsServingCertDynamicCertProvider, c.impersonationSigningCertProvider, ) @@ -436,7 +438,7 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStopped(shouldCloseEr return nil } - c.infoLog.Info("stopping impersonation proxy", "port", impersonationProxyPort) + c.infoLog.Info("stopping impersonation proxy", "port", c.impersonationProxyPort) close(c.serverStopCh) stopErr := <-c.errorCh @@ -457,7 +459,7 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.C Type: v1.ServiceTypeLoadBalancer, Ports: []v1.ServicePort{ { - TargetPort: intstr.FromInt(impersonationProxyPort), + TargetPort: intstr.FromInt(c.impersonationProxyPort), Port: defaultHTTPSPort, Protocol: v1.ProtocolTCP, }, @@ -503,7 +505,7 @@ func (c *impersonatorConfigController) ensureClusterIPServiceIsStarted(ctx conte Type: v1.ServiceTypeClusterIP, Ports: []v1.ServicePort{ { - TargetPort: intstr.FromInt(impersonationProxyPort), + TargetPort: intstr.FromInt(c.impersonationProxyPort), Port: defaultHTTPSPort, Protocol: v1.ProtocolTCP, }, diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 7e17d15e..ec43849c 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -51,6 +51,7 @@ import ( func TestImpersonatorConfigControllerOptions(t *testing.T) { spec.Run(t, "options", func(t *testing.T, when spec.G, it spec.S) { const installedInNamespace = "some-namespace" + const impersonationProxyPort = 8444 const credentialIssuerResourceName = "some-credential-issuer-resource-name" const generatedLoadBalancerServiceName = "some-service-resource-name" const generatedClusterIPServiceName = "some-cluster-ip-resource-name" @@ -84,6 +85,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) { servicesInformer, secretsInformer, observableWithInformerOption.WithInformer, + impersonationProxyPort, generatedLoadBalancerServiceName, generatedClusterIPServiceName, tlsSecretName, @@ -252,6 +254,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { name := t.Name() spec.Run(t, "Sync", func(t *testing.T, when spec.G, it spec.S) { const installedInNamespace = "some-namespace" + const impersonationProxyPort = 8444 const credentialIssuerResourceName = "some-credential-issuer-resource-name" const loadBalancerServiceName = "some-service-resource-name" const clusterIPServiceName = "some-cluster-ip-resource-name" @@ -553,6 +556,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { kubeInformers.Core().V1().Services(), kubeInformers.Core().V1().Secrets(), controllerlib.WithInformer, + impersonationProxyPort, loadBalancerServiceName, clusterIPServiceName, tlsSecretName, diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 6129a974..be3af899 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -59,6 +59,9 @@ type Config struct { // the kubecertagent package's controllers should manage the agent pods. KubeCertAgentConfig *concierge.KubeCertAgentSpec + // ImpersonationProxyServerPort decides which port the impersonation proxy should bind. + ImpersonationProxyServerPort int + // DiscoveryURLOverride allows a caller to inject a hardcoded discovery URL into Pinniped // discovery document. DiscoveryURLOverride *string @@ -93,7 +96,7 @@ type Config struct { Labels map[string]string } -// Prepare the controllers and their informers and return a function that will start them when called. +// PrepareControllers prepares the controllers and their informers and returns a function that will start them when called. //nolint:funlen // Eh, fair, it is a really long function...but it is wiring the world...so... func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { loginConciergeGroupData, identityConciergeGroupData := groupsuffix.ConciergeAggregatedGroups(c.APIGroupSuffix) @@ -262,6 +265,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { informers.installationNamespaceK8s.Core().V1().Services(), informers.installationNamespaceK8s.Core().V1().Secrets(), controllerlib.WithInformer, + c.ImpersonationProxyServerPort, c.NamesConfig.ImpersonationLoadBalancerService, c.NamesConfig.ImpersonationClusterIPService, c.NamesConfig.ImpersonationTLSCertificateSecret,