From 7dbcbd39e2c74fd68dbdbb63b31b34228f7fb4b3 Mon Sep 17 00:00:00 2001 From: Marian Lobur Date: Thu, 5 Jul 2018 13:57:17 +0200 Subject: [PATCH] Remove deprecated legacy audit logging code. Kubernetes-commit: 3f730d4c255e7c8ee67a020eed0b8f0a8f634750 --- pkg/endpoints/filters/legacy_audit.go | 161 --------------------- pkg/endpoints/filters/legacy_audit_test.go | 104 ------------- pkg/features/kube_features.go | 3 +- pkg/server/config.go | 13 +- pkg/server/options/audit.go | 104 +++++-------- 5 files changed, 39 insertions(+), 346 deletions(-) delete mode 100644 pkg/endpoints/filters/legacy_audit.go delete mode 100644 pkg/endpoints/filters/legacy_audit_test.go diff --git a/pkg/endpoints/filters/legacy_audit.go b/pkg/endpoints/filters/legacy_audit.go deleted file mode 100644 index bdf13c58e..000000000 --- a/pkg/endpoints/filters/legacy_audit.go +++ /dev/null @@ -1,161 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -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 filters - -import ( - "bufio" - "fmt" - "io" - "net" - "net/http" - "strings" - "time" - - "github.com/golang/glog" - "github.com/pborman/uuid" - - authenticationapi "k8s.io/api/authentication/v1" - utilnet "k8s.io/apimachinery/pkg/util/net" - "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" -) - -var _ http.ResponseWriter = &legacyAuditResponseWriter{} - -type legacyAuditResponseWriter struct { - http.ResponseWriter - out io.Writer - id string -} - -func (a *legacyAuditResponseWriter) printResponse(code int) { - line := fmt.Sprintf("%s AUDIT: id=%q response=\"%d\"\n", time.Now().Format(time.RFC3339Nano), a.id, code) - if _, err := fmt.Fprint(a.out, line); err != nil { - glog.Errorf("Unable to write audit log: %s, the error is: %v", line, err) - } -} - -func (a *legacyAuditResponseWriter) WriteHeader(code int) { - a.printResponse(code) - a.ResponseWriter.WriteHeader(code) -} - -// fancyLegacyResponseWriterDelegator implements http.CloseNotifier, http.Flusher and -// http.Hijacker which are needed to make certain http operation (e.g. watch, rsh, etc) -// working. -type fancyLegacyResponseWriterDelegator struct { - *legacyAuditResponseWriter -} - -func (f *fancyLegacyResponseWriterDelegator) CloseNotify() <-chan bool { - return f.ResponseWriter.(http.CloseNotifier).CloseNotify() -} - -func (f *fancyLegacyResponseWriterDelegator) Flush() { - f.ResponseWriter.(http.Flusher).Flush() -} - -func (f *fancyLegacyResponseWriterDelegator) Hijack() (net.Conn, *bufio.ReadWriter, error) { - // fake a response status before protocol switch happens - f.printResponse(http.StatusSwitchingProtocols) - return f.ResponseWriter.(http.Hijacker).Hijack() -} - -var _ http.CloseNotifier = &fancyLegacyResponseWriterDelegator{} -var _ http.Flusher = &fancyLegacyResponseWriterDelegator{} -var _ http.Hijacker = &fancyLegacyResponseWriterDelegator{} - -// WithLegacyAudit decorates a http.Handler with audit logging information for all the -// requests coming to the server. If out is nil, no decoration takes place. -// Each audit log contains two entries: -// 1. the request line containing: -// - unique id allowing to match the response line (see 2) -// - source ip of the request -// - HTTP method being invoked -// - original user invoking the operation -// - original user's groups info -// - impersonated user for the operation -// - impersonated groups info -// - namespace of the request or -// - uri is the full URI as requested -// 2. the response line containing: -// - the unique id from 1 -// - response code -func WithLegacyAudit(handler http.Handler, out io.Writer) http.Handler { - if out == nil { - return handler - } - return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - ctx := req.Context() - attribs, err := GetAuthorizerAttributes(ctx) - if err != nil { - responsewriters.InternalError(w, req, err) - return - } - - username := "" - groups := "" - if attribs.GetUser() != nil { - username = attribs.GetUser().GetName() - if userGroups := attribs.GetUser().GetGroups(); len(userGroups) > 0 { - groups = auditStringSlice(userGroups) - } - } - asuser := req.Header.Get(authenticationapi.ImpersonateUserHeader) - if len(asuser) == 0 { - asuser = "" - } - asgroups := "" - requestedGroups := req.Header[authenticationapi.ImpersonateGroupHeader] - if len(requestedGroups) > 0 { - asgroups = auditStringSlice(requestedGroups) - } - namespace := attribs.GetNamespace() - if len(namespace) == 0 { - namespace = "" - } - id := uuid.NewRandom().String() - - line := fmt.Sprintf("%s AUDIT: id=%q ip=%q method=%q user=%q groups=%q as=%q asgroups=%q namespace=%q uri=%q\n", - time.Now().Format(time.RFC3339Nano), id, utilnet.GetClientIP(req), req.Method, username, groups, asuser, asgroups, namespace, req.URL) - if _, err := fmt.Fprint(out, line); err != nil { - glog.Errorf("Unable to write audit log: %s, the error is: %v", line, err) - } - respWriter := legacyDecorateResponseWriter(w, out, id) - handler.ServeHTTP(respWriter, req) - }) -} - -func auditStringSlice(inList []string) string { - quotedElements := make([]string, len(inList)) - for i, in := range inList { - quotedElements[i] = fmt.Sprintf("%q", in) - } - return strings.Join(quotedElements, ",") -} - -func legacyDecorateResponseWriter(responseWriter http.ResponseWriter, out io.Writer, id string) http.ResponseWriter { - delegate := &legacyAuditResponseWriter{ResponseWriter: responseWriter, out: out, id: id} - // check if the ResponseWriter we're wrapping is the fancy one we need - // or if the basic is sufficient - _, cn := responseWriter.(http.CloseNotifier) - _, fl := responseWriter.(http.Flusher) - _, hj := responseWriter.(http.Hijacker) - if cn && fl && hj { - return &fancyLegacyResponseWriterDelegator{delegate} - } - return delegate -} diff --git a/pkg/endpoints/filters/legacy_audit_test.go b/pkg/endpoints/filters/legacy_audit_test.go deleted file mode 100644 index 9e1e1ee1e..000000000 --- a/pkg/endpoints/filters/legacy_audit_test.go +++ /dev/null @@ -1,104 +0,0 @@ -/* -Copyright 2016 The Kubernetes Authors. - -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 filters - -import ( - "bytes" - "io/ioutil" - "net/http" - "net/http/httptest" - "reflect" - "regexp" - "strings" - "testing" - - "k8s.io/apiserver/pkg/authentication/user" -) - -func TestLegacyConstructResponseWriter(t *testing.T) { - actual := legacyDecorateResponseWriter(&simpleResponseWriter{}, ioutil.Discard, "") - switch v := actual.(type) { - case *legacyAuditResponseWriter: - default: - t.Errorf("Expected auditResponseWriter, got %v", reflect.TypeOf(v)) - } - - actual = legacyDecorateResponseWriter(&fancyResponseWriter{}, ioutil.Discard, "") - switch v := actual.(type) { - case *fancyLegacyResponseWriterDelegator: - default: - t.Errorf("Expected fancyResponseWriterDelegator, got %v", reflect.TypeOf(v)) - } -} - -func TestLegacyAudit(t *testing.T) { - var buf bytes.Buffer - - handler := WithLegacyAudit(&fakeHTTPHandler{}, &buf) - - req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) - req.RemoteAddr = "127.0.0.1" - req = withTestContext(req, &user.DefaultInfo{Name: "admin"}, nil) - handler.ServeHTTP(httptest.NewRecorder(), req) - line := strings.Split(strings.TrimSpace(buf.String()), "\n") - if len(line) != 2 { - t.Fatalf("Unexpected amount of lines in audit log: %d", len(line)) - } - match, err := regexp.MatchString(`[\d\:\-\.\+TZ]+ AUDIT: id="[\w-]+" ip="127.0.0.1" method="GET" user="admin" groups="" as="" asgroups="" namespace="default" uri="/api/v1/namespaces/default/pods"`, line[0]) - if err != nil { - t.Errorf("Unexpected error matching first line: %v", err) - } - if !match { - t.Errorf("Unexpected first line of audit: %s", line[0]) - } - match, err = regexp.MatchString(`[\d\:\-\.\+TZ]+ AUDIT: id="[\w-]+" response="200"`, line[1]) - if err != nil { - t.Errorf("Unexpected error matching second line: %v", err) - } - if !match { - t.Errorf("Unexpected second line of audit: %s", line[1]) - } -} - -func TestLegacyAuditNoPanicOnNilUser(t *testing.T) { - var buf bytes.Buffer - - handler := WithLegacyAudit(&fakeHTTPHandler{}, &buf) - - req, _ := http.NewRequest("GET", "/api/v1/namespaces/default/pods", nil) - req.RemoteAddr = "127.0.0.1" - req = withTestContext(req, nil, nil) - handler.ServeHTTP(httptest.NewRecorder(), req) - line := strings.Split(strings.TrimSpace(buf.String()), "\n") - if len(line) != 2 { - t.Fatalf("Unexpected amount of lines in audit log: %d", len(line)) - } - match, err := regexp.MatchString(`[\d\:\-\.\+TZ]+ AUDIT: id="[\w-]+" ip="127.0.0.1" method="GET" user="" groups="" as="" asgroups="" namespace="default" uri="/api/v1/namespaces/default/pods"`, line[0]) - if err != nil { - t.Errorf("Unexpected error matching first line: %v", err) - } - if !match { - t.Errorf("Unexpected first line of audit: %s", line[0]) - } - match, err = regexp.MatchString(`[\d\:\-\.\+TZ]+ AUDIT: id="[\w-]+" response="200"`, line[1]) - if err != nil { - t.Errorf("Unexpected error matching second line: %v", err) - } - if !match { - t.Errorf("Unexpected second line of audit: %s", line[1]) - } -} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 61cf50aca..3b6925722 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -37,6 +37,7 @@ const ( // owner: @tallclair // alpha: v1.7 // beta: v1.8 + // GA: v1.12 // // AdvancedAuditing enables a much more general API auditing pipeline, which includes support for // pluggable output backends and an audit policy specifying how different requests should be @@ -82,7 +83,7 @@ func init() { // available throughout Kubernetes binaries. var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureSpec{ StreamingProxyRedirects: {Default: true, PreRelease: utilfeature.Beta}, - AdvancedAuditing: {Default: true, PreRelease: utilfeature.Beta}, + AdvancedAuditing: {Default: true, PreRelease: utilfeature.GA}, APIResponseCompression: {Default: false, PreRelease: utilfeature.Alpha}, Initializers: {Default: false, PreRelease: utilfeature.Alpha}, APIListChunking: {Default: true, PreRelease: utilfeature.Beta}, diff --git a/pkg/server/config.go b/pkg/server/config.go index 7587ecd41..e7b0a8ecc 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -20,7 +20,6 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "io" "net" "net/http" goruntime "runtime" @@ -114,8 +113,6 @@ type Config struct { // Version will enable the /version endpoint if non-nil Version *version.Info - // LegacyAuditWriter is the destination for audit logs. If nil, they will not be written. - LegacyAuditWriter io.Writer // AuditBackend is where audit events are sent to. AuditBackend audit.Backend // AuditPolicyChecker makes the decision of whether and how to audit log a request. @@ -534,15 +531,9 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler { handler := genericapifilters.WithAuthorization(apiHandler, c.Authorization.Authorizer, c.Serializer) handler = genericfilters.WithMaxInFlightLimit(handler, c.MaxRequestsInFlight, c.MaxMutatingRequestsInFlight, c.LongRunningFunc) handler = genericapifilters.WithImpersonation(handler, c.Authorization.Authorizer, c.Serializer) - if utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) { - handler = genericapifilters.WithAudit(handler, c.AuditBackend, c.AuditPolicyChecker, c.LongRunningFunc) - } else { - handler = genericapifilters.WithLegacyAudit(handler, c.LegacyAuditWriter) - } + handler = genericapifilters.WithAudit(handler, c.AuditBackend, c.AuditPolicyChecker, c.LongRunningFunc) failedHandler := genericapifilters.Unauthorized(c.Serializer, c.Authentication.SupportsBasicAuth) - if utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) { - failedHandler = genericapifilters.WithFailedAuthenticationAudit(failedHandler, c.AuditBackend, c.AuditPolicyChecker) - } + failedHandler = genericapifilters.WithFailedAuthenticationAudit(failedHandler, c.AuditBackend, c.AuditPolicyChecker) handler = genericapifilters.WithAuthentication(handler, c.Authentication.Authenticator, failedHandler) handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true") handler = genericfilters.WithTimeoutForNonLongRunningRequests(handler, c.LongRunningFunc, c.RequestTimeout) diff --git a/pkg/server/options/audit.go b/pkg/server/options/audit.go index 29269a228..71476c229 100644 --- a/pkg/server/options/audit.go +++ b/pkg/server/options/audit.go @@ -33,9 +33,7 @@ import ( auditv1beta1 "k8s.io/apiserver/pkg/apis/audit/v1beta1" "k8s.io/apiserver/pkg/audit" "k8s.io/apiserver/pkg/audit/policy" - "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/server" - utilfeature "k8s.io/apiserver/pkg/util/feature" pluginbuffered "k8s.io/apiserver/plugin/pkg/audit/buffered" pluginlog "k8s.io/apiserver/plugin/pkg/audit/log" plugintruncate "k8s.io/apiserver/plugin/pkg/audit/truncate" @@ -59,17 +57,12 @@ func appendBackend(existing, newBackend audit.Backend) audit.Backend { return audit.Union(existing, newBackend) } -func advancedAuditingEnabled() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.AdvancedAuditing) -} - type AuditOptions struct { // Policy configuration file for filtering audit events that are captured. // If unspecified, a default is provided. PolicyFile string // Plugin options - LogOptions AuditLogOptions WebhookOptions AuditWebhookOptions } @@ -110,8 +103,6 @@ type AuditTruncateOptions struct { } // AuditLogOptions determines the output of the structured audit log by default. -// If the AdvancedAuditing feature is set to false, AuditLogOptions holds the legacy -// audit log writer. type AuditLogOptions struct { Path string MaxAge int @@ -179,17 +170,7 @@ func (o *AuditOptions) Validate() []error { return nil } - allErrors := []error{} - - if !advancedAuditingEnabled() { - if len(o.PolicyFile) > 0 { - allErrors = append(allErrors, fmt.Errorf("feature '%s' must be enabled to set option --audit-policy-file", features.AdvancedAuditing)) - } - if len(o.WebhookOptions.ConfigFile) > 0 { - allErrors = append(allErrors, fmt.Errorf("feature '%s' must be enabled to set option --audit-webhook-config-file", features.AdvancedAuditing)) - } - } - + var allErrors []error allErrors = append(allErrors, o.LogOptions.Validate()...) allErrors = append(allErrors, o.WebhookOptions.Validate()...) @@ -263,8 +244,7 @@ func (o *AuditOptions) AddFlags(fs *pflag.FlagSet) { } fs.StringVar(&o.PolicyFile, "audit-policy-file", o.PolicyFile, - "Path to the file that defines the audit policy configuration. Requires the 'AdvancedAuditing' feature gate."+ - " With AdvancedAuditing, a profile is required to enable auditing.") + "Path to the file that defines the audit policy configuration.") o.LogOptions.AddFlags(fs) o.LogOptions.BatchOptions.AddFlags(pluginlog.PluginName, fs) @@ -279,19 +259,14 @@ func (o *AuditOptions) ApplyTo(c *server.Config) error { return nil } - // Apply legacy audit options if advanced audit is not enabled. - if !advancedAuditingEnabled() { - return o.LogOptions.legacyApplyTo(c) - } - - // Apply advanced options if advanced audit is enabled. + // Apply advanced options. // 1. Apply generic options. if err := o.applyTo(c); err != nil { return err } // 2. Apply plugin options. - if err := o.LogOptions.advancedApplyTo(c); err != nil { + if err := o.LogOptions.applyTo(c); err != nil { return err } if err := o.WebhookOptions.applyTo(c); err != nil { @@ -390,8 +365,8 @@ func (o *AuditLogOptions) AddFlags(fs *pflag.FlagSet) { "The maximum size in megabytes of the audit log file before it gets rotated.") fs.StringVar(&o.Format, "audit-log-format", o.Format, "Format of saved audits. \"legacy\" indicates 1-line text format for each event."+ - " \"json\" indicates structured json format. Requires the 'AdvancedAuditing' feature"+ - " gate. Known formats are "+strings.Join(pluginlog.AllowedFormats, ",")+".") + " \"json\" indicates structured json format. Known formats are "+ + strings.Join(pluginlog.AllowedFormats, ",")+".") fs.StringVar(&o.GroupVersionString, "audit-log-version", o.GroupVersionString, "API group and version used for serializing audit events written to log.") } @@ -403,30 +378,29 @@ func (o *AuditLogOptions) Validate() []error { } var allErrors []error - if advancedAuditingEnabled() { - if err := validateBackendBatchOptions(pluginlog.PluginName, o.BatchOptions); err != nil { - allErrors = append(allErrors, err) - } - if err := o.TruncateOptions.Validate(pluginlog.PluginName); err != nil { - allErrors = append(allErrors, err) - } - if err := validateGroupVersionString(o.GroupVersionString); err != nil { - allErrors = append(allErrors, err) - } + if err := validateBackendBatchOptions(pluginlog.PluginName, o.BatchOptions); err != nil { + allErrors = append(allErrors, err) + } + if err := o.TruncateOptions.Validate(pluginlog.PluginName); err != nil { + allErrors = append(allErrors, err) + } - // Check log format - validFormat := false - for _, f := range pluginlog.AllowedFormats { - if f == o.Format { - validFormat = true - break - } - } - if !validFormat { - allErrors = append(allErrors, fmt.Errorf("invalid audit log format %s, allowed formats are %q", o.Format, strings.Join(pluginlog.AllowedFormats, ","))) + if err := validateGroupVersionString(o.GroupVersionString); err != nil { + allErrors = append(allErrors, err) + } + + // Check log format + validFormat := false + for _, f := range pluginlog.AllowedFormats { + if f == o.Format { + validFormat = true + break } } + if !validFormat { + allErrors = append(allErrors, fmt.Errorf("invalid audit log format %s, allowed formats are %q", o.Format, strings.Join(pluginlog.AllowedFormats, ","))) + } // Check validities of MaxAge, MaxBackups and MaxSize of log options, if file log backend is enabled. if o.MaxAge < 0 { @@ -464,7 +438,7 @@ func (o *AuditLogOptions) getWriter() io.Writer { return w } -func (o *AuditLogOptions) advancedApplyTo(c *server.Config) error { +func (o *AuditLogOptions) applyTo(c *server.Config) error { if w := o.getWriter(); w != nil { groupVersion, _ := schema.ParseGroupVersion(o.GroupVersionString) log := pluginlog.NewBackend(w, o.Format, groupVersion) @@ -475,15 +449,9 @@ func (o *AuditLogOptions) advancedApplyTo(c *server.Config) error { return nil } -func (o *AuditLogOptions) legacyApplyTo(c *server.Config) error { - c.LegacyAuditWriter = o.getWriter() - return nil -} - func (o *AuditWebhookOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.ConfigFile, "audit-webhook-config-file", o.ConfigFile, - "Path to a kubeconfig formatted file that defines the audit webhook configuration."+ - " Requires the 'AdvancedAuditing' feature gate.") + "Path to a kubeconfig formatted file that defines the audit webhook configuration.") fs.DurationVar(&o.InitialBackoff, "audit-webhook-initial-backoff", o.InitialBackoff, "The amount of time to wait before retrying the first failed request.") fs.DurationVar(&o.InitialBackoff, "audit-webhook-batch-initial-backoff", @@ -500,17 +468,15 @@ func (o *AuditWebhookOptions) Validate() []error { } var allErrors []error - if advancedAuditingEnabled() { - if err := validateBackendBatchOptions(pluginwebhook.PluginName, o.BatchOptions); err != nil { - allErrors = append(allErrors, err) - } - if err := o.TruncateOptions.Validate(pluginwebhook.PluginName); err != nil { - allErrors = append(allErrors, err) - } + if err := validateBackendBatchOptions(pluginwebhook.PluginName, o.BatchOptions); err != nil { + allErrors = append(allErrors, err) + } + if err := o.TruncateOptions.Validate(pluginwebhook.PluginName); err != nil { + allErrors = append(allErrors, err) + } - if err := validateGroupVersionString(o.GroupVersionString); err != nil { - allErrors = append(allErrors, err) - } + if err := validateGroupVersionString(o.GroupVersionString); err != nil { + allErrors = append(allErrors, err) } return allErrors }