From a1de5a86ffcae7c0baa5b0b266587fedb9b44103 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Tue, 9 Jun 2020 17:49:01 -0400 Subject: [PATCH] Migrate a single node_authorizer.go klog.Infof call to klog.InfoS (#91591) * Migrate a single node_authorizer.go klog.Infof call to klog.InfoS We are starting with the log lines that show up most often. Signed-off-by: Andrew Keesler * Remove quotes from error for readability Signed-off-by: Andrew Keesler * node_authorizer.go: use %s for node names for log uniformity Signed-off-by: Andrew Keesler * node_authorizer.go: single-quote node name for readability++ This is good because: 1) the node name is clear in the log line 2) the node names shows up the same in {un-,}structured logs Signed-off-by: Andrew Keesler --- .../auth/authorizer/node/node_authorizer.go | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index 1916f9ca523..39157c73f6c 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -151,12 +151,12 @@ func (r *NodeAuthorizer) authorizeStatusUpdate(nodeName string, startingType ver case "update", "patch": // ok default: - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get/update/patch this type", nil } if attrs.GetSubresource() != "status" { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only update status subresource", nil } @@ -166,11 +166,11 @@ func (r *NodeAuthorizer) authorizeStatusUpdate(nodeName string, startingType ver // authorizeGet authorizes "get" requests to objects of the specified type if they are related to the specified node func (r *NodeAuthorizer) authorizeGet(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { if attrs.GetVerb() != "get" { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get individual resources of this type", nil } if len(attrs.GetSubresource()) > 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "cannot get subresource", nil } return r.authorize(nodeName, startingType, attrs) @@ -183,16 +183,16 @@ func (r *NodeAuthorizer) authorizeReadNamespacedObject(nodeName string, starting case "get", "list", "watch": //ok default: - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only read resources of this type", nil } if len(attrs.GetSubresource()) > 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "cannot read subresource", nil } if len(attrs.GetNamespace()) == 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only read namespaced object of this type", nil } return r.authorize(nodeName, startingType, attrs) @@ -200,18 +200,18 @@ func (r *NodeAuthorizer) authorizeReadNamespacedObject(nodeName string, starting func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { if len(attrs.GetName()) == 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "No Object name found", nil } ok, err := r.hasPathFrom(nodeName, startingType, attrs.GetNamespace(), attrs.GetName()) if err != nil { - klog.V(2).Infof("NODE DENY: %v", err) - return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node %q and this object", nodeName), nil + klog.V(2).InfoS("NODE DENY", "err", err) + return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node '%s' and this object", nodeName), nil } if !ok { - klog.V(2).Infof("NODE DENY: %q %#v", nodeName, attrs) - return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node %q and this object", nodeName), nil + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node '%s' and this object", nodeName), nil } return authorizer.DecisionAllow, "", nil } @@ -220,23 +220,23 @@ func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, att // subresource of pods running on a node func (r *NodeAuthorizer) authorizeCreateToken(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { if attrs.GetVerb() != "create" || len(attrs.GetName()) == 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only create tokens for individual service accounts", nil } if attrs.GetSubresource() != "token" { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only create token subresource of serviceaccount", nil } ok, err := r.hasPathFrom(nodeName, startingType, attrs.GetNamespace(), attrs.GetName()) if err != nil { klog.V(2).Infof("NODE DENY: %v", err) - return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node %q and this object", nodeName), nil + return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node '%s' and this object", nodeName), nil } if !ok { - klog.V(2).Infof("NODE DENY: %q %#v", nodeName, attrs) - return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node %q and this object", nodeName), nil + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, fmt.Sprintf("no relationship found between node '%s' and this object", nodeName), nil } return authorizer.DecisionAllow, "", nil } @@ -249,13 +249,13 @@ func (r *NodeAuthorizer) authorizeLease(nodeName string, attrs authorizer.Attrib case "get", "create", "update", "patch", "delete": //ok default: - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a node lease", nil } // the request must be against the system namespace reserved for node leases if attrs.GetNamespace() != api.NamespaceNodeLease { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, fmt.Sprintf("can only access leases in the %q system namespace", api.NamespaceNodeLease), nil } @@ -263,7 +263,7 @@ func (r *NodeAuthorizer) authorizeLease(nodeName string, attrs authorizer.Attrib // note we skip this check for create, since the authorizer doesn't know the name on create // the noderestriction admission plugin is capable of performing this check at create time if verb != "create" && attrs.GetName() != nodeName { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only access node lease with the same name as the requesting node", nil } @@ -278,12 +278,12 @@ func (r *NodeAuthorizer) authorizeCSINode(nodeName string, attrs authorizer.Attr case "get", "create", "update", "patch", "delete": //ok default: - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get, create, update, patch, or delete a CSINode", nil } if len(attrs.GetSubresource()) > 0 { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "cannot authorize CSINode subresources", nil } @@ -291,7 +291,7 @@ func (r *NodeAuthorizer) authorizeCSINode(nodeName string, attrs authorizer.Attr // note we skip this check for create, since the authorizer doesn't know the name on create // the noderestriction admission plugin is capable of performing this check at create time if verb != "create" && attrs.GetName() != nodeName { - klog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + klog.V(2).Infof("NODE DENY: '%s' %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only access CSINode with the same name as the requesting node", nil } @@ -305,12 +305,12 @@ func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, s nodeVertex, exists := r.graph.getVertex_rlocked(nodeVertexType, "", nodeName) if !exists { - return false, fmt.Errorf("unknown node %q cannot get %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName) + return false, fmt.Errorf("unknown node '%s' cannot get %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName) } startingVertex, exists := r.graph.getVertex_rlocked(startingType, startingNamespace, startingName) if !exists { - return false, fmt.Errorf("node %q cannot get unknown %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName) + return false, fmt.Errorf("node '%s' cannot get unknown %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName) } // Fast check to see if we know of a destination edge @@ -342,7 +342,7 @@ func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, s return found }) if !found { - return false, fmt.Errorf("node %q cannot get %s %s/%s, no relationship to this object was found in the node authorizer graph", nodeName, vertexTypes[startingType], startingNamespace, startingName) + return false, fmt.Errorf("node '%s' cannot get %s %s/%s, no relationship to this object was found in the node authorizer graph", nodeName, vertexTypes[startingType], startingNamespace, startingName) } return true, nil }