policy: Track and return multiple AuthTypes for GetAuthTypes()

Track and return multiple AuthTypes for GetAuthTypes(), add multiple auth
types to the distillery unit test.

While we currently plan to only support Spire AuthType, the policy engine
is wired for supporting multiple auth types. Honor this in the new
GetAuthTypes() internal API.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

pkg/policy/distillery.go

@@ -153,32 +153,43 @@ func (cache *PolicyCache) UpdatePolicy(identity *identityPkg.Identity) error {
 	return err
 }
 
-// GetAuthType returns the AuthType required by the policy between the localID and remoteID, or
-// AuthTypeDisabled if none.
-//
-// Note that the first found AuthType is returned. This is correct as the policy should only
-// support a single auth type between a pair of identities and the datapath has the same
-// restriction.
-func (cache *PolicyCache) GetAuthType(localID, remoteID identityPkg.NumericIdentity) AuthType {
+// GetAuthTypes returns the AuthTypes required by the policy between the localID and remoteID, if
+// any, otherwise returns nil.
+func (cache *PolicyCache) GetAuthTypes(localID, remoteID identityPkg.NumericIdentity) AuthTypes {
 	cache.Lock()
 	cip, ok := cache.policies[localID]
 	cache.Unlock()
 	if !ok {
-		return AuthTypeDisabled // No policy for localID (no endpoint with localID)
+		return nil // No policy for localID (no endpoint with localID)
 	}
 
 	// SelectorPolicy is const after it has been created, so no locking needed to access it
 	selPolicy := cip.getPolicy()
 	if selPolicy.L4Policy == nil {
-		return AuthTypeDisabled // Local identity added, but policy not computed yet
+		return nil // Local identity added, but policy not computed yet
 	}
 
-	for cs, authType := range selPolicy.L4Policy.AuthMap {
-		if cs.Selects(remoteID) {
-			return authType
+	var resTypes AuthTypes
+	for cs, authTypes := range selPolicy.L4Policy.AuthMap {
+		missing := false
+		for authType := range authTypes {
+			if _, exists := resTypes[authType]; !exists {
+				missing = true
+				break
+			}
+		}
+		// Only check if 'cs' selects 'remoteID' if one of the authTypes is still missing
+		// from the result
+		if missing && cs.Selects(remoteID) {
+			if resTypes == nil {
+				resTypes = make(AuthTypes, 1)
+			}
+			for authType := range authTypes {
+				resTypes[authType] = struct{}{}
+			}
 		}
 	}
-	return AuthTypeDisabled
+	return resTypes
 }
 
 // cachedSelectorPolicy is a wrapper around a selectorPolicy (stored in the

pkg/policy/distillery_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 
 	. "github.com/cilium/checkmate"
+	"golang.org/x/exp/maps"
 
 	"github.com/cilium/cilium/pkg/checker"
 	"github.com/cilium/cilium/pkg/identity"
@@ -228,6 +229,14 @@ var (
 			WithIngressRules([]api.IngressRule{{
 			ToPorts: allowPort80,
 		}})
+	rule__L4__AllowAuth = api.NewRule().
+				WithLabels(lbls__L4__Allow).
+				WithIngressRules([]api.IngressRule{{
+			ToPorts: allowPort80,
+			Authentication: &api.Authentication{
+				Mode: api.AuthenticationModeRequired,
+			},
+		}})
 	rule__npL4__Allow = api.NewRule().
 				WithLabels(lbls__L4__Allow).
 				WithIngressRules([]api.IngressRule{{
@@ -324,8 +333,8 @@ var (
 	mapEntryL7None_ = func(lbls ...labels.LabelArray) MapStateEntry {
 		return NewMapStateEntry(nil, labels.LabelArrayList(lbls).Sort(), false, false, AuthTypeDisabled).WithOwners()
 	}
-	mapEntryL7Auth_ = func(lbls ...labels.LabelArray) MapStateEntry {
-		return NewMapStateEntry(nil, labels.LabelArrayList(lbls).Sort(), false, false, AuthTypeAlwaysFail).WithOwners()
+	mapEntryL7Auth_ = func(at AuthType, lbls ...labels.LabelArray) MapStateEntry {
+		return NewMapStateEntry(nil, labels.LabelArrayList(lbls).Sort(), false, false, at).WithOwners()
 	}
 	mapEntryL7Deny_ = func(lbls ...labels.LabelArray) MapStateEntry {
 		return NewMapStateEntry(nil, labels.LabelArrayList(lbls).Sort(), false, true, AuthTypeDisabled).WithOwners()
@@ -406,7 +415,7 @@ func Test_MergeL3(t *testing.T) {
 	}
 	selectorCache := testNewSelectorCache(identityCache)
 
-	type authResult map[identity.NumericIdentity]AuthType
+	type authResult map[identity.NumericIdentity]AuthTypes
 	tests := []struct {
 		test   int
 		rules  api.Rules
@@ -415,20 +424,24 @@ func Test_MergeL3(t *testing.T) {
 	}{
 		{
 			0,
-			api.Rules{rule__L3AllowFoo, rule__L3AllowBarAuth},
-			MapState{mapKeyAllowFoo__: mapEntryL7None_(lbls__L3AllowFoo), mapKeyAllowBar__: mapEntryL7Auth_(lbls__L3AllowBar)},
+			api.Rules{rule__L3AllowFoo, rule__L3AllowBarAuth, rule__L4__AllowAuth},
+			MapState{
+				mapKeyAllow___L4: mapEntryL7Auth_(AuthTypeSpire, lbls__L4__Allow),
+				mapKeyAllowFoo__: mapEntryL7None_(lbls__L3AllowFoo),
+				mapKeyAllowBar__: mapEntryL7Auth_(AuthTypeAlwaysFail, lbls__L3AllowBar),
+			},
 			authResult{
-				identity.NumericIdentity(identityBar): AuthTypeAlwaysFail,
-				identity.NumericIdentity(identityFoo): AuthTypeDisabled,
+				identity.NumericIdentity(identityBar): AuthTypes{AuthTypeAlwaysFail: struct{}{}, AuthTypeSpire: struct{}{}},
+				identity.NumericIdentity(identityFoo): AuthTypes{AuthTypeSpire: struct{}{}},
 			},
 		},
 		{
 			1,
 			api.Rules{rule__L3AllowFoo, ruleL3L4__Allow},
 			MapState{mapKeyAllowFoo__: mapEntryL7None_(lbls__L3AllowFoo), mapKeyAllowFooL4: mapEntryL7None_(lblsL3L4__Allow)},
 			authResult{
-				identity.NumericIdentity(identityBar): AuthTypeDisabled,
-				identity.NumericIdentity(identityFoo): AuthTypeDisabled,
+				identity.NumericIdentity(identityBar): AuthTypes{},
+				identity.NumericIdentity(identityFoo): AuthTypes{},
 			},
 		},
 	}
@@ -454,10 +467,10 @@ func Test_MergeL3(t *testing.T) {
 				t.Logf("Policy Trace: \n%s\n", logBuffer.String())
 				t.Errorf("Policy obtained didn't match expected for endpoint %s:\n%s", labelsFoo, err)
 			}
-			for remoteID, expectedAuthType := range tt.auths {
-				authType := repo.GetAuthType(identity.ID, remoteID)
-				if authType != expectedAuthType {
-					t.Errorf("Incorrect AuthTypes result for remote ID %d: obtained %v, expected %v", remoteID, authType, expectedAuthType)
+			for remoteID, expectedAuthTypes := range tt.auths {
+				authTypes := repo.GetAuthTypes(identity.ID, remoteID)
+				if !maps.Equal(authTypes, expectedAuthTypes) {
+					t.Errorf("Incorrect AuthTypes result for remote ID %d: obtained %v, expected %v", remoteID, authTypes, expectedAuthTypes)
 				}
 			}
 		})

pkg/policy/l4.go

@@ -219,8 +219,11 @@ func (a *PerSelectorPolicy) Equal(b *PerSelectorPolicy) bool {
 // AuthType enumerates the supported authentication types in api.
 type AuthType uint8
 
-// Authmap maps remote selectors to their needed AuthType, if any
-type AuthMap map[CachedSelector]AuthType
+// AuthTypes is a set of AuthTypes, usually nil if empty
+type AuthTypes map[AuthType]struct{}
+
+// Authmap maps remote selectors to their needed AuthTypes, if any
+type AuthMap map[CachedSelector]AuthTypes
 
 const (
 	// AuthTypeDisabled means no authentication required
@@ -859,7 +862,12 @@ func (l4 *L4Filter) attach(ctx PolicyContext, l4Policy *L4Policy) {
 					if l4Policy.AuthMap == nil {
 						l4Policy.AuthMap = make(AuthMap, 1)
 					}
-					l4Policy.AuthMap[cs] = authType
+					authTypes := l4Policy.AuthMap[cs]
+					if authTypes == nil {
+						authTypes = make(AuthTypes, 1)
+					}
+					authTypes[authType] = struct{}{}
+					l4Policy.AuthMap[cs] = authTypes
 				}
 			}
 		}

pkg/policy/repository.go

@@ -144,9 +144,9 @@ func (p *Repository) GetSelectorCache() *SelectorCache {
 	return p.selectorCache
 }
 
-// GetAuthType returns the AuthTypes required by the policy between the localID and remoteID
-func (p *Repository) GetAuthType(localID, remoteID identity.NumericIdentity) AuthType {
-	return p.policyCache.GetAuthType(localID, remoteID)
+// GetAuthTypes returns the AuthTypes required by the policy between the localID and remoteID
+func (p *Repository) GetAuthTypes(localID, remoteID identity.NumericIdentity) AuthTypes {
+	return p.policyCache.GetAuthTypes(localID, remoteID)
 }
 
 func (p *Repository) SetEnvoyRulesFunc(f func(certificatemanager.SecretManager, *api.L7Rules, string) (*cilium.HttpNetworkPolicyRules, bool)) {