Merge pull request #8808 from BlaineEXE/add-trace-log-option

core: create TRACE_INSECURE log level
This commit is contained in:
Sébastien Han authored and GitHub committed 2021-10-06 11:35:32 +02:00
commit 0e0ccf8640
10 files changed
+125 -45

No files matched your search

+1 -1
View File
@@ -41,7 +41,7 @@ currentNamespaceOnly: false
## Annotations to be added to pod
annotations: {}
## LogLevel can be set to: TRACE, DEBUG, INFO, NOTICE, WARNING, ERROR or CRITICAL
## The logging level for the operator: ERROR | WARNING | INFO | DEBUG
logLevel: INFO
## If true, create & use RBAC resources
@@ -100,7 +100,7 @@ metadata:
# should be in the namespace of the operator
namespace: rook-ceph # namespace:operator
data:
# The logging level for the operator: INFO | DEBUG
# The logging level for the operator: ERROR | WARNING | INFO | DEBUG
ROOK_LOG_LEVEL: "INFO"
# Enable the CSI driver.
@@ -22,7 +22,7 @@ metadata:
# should be in the namespace of the operator
namespace: rook-ceph # namespace:operator
data:
# The logging level for the operator: INFO | DEBUG
# The logging level for the operator: ERROR | WARNING | INFO | DEBUG
ROOK_LOG_LEVEL: "INFO"
# Enable the CSI driver.
+3 -14
View File
@@ -28,6 +28,7 @@ import (
rookclient "github.com/rook/rook/pkg/client/clientset/versioned"
"github.com/rook/rook/pkg/clusterd"
"github.com/rook/rook/pkg/operator/k8sutil"
"github.com/rook/rook/pkg/util"
"github.com/rook/rook/pkg/util/exec"
"github.com/rook/rook/pkg/util/flags"
"github.com/rook/rook/pkg/version"
@@ -55,20 +56,15 @@ var (
logLevelRaw string
operatorImage string
serviceAccountName string
Cfg = &Config{}
logger = capnslog.NewPackageLogger("github.com/rook/rook", "rookcmd")
)
type Config struct {
LogLevel capnslog.LogLevel
}
// Initialize the configuration parameters. The precedence from lowest to highest is:
// 1) default value (at compilation)
// 2) environment variables (upper case, replace - with _, and rook prefix. For example, discovery-url is ROOK_DISCOVERY_URL)
// 3) command line parameter
func init() {
RootCmd.PersistentFlags().StringVar(&logLevelRaw, "log-level", "INFO", "logging level for logging/tracing output (valid values: CRITICAL,ERROR,WARNING,NOTICE,INFO,DEBUG,TRACE)")
RootCmd.PersistentFlags().StringVar(&logLevelRaw, "log-level", "INFO", "logging level for logging/tracing output (valid values: ERROR,WARNING,INFO,DEBUG)")
RootCmd.PersistentFlags().StringVar(&operatorImage, "operator-image", "", "Override the image url that the operator uses. The default is read from the operator pod.")
RootCmd.PersistentFlags().StringVar(&serviceAccountName, "service-account", "", "Override the service account that the operator uses. The default is read from the operator pod.")
@@ -79,13 +75,7 @@ func init() {
// SetLogLevel set log level based on provided log option.
func SetLogLevel() {
// parse given log level string then set up corresponding global logging level
ll, err := capnslog.ParseLevel(logLevelRaw)
if err != nil {
logger.Warningf("failed to set log level %s. %+v", logLevelRaw, err)
}
Cfg.LogLevel = ll
capnslog.SetGlobalLogLevel(Cfg.LogLevel)
util.SetGlobalLogLevel(logLevelRaw, logger)
}
// LogStartupInfo log the version number, arguments, and all final flag values (environment variable overrides have already been taken into account)
@@ -103,7 +93,6 @@ func NewContext() *clusterd.Context {
context := &clusterd.Context{
Executor: &exec.CommandExecutor{},
ConfigDir: k8sutil.DataDir,
LogLevel: Cfg.LogLevel,
}
// Try to read config from in-cluster env
-4
View File
@@ -17,7 +17,6 @@ limitations under the License.
package clusterd
import (
"github.com/coreos/pkg/capnslog"
netclient "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/client/clientset/versioned/typed/k8s.cni.cncf.io/v1"
rookclient "github.com/rook/rook/pkg/client/clientset/versioned"
"github.com/rook/rook/pkg/util/exec"
@@ -58,9 +57,6 @@ type Context struct {
// The root configuration directory used by services
ConfigDir string
// A value indicating the desired logging/tracing level
LogLevel capnslog.LogLevel
// The full path to a config file that can be used to override generated settings
ConfigFileOverride string
+1 -7
View File
@@ -29,7 +29,6 @@ import (
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/coreos/pkg/capnslog"
"github.com/go-ini/ini"
"github.com/rook/rook/pkg/clusterd"
cephver "github.com/rook/rook/pkg/operator/ceph/version"
@@ -49,9 +48,7 @@ func TestCreateDefaultCephConfig(t *testing.T) {
}
// start with INFO level logging
context := &clusterd.Context{
LogLevel: capnslog.INFO,
}
context := &clusterd.Context{}
cephConfig, err := CreateDefaultCephConfig(context, clusterInfo)
if err != nil {
@@ -59,9 +56,6 @@ func TestCreateDefaultCephConfig(t *testing.T) {
}
verifyConfig(t, cephConfig, clusterInfo, 0)
// now use DEBUG level logging
context.LogLevel = capnslog.DEBUG
cephConfig, err = CreateDefaultCephConfig(context, clusterInfo)
if err != nil {
t.Fatalf("failed to create default ceph config. %+v", err)
+5 -15
View File
@@ -18,7 +18,6 @@ package operator
import (
"context"
"strings"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -27,12 +26,12 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
"github.com/coreos/pkg/capnslog"
"github.com/pkg/errors"
"github.com/rook/rook/pkg/clusterd"
opcontroller "github.com/rook/rook/pkg/operator/ceph/controller"
"github.com/rook/rook/pkg/operator/discover"
"github.com/rook/rook/pkg/operator/k8sutil"
"github.com/rook/rook/pkg/util"
v1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -127,10 +126,7 @@ func (r *ReconcileConfig) reconcile(request reconcile.Request) (reconcile.Result
opcontroller.SetCephCommandsTimeout(r.config.Parameters)
// Reconcile Operator's logging level
err = reconcileOperatorLogLevel(opConfig.Data)
if err != nil {
return opcontroller.ImmediateRetryResult, err
}
reconcileOperatorLogLevel(opConfig.Data)
// Reconcile discovery daemon
err = r.reconcileDiscoveryDaemon()
@@ -145,15 +141,9 @@ func (r *ReconcileConfig) reconcile(request reconcile.Request) (reconcile.Result
return reconcile.Result{}, nil
}
func reconcileOperatorLogLevel(data map[string]string) error {
rookLogLevel := k8sutil.GetValue(data, "ROOK_LOG_LEVEL", "INFO")
logLevel, err := capnslog.ParseLevel(strings.ToUpper(rookLogLevel))
if err != nil {
return errors.Wrapf(err, "failed to load ROOK_LOG_LEVEL %q.", rookLogLevel)
}
capnslog.SetGlobalLogLevel(logLevel)
return nil
func reconcileOperatorLogLevel(data map[string]string) {
rookLogLevel := k8sutil.GetValue(data, "ROOK_LOG_LEVEL", util.DefaultLogLevel.String())
util.SetGlobalLogLevel(rookLogLevel, logger)
}
func (r *ReconcileConfig) reconcileDiscoveryDaemon() error {
+4 -2
View File
@@ -72,7 +72,8 @@ func (c *debugHTTPClient) Do(req *http.Request) (*http.Response, error) {
if err != nil {
return nil, err
}
c.logger.Debugf("\n%s\n", string(dump))
// this can leak credentials for making requests
c.logger.Tracef("\n%s\n", string(dump))
resp, err := c.client.Do(req)
if err != nil {
@@ -84,7 +85,8 @@ func (c *debugHTTPClient) Do(req *http.Request) (*http.Response, error) {
if err != nil {
return nil, err
}
c.logger.Debugf("\n%s\n", string(dump))
// this can leak any sensitive info like credentials in the response
c.logger.Tracef("\n%s\n", string(dump))
return resp, nil
}
+54
View File
@@ -0,0 +1,54 @@
/*
Copyright 2021 The Rook Authors. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package util
import (
"github.com/coreos/pkg/capnslog"
)
const DefaultLogLevel = capnslog.INFO
func SetGlobalLogLevel(userLogLevelSelection string, logger *capnslog.PackageLogger) {
// capnslog supports trace level logging, but in Rook we want to treat trace logging as insecure
// and block users from finding the value in most circumstances. If they request "TRACE" level
// logging, just output debug logs.
if userLogLevelSelection == "TRACE" {
userLogLevelSelection = "DEBUG"
}
// only if users give the super secret "TRACE_INSECURE" log level will they get real trace
// logging, which might leak credentials and other insecure nasties into their logs.
if userLogLevelSelection == "TRACE_INSECURE" {
userLogLevelSelection = "TRACE"
}
// parse given log level string then set up corresponding global logging level
logLevel, err := capnslog.ParseLevel(userLogLevelSelection)
if err != nil {
logger.Errorf("failed to parse log level %q. defaulting to %q. %v", userLogLevelSelection, DefaultLogLevel.String(), err)
logLevel = DefaultLogLevel
}
// If capnslog changes in the future to allow a more verbose level than TRACE and a user somehow
// enters it, then reject that log level, and revert to default. This can't be unit tested, but
// it'll probably never happen in the wild anyway, just here for safety.
if logLevel > capnslog.TRACE {
logger.Infof("not setting log level %q more verbose than TRACE. reverting to default %q", logLevel.String(), DefaultLogLevel.String())
logLevel = DefaultLogLevel
}
capnslog.SetGlobalLogLevel(logLevel)
}
+55
View File
@@ -0,0 +1,55 @@
/*
Copyright 2021 The Rook Authors. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package util
import (
"testing"
"github.com/coreos/pkg/capnslog"
"github.com/stretchr/testify/assert"
)
func TestSetGlobalLogLevel(t *testing.T) {
logger := capnslog.NewPackageLogger("github.com/rook/rook", "pkg/util/logging_test")
tests := []struct {
name string
userLogLevelSelection string
desiredLogLevel capnslog.LogLevel
}{
{"INFO is supported", "INFO", capnslog.INFO},
{"DEBUG is supported", "DEBUG", capnslog.DEBUG},
{"WARNING is supported", "WARNING", capnslog.WARNING},
{"ERROR is supported", "ERROR", capnslog.ERROR},
{"TRACE will be turned into DEBUG", "TRACE", capnslog.DEBUG},
{"TRACE_INSECURE will be turned into TRACE", "TRACE_INSECURE", capnslog.TRACE},
{"an invalid input will be turned into INFO", "INVALID", capnslog.INFO},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
SetGlobalLogLevel(tt.userLogLevelSelection, logger)
t.Logf("asserting that log level %d can be shown", tt.desiredLogLevel)
assert.True(t, logger.LevelAt(tt.desiredLogLevel))
// logger.LevelAt will only tell whether the logger will output logs at the desired log
// level, but it won't actually tell what the log level is specifically. To make sure we
// won't output MORE logs, we should check for the next more verbose log level also.
nextMostVerboseLevel := tt.desiredLogLevel + 1
t.Logf("asserting that log level %d can NOT be shown", nextMostVerboseLevel)
assert.False(t, logger.LevelAt(nextMostVerboseLevel))
})
}
}