authz: Rbac engine audit logging

authz/audit/audit_logger.go

@@ -22,8 +22,6 @@ package audit
 import (
 	"encoding/json"
 	"sync"
-
-	"google.golang.org/grpc/internal"
 )
 
 // loggerBuilderRegistry holds a map of audit logger builders and a mutex
@@ -39,10 +37,6 @@ var (
 	}
 )
 
-func init() {
-	internal.UnregisterAuditLoggerBuilderForTesting = unregisterLoggerBuilder
-}
-
 // RegisterLoggerBuilder registers the builder in a global map
 // using b.Name() as the key.
 //
@@ -63,12 +57,6 @@ func GetLoggerBuilder(name string) LoggerBuilder {
 	return registry.builders[name]
 }
 
-func unregisterLoggerBuilder(name string) {
-	registry.mu.Lock()
-	defer registry.mu.Unlock()
-	delete(registry.builders, name)
-}
-
 // Event contains information passed to the audit logger as part of an
 // audit logging event.
 type Event struct {

authz/rbac_translator.go

@@ -39,7 +39,7 @@ import (
 
 // This is used when converting a custom config from raw JSON to a TypedStruct
 // The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/<name>"
-const typedURLPrefix = "grpc.authz.audit_logging/"
+const typeURLPrefix = "grpc.authz.audit_logging/"
 
 type header struct {
 	Key    string
@@ -308,7 +308,7 @@ func (options *auditLoggingOptions) toProtos() (allow *v3rbacpb.RBAC_AuditLoggin
 			return nil, nil, fmt.Errorf("AuditLogger Config field cannot be nil")
 		}
 		typedStruct := &v1xdsudpatypepb.TypedStruct{
-			TypeUrl: typedURLPrefix + config.Name,
+			TypeUrl: typeURLPrefix + config.Name,
 			Value:   config.Config,
 		}
 		customConfig, err := anypb.New(typedStruct)
@@ -355,8 +355,8 @@ func toDenyCondition(condition v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition)
 
 // translatePolicy translates SDK authorization policy in JSON format to two
 // Envoy RBAC polices (deny followed by allow policy) or only one Envoy RBAC
-// allow policy. If the input policy cannot be parsed or is invalid, an error
-// will be returned.
+// allow policy. Also returns the overall policy name. If the input policy
+// cannot be parsed or is invalid, an error will be returned.
 func translatePolicy(policyStr string) ([]*v3rbacpb.RBAC, string, error) {
 	policy := &authorizationPolicy{}
 	d := json.NewDecoder(bytes.NewReader([]byte(policyStr)))

authz/rbac_translator_test.go

@@ -951,7 +951,7 @@ func anyPbHelper(t *testing.T, in map[string]interface{}, name string) *anypb.An
 	t.Helper()
 	pb, err := structpb.NewStruct(in)
 	typedStruct := &v1xdsudpatypepb.TypedStruct{
-		TypeUrl: typedURLPrefix + name,
+		TypeUrl: typeURLPrefix + name,
 		Value:   pb,
 	}
 	if err != nil {

internal/internal.go

@@ -164,12 +164,6 @@ var (
 
 	// ORCAAllowAnyMinReportingInterval is for examples/orca use ONLY.
 	ORCAAllowAnyMinReportingInterval interface{} // func(so *orca.ServiceOptions)
-
-	// UnregisterAuditLoggerBuilderForTesting unregisters the AuditLoggerBuilder
-	// by name for testing purposes. This is needed because there is no way to
-	// unregister for repeated tests. It is set by package authz/audit in
-	// audit_logger.go
-	UnregisterAuditLoggerBuilderForTesting func(name string)
 )
 
 // HealthChecker defines the signature of the client-side LB channel health checking function.

internal/xds/rbac/converter_test.go

@@ -23,7 +23,6 @@ import (
 	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3"
 	"google.golang.org/grpc/authz/audit"
-	"google.golang.org/grpc/internal"
 	"google.golang.org/protobuf/types/known/anypb"
 )
 
@@ -79,7 +78,7 @@ func (s) TestBuildLoggerErrors(t *testing.T) {
 			loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{
 				AuditLogger: &v3corepb.TypedExtensionConfig{
 					Name:        "TestAuditLoggerCustomConfig",
-					TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": "BADVALUE", "xyz": "123"}, "TestAuditLoggerCustomConfig")},
+					TypedConfig: createUDPATypedStruct(t, map[string]interface{}{"abc": "BADVALUE", "xyz": "123"}, "fail to parse custom config_TestAuditLoggerCustomConfig")},
 				IsOptional: false,
 			},
 			expectedError: "AuditLogger custom config could not be parsed",
@@ -89,7 +88,7 @@ func (s) TestBuildLoggerErrors(t *testing.T) {
 			loggerConfig: &v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig{
 				AuditLogger: &v3corepb.TypedExtensionConfig{
 					Name:        "UnregisteredLogger",
-					TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "UnregisteredLogger"),
+					TypedConfig: createUDPATypedStruct(t, map[string]interface{}{}, "No registered logger but optional passes_UnregisteredLogger"),
 				},
 				IsOptional: true,
 			},
@@ -98,9 +97,8 @@ func (s) TestBuildLoggerErrors(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			b := TestAuditLoggerCustomConfigBuilder{}
+			b := TestAuditLoggerCustomConfigBuilder{testName: test.name}
 			audit.RegisterLoggerBuilder(&b)
-			defer internal.UnregisterAuditLoggerBuilderForTesting(b.Name())
 			logger, err := buildLogger(test.loggerConfig)
 			if err != nil && !strings.HasPrefix(err.Error(), test.expectedError) {
 				t.Fatalf("expected error: %v. got error: %v", test.expectedError, err)

internal/xds/rbac/rbac_engine.go

@@ -122,7 +122,7 @@ type engine struct {
 	auditCondition v3rbacpb.RBAC_AuditLoggingOptions_AuditCondition
 }
 
-// newEngine creates an RBAC Engine based on the contents of policy. Returns a
+// newEngine creates an RBAC Engine based on the contents of a policy. Returns a
 // non-nil error if the policy is invalid.
 func newEngine(config *v3rbacpb.RBAC, policyName string) (*engine, error) {
 	a := config.GetAction()
@@ -144,11 +144,11 @@ func newEngine(config *v3rbacpb.RBAC, policyName string) (*engine, error) {
 		return nil, err
 	}
 	return &engine{
+		policyName:     policyName,
 		policies:       policies,
 		action:         a,
 		auditLoggers:   auditLoggers,
 		auditCondition: auditCondition,
-		policyName:     policyName,
 	}, nil
 }
 
@@ -163,6 +163,8 @@ func parseAuditOptions(opts *v3rbacpb.RBAC_AuditLoggingOptions) ([]audit.Logger,
 			return nil, v3rbacpb.RBAC_AuditLoggingOptions_NONE, err
 		}
 		if auditLogger == nil {
+			// This occurs when the audit logger is not registered but also
+			// marked optional.
 			continue
 		}
 		auditLoggers = append(auditLoggers, auditLogger)
@@ -304,6 +306,6 @@ func (e *engine) doAuditLogging(rpcData *rpcData, rule string, authorized bool)
 	}
 }
 
-// This is used when converting a custom config from raw JSON to a TypedStruct
-// The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/<name>"
-const typedURLPrefix = "grpc.authz.audit_logging/"
+// This is used when converting a custom config from raw JSON to a TypedStruct.
+// The TypeURL of the TypeStruct will be "grpc.authz.audit_logging/<name>".
+const typeURLPrefix = "grpc.authz.audit_logging/"