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..d1999886 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -58,6 +58,7 @@ 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 names: servingCertificateSecret: (@= defaultResourceNameWithSuffix("api-tls-serving-certificate") @) credentialIssuer: (@= defaultResourceNameWithSuffix("config") @) @@ -175,7 +176,7 @@ spec: livenessProbe: httpGet: path: /healthz - port: 8443 + port: 10250 scheme: HTTPS initialDelaySeconds: 2 timeoutSeconds: 15 @@ -184,7 +185,7 @@ spec: readinessProbe: httpGet: path: /healthz - port: 8443 + port: 10250 scheme: HTTPS initialDelaySeconds: 2 timeoutSeconds: 3 @@ -251,7 +252,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 71e583cc..ff8e8ebc 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -168,6 +168,7 @@ func (a *App) runServer(ctx context.Context) error { certIssuer, buildControllers, *cfg.APIGroupSuffix, + *cfg.AggregatedAPIServerPort, scheme, loginGV, identityGV, @@ -193,6 +194,7 @@ func getAggregatedAPIServerConfig( issuer issuer.ClientCertIssuer, buildControllers controllerinit.RunnerBuilder, apiGroupSuffix string, + aggregatedAPIServerPort int64, scheme *runtime.Scheme, loginConciergeGroupVersion, identityConciergeGroupVersion schema.GroupVersion, ) (*apiserver.Config, error) { @@ -207,7 +209,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) serverConfig := genericapiserver.NewRecommendedConfig(codecs) // Note that among other things, this ApplyTo() function copies diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index cbe9d7f9..6c503178 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -21,6 +21,12 @@ 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 ) // FromPath loads an Config from a provided local file path, inserts any @@ -42,6 +48,7 @@ func FromPath(path string) (*Config, error) { } maybeSetAPIDefaults(&config.APIConfig) + maybeSetAggregatedAPIServerPortDefaults(&config.AggregatedAPIServerPort) maybeSetAPIGroupSuffixDefault(&config.APIGroupSuffix) maybeSetKubeCertAgentDefaults(&config.KubeCertAgentConfig) @@ -53,6 +60,10 @@ func FromPath(path string) (*Config, error) { return nil, fmt.Errorf("validate apiGroupSuffix: %w", err) } + if err := validateAggregatedAPIServerPort(config.AggregatedAPIServerPort); err != nil { + return nil, fmt.Errorf("validate aggregatedAPIServerPort: %w", err) + } + if err := validateNames(&config.NamesConfig); err != nil { return nil, fmt.Errorf("validate names: %w", err) } @@ -84,6 +95,12 @@ func maybeSetAPIGroupSuffixDefault(apiGroupSuffix **string) { } } +func maybeSetAggregatedAPIServerPortDefaults(aggregatedAPIServerPort **int64) { + if *aggregatedAPIServerPort == nil { + *aggregatedAPIServerPort = pointer.Int64Ptr(aggregatedAPIServerPortDefault) + } +} + func maybeSetKubeCertAgentDefaults(cfg *KubeCertAgentSpec) { if cfg.NamePrefix == nil { cfg.NamePrefix = pointer.StringPtr("pinniped-kube-cert-agent-") @@ -147,3 +164,11 @@ func validateAPI(apiConfig *APIConfigSpec) error { func validateAPIGroupSuffix(apiGroupSuffix string) error { return groupsuffix.Validate(apiGroupSuffix) } + +func validateAggregatedAPIServerPort(aggregatedAPIServerPort *int64) error { + // It cannot be below 1024 because the container is not running as root. + if *aggregatedAPIServerPort < 1024 || *aggregatedAPIServerPort > 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..77bdf1d8 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -33,6 +33,7 @@ func TestFromPath(t *testing.T) { durationSeconds: 3600 renewBeforeSeconds: 2400 apiGroupSuffix: some.suffix.com + aggregatedAPIServerPort: 12345 names: servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate credentialIssuer: pinniped-config @@ -65,7 +66,8 @@ 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), NamesConfig: NamesConfigSpec{ ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate", CredentialIssuer: "pinniped-config", @@ -108,7 +110,8 @@ func TestFromPath(t *testing.T) { DiscoveryInfo: DiscoveryInfoSpec{ URL: nil, }, - APIGroupSuffix: pointer.StringPtr("pinniped.dev"), + APIGroupSuffix: pointer.StringPtr("pinniped.dev"), + AggregatedAPIServerPort: pointer.Int64Ptr(10250), APIConfig: APIConfigSpec{ ServingCertificateConfig: ServingCertificateConfigSpec{ DurationSeconds: pointer.Int64Ptr(60 * 60 * 24 * 365), // about a year @@ -323,6 +326,22 @@ 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: "ZeroRenewBefore", yaml: here.Doc(` diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index 6aa6733a..cb93cb5c 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -7,13 +7,14 @@ 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"` + 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