diff --git a/deploy/concierge/deployment.yaml b/deploy/concierge/deployment.yaml index f2498cadd..26b892524 100644 --- a/deploy/concierge/deployment.yaml +++ b/deploy/concierge/deployment.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") @@ -96,14 +96,9 @@ data: imagePullSecrets: - image-pull-secret (@ end @) - (@ if data.values.log_level or data.values.deprecated_log_format: @) + (@ if data.values.log_level: @) log: - (@ if data.values.log_level: @) level: (@= getAndValidateLogLevel() @) - (@ end @) - (@ if data.values.deprecated_log_format: @) - format: (@= data.values.deprecated_log_format @) - (@ end @) (@ end @) --- #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "": diff --git a/deploy/concierge/values.yaml b/deploy/concierge/values.yaml index 582641b80..0e5708956 100644 --- a/deploy/concierge/values.yaml +++ b/deploy/concierge/values.yaml @@ -124,17 +124,6 @@ api_serving_certificate_renew_before_seconds: 2160000 #@schema/validation one_of=["info", "debug", "trace", "all"] log_level: "" -#@schema/title "Log format" -#@ deprecated_log_format_desc = "Specify the format of logging: json (for machine parsable logs) and text (for legacy klog formatted logs). \ -#@ By default, when this value is left unset, logs are formatted in json. \ -#@ This configuration is deprecated and will be removed in a future release at which point logs will always be formatted as json." -#@schema/desc deprecated_log_format_desc -#@schema/examples ("Set logs to JSON format","json") -#@schema/nullable -#@schema/validation one_of=["json", "text"] -#@schema/deprecated "This configuration is deprecated and will be removed in a future release at which point logs will always be formatted as json." -deprecated_log_format: "" - #@schema/title "Run as user" #@schema/desc "The user ID that will own the process." #! See the Dockerfile for the reasoning behind this default value. diff --git a/deploy/supervisor/helpers.lib.yaml b/deploy/supervisor/helpers.lib.yaml index fbb60a2d9..818b673a8 100644 --- a/deploy/supervisor/helpers.lib.yaml +++ b/deploy/supervisor/helpers.lib.yaml @@ -1,4 +1,4 @@ -#! Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") @@ -53,17 +53,11 @@ _: #@ template.replace(data.values.custom_labels) #@ "apiService": defaultResourceNameWithSuffix("api"), #@ }, #@ "labels": labels(), -#@ "insecureAcceptExternalUnencryptedHttpRequests": data.values.deprecated_insecure_accept_external_unencrypted_http_requests #@ } -#@ if data.values.log_level or data.values.deprecated_log_format: -#@ config["log"] = {} -#@ end #@ if data.values.log_level: +#@ config["log"] = {} #@ config["log"]["level"] = getAndValidateLogLevel() #@ end -#@ if data.values.deprecated_log_format: -#@ config["log"]["format"] = data.values.deprecated_log_format -#@ end #@ if data.values.endpoints: #@ config["endpoints"] = data.values.endpoints #@ end diff --git a/deploy/supervisor/service.yaml b/deploy/supervisor/service.yaml index fd6b96232..fe8a0f34d 100644 --- a/deploy/supervisor/service.yaml +++ b/deploy/supervisor/service.yaml @@ -1,24 +1,10 @@ -#! Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +#! Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. #! SPDX-License-Identifier: Apache-2.0 #@ load("@ytt:data", "data") -#@ load("@ytt:assert", "assert") #@ load("helpers.lib.yaml", "labels", "deploymentPodLabel", "namespace", "defaultResourceName", "defaultResourceNameWithSuffix") -#@ if hasattr(data.values, "service_http_nodeport_port"): -#@ assert.fail('value "service_http_nodeport_port" has been renamed to "deprecated_service_http_nodeport_port" and will be removed in a future release') -#@ end -#@ if hasattr(data.values, "service_http_nodeport_nodeport"): -#@ assert.fail('value "service_http_nodeport_nodeport" has been renamed to "deprecated_service_http_nodeport_nodeport" and will be removed in a future release') -#@ end -#@ if hasattr(data.values, "service_http_loadbalancer_port"): -#@ assert.fail('value "service_http_loadbalancer_port" has been renamed to "deprecated_service_http_loadbalancer_port" and will be removed in a future release') -#@ end -#@ if hasattr(data.values, "service_http_clusterip_port"): -#@ assert.fail('value "service_http_clusterip_port" has been renamed to "deprecated_service_http_clusterip_port" and will be removed in a future release') -#@ end - -#@ if data.values.deprecated_service_http_nodeport_port or data.values.service_https_nodeport_port: +#@ if data.values.service_https_nodeport_port: --- apiVersion: v1 kind: Service @@ -33,15 +19,6 @@ spec: type: NodePort selector: #@ deploymentPodLabel() ports: - #@ if data.values.deprecated_service_http_nodeport_port: - - name: http - protocol: TCP - port: #@ data.values.deprecated_service_http_nodeport_port - targetPort: 8080 - #@ if data.values.deprecated_service_http_nodeport_nodeport: - nodePort: #@ data.values.deprecated_service_http_nodeport_nodeport - #@ end - #@ end #@ if data.values.service_https_nodeport_port: - name: https protocol: TCP @@ -53,7 +30,7 @@ spec: #@ end #@ end -#@ if data.values.deprecated_service_http_clusterip_port or data.values.service_https_clusterip_port: +#@ if data.values.service_https_clusterip_port: --- apiVersion: v1 kind: Service @@ -68,12 +45,6 @@ spec: type: ClusterIP selector: #@ deploymentPodLabel() ports: - #@ if data.values.deprecated_service_http_clusterip_port: - - name: http - protocol: TCP - port: #@ data.values.deprecated_service_http_clusterip_port - targetPort: 8080 - #@ end #@ if data.values.service_https_clusterip_port: - name: https protocol: TCP @@ -82,7 +53,7 @@ spec: #@ end #@ end -#@ if data.values.deprecated_service_http_loadbalancer_port or data.values.service_https_loadbalancer_port: +#@ if data.values.service_https_loadbalancer_port: --- apiVersion: v1 kind: Service @@ -100,12 +71,6 @@ spec: loadBalancerIP: #@ data.values.service_loadbalancer_ip #@ end ports: - #@ if data.values.deprecated_service_http_loadbalancer_port: - - name: http - protocol: TCP - port: #@ data.values.deprecated_service_http_loadbalancer_port - targetPort: 8080 - #@ end #@ if data.values.service_https_loadbalancer_port: - name: https protocol: TCP diff --git a/deploy/supervisor/values.yaml b/deploy/supervisor/values.yaml index e3c31a5b3..750a5e075 100644 --- a/deploy/supervisor/values.yaml +++ b/deploy/supervisor/values.yaml @@ -79,34 +79,6 @@ image_tag: latest #@schema/validation min_len=1 image_pull_dockerconfigjson: "" -#@schema/title "Deprecated service HTTP nodeport port" -#@schema/desc "When specified, creates a NodePort Service with this `port` value, with port 8080 as its `targetPort`" -#@schema/examples ("Specify port",31234) -#@schema/nullable -#@schema/deprecated "This data value will be removed in a future release" -deprecated_service_http_nodeport_port: 0 - -#@schema/title "Deprecated service http nodeport nodeport" -#@schema/desc "The `nodePort` value of the NodePort Service, optional when `deprecated_service_http_nodeport_port` is specified" -#@schema/examples ("Specify port",31234) -#@schema/nullable -#@schema/deprecated "This data value will be removed in a future release" -deprecated_service_http_nodeport_nodeport: 0 - -#@schema/title "Deprecated service http loadbalancer port" -#@schema/desc "When specified, creates a LoadBalancer Service with this `port` value, with port 8080 as its `targetPort`" -#@schema/examples ("Specify port",8443) -#@schema/nullable -#@schema/deprecated "This data value will be removed in a future release" -deprecated_service_http_loadbalancer_port: 0 - -#@schema/title "Deprecated service http clusterip port" -#@schema/desc "Creates a ClusterIP Service with this `port` value, with port 8080 as its `targetPort`" -#@schema/examples ("Specify port",8443) -#@schema/nullable -#@schema/deprecated "This data value will be removed in a future release" -deprecated_service_http_clusterip_port: 0 - #@schema/title "Service https nodeport port" #@schema/desc "When specified, creates a NodePort Service with this `port` value, with port 8443 as its `targetPort`" #@schema/examples ("Specify port",31243) @@ -147,17 +119,6 @@ service_loadbalancer_ip: "" #@schema/validation one_of=["info", "debug", "trace", "all"] log_level: "" -#@schema/title "Log format" -#@ deprecated_log_format_desc = "Specify the format of logging: json (for machine parsable logs) and text (for legacy klog formatted logs). \ -#@ By default, when this value is left unset, logs are formatted in json. \ -#@ This configuration is deprecated and will be removed in a future release at which point logs will always be formatted as json." -#@schema/desc deprecated_log_format_desc -#@schema/examples ("Set logs to JSON format","json") -#@schema/nullable -#@schema/validation one_of=["json", "text"] -#@schema/deprecated "This configuration is deprecated and will be removed in a future release at which point logs will always be formatted as json." -deprecated_log_format: "" - #@schema/title "Run as user" #@schema/desc "The user ID that will own the process." #! See the Dockerfile for the reasoning behind this default value. @@ -242,19 +203,3 @@ no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,. #@schema/nullable #@schema/validation ("a map with keys 'http' and 'https', whose values are either the string 'disabled' or a map having keys 'network' and 'address', and the value of 'network' must be one of the allowed values", validate_endpoints) endpoints: { } - -#@ deprecated_insecure_accept_external_unencrypted_http_requests_desc = "Optionally override the validation on the endpoints.http \ -#@ value which checks that only loopback interfaces are used. \ -#@ When deprecated_insecure_accept_external_unencrypted_http_requests is true, the HTTP listener is allowed to bind to any \ -#@ interface, including interfaces that are listening for traffic from outside the pod. This value is being introduced \ -#@ to ease the transition to the new loopback interface validation for the HTTP port for any users who need more time \ -#@ to change their ingress strategy to avoid using plain HTTP into the Supervisor pods. \ -#@ This value is immediately deprecated upon its introduction. It will be removed in some future release, at which time \ -#@ traffic from outside the pod will need to be sent to the HTTPS listener instead, with no simple workaround available. \ -#@ Allowed values are true (boolean), 'true' (string), false (boolean), and 'false' (string). The default is false." -#@schema/desc deprecated_insecure_accept_external_unencrypted_http_requests_desc -#@schema/type any=True -#@schema/validation ("a boolean or string version of boolean", lambda v: type(v) in ["string", "boolean"]) -#@schema/validation one_of=["true", "false", True, False] -#@schema/deprecated "This data value will be removed in a future release" -deprecated_insecure_accept_external_unencrypted_http_requests: false diff --git a/internal/config/concierge/config.go b/internal/config/concierge/config.go index ee4c97e1d..a95619565 100644 --- a/internal/config/concierge/config.go +++ b/internal/config/concierge/config.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package concierge contains functionality to load/store Config's from/to @@ -79,7 +79,6 @@ func FromPath(ctx context.Context, path string) (*Config, error) { return nil, fmt.Errorf("validate names: %w", err) } - plog.MaybeSetDeprecatedLogLevel(config.LogLevel, &config.Log) if err := plog.ValidateAndSetLogLevelAndFormatGlobally(ctx, config.Log); err != nil { return nil, fmt.Errorf("validate log level: %w", err) } diff --git a/internal/config/concierge/config_test.go b/internal/config/concierge/config_test.go index 61b145166..ec7573941 100644 --- a/internal/config/concierge/config_test.go +++ b/internal/config/concierge/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package concierge @@ -57,7 +57,8 @@ func TestFromPath(t *testing.T) { namePrefix: kube-cert-agent-name-prefix- image: kube-cert-agent-image imagePullSecrets: [kube-cert-agent-image-pull-secret] - logLevel: debug + log: + level: debug `), wantConfig: &Config{ DiscoveryInfo: DiscoveryInfoSpec{ @@ -94,14 +95,13 @@ func TestFromPath(t *testing.T) { Image: ptr.To("kube-cert-agent-image"), ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, }, - LogLevel: func(level plog.LogLevel) *plog.LogLevel { return &level }(plog.LevelDebug), Log: plog.LogSpec{ Level: plog.LevelDebug, }, }, }, { - name: "Fully filled out new log struct", + name: "fully filled out including log format", yaml: here.Doc(` --- discovery: @@ -180,88 +180,6 @@ func TestFromPath(t *testing.T) { }, }, }, - { - name: "Fully filled out old log and new log struct", - yaml: here.Doc(` - --- - discovery: - url: https://some.discovery/url - api: - servingCertificate: - durationSeconds: 3600 - renewBeforeSeconds: 2400 - apiGroupSuffix: some.suffix.com - aggregatedAPIServerPort: 12345 - impersonationProxyServerPort: 4242 - names: - servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate - credentialIssuer: pinniped-config - apiService: pinniped-api - kubeCertAgentPrefix: kube-cert-agent-prefix - impersonationLoadBalancerService: impersonationLoadBalancerService-value - impersonationClusterIPService: impersonationClusterIPService-value - impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value - impersonationCACertificateSecret: impersonationCACertificateSecret-value - impersonationSignerSecret: impersonationSignerSecret-value - impersonationSignerSecret: impersonationSignerSecret-value - agentServiceAccount: agentServiceAccount-value - impersonationProxyServiceAccount: impersonationProxyServiceAccount-value - impersonationProxyLegacySecret: impersonationProxyLegacySecret-value - extraName: extraName-value - labels: - myLabelKey1: myLabelValue1 - myLabelKey2: myLabelValue2 - kubeCertAgent: - namePrefix: kube-cert-agent-name-prefix- - image: kube-cert-agent-image - imagePullSecrets: [kube-cert-agent-image-pull-secret] - logLevel: debug - log: - level: all - format: json - `), - wantConfig: &Config{ - DiscoveryInfo: DiscoveryInfoSpec{ - URL: ptr.To("https://some.discovery/url"), - }, - APIConfig: APIConfigSpec{ - ServingCertificateConfig: ServingCertificateConfigSpec{ - DurationSeconds: ptr.To[int64](3600), - RenewBeforeSeconds: ptr.To[int64](2400), - }, - }, - APIGroupSuffix: ptr.To("some.suffix.com"), - AggregatedAPIServerPort: ptr.To[int64](12345), - ImpersonationProxyServerPort: ptr.To[int64](4242), - NamesConfig: NamesConfigSpec{ - ServingCertificateSecret: "pinniped-concierge-api-tls-serving-certificate", - CredentialIssuer: "pinniped-config", - APIService: "pinniped-api", - ImpersonationLoadBalancerService: "impersonationLoadBalancerService-value", - ImpersonationClusterIPService: "impersonationClusterIPService-value", - ImpersonationTLSCertificateSecret: "impersonationTLSCertificateSecret-value", - ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value", - ImpersonationSignerSecret: "impersonationSignerSecret-value", - AgentServiceAccount: "agentServiceAccount-value", - ImpersonationProxyServiceAccount: "impersonationProxyServiceAccount-value", - ImpersonationProxyLegacySecret: "impersonationProxyLegacySecret-value", - }, - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - KubeCertAgentConfig: KubeCertAgentSpec{ - NamePrefix: ptr.To("kube-cert-agent-name-prefix-"), - Image: ptr.To("kube-cert-agent-image"), - ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"}, - }, - LogLevel: func(level plog.LogLevel) *plog.LogLevel { return &level }(plog.LevelDebug), - Log: plog.LogSpec{ - Level: plog.LevelDebug, - Format: plog.FormatJSON, - }, - }, - }, { name: "invalid log format", yaml: here.Doc(` @@ -281,7 +199,28 @@ func TestFromPath(t *testing.T) { level: all format: snorlax `), - wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: invalid log format, valid choices are the empty string, json and text", + wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: invalid log format, valid choices are the empty string or 'json'", + }, + { + name: "cli is a bad log format when configured by the user", + yaml: here.Doc(` + --- + names: + servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate + credentialIssuer: pinniped-config + apiService: pinniped-api + impersonationLoadBalancerService: impersonationLoadBalancerService-value + impersonationClusterIPService: impersonationClusterIPService-value + impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value + impersonationCACertificateSecret: impersonationCACertificateSecret-value + impersonationSignerSecret: impersonationSignerSecret-value + agentServiceAccount: agentServiceAccount-value + impersonationProxyServiceAccount: impersonationProxyServiceAccount-value + log: + level: all + format: cli + `), + wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: invalid log format, valid choices are the empty string or 'json'", }, { name: "When only the required fields are present, causes other fields to be defaulted", diff --git a/internal/config/concierge/types.go b/internal/config/concierge/types.go index 967a38787..ffcaac5b0 100644 --- a/internal/config/concierge/types.go +++ b/internal/config/concierge/types.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package concierge @@ -15,9 +15,7 @@ type Config struct { NamesConfig NamesConfigSpec `json:"names"` KubeCertAgentConfig KubeCertAgentSpec `json:"kubeCertAgent"` Labels map[string]string `json:"labels"` - // Deprecated: use log.level instead - LogLevel *plog.LogLevel `json:"logLevel"` - Log plog.LogSpec `json:"log"` + Log plog.LogSpec `json:"log"` } // DiscoveryInfoSpec contains configuration knobs specific to diff --git a/internal/config/supervisor/config.go b/internal/config/supervisor/config.go index 36be3fb51..1e7c60d1b 100644 --- a/internal/config/supervisor/config.go +++ b/internal/config/supervisor/config.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package supervisor contains functionality to load/store Config's from/to @@ -66,7 +66,6 @@ func FromPath(ctx context.Context, path string) (*Config, error) { return nil, fmt.Errorf("validate names: %w", err) } - plog.MaybeSetDeprecatedLogLevel(config.LogLevel, &config.Log) if err := plog.ValidateAndSetLogLevelAndFormatGlobally(ctx, config.Log); err != nil { return nil, fmt.Errorf("validate log level: %w", err) } @@ -90,7 +89,7 @@ func FromPath(ctx context.Context, path string) (*Config, error) { if err := validateEndpoint(*config.Endpoints.HTTP); err != nil { return nil, fmt.Errorf("validate http endpoint: %w", err) } - if err := validateAdditionalHTTPEndpointRequirements(*config.Endpoints.HTTP, config.AllowExternalHTTP); err != nil { + if err := validateAdditionalHTTPEndpointRequirements(*config.Endpoints.HTTP); err != nil { return nil, fmt.Errorf("validate http endpoint: %w", err) } if err := validateAtLeastOneEnabledEndpoint(*config.Endpoints.HTTPS, *config.Endpoints.HTTP); err != nil { @@ -151,16 +150,8 @@ func validateEndpoint(endpoint Endpoint) error { } } -func validateAdditionalHTTPEndpointRequirements(endpoint Endpoint, allowExternalHTTP stringOrBoolAsBool) error { +func validateAdditionalHTTPEndpointRequirements(endpoint Endpoint) error { if endpoint.Network == NetworkTCP && !addrIsOnlyOnLoopback(endpoint.Address) { - if allowExternalHTTP { - // Log that the validation should have been triggered. - plog.Warning("Listening on non-loopback interfaces for the HTTP port is deprecated and will be removed " + - "in a future release. Your current configuration would not be allowed in that future release. " + - "Please see comments in deploy/supervisor/values.yaml and review your settings.") - // Skip enforcement of the validation. - return nil - } return fmt.Errorf( "http listener address %q for %q network may only bind to loopback interfaces", endpoint.Address, diff --git a/internal/config/supervisor/config_test.go b/internal/config/supervisor/config_test.go index a90f662db..61834a0eb 100644 --- a/internal/config/supervisor/config_test.go +++ b/internal/config/supervisor/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisor @@ -25,54 +25,6 @@ func TestFromPath(t *testing.T) { }{ { name: "Happy", - yaml: here.Doc(` - --- - apiGroupSuffix: some.suffix.com - labels: - myLabelKey1: myLabelValue1 - myLabelKey2: myLabelValue2 - names: - defaultTLSCertificateSecret: my-secret-name - endpoints: - https: - network: unix - address: :1234 - http: - network: tcp - address: 127.0.0.1:1234 - insecureAcceptExternalUnencryptedHttpRequests: false - logLevel: trace - aggregatedAPIServerPort: 12345 - `), - wantConfig: &Config{ - APIGroupSuffix: ptr.To("some.suffix.com"), - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - NamesConfig: NamesConfigSpec{ - DefaultTLSCertificateSecret: "my-secret-name", - }, - Endpoints: &Endpoints{ - HTTPS: &Endpoint{ - Network: "unix", - Address: ":1234", - }, - HTTP: &Endpoint{ - Network: "tcp", - Address: "127.0.0.1:1234", - }, - }, - AllowExternalHTTP: false, - LogLevel: func(level plog.LogLevel) *plog.LogLevel { return &level }(plog.LevelTrace), - Log: plog.LogSpec{ - Level: plog.LevelTrace, - }, - AggregatedAPIServerPort: ptr.To[int64](12345), - }, - }, - { - name: "Happy with new log field", yaml: here.Doc(` --- apiGroupSuffix: some.suffix.com @@ -91,7 +43,7 @@ func TestFromPath(t *testing.T) { insecureAcceptExternalUnencryptedHttpRequests: false log: level: info - format: text + format: json aggregatedAPIServerPort: 12345 `), wantConfig: &Config{ @@ -113,67 +65,15 @@ func TestFromPath(t *testing.T) { Address: "127.0.0.1:1234", }, }, - AllowExternalHTTP: false, Log: plog.LogSpec{ Level: plog.LevelInfo, - Format: plog.FormatText, + Format: plog.FormatJSON, }, AggregatedAPIServerPort: ptr.To[int64](12345), }, }, { - name: "Happy with old and new log field", - yaml: here.Doc(` - --- - apiGroupSuffix: some.suffix.com - labels: - myLabelKey1: myLabelValue1 - myLabelKey2: myLabelValue2 - names: - defaultTLSCertificateSecret: my-secret-name - endpoints: - https: - network: unix - address: :1234 - http: - network: tcp - address: 127.0.0.1:1234 - insecureAcceptExternalUnencryptedHttpRequests: false - logLevel: trace - log: - level: info - format: text - `), - wantConfig: &Config{ - APIGroupSuffix: ptr.To("some.suffix.com"), - Labels: map[string]string{ - "myLabelKey1": "myLabelValue1", - "myLabelKey2": "myLabelValue2", - }, - NamesConfig: NamesConfigSpec{ - DefaultTLSCertificateSecret: "my-secret-name", - }, - Endpoints: &Endpoints{ - HTTPS: &Endpoint{ - Network: "unix", - Address: ":1234", - }, - HTTP: &Endpoint{ - Network: "tcp", - Address: "127.0.0.1:1234", - }, - }, - AllowExternalHTTP: false, - LogLevel: func(level plog.LogLevel) *plog.LogLevel { return &level }(plog.LevelTrace), - Log: plog.LogSpec{ - Level: plog.LevelTrace, - Format: plog.FormatText, - }, - AggregatedAPIServerPort: ptr.To[int64](10250), - }, - }, - { - name: "bad log format", + name: "cli is a bad log format when configured by the user", yaml: here.Doc(` --- names: @@ -182,7 +82,7 @@ func TestFromPath(t *testing.T) { level: info format: cli `), - wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: invalid log format, valid choices are the empty string, json and text", + wantError: "decode yaml: error unmarshaling JSON: while decoding JSON: invalid log format, valid choices are the empty string or 'json'", }, { name: "When only the required fields are present, causes other fields to be defaulted", @@ -206,7 +106,6 @@ func TestFromPath(t *testing.T) { Network: "disabled", }, }, - AllowExternalHTTP: false, AggregatedAPIServerPort: ptr.To[int64](10250), }, }, @@ -268,7 +167,7 @@ func TestFromPath(t *testing.T) { wantError: `validate http endpoint: http listener address ":8080" for "tcp" network may only bind to loopback interfaces`, }, { - name: "http endpoint uses tcp but binds to more than only loopback interfaces with insecureAcceptExternalUnencryptedHttpRequests set to boolean false", + name: "http endpoint uses tcp but binds to more than only loopback interfaces", yaml: here.Doc(` --- names: @@ -279,100 +178,9 @@ func TestFromPath(t *testing.T) { http: network: tcp address: :8080 - insecureAcceptExternalUnencryptedHttpRequests: false `), wantError: `validate http endpoint: http listener address ":8080" for "tcp" network may only bind to loopback interfaces`, }, - { - name: "http endpoint uses tcp but binds to more than only loopback interfaces with insecureAcceptExternalUnencryptedHttpRequests set to unsupported value", - yaml: here.Doc(` - --- - names: - defaultTLSCertificateSecret: my-secret-name - insecureAcceptExternalUnencryptedHttpRequests: "garbage" # this will be treated as the default, which is false - `), - wantError: `decode yaml: error unmarshaling JSON: while decoding JSON: invalid value for boolean`, - }, - { - name: "http endpoint uses tcp but binds to more than only loopback interfaces with insecureAcceptExternalUnencryptedHttpRequests set to string false", - yaml: here.Doc(` - --- - names: - defaultTLSCertificateSecret: my-secret-name - endpoints: - https: - network: disabled - http: - network: tcp - address: :8080 - insecureAcceptExternalUnencryptedHttpRequests: "false" - `), - wantError: `validate http endpoint: http listener address ":8080" for "tcp" network may only bind to loopback interfaces`, - }, - { - name: "http endpoint uses tcp but binds to more than only loopback interfaces with insecureAcceptExternalUnencryptedHttpRequests set to boolean true", - yaml: here.Doc(` - --- - names: - defaultTLSCertificateSecret: my-secret-name - endpoints: - http: - network: tcp - address: :1234 - insecureAcceptExternalUnencryptedHttpRequests: true - `), - wantConfig: &Config{ - APIGroupSuffix: ptr.To("pinniped.dev"), - Labels: map[string]string{}, - NamesConfig: NamesConfigSpec{ - DefaultTLSCertificateSecret: "my-secret-name", - }, - Endpoints: &Endpoints{ - HTTPS: &Endpoint{ - Network: "tcp", - Address: ":8443", - }, - HTTP: &Endpoint{ - Network: "tcp", - Address: ":1234", - }, - }, - AllowExternalHTTP: true, - AggregatedAPIServerPort: ptr.To[int64](10250), - }, - }, - { - name: "http endpoint uses tcp but binds to more than only loopback interfaces with insecureAcceptExternalUnencryptedHttpRequests set to string true", - yaml: here.Doc(` - --- - names: - defaultTLSCertificateSecret: my-secret-name - endpoints: - http: - network: tcp - address: :1234 - insecureAcceptExternalUnencryptedHttpRequests: "true" - `), - wantConfig: &Config{ - APIGroupSuffix: ptr.To("pinniped.dev"), - Labels: map[string]string{}, - NamesConfig: NamesConfigSpec{ - DefaultTLSCertificateSecret: "my-secret-name", - }, - Endpoints: &Endpoints{ - HTTPS: &Endpoint{ - Network: "tcp", - Address: ":8443", - }, - HTTP: &Endpoint{ - Network: "tcp", - Address: ":1234", - }, - }, - AllowExternalHTTP: true, - AggregatedAPIServerPort: ptr.To[int64](10250), - }, - }, { name: "endpoint disabled with non-empty address", yaml: here.Doc(` diff --git a/internal/config/supervisor/types.go b/internal/config/supervisor/types.go index bd89e2c7a..918375f67 100644 --- a/internal/config/supervisor/types.go +++ b/internal/config/supervisor/types.go @@ -1,25 +1,20 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisor import ( - "errors" - "go.pinniped.dev/internal/plog" ) // Config contains knobs to setup an instance of the Pinniped Supervisor. type Config struct { - APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` - Labels map[string]string `json:"labels"` - NamesConfig NamesConfigSpec `json:"names"` - // Deprecated: use log.level instead - LogLevel *plog.LogLevel `json:"logLevel"` - Log plog.LogSpec `json:"log"` - Endpoints *Endpoints `json:"endpoints"` - AllowExternalHTTP stringOrBoolAsBool `json:"insecureAcceptExternalUnencryptedHttpRequests"` - AggregatedAPIServerPort *int64 `json:"aggregatedAPIServerPort"` + APIGroupSuffix *string `json:"apiGroupSuffix,omitempty"` + Labels map[string]string `json:"labels"` + NamesConfig NamesConfigSpec `json:"names"` + Log plog.LogSpec `json:"log"` + Endpoints *Endpoints `json:"endpoints"` + AggregatedAPIServerPort *int64 `json:"aggregatedAPIServerPort"` } // NamesConfigSpec configures the names of some Kubernetes resources for the Supervisor. @@ -37,17 +32,3 @@ type Endpoint struct { Network string `json:"network"` Address string `json:"address"` } - -type stringOrBoolAsBool bool - -func (sb *stringOrBoolAsBool) UnmarshalJSON(b []byte) error { - switch string(b) { - case "true", `"true"`: - *sb = true - case "false", `"false"`: - *sb = false - default: - return errors.New("invalid value for boolean") - } - return nil -} diff --git a/internal/plog/config.go b/internal/plog/config.go index b5e7ad3ce..b2c60416f 100644 --- a/internal/plog/config.go +++ b/internal/plog/config.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package plog @@ -22,8 +22,6 @@ func (l *LogFormat) UnmarshalJSON(b []byte) error { switch string(b) { case `""`, `"json"`: *l = FormatJSON - case `"text"`: - *l = FormatText // there is no "cli" case because it is not a supported option via our config default: return errInvalidLogFormat @@ -33,11 +31,10 @@ func (l *LogFormat) UnmarshalJSON(b []byte) error { const ( FormatJSON LogFormat = "json" - FormatText LogFormat = "text" FormatCLI LogFormat = "cli" // only used by the pinniped CLI and not the server components errInvalidLogLevel = constable.Error("invalid log level, valid choices are the empty string, info, debug, trace and all") - errInvalidLogFormat = constable.Error("invalid log format, valid choices are the empty string, json and text") + errInvalidLogFormat = constable.Error("invalid log format, valid choices are the empty string or 'json'") ) var _ json.Unmarshaler = func() *LogFormat { @@ -50,13 +47,6 @@ type LogSpec struct { Format LogFormat `json:"format,omitempty"` } -func MaybeSetDeprecatedLogLevel(level *LogLevel, log *LogSpec) { - if level != nil { - Warning("logLevel is deprecated, set log.level instead") - log.Level = *level - } -} - func ValidateAndSetLogLevelAndFormatGlobally(ctx context.Context, spec LogSpec) error { klogLevel := klogLevelForPlogLevel(spec.Level) if klogLevel < 0 { @@ -75,8 +65,6 @@ func ValidateAndSetLogLevelAndFormatGlobally(ctx context.Context, spec LogSpec) encoding = "json" case FormatCLI: encoding = "console" - case FormatText: - encoding = "text" default: return errInvalidLogFormat } @@ -88,12 +76,8 @@ func ValidateAndSetLogLevelAndFormatGlobally(ctx context.Context, spec LogSpec) setGlobalLoggers(log, flush) - //nolint:exhaustive // the switch above is exhaustive for format already - switch spec.Format { - case FormatCLI: + if spec.Format == FormatCLI { return nil // do not spawn go routines on the CLI to allow the CLI to call this more than once - case FormatText: - Warning("setting log.format to 'text' is deprecated - this option will be removed in a future release") } // do spawn go routines on the server diff --git a/internal/plog/config_test.go b/internal/plog/config_test.go index 1c7888631..c5e750377 100644 --- a/internal/plog/config_test.go +++ b/internal/plog/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package plog @@ -101,7 +101,7 @@ func TestFormat(t *testing.T) { "timestamp": "2022-11-21T23:37:26.953313Z", "caller": "%s/config_test.go:%d$plog.TestFormat.func1", "message": "something happened", - "error": "invalid log format, valid choices are the empty string, json and text", + "error": "invalid log format, valid choices are the empty string or 'json'", "an": "item" }`, wd, getLineNumberOfCaller()-11), scanner.Text()) @@ -148,7 +148,7 @@ testing.tRunner DebugErr("something happened", errInvalidLogFormat, "an", "item") require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(nowStr+` plog/config_test.go:%d something happened {"error": "invalid log format, valid choices are the empty string, json and text", "an": "item"}`, + require.Equal(t, fmt.Sprintf(nowStr+` plog/config_test.go:%d something happened {"error": "invalid log format, valid choices are the empty string or 'json'", "an": "item"}`, getLineNumberOfCaller()-4), scanner.Text()) Logr().WithName("burrito").Error(errInvalidLogLevel, "wee", "a", "b", "slightly less than a year", 363*24*time.Hour, "slightly more than 2 years", 2*367*24*time.Hour) @@ -157,74 +157,6 @@ testing.tRunner require.Equal(t, fmt.Sprintf(nowStr+` burrito plog/config_test.go:%d wee {"a": "b", "slightly less than a year": "363d", "slightly more than 2 years": "2y4d", "error": "invalid log level, valid choices are the empty string, info, debug, trace and all"}`, getLineNumberOfCaller()-4), scanner.Text()) - old := New().WithName("created before mode change").WithValues("is", "old") - - err = ValidateAndSetLogLevelAndFormatGlobally(ctx, LogSpec{Level: LevelDebug, Format: FormatText}) - require.NoError(t, err) - pid := os.Getpid() - - // check for the deprecation warning - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config.go:96] "setting log.format to 'text' is deprecated - this option will be removed in a future release" warning=true`, - pid), scanner.Text()) - - Debug("what is happening", "does klog", "work?") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "what is happening" does klog="work?"`, - pid, getLineNumberOfCaller()-4), scanner.Text()) - - Logr().WithName("panda").V(KlogLevelDebug).Info("are the best", "yes?", "yes.") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "are the best" logger="panda" yes?="yes."`, - pid, getLineNumberOfCaller()-4), scanner.Text()) - - New().WithName("hi").WithName("there").WithValues("a", 1, "b", 2).Always("do it") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "do it" logger="hi.there" a=1 b=2`, - pid, getLineNumberOfCaller()-4), scanner.Text()) - - l := WithValues("x", 33, "z", 22) - l.Debug("what to do") - l.Debug("and why") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "what to do" x=33 z=22`, - pid, getLineNumberOfCaller()-5), scanner.Text()) - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "and why" x=33 z=22`, - pid, getLineNumberOfCaller()-8), scanner.Text()) - - old.Always("should be klog text format", "for", "sure") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "should be klog text format" logger="created before mode change" is="old" for="sure"`, - pid, getLineNumberOfCaller()-4), scanner.Text()) - - // make sure child loggers do not share state - old1 := old.WithValues("i am", "old1") - old2 := old.WithName("old2") - old1.Warning("warn") - old2.Info("info") - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "warn" logger="created before mode change" is="old" i am="old1" warning=true`, - pid, getLineNumberOfCaller()-5), scanner.Text()) - require.True(t, scanner.Scan()) - require.NoError(t, scanner.Err()) - require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "info" logger="created before mode change.old2" is="old"`, - pid, getLineNumberOfCaller()-8), scanner.Text()) - - Trace("should not be logged", "for", "sure") - require.Empty(t, buf.String()) - - Logr().V(klogLevelAll).Info("also should not be logged", "open", "close") - require.Empty(t, buf.String()) - require.False(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Empty(t, scanner.Text()) diff --git a/site/content/docs/howto/supervisor/configure-supervisor.md b/site/content/docs/howto/supervisor/configure-supervisor.md index ef7d77830..15adbfe3c 100644 --- a/site/content/docs/howto/supervisor/configure-supervisor.md +++ b/site/content/docs/howto/supervisor/configure-supervisor.md @@ -61,17 +61,8 @@ It is recommended that the traffic to these endpoints should be encrypted via TL Supervisor pods, even when crossing boundaries that are entirely inside the Kubernetes cluster. The credentials and tokens that are handled by these endpoints are too sensitive to transmit without encryption. -In previous versions of the Supervisor app, there were both HTTP and HTTPS ports available for use by default. -These ports each host all the Supervisor's endpoints. Unfortunately, this has caused some confusion in the community -and some blog posts have been written which demonstrate using the HTTP port in such a way that a portion of the traffic's -path is unencrypted. Newer versions of the Supervisor disable the HTTP port by default to make it more clear that -the Supervisor app is not intended to receive non-TLS HTTP traffic from outside the Pod. Furthermore, in these newer versions, -when the HTTP listener is configured to be enabled it may only listen on loopback interfaces for traffic from within its own pod. -To aid in transition for impacted users, the old behavior of allowing the HTTP listener to receive traffic from -outside the pod may be re-enabled using the -`deprecated_insecure_accept_external_unencrypted_http_requests` value in -[values.yaml](https://github.com/vmware-tanzu/pinniped/blob/main/deploy/supervisor/values.yaml), -until that setting is removed in a future release. +The Supervisor only listens on an HTTPS port by default. Incoming traffic must use TLS. The only exception is for +an advanced configuration style using a service mesh to deliver traffic into the Supervisor (discussed below). Because there are many ways to expose TLS services from a Kubernetes cluster, the Supervisor app leaves this up to the user. Some common approaches are: