Adds more comments to pkg for context and cleans up some funcs

Signed-off-by: JoshVanL <[email protected]>

Author: JoshVanL

cmd/app/run.go

@@ -110,7 +110,7 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma
 
 			<-waitCh
 
-			if err := p.RunShutdownHooks(); err != nil {
+			if err := p.RunPreShutdownHooks(); err != nil {
 				return err
 			}
 

pkg/proxy/audit/audit.go

@@ -14,34 +14,39 @@ import (
 )
 
 type Audit struct {
-	options      *options.AuditOptions
+	opts         *options.AuditOptions
 	serverConfig *server.CompletedConfig
 }
 
-func New(options *options.AuditOptions, externalAddress string, secureServingInfo *server.SecureServingInfo) (*Audit, error) {
+// New creates a new Audit struct to handle auditing for proxy requests. This
+// is mostly a wrapper for the apiserver auditing handlers to combine them with
+// the proxy.
+func New(opts *options.AuditOptions, externalAddress string, secureServingInfo *server.SecureServingInfo) (*Audit, error) {
 	serverConfig := &server.Config{
 		ExternalAddress: externalAddress,
 		SecureServing:   secureServingInfo,
 
-		// Default to treating watch as a long-running operation
-		// Generic API servers have no inherent long-running subresources
+		// Default to treating watch as a long-running operation.
+		// Generic API servers have no inherent long-running subresources.
+		// This is so watch requests are handled correctly in the audit log.
 		LongRunningFunc: genericfilters.BasicLongRunningRequestCheck(
 			sets.NewString("watch"), sets.NewString()),
 	}
 
 	// We do not support dynamic auditing, so leave nil
-	if err := options.ApplyTo(serverConfig, nil, nil, nil, nil); err != nil {
+	if err := opts.ApplyTo(serverConfig, nil, nil, nil, nil); err != nil {
 		return nil, err
 	}
 
 	completed := serverConfig.Complete(nil)
 
 	return &Audit{
-		options:      options,
+		opts:         opts,
 		serverConfig: &completed,
 	}, nil
 }
 
+// Run will run the audit backend if configured.
 func (a *Audit) Run(stopCh <-chan struct{}) error {
 	if a.serverConfig.AuditBackend != nil {
 		if err := a.serverConfig.AuditBackend.Run(stopCh); err != nil {
@@ -52,6 +57,7 @@ func (a *Audit) Run(stopCh <-chan struct{}) error {
 	return nil
 }
 
+// Shutdown will shutdown the audit backend if configured.
 func (a *Audit) Shutdown() error {
 	if a.serverConfig.AuditBackend != nil {
 		a.serverConfig.AuditBackend.Shutdown()
@@ -60,11 +66,16 @@ func (a *Audit) Shutdown() error {
 	return nil
 }
 
+// WithRequest will wrap the given handler to inject the request information
+// into the context which is then used by the wrapped audit handler.
 func (a *Audit) WithRequest(handler http.Handler) http.Handler {
 	handler = genericapifilters.WithAudit(handler, a.serverConfig.AuditBackend, a.serverConfig.AuditPolicyChecker, a.serverConfig.LongRunningFunc)
 	return genericapifilters.WithRequestInfo(handler, a.serverConfig.RequestInfoResolver)
 }
 
+// WithUnauthorized will wrap the given handler to inject the request
+// information into the context which is then used by the wrapped audit
+// handler.
 func (a *Audit) WithUnauthorized(handler http.Handler) http.Handler {
 	handler = genericapifilters.WithFailedAuthenticationAudit(handler, a.serverConfig.AuditBackend, a.serverConfig.AuditPolicyChecker)
 	return genericapifilters.WithRequestInfo(handler, a.serverConfig.RequestInfoResolver)

pkg/proxy/proxy.go

@@ -261,6 +261,6 @@ func (p *Proxy) OIDCTokenAuthenticator() authenticator.Token {
 	return p.tokenAuther
 }
 
-func (p *Proxy) RunShutdownHooks() error {
+func (p *Proxy) RunPreShutdownHooks() error {
 	return p.hooks.RunPreShutdownHooks()
 }

pkg/proxy/proxy_test.go

@@ -254,7 +254,7 @@ func TestHasImpersonation(t *testing.T) {
 	}
 }
 
-func newTestProxy(t *testing.T) (*fakeProxy, error) {
+func newTestProxy(t *testing.T) *fakeProxy {
 	ctrl := gomock.NewController(t)
 	fakeToken := mocks.NewMockToken(ctrl)
 	fakeRT := &fakeRT{t: t}
@@ -274,13 +274,13 @@ func newTestProxy(t *testing.T) (*fakeProxy, error) {
 
 	auditor, err := audit.New(new(options.AuditOptions), "0.0.0.0:1234", new(server.SecureServingInfo))
 	if err != nil {
-		return nil, err
+		t.Fatalf("failed to create auditor: %s", err)
 	}
 	p.auditor = auditor
 
 	p.handleError = p.newErrorHandler()
 
-	return p, nil
+	return p
 }
 
 func TestHandlers(t *testing.T) {
@@ -537,11 +537,7 @@ func TestHandlers(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			p, err := newTestProxy(t)
-			if err != nil {
-				t.Errorf("unexpected error: %s", err)
-				t.FailNow()
-			}
+			p := newTestProxy(t)
 
 			w := httptest.NewRecorder()
 
@@ -649,11 +645,7 @@ func TestHeadersConfig(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			p, err := newTestProxy(t)
-			if err != nil {
-				t.Errorf("unexpected error: %s", err)
-				t.FailNow()
-			}
+			p := newTestProxy(t)
 
 			p.config = test.config
 			w := httptest.NewRecorder()