chore: move to mlog.New and drop klog and logr as direct deps (#722)

Signed-off-by: Monis Khan <[email protected]>

cmd/proxy/main.go

@@ -48,9 +48,7 @@ func mainErr() error {
 		return nil
 	}
 
-	// nolint:staticcheck
-	// we will migrate to mlog.New in a future change
-	p, err := proxy.NewProxy(proxyPort, mlog.Logr().WithName("proxy"))
+	p, err := proxy.NewProxy(proxyPort, mlog.New().WithName("proxy"))
 	if err != nil {
 		return fmt.Errorf("setup: failed to create proxy: %w", err)
 	}

cmd/webhook/main.go

@@ -53,9 +53,7 @@ var (
 	dnsName = fmt.Sprintf("%s.%s.svc", serviceName, util.GetNamespace())
 	scheme  = runtime.NewScheme()
 
-	// nolint:staticcheck
-	// we will migrate to mlog.New in a future change
-	entryLog = mlog.Logr().WithName("entrypoint")
+	entryLog = mlog.New().WithName("entrypoint")
 )
 
 func init() {

config/manager/manager.yaml

@@ -28,6 +28,8 @@ spec:
       containers:
       - command:
         - /manager
+        args:
+        - --log-level=info
         image: manager:latest
         imagePullPolicy: IfNotPresent
         name: manager

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/Azure/go-autorest/autorest/azure/cli v0.4.6
 	github.com/Azure/go-autorest/autorest/to v0.4.0
 	github.com/AzureAD/microsoft-authentication-library-for-go v0.7.0
-	github.com/go-logr/logr v1.2.3
 	github.com/golang/mock v1.6.0
 	github.com/google/uuid v1.3.0
 	github.com/gorilla/mux v1.8.0
@@ -35,7 +34,6 @@ require (
 	k8s.io/api v0.25.6
 	k8s.io/apimachinery v0.25.6
 	k8s.io/client-go v0.25.6
-	k8s.io/klog/v2 v2.80.1
 	k8s.io/utils v0.0.0-20221128185143-99ec85e7a448
 	monis.app/mlog v0.0.2
 	sigs.k8s.io/controller-runtime v0.13.1
@@ -60,6 +58,7 @@ require (
 	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
 	github.com/fsnotify/fsnotify v1.5.4 // indirect
+	github.com/go-logr/logr v1.2.3 // indirect
 	github.com/go-logr/zapr v1.2.3 // indirect
 	github.com/go-openapi/jsonpointer v0.19.5 // indirect
 	github.com/go-openapi/jsonreference v0.19.5 // indirect
@@ -115,6 +114,7 @@ require (
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/apiextensions-apiserver v0.25.3 // indirect
 	k8s.io/component-base v0.25.3 // indirect
+	k8s.io/klog/v2 v2.80.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect
 	sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect

manifest_staging/charts/workload-identity-webhook/README.md

@@ -45,7 +45,7 @@ helm upgrade -n azure-workload-identity-system [RELEASE_NAME] azure-workload-ide
 | service.targetPort               | Service target port                                                                                                               | `9443`                                                  |
 | azureTenantID                    | [**REQUIRED**] Azure tenant ID                                                                                                    | ``                                                      |
 | azureEnvironment                 | Azure Environment                                                                                                                 | `AzurePublicCloud`                                      |
-| logLevel                         | The log level to use for the webhook manager. In order of increasing verbosity: unset (empty string), info, debug, trace and all. | ``                                                      |
+| logLevel                         | The log level to use for the webhook manager. In order of increasing verbosity: unset (empty string), info, debug, trace and all. | `info`                                                  |
 | metricsAddr                      | The address to bind the metrics server to                                                                                         | `:8095`                                                 |
 | metricsBackend                   | The metrics backend to use (`prometheus`)                                                                                         | `prometheus`                                            |
 | priorityClassName                | The priority class name for webhook manager                                                                                       | `system-cluster-critical`                               |

manifest_staging/charts/workload-identity-webhook/values.yaml

@@ -26,7 +26,7 @@ service:
   targetPort: 9443
 azureEnvironment: AzurePublicCloud
 azureTenantID:
-logLevel:
+logLevel: info
 metricsAddr: ":8095"
 metricsBackend: prometheus
 priorityClassName: system-cluster-critical

manifest_staging/deploy/azure-wi-webhook.yaml

@@ -155,7 +155,9 @@ spec:
         azure-workload-identity.io/system: "true"
     spec:
       containers:
-      - command:
+      - args:
+        - --log-level=info
+        command:
         - /manager
         env:
         - name: POD_NAMESPACE

pkg/cmd/podidentity/detect.go

@@ -9,11 +9,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-workload-identity/pkg/cmd/podidentity/k8s"
-	"github.com/Azure/azure-workload-identity/pkg/cmd/serviceaccount/options"
-	"github.com/Azure/azure-workload-identity/pkg/kuberneteshelper"
-	"github.com/Azure/azure-workload-identity/pkg/webhook"
-
 	aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -26,7 +21,13 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"monis.app/mlog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/Azure/azure-workload-identity/pkg/cmd/podidentity/k8s"
+	"github.com/Azure/azure-workload-identity/pkg/cmd/serviceaccount/options"
+	"github.com/Azure/azure-workload-identity/pkg/kuberneteshelper"
+	"github.com/Azure/azure-workload-identity/pkg/webhook"
 )
 
 var (
@@ -362,12 +363,14 @@ func (dc *detectCmd) addProxyContainer(containers []corev1.Container) []corev1.C
 		}
 	}
 
+	logLevel := mlog.LevelInfo // somewhat arbitrary decision
 	proxyContainer := corev1.Container{
 		Name:            proxyContainerName,
 		Image:           proxyImage,
 		ImagePullPolicy: corev1.PullIfNotPresent,
 		Args: []string{
 			fmt.Sprintf("--proxy-port=%d", dc.proxyPort),
+			fmt.Sprintf("--log-level=%s", logLevel),
 		},
 		Ports: []corev1.ContainerPort{
 			{
@@ -381,6 +384,7 @@ func (dc *detectCmd) addProxyContainer(containers []corev1.Container) []corev1.C
 						"/proxy",
 						fmt.Sprintf("--proxy-port=%d", dc.proxyPort),
 						"--probe",
+						fmt.Sprintf("--log-level=%s", logLevel),
 					},
 				},
 			},

pkg/cmd/podidentity/detect_test.go

@@ -51,6 +51,7 @@ var (
 		ImagePullPolicy: corev1.PullIfNotPresent,
 		Args: []string{
 			fmt.Sprintf("--proxy-port=%d", 8000),
+			"--log-level=info",
 		},
 		Ports: []corev1.ContainerPort{
 			{
@@ -64,6 +65,7 @@ var (
 						"/proxy",
 						fmt.Sprintf("--proxy-port=%d", 8000),
 						"--probe",
+						"--log-level=info",
 					},
 				},
 			},

pkg/proxy/probe_test.go

@@ -3,14 +3,14 @@ package proxy
 import (
 	"testing"
 
-	"k8s.io/klog/v2/klogr"
+	"monis.app/mlog"
 )
 
 func TestProbe(t *testing.T) {
 	setup()
 	defer teardown()
 
-	p := &proxy{logger: klogr.New()}
+	p := &proxy{logger: mlog.New()}
 	rtr.PathPrefix("/readyz").HandlerFunc(p.readyzHandler)
 
 	if err := probe(server.URL + "/readyz"); err != nil {

pkg/proxy/proxy.go

@@ -11,13 +11,13 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-workload-identity/pkg/version"
-	"github.com/Azure/azure-workload-identity/pkg/webhook"
-
 	"github.com/AzureAD/microsoft-authentication-library-for-go/apps/confidential"
-	"github.com/go-logr/logr"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
+	"monis.app/mlog"
+
+	"github.com/Azure/azure-workload-identity/pkg/version"
+	"github.com/Azure/azure-workload-identity/pkg/webhook"
 )
 
 const (
@@ -47,7 +47,7 @@ type proxy struct {
 	port          int
 	tenantID      string
 	authorityHost string
-	logger        logr.Logger
+	logger        mlog.Logger
 }
 
 // using this from https://github.com/Azure/go-autorest/blob/b3899c1057425994796c92293e931f334af63b4e/autorest/adal/token.go#L1055-L1067
@@ -67,7 +67,7 @@ type token struct {
 }
 
 // NewProxy returns a proxy instance
-func NewProxy(port int, logger logr.Logger) (Proxy, error) {
+func NewProxy(port int, logger mlog.Logger) (Proxy, error) {
 	// tenantID is required for fetching a token using client assertions
 	// the mutating webhook will inject the tenantID for the cluster
 	tenantID := os.Getenv(webhook.AzureTenantIDEnvVar)
@@ -127,23 +127,23 @@ func (p *proxy) msiHandler(w http.ResponseWriter, r *http.Request) {
 	// get the token using the msal
 	token, err := doTokenRequest(r.Context(), clientID, resource, p.tenantID, p.authorityHost)
 	if err != nil {
-		p.logger.Error(err, "failed to get token")
+		p.logger.Error("failed to get token", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	p.logger.Info("successfully acquired token", "method", r.Method, "uri", r.RequestURI)
 	// write the token to the response
 	w.Header().Set("Content-Type", "application/json")
 	if err := json.NewEncoder(w).Encode(token); err != nil {
-		p.logger.Error(err, "failed to encode token")
+		p.logger.Error("failed to encode token", err)
 	}
 }
 
 func (p *proxy) defaultPathHandler(w http.ResponseWriter, r *http.Request) {
 	client := &http.Client{}
 	req, err := http.NewRequest(r.Method, r.URL.String(), r.Body)
 	if err != nil || req == nil {
-		p.logger.Error(err, "failed to create new request")
+		p.logger.Error("failed to create new request", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
@@ -156,15 +156,15 @@ func (p *proxy) defaultPathHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	resp, err := client.Do(req)
 	if err != nil {
-		p.logger.Error(err, "failed executing request", "url", req.URL.String())
+		p.logger.Error("failed executing request", err, "url", req.URL.String())
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		p.logger.Error(err, "failed to read response body", "url", req.URL.String())
+		p.logger.Error("failed to read response body", err, "url", req.URL.String())
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 	}
 	p.logger.Info("received response from IMDS", "method", r.Method, "uri", r.RequestURI, "status", resp.StatusCode)

pkg/proxy/proxy_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 
 	"github.com/gorilla/mux"
-	"k8s.io/klog/v2/klogr"
+	"monis.app/mlog"
 
 	"github.com/Azure/azure-workload-identity/pkg/webhook"
 )
@@ -61,7 +61,7 @@ func TestProxy_MSIHandler(t *testing.T) {
 			setup()
 			defer teardown()
 
-			p := &proxy{logger: klogr.New()}
+			p := &proxy{logger: mlog.New()}
 			rtr.PathPrefix(tokenPathPrefix).HandlerFunc(p.msiHandler)
 			rtr.PathPrefix("/").HandlerFunc(p.defaultPathHandler)
 
@@ -291,7 +291,7 @@ func TestProxy_ReadyZHandler(t *testing.T) {
 			setup()
 			defer teardown()
 
-			p := &proxy{logger: klogr.New()}
+			p := &proxy{logger: mlog.New()}
 			rtr.PathPrefix("/readyz").HandlerFunc(p.readyzHandler)
 
 			req, err := http.NewRequest(http.MethodGet, server.URL+test.path, nil)

pkg/webhook/webhook.go

@@ -108,28 +108,26 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
 		serviceAccountName = "default"
 	}
 
-	// nolint:staticcheck
-	// we will migrate to mlog.New in a future change
-	logger := mlog.Logr().WithName("handler").WithValues("pod", podName, "namespace", pod.Namespace, "service-account", serviceAccountName)
+	logger := mlog.New().WithName("handler").WithValues("pod", podName, "namespace", pod.Namespace, "service-account", serviceAccountName)
 	// get service account associated with the pod
 	serviceAccount := &corev1.ServiceAccount{}
 	if err = m.client.Get(ctx, types.NamespacedName{Name: serviceAccountName, Namespace: pod.Namespace}, serviceAccount); err != nil {
 		if !apierrors.IsNotFound(err) {
-			logger.Error(err, "failed to get service account")
+			logger.Error("failed to get service account", err)
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 		// bypass cache and get from the API server as it's not found in cache
 		err = m.reader.Get(ctx, types.NamespacedName{Name: serviceAccountName, Namespace: pod.Namespace}, serviceAccount)
 		if err != nil {
-			logger.Error(err, "failed to get service account")
+			logger.Error("failed to get service account", err)
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 	}
 
 	if shouldInjectProxySidecar(pod) {
 		proxyPort, err := getProxyPort(pod)
 		if err != nil {
-			logger.Error(err, "failed to get proxy port")
+			logger.Error("failed to get proxy port", err)
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 
@@ -140,7 +138,7 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
 	// get service account token expiration
 	serviceAccountTokenExpiration, err := getServiceAccountTokenExpiration(pod, serviceAccount)
 	if err != nil {
-		logger.Error(err, "failed to get service account token expiration")
+		logger.Error("failed to get service account token expiration", err)
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 	// get the clientID
@@ -154,13 +152,13 @@ func (m *podMutator) Handle(ctx context.Context, req admission.Request) (respons
 
 	// add the projected service account token volume to the pod if not exists
 	if err = addProjectedServiceAccountTokenVolume(pod, serviceAccountTokenExpiration, m.audience); err != nil {
-		logger.Error(err, "failed to add projected service account volume")
+		logger.Error("failed to add projected service account volume", err)
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
 	marshaledPod, err := json.Marshal(pod)
 	if err != nil {
-		logger.Error(err, "failed to marshal pod object")
+		logger.Error("failed to marshal pod object", err)
 		return admission.Errored(http.StatusInternalServerError, err)
 	}
 	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
@@ -229,12 +227,14 @@ func (m *podMutator) injectProxySidecarContainer(containers []corev1.Container,
 		}
 	}
 
+	logLevel := currentLogLevel() // run the proxy at the same log level as the webhook
 	containers = append(containers, corev1.Container{
 		Name:            ProxySidecarContainerName,
 		Image:           strings.Join([]string{imageRepository, ProxyImageVersion}, ":"),
 		ImagePullPolicy: corev1.PullIfNotPresent,
 		Args: []string{
 			fmt.Sprintf("--proxy-port=%d", proxyPort),
+			fmt.Sprintf("--log-level=%s", logLevel),
 		},
 		Ports: []corev1.ContainerPort{{
 			ContainerPort: proxyPort,
@@ -246,6 +246,7 @@ func (m *podMutator) injectProxySidecarContainer(containers []corev1.Container,
 						"/proxy",
 						fmt.Sprintf("--proxy-port=%d", proxyPort),
 						"--probe",
+						fmt.Sprintf("--log-level=%s", logLevel),
 					},
 				},
 			},
@@ -435,3 +436,19 @@ func getAzureAuthorityHost(c *config.Config) (string, error) {
 	}
 	return env.ActiveDirectoryEndpoint, err
 }
+
+func currentLogLevel() string {
+	for _, level := range []mlog.LogLevel{
+		// iterate in reverse order
+		mlog.LevelAll,
+		mlog.LevelTrace,
+		mlog.LevelDebug,
+		mlog.LevelInfo,
+		mlog.LevelWarning,
+	} {
+		if mlog.Enabled(level) {
+			return string(level)
+		}
+	}
+	return "" // this is unreachable
+}