Skip to content

Commit

Permalink
API request logging improvements (#3180)
Browse files Browse the repository at this point in the history
* cli arg improvements

* code improvements per marcin

* store structs not bools
  • Loading branch information
jeefy authored and k8s-ci-robot committed Aug 2, 2018
1 parent 6ed35fc commit 10bd78f
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 14 deletions.
6 changes: 6 additions & 0 deletions src/app/backend/args/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ func (self *holderBuilder) SetSystemBannerSeverity(systemBannerSeverity string)
return self
}

// SetLogLevel 'api-log-level' argument of Dashboard binary.
func (self *holderBuilder) SetAPILogLevel(apiLogLevel string) *holderBuilder {
self.holder.apiLogLevel = apiLogLevel
return self
}

// SetAuthenticationMode 'authentication-mode' argument of Dashboard binary.
func (self *holderBuilder) SetAuthenticationMode(authMode []string) *holderBuilder {
self.holder.authenticationMode = authMode
Expand Down
8 changes: 7 additions & 1 deletion src/app/backend/args/holder.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ type holder struct {
kubeConfigFile string
systemBanner string
systemBannerSeverity string
apiLogLevel string

authenticationMode []string

autoGenerateCertificates bool
enableInsecureLogin bool
disableSettingsAuthorizer bool

disableSkipButton bool
disableSkipButton bool
}

// GetInsecurePort 'insecure-port' argument of Dashboard binary.
Expand Down Expand Up @@ -129,6 +130,11 @@ func (self *holder) GetSystemBannerSeverity() string {
return self.systemBannerSeverity
}

// LogLevel 'api-log-level' argument of Dashboard binary.
func (self *holder) GetAPILogLevel() string {
return self.apiLogLevel
}

// GetAuthenticationMode 'authentication-mode' argument of Dashboard binary.
func (self *holder) GetAuthenticationMode() []string {
return self.authenticationMode
Expand Down
2 changes: 2 additions & 0 deletions src/app/backend/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ var (
argDisableSkip = pflag.Bool("disable-skip", false, "When enabled, the skip button on the login page will not be shown. Default: false.")
argSystemBanner = pflag.String("system-banner", "", "When non-empty displays message to Dashboard users. Accepts simple HTML tags. Default: ''.")
argSystemBannerSeverity = pflag.String("system-banner-severity", "INFO", "Severity of system banner. Should be one of 'INFO|WARNING|ERROR'. Default: 'INFO'.")
argAPILogLevel = pflag.String("api-log-level", "INFO", "Level of API request logging. Should be one of 'INFO|NONE|DEBUG'. Default: 'INFO'.")
argDisableSettingsAuthorizer = pflag.Bool("disable-settings-authorizer", false, "When enabled, Dashboard settings page will not require user to be logged in and authorized to access settings page.")
)

Expand Down Expand Up @@ -217,6 +218,7 @@ func initArgHolder() {
builder.SetKubeConfigFile(*argKubeConfigFile)
builder.SetSystemBanner(*argSystemBanner)
builder.SetSystemBannerSeverity(*argSystemBannerSeverity)
builder.SetAPILogLevel(*argAPILogLevel)
builder.SetAuthenticationMode(*argAuthenticationMode)
builder.SetAutoGenerateCertificates(*argAutoGenerateCertificates)
builder.SetEnableInsecureLogin(*argEnableInsecureLogin)
Expand Down
68 changes: 57 additions & 11 deletions src/app/backend/handler/apihandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package handler

import (
"encoding/json"
"net/http"
"testing"

Expand All @@ -23,6 +24,7 @@ import (
"strings"

restful "github.com/emicklei/go-restful"
"github.com/kubernetes/dashboard/src/app/backend/args"
"github.com/kubernetes/dashboard/src/app/backend/auth"
authApi "github.com/kubernetes/dashboard/src/app/backend/auth/api"
"github.com/kubernetes/dashboard/src/app/backend/auth/jwe"
Expand Down Expand Up @@ -103,25 +105,69 @@ func TestMapUrlToResource(t *testing.T) {
}

func TestFormatRequestLog(t *testing.T) {
req, err := http.NewRequest("PUT", "/api/v1/pod", bytes.NewReader([]byte("{}")))
if err != nil {
t.Error("Cannot mockup request")
}
cases := []struct {
request *restful.Request
expected string
method string
uri string
content map[string]string
expected string
apiLogLevel string
}{
{
&restful.Request{
Request: req,
},
"PUT",
"/api/v1/pod",
map[string]string{},
"Incoming HTTP/1.1 PUT /api/v1/pod request",
"DEFAULT",
},
{
"PUT",
"/api/v1/pod",
map[string]string{},
"",
"NONE",
},
{
"POST",
"/api/v1/login",
map[string]string{"password": "abc123"},
"Incoming HTTP/1.1 POST /api/v1/login request from : { contents hidden }",
"DEFAULT",
},
{
"POST",
"/api/v1/login",
map[string]string{},
"",
"NONE",
},
{
"POST",
"/api/v1/login",
map[string]string{"password": "abc123"},
"Incoming HTTP/1.1 POST /api/v1/login request from : {\n \"password\": \"abc123\"\n}",
"DEBUG",
},
}

for _, c := range cases {
actual := formatRequestLog(c.request)
jsonValue, _ := json.Marshal(c.content)

req, err := http.NewRequest(c.method, c.uri, bytes.NewReader(jsonValue))
req.Header.Set("Content-Type", "application/json")

if err != nil {
t.Error("Cannot mockup request")
}

builder := args.GetHolderBuilder()
builder.SetAPILogLevel(c.apiLogLevel)

var restfulRequest restful.Request
restfulRequest.Request = req

actual := formatRequestLog(&restfulRequest)
if !strings.Contains(actual, c.expected) {
t.Errorf("formatRequestLog(%#v) returns %#v, expected to contain %#v", c.request, actual, c.expected)
t.Errorf("formatRequestLog(%#v) returns %#v, expected to contain %#v", req, actual, c.expected)
}
}
}
32 changes: 30 additions & 2 deletions src/app/backend/handler/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

restful "github.com/emicklei/go-restful"
"github.com/kubernetes/dashboard/src/app/backend/args"
authApi "github.com/kubernetes/dashboard/src/app/backend/auth/api"
clientapi "github.com/kubernetes/dashboard/src/app/backend/client/api"
kdErrors "github.com/kubernetes/dashboard/src/app/backend/errors"
Expand Down Expand Up @@ -54,9 +55,15 @@ func restrictedResourcesFilter(request *restful.Request, response *restful.Respo
// web-service filter function used for request and response logging.
func requestAndResponseLogger(request *restful.Request, response *restful.Response,
chain *restful.FilterChain) {
log.Printf(formatRequestLog(request))
if args.Holder.GetAPILogLevel() != "NONE" {
log.Printf(formatRequestLog(request))
}

chain.ProcessFilter(request, response)
log.Printf(formatResponseLog(response, request))

if args.Holder.GetAPILogLevel() != "NONE" {
log.Printf(formatResponseLog(response, request))
}
}

// formatRequestLog formats request log string.
Expand All @@ -76,6 +83,12 @@ func formatRequestLog(request *restful.Request) string {
}
}

// Is DEBUG level logging enabled? Yes?
// Great now let's filter out any content from sensitive URLs
if args.Holder.GetAPILogLevel() != "DEBUG" && checkSensitiveURL(&uri) {
content = "{ contents hidden }"
}

return fmt.Sprintf(RequestLogString, time.Now().Format(time.RFC3339), request.Request.Proto,
request.Request.Method, uri, request.Request.RemoteAddr, content)
}
Expand All @@ -86,6 +99,21 @@ func formatResponseLog(response *restful.Response, request *restful.Request) str
request.Request.RemoteAddr, response.StatusCode())
}

// checkSensitiveUrl checks if a string matches against a sensitive URL
// true if sensitive. false if not.
func checkSensitiveURL(url *string) bool {
var s struct{}
var sensitiveUrls = make(map[string]struct{})
sensitiveUrls["/api/v1/login"] = s
sensitiveUrls["/api/v1/csrftoken/login"] = s

if _, ok := sensitiveUrls[*url]; ok {
return true
}
return false

}

func metricsFilter(req *restful.Request, resp *restful.Response,
chain *restful.FilterChain) {
resource := mapUrlToResource(req.SelectedRoutePath())
Expand Down

0 comments on commit 10bd78f

Please sign in to comment.