Merge branch 'main' into github_identity_provider

This commit is contained in:
Ryan Richard
2024-05-07 13:08:50 -07:00
25 changed files with 228 additions and 620 deletions
@@ -5,7 +5,6 @@ package admissionpluginconfig
import (
"fmt"
"strings"
"github.com/pkg/errors"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
@@ -82,17 +81,16 @@ func k8sAPIServerHasValidatingAdmissionPolicyResource(discoveryClient discovery.
return false, fmt.Errorf("failed to perform k8s API discovery: %w", err)
}
// Now look at all discovered groups until we find admissionregistration.k8s.io.
wantedGroupWithSlash := fmt.Sprintf("%s/", admissionregistrationv1.GroupName)
// Now look at all discovered groups until we find version v1 of group admissionregistration.k8s.io.
for _, resourcesPerGV := range resources {
if strings.HasPrefix(resourcesPerGV.GroupVersion, wantedGroupWithSlash) {
if resourcesPerGV.GroupVersion == admissionregistrationv1.SchemeGroupVersion.String() {
// Found the group, so now look to see if it includes ValidatingAdmissionPolicy as a resource,
// which went GA in Kubernetes 1.30, and could be enabled by a feature flag in previous versions.
for _, resource := range resourcesPerGV.APIResources {
if resource.Kind == "ValidatingAdmissionPolicy" {
// Found it!
plog.Info("found ValidatingAdmissionPolicy resource on this Kubernetes cluster",
"group", resource.Group, "version", resource.Version, "kind", resource.Kind)
"groupVersion", resourcesPerGV.GroupVersion, "kind", resource.Kind)
return true, nil
}
}
@@ -57,6 +57,14 @@ func TestConfigureAdmissionPlugins(t *testing.T) {
},
}
newStyleAdmissionResourcesWithValidatingAdmissionPoliciesAtOlderAPIVersion := &metav1.APIResourceList{
GroupVersion: admissionregistrationv1.SchemeGroupVersion.Group + "/v1beta1",
APIResources: []metav1.APIResource{
{Name: "validatingwebhookconfigurations", Kind: "ValidatingWebhookConfiguration"},
{Name: "validatingadmissionpolicies", Kind: "ValidatingAdmissionPolicy"},
},
}
oldStyleAdmissionResourcesWithoutValidatingAdmissionPolicies := &metav1.APIResourceList{
GroupVersion: admissionregistrationv1.SchemeGroupVersion.String(),
APIResources: []metav1.APIResource{
@@ -92,6 +100,16 @@ func TestConfigureAdmissionPlugins(t *testing.T) {
wantRegisteredPlugins: customOldStylePluginsRegistered,
wantRecommendedPluginOrder: customOldStyleRecommendedPluginOrder,
},
{
name: "when there is only an older version of ValidatingAdmissionPolicy resource, as there would be in an old Kubernetes cluster with the feature flag enabled, then we change the plugin configuration to be more like it was for old versions of Kubernetes (because the admission code wants to watch v1)",
availableAPIResources: []*metav1.APIResourceList{
coreResources,
newStyleAdmissionResourcesWithValidatingAdmissionPoliciesAtOlderAPIVersion,
appsResources,
},
wantRegisteredPlugins: customOldStylePluginsRegistered,
wantRecommendedPluginOrder: customOldStyleRecommendedPluginOrder,
},
{
name: "when there is a total error returned by discovery",
discoveryErr: errors.New("total error from API discovery client"),
+1 -2
View File
@@ -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)
}
+26 -87
View File
@@ -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",
+2 -4
View File
@@ -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
+3 -12
View File
@@ -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,
+6 -198
View File
@@ -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(`
+7 -26
View File
@@ -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
}
@@ -428,7 +428,13 @@ func TestFuzzAndJSONNewValidEmptyAuthorizeCodeSession(t *testing.T) {
// cause those old session Secrets to be discarded upon read after an upgrade.
// Note that when you change the storage version, you will also need to change it in the JSON content of the
// expected value for this assertion.
require.JSONEq(t, ExpectedAuthorizeCodeSessionJSONFromFuzzing, authorizeCodeSessionJSONFromFuzzing, "actual:\n%s", authorizeCodeSessionJSONFromFuzzing)
require.JSONEq(t,
ExpectedAuthorizeCodeSessionJSONFromFuzzing,
authorizeCodeSessionJSONFromFuzzing,
"actual:\n%s\n\n(NOTICE: This test may fail when storage structure is updated. "+
"Be sure to update relevant version variables (authorizeCodeStorageVersion, oidcStorageVersion, pkceStorageVersion, "+
"refreshTokenStorageVersion, accessTokenStorageVersion) to a new value and leave a comment documenting the change. "+
"Updating the version ensures new secret generation, therefore smooth upgrades of Pinniped.)", authorizeCodeSessionJSONFromFuzzing)
}
func TestReadFromSecret(t *testing.T) {
+3 -19
View File
@@ -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
+3 -71
View File
@@ -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())
@@ -1,4 +1,4 @@
// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved.
// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package clientsecretrequest
@@ -33,6 +33,7 @@ import (
supervisorfake "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake"
"go.pinniped.dev/internal/oidcclientsecretstorage"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/testutil"
)
func TestNew(t *testing.T) {
@@ -1563,8 +1564,12 @@ func TestCreate(t *testing.T) {
var log bytes.Buffer
logger := plog.TestZapr(t, &log)
klog.SetLogger(logger)
originalKLogLevel := testutil.GetGlobalKlogLevel()
// trace.Log() utility will only log at level 2 or above, so set that for this test.
testutil.SetGlobalKlogLevel(t, 2) //nolint:staticcheck // old test of code using trace.Log()
t.Cleanup(func() {
klog.ClearLogger()
testutil.SetGlobalKlogLevel(t, originalKLogLevel) //nolint:staticcheck // old test of code using trace.Log()
})
kubeClient := kubefake.NewSimpleClientset()
@@ -68,16 +68,21 @@ func TestCreate(t *testing.T) {
var r *require.Assertions
var ctrl *gomock.Controller
var logger *testutil.TranscriptLogger
var originalKLogLevel klog.Level
it.Before(func() {
r = require.New(t)
ctrl = gomock.NewController(t)
logger = testutil.NewTranscriptLogger(t) //nolint:staticcheck // old test with lots of log statements
klog.SetLogger(logr.New(logger)) // this is unfortunately a global logger, so can't run these tests in parallel :(
originalKLogLevel = testutil.GetGlobalKlogLevel()
// trace.Log() utility will only log at level 2 or above, so set that for this test.
testutil.SetGlobalKlogLevel(t, 2) //nolint:staticcheck // old test of code using trace.Log()
})
it.After(func() {
klog.ClearLogger()
testutil.SetGlobalKlogLevel(t, originalKLogLevel) //nolint:staticcheck // old test of code using trace.Log()
ctrl.Finish()
})
+32
View File
@@ -0,0 +1,32 @@
// Copyright 2024 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package testutil
import (
"strconv"
"testing"
"github.com/stretchr/testify/require"
"k8s.io/component-base/logs"
"k8s.io/klog/v2"
)
// Deprecated: This is meant for old tests only.
func SetGlobalKlogLevel(t *testing.T, l klog.Level) {
t.Helper()
_, err := logs.GlogSetter(strconv.Itoa(int(l)))
require.NoError(t, err)
}
func GetGlobalKlogLevel() klog.Level {
// hack around klog not exposing a Get method
for i := klog.Level(0); i < 256; i++ {
if klog.V(i).Enabled() {
continue
}
return i - 1
}
return -1
}