-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Switch to use upstream audit handler #11192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a371261
d069dca
fa8395e
59e59e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"crypto/tls" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"net/http" | ||
"os" | ||
"regexp" | ||
|
@@ -16,6 +17,7 @@ import ( | |
"github.com/go-openapi/spec" | ||
"github.com/golang/glog" | ||
"github.com/prometheus/client_golang/prometheus" | ||
"gopkg.in/natefinch/lumberjack.v2" | ||
|
||
kapi "k8s.io/kubernetes/pkg/api" | ||
"k8s.io/kubernetes/pkg/api/meta" | ||
|
@@ -25,6 +27,7 @@ import ( | |
"k8s.io/kubernetes/pkg/apimachinery/registered" | ||
v1beta1extensions "k8s.io/kubernetes/pkg/apis/extensions/v1beta1" | ||
"k8s.io/kubernetes/pkg/apiserver" | ||
"k8s.io/kubernetes/pkg/apiserver/audit" | ||
"k8s.io/kubernetes/pkg/client/restclient" | ||
kclient "k8s.io/kubernetes/pkg/client/unversioned" | ||
clientadapter "k8s.io/kubernetes/pkg/client/unversioned/adapters/internalclientset" | ||
|
@@ -180,7 +183,22 @@ func (c *MasterConfig) Run(protected []APIInstaller, unprotected []APIInstaller) | |
handler = c.authorizationFilter(handler) | ||
handler = c.impersonationFilter(handler) | ||
// audit handler must comes before the impersonationFilter to read the original user | ||
handler = c.auditHandler(handler) | ||
if c.Options.AuditConfig.Enabled { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kind of ugly to have this code block here. Hopefully, we are able to get rid of this in a future rebase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll be able to move it where necessary, but the option will stay forever with us ;) |
||
attributeGetter := apiserver.NewRequestAttributeGetter(c.getRequestContextMapper(), c.getRequestInfoResolver()) | ||
var writer io.Writer | ||
if len(c.Options.AuditConfig.AuditFilePath) > 0 { | ||
writer = &lumberjack.Logger{ | ||
Filename: c.Options.AuditConfig.AuditFilePath, | ||
MaxAge: c.Options.AuditConfig.MaximumFileRetentionDays, | ||
MaxBackups: c.Options.AuditConfig.MaximumRetainedFiles, | ||
MaxSize: c.Options.AuditConfig.MaximumFileSizeMegabytes, | ||
} | ||
} else { | ||
// backwards compatible writer to regular log | ||
writer = cmdutil.NewGLogWriterV(0) | ||
} | ||
handler = audit.WithAudit(handler, attributeGetter, writer) | ||
} | ||
handler = authenticationHandlerFilter(handler, c.Authenticator, c.getRequestContextMapper()) | ||
handler = namespacingFilter(handler, c.getRequestContextMapper()) | ||
handler = cacheControlFilter(handler, "no-store") // protected endpoints should not be cached | ||
|
@@ -876,6 +894,23 @@ func (c *MasterConfig) getRequestContextMapper() kapi.RequestContextMapper { | |
return c.RequestContextMapper | ||
} | ||
|
||
// getRequestInfoResolver returns a request resolver. | ||
func (c *MasterConfig) getRequestInfoResolver() *apiserver.RequestInfoResolver { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will bite us in the next rebase. Upstream genericapiserver only knows about one APIPrefix and one APIGroupPrefix. Looks like we have to turn those into lists in order to support origin's setup. /cc @deads2k There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do hope the next rebase (I'm the lucky looser/winner) will straight this out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WIP PR for multiple api group prefixes in genericapiserver: kubernetes/kubernetes#35021 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me know when you start the master.go rebase. We have moved around so much stuff, hopefully to the better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember, we'll be in touch, definitely. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Those are all legacy prefixes which I just made have a list upstream: https://github.com/kubernetes/kubernetes/blob/master/pkg/genericapiserver/config.go#L175 |
||
if c.RequestInfoResolver == nil { | ||
c.RequestInfoResolver = &apiserver.RequestInfoResolver{ | ||
APIPrefixes: sets.NewString(strings.Trim(LegacyOpenShiftAPIPrefix, "/"), | ||
strings.Trim(OpenShiftAPIPrefix, "/"), | ||
strings.Trim(KubernetesAPIPrefix, "/"), | ||
strings.Trim(KubernetesAPIGroupPrefix, "/")), // all possible API prefixes | ||
GrouplessAPIPrefixes: sets.NewString(strings.Trim(LegacyOpenShiftAPIPrefix, "/"), | ||
strings.Trim(OpenShiftAPIPrefix, "/"), | ||
strings.Trim(KubernetesAPIPrefix, "/"), | ||
), // APIPrefixes that won't have groups (legacy) | ||
} | ||
} | ||
return c.RequestInfoResolver | ||
} | ||
|
||
// RouteAllocator returns a route allocation controller. | ||
func (c *MasterConfig) RouteAllocator() *routeallocationcontroller.RouteAllocationController { | ||
osclient, kclient := c.RouteAllocatorClients() | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see validation on these fields and I don't see this path listed in the list of paths for resolution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deads2k what kind of validation you're thinking about? that the path field is non-empty and ints are >= 0? I don't see any value in that.