.github/workflows/codeql-analysis.yml

@@ -3,9 +3,6 @@ name: "CodeQL"
 on:
   push:
     branches: [ master ]
-  pull_request:
-    # The branches below must be a subset of the branches above
-    branches: [ master ]
   schedule:
     - cron: '24 20 * * 3'
 
@@ -19,6 +16,7 @@ jobs:
   analyze:
     name: Analyze
     runs-on: ubuntu-latest
+    timeout-minutes: 30
 
     strategy:
       fail-fast: false

.github/workflows/testing.yml

@@ -18,6 +18,7 @@ jobs:
   # Check generated protos match their source repos (optional for PRs).
   vet-proto:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     steps:
       # Setup the environment.
       - name: Setup Go
@@ -37,6 +38,7 @@ jobs:
     env:
       VET_SKIP_PROTO: 1
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     strategy:
       matrix:
         include:
@@ -112,3 +114,4 @@ jobs:
           examples/examples_test.sh
           security/advancedtls/examples/examples_test.sh
           interop/interop_test.sh
+          xds/internal/test/e2e/run.sh

admin/admin.go

@@ -20,6 +20,7 @@
 // administration services to a gRPC server. The services registered are:
 //
 // - Channelz: https://github.com/grpc/proposal/blob/master/A14-channelz.md
+//
 // - CSDS: https://github.com/grpc/proposal/blob/master/A40-csds-support.md
 //
 // Experimental

attributes/attributes.go

@@ -25,55 +25,75 @@
 // later release.
 package attributes
 
-import "fmt"
-
 // Attributes is an immutable struct for storing and retrieving generic
 // key/value pairs.  Keys must be hashable, and users should define their own
-// types for keys.
+// types for keys.  Values should not be modified after they are added to an
+// Attributes or if they were received from one.  If values implement 'Equal(o
+// interface{}) bool', it will be called by (*Attributes).Equal to determine
+// whether two values with the same key should be considered equal.
 type Attributes struct {
 	m map[interface{}]interface{}
 }
 
-// New returns a new Attributes containing all key/value pairs in kvs.  If the
-// same key appears multiple times, the last value overwrites all previous
-// values for that key.  Panics if len(kvs) is not even.
-func New(kvs ...interface{}) *Attributes {
-	if len(kvs)%2 != 0 {
-		panic(fmt.Sprintf("attributes.New called with unexpected input: len(kvs) = %v", len(kvs)))
-	}
-	a := &Attributes{m: make(map[interface{}]interface{}, len(kvs)/2)}
-	for i := 0; i < len(kvs)/2; i++ {
-		a.m[kvs[i*2]] = kvs[i*2+1]
-	}
-	return a
+// New returns a new Attributes containing the key/value pair.
+func New(key, value interface{}) *Attributes {
+	return &Attributes{m: map[interface{}]interface{}{key: value}}
 }
 
-// WithValues returns a new Attributes containing all key/value pairs in a and
-// kvs.  Panics if len(kvs) is not even.  If the same key appears multiple
-// times, the last value overwrites all previous values for that key.  To
-// remove an existing key, use a nil value.
-func (a *Attributes) WithValues(kvs ...interface{}) *Attributes {
+// WithValue returns a new Attributes containing the previous keys and values
+// and the new key/value pair.  If the same key appears multiple times, the
+// last value overwrites all previous values for that key.  To remove an
+// existing key, use a nil value.  value should not be modified later.
+func (a *Attributes) WithValue(key, value interface{}) *Attributes {
 	if a == nil {
-		return New(kvs...)
+		return New(key, value)
 	}
-	if len(kvs)%2 != 0 {
-		panic(fmt.Sprintf("attributes.New called with unexpected input: len(kvs) = %v", len(kvs)))
-	}
-	n := &Attributes{m: make(map[interface{}]interface{}, len(a.m)+len(kvs)/2)}
+	n := &Attributes{m: make(map[interface{}]interface{}, len(a.m)+1)}
 	for k, v := range a.m {
 		n.m[k] = v
 	}
-	for i := 0; i < len(kvs)/2; i++ {
-		n.m[kvs[i*2]] = kvs[i*2+1]
-	}
+	n.m[key] = value
 	return n
 }
 
 // Value returns the value associated with these attributes for key, or nil if
-// no value is associated with key.
+// no value is associated with key.  The returned value should not be modified.
 func (a *Attributes) Value(key interface{}) interface{} {
 	if a == nil {
 		return nil
 	}
 	return a.m[key]
 }
+
+// Equal returns whether a and o are equivalent.  If 'Equal(o interface{})
+// bool' is implemented for a value in the attributes, it is called to
+// determine if the value matches the one stored in the other attributes.  If
+// Equal is not implemented, standard equality is used to determine if the two
+// values are equal.
+func (a *Attributes) Equal(o *Attributes) bool {
+	if a == nil && o == nil {
+		return true
+	}
+	if a == nil || o == nil {
+		return false
+	}
+	if len(a.m) != len(o.m) {
+		return false
+	}
+	for k, v := range a.m {
+		ov, ok := o.m[k]
+		if !ok {
+			// o missing element of a
+			return false
+		}
+		if eq, ok := v.(interface{ Equal(o interface{}) bool }); ok {
+			if !eq.Equal(ov) {
+				return false
+			}
+		} else if v != ov {
+			// Fallback to a standard equality check if Value is unimplemented.
+			return false
+		}
+	}
+	return true
+}

attributes/attributes_test.go

@@ -20,41 +20,71 @@ package attributes_test
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 
 	"google.golang.org/grpc/attributes"
 )
 
+type stringVal struct {
+	s string
+}
+
+func (s stringVal) Equal(o interface{}) bool {
+	os, ok := o.(stringVal)
+	return ok && s.s == os.s
+}
+
 func ExampleAttributes() {
 	type keyOne struct{}
 	type keyTwo struct{}
-	a := attributes.New(keyOne{}, 1, keyTwo{}, "two")
+	a := attributes.New(keyOne{}, 1).WithValue(keyTwo{}, stringVal{s: "two"})
 	fmt.Println("Key one:", a.Value(keyOne{}))
 	fmt.Println("Key two:", a.Value(keyTwo{}))
 	// Output:
 	// Key one: 1
-	// Key two: two
+	// Key two: {two}
 }
 
-func ExampleAttributes_WithValues() {
+func ExampleAttributes_WithValue() {
 	type keyOne struct{}
 	type keyTwo struct{}
 	a := attributes.New(keyOne{}, 1)
-	a = a.WithValues(keyTwo{}, "two")
+	a = a.WithValue(keyTwo{}, stringVal{s: "two"})
 	fmt.Println("Key one:", a.Value(keyOne{}))
 	fmt.Println("Key two:", a.Value(keyTwo{}))
 	// Output:
 	// Key one: 1
-	// Key two: two
+	// Key two: {two}
 }
 
-// Test that two attributes with the same content are `reflect.DeepEqual`.
-func TestDeepEqual(t *testing.T) {
+// Test that two attributes with the same content are Equal.
+func TestEqual(t *testing.T) {
 	type keyOne struct{}
-	a1 := attributes.New(keyOne{}, 1)
-	a2 := attributes.New(keyOne{}, 1)
-	if !reflect.DeepEqual(a1, a2) {
-		t.Fatalf("reflect.DeepEqual(%+v, %+v), want true, got false", a1, a2)
+	type keyTwo struct{}
+	a1 := attributes.New(keyOne{}, 1).WithValue(keyTwo{}, stringVal{s: "two"})
+	a2 := attributes.New(keyOne{}, 1).WithValue(keyTwo{}, stringVal{s: "two"})
+	if !a1.Equal(a2) {
+		t.Fatalf("%+v.Equals(%+v) = false; want true", a1, a2)
+	}
+	if !a2.Equal(a1) {
+		t.Fatalf("%+v.Equals(%+v) = false; want true", a2, a1)
+	}
+}
+
+// Test that two attributes with different content are not Equal.
+func TestNotEqual(t *testing.T) {
+	type keyOne struct{}
+	type keyTwo struct{}
+	a1 := attributes.New(keyOne{}, 1).WithValue(keyTwo{}, stringVal{s: "two"})
+	a2 := attributes.New(keyOne{}, 2).WithValue(keyTwo{}, stringVal{s: "two"})
+	a3 := attributes.New(keyOne{}, 1).WithValue(keyTwo{}, stringVal{s: "one"})
+	if a1.Equal(a2) {
+		t.Fatalf("%+v.Equals(%+v) = true; want false", a1, a2)
+	}
+	if a2.Equal(a1) {
+		t.Fatalf("%+v.Equals(%+v) = true; want false", a2, a1)
+	}
+	if a3.Equal(a1) {
+		t.Fatalf("%+v.Equals(%+v) = true; want false", a3, a1)
 	}
 }

authz/rbac_translator.go

@@ -155,14 +155,20 @@ func parsePrincipalNames(principalNames []string) []*v3rbacpb.Principal {
 }
 
 func parsePeer(source peer) (*v3rbacpb.Principal, error) {
-	if len(source.Principals) > 0 {
-		return principalOr(parsePrincipalNames(source.Principals)), nil
+	if source.Principals == nil {
+		return &v3rbacpb.Principal{
+			Identifier: &v3rbacpb.Principal_Any{
+				Any: true,
+			},
+		}, nil
 	}
-	return &v3rbacpb.Principal{
-		Identifier: &v3rbacpb.Principal_Any{
-			Any: true,
-		},
-	}, nil
+	if len(source.Principals) == 0 {
+		return &v3rbacpb.Principal{
+			Identifier: &v3rbacpb.Principal_Authenticated_{
+				Authenticated: &v3rbacpb.Principal_Authenticated{},
+			}}, nil
+	}
+	return principalOr(parsePrincipalNames(source.Principals)), nil
 }
 
 func parsePaths(paths []string) []*v3rbacpb.Permission {

authz/rbac_translator_test.go

@@ -205,6 +205,32 @@ func TestTranslatePolicy(t *testing.T) {
 				},
 			},
 		},
+		"empty principal field": {
+			authzPolicy: `{
+				"name": "authz",
+				"allow_rules": [{
+					"name": "allow_authenticated",
+					"source": {"principals":[]}
+				}]
+			}`,
+			wantPolicies: []*v3rbacpb.RBAC{
+				{
+					Action: v3rbacpb.RBAC_ALLOW,
+					Policies: map[string]*v3rbacpb.Policy{
+						"authz_allow_authenticated": {
+							Principals: []*v3rbacpb.Principal{
+								{Identifier: &v3rbacpb.Principal_Authenticated_{
+									Authenticated: &v3rbacpb.Principal_Authenticated{},
+								}},
+							},
+							Permissions: []*v3rbacpb.Permission{
+								{Rule: &v3rbacpb.Permission_Any{Any: true}},
+							},
+						},
+					},
+				},
+			},
+		},
 		"unknown field": {
 			authzPolicy: `{"random": 123}`,
 			wantErr:     "failed to unmarshal policy",