Skip to content

Commit

Permalink
auth: depend from nodeIDHandler directly
Browse files Browse the repository at this point in the history
[ upstream commit eb3c1d8 ]

After removing the node IDS from the IPCache with cilium#27029, the auth
module should directly depend on the NodeIDHandler instead of the
IPCache.

This commit refactors the dependency from the IPCache to the
NodeIDHandler.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
  • Loading branch information
mhofstetter authored and michi-covalent committed Aug 18, 2023
1 parent c49638e commit 1602494
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 30 deletions.
1 change: 1 addition & 0 deletions daemon/cmd/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func (ds *DaemonSuite) SetUpTest(c *C) {
return cs
},
func() datapath.Datapath { return fakeDatapath.NewDatapath() },
func(dp datapath.Datapath) datapath.NodeIDHandler { return dp.NodeIDs() },
func() *option.DaemonConfig { return option.Config },
func() cnicell.CNIConfigManager { return &fakecni.FakeCNIConfigManager{} },
func() signalmap.Map { return fakesignalmap.NewFakeSignalMap([][]byte{}, time.Second) },
Expand Down
20 changes: 10 additions & 10 deletions pkg/auth/authmap_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
)

type authMapGarbageCollector struct {
logger logrus.FieldLogger
authmap authMap
ipCache ipCache
policyRepo policyRepository
logger logrus.FieldLogger
authmap authMap
nodeIDHandler datapathTypes.NodeIDHandler
policyRepo policyRepository

ciliumNodesMutex lock.Mutex
ciliumNodesDiscovered map[uint16]struct{}
Expand All @@ -42,12 +42,12 @@ type policyRepository interface {
GetAuthTypes(localID, remoteID identity.NumericIdentity) policy.AuthTypes
}

func newAuthMapGC(logger logrus.FieldLogger, authmap authMap, ipCache ipCache, policyRepo policyRepository) *authMapGarbageCollector {
func newAuthMapGC(logger logrus.FieldLogger, authmap authMap, nodeIDHandler datapathTypes.NodeIDHandler, policyRepo policyRepository) *authMapGarbageCollector {
return &authMapGarbageCollector{
logger: logger,
authmap: authmap,
ipCache: ipCache,
policyRepo: policyRepo,
logger: logger,
authmap: authmap,
nodeIDHandler: nodeIDHandler,
policyRepo: policyRepo,

ciliumNodesDiscovered: map[uint16]struct{}{
0: {}, // Local node 0 is always available
Expand Down Expand Up @@ -210,7 +210,7 @@ func (r *authMapGarbageCollector) remoteNodeIDs(node nodeTypes.Node) []uint16 {

for _, addr := range node.IPAddresses {
if addr.Type == addressing.NodeInternalIP {
nodeID, exists := r.ipCache.GetNodeID(addr.IP)
nodeID, exists := r.nodeIDHandler.GetNodeID(addr.IP)
if !exists {
// This might be the case at startup, when new nodes aren't yet known to the nodehandler
// and therefore no node id has been assigned to them.
Expand Down
8 changes: 4 additions & 4 deletions pkg/auth/cell.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import (
"github.com/spf13/pflag"

"github.com/cilium/cilium/pkg/auth/spire"
"github.com/cilium/cilium/pkg/datapath/types"
"github.com/cilium/cilium/pkg/hive"
"github.com/cilium/cilium/pkg/hive/cell"
"github.com/cilium/cilium/pkg/hive/job"
"github.com/cilium/cilium/pkg/identity/cache"
"github.com/cilium/cilium/pkg/ipcache"
"github.com/cilium/cilium/pkg/maps/authmap"
nodeManager "github.com/cilium/cilium/pkg/node/manager"
"github.com/cilium/cilium/pkg/policy"
Expand Down Expand Up @@ -76,7 +76,7 @@ type authManagerParams struct {
AuthHandlers []authHandler `group:"authHandlers"`

SignalManager signal.SignalManager
IPCache *ipcache.IPCache
NodeIDHandler types.NodeIDHandler
IdentityChanges stream.Observable[cache.IdentityChange]
NodeManager nodeManager.NodeManager
PolicyRepo *policy.Repository
Expand All @@ -93,12 +93,12 @@ func registerAuthManager(params authManagerParams) error {
mapWriter := newAuthMapWriter(params.Logger, params.AuthMap)
mapCache := newAuthMapCache(params.Logger, mapWriter)

mgr, err := newAuthManager(params.Logger, params.AuthHandlers, mapCache, params.IPCache)
mgr, err := newAuthManager(params.Logger, params.AuthHandlers, mapCache, params.NodeIDHandler)
if err != nil {
return fmt.Errorf("failed to create auth manager: %w", err)
}

mapGC := newAuthMapGC(params.Logger, mapCache, params.IPCache, params.PolicyRepo)
mapGC := newAuthMapGC(params.Logger, mapCache, params.NodeIDHandler, params.PolicyRepo)

// Register auth components to lifecycle hooks & jobs

Expand Down
22 changes: 8 additions & 14 deletions pkg/auth/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ package auth
import (
"context"
"fmt"
"net"
"time"

"github.com/sirupsen/logrus"

"github.com/cilium/cilium/pkg/auth/certs"
"github.com/cilium/cilium/pkg/datapath/types"
"github.com/cilium/cilium/pkg/identity"
"github.com/cilium/cilium/pkg/lock"
"github.com/cilium/cilium/pkg/maps/authmap"
Expand All @@ -27,22 +27,16 @@ func (key signalAuthKey) String() string {
}

type authManager struct {
logger logrus.FieldLogger
ipCache ipCache
authHandlers map[policy.AuthType]authHandler
authmap authMap
logger logrus.FieldLogger
nodeIDHandler types.NodeIDHandler
authHandlers map[policy.AuthType]authHandler
authmap authMap

mutex lock.Mutex
pending map[authKey]struct{}
handleAuthenticationFunc func(a *authManager, k authKey, reAuth bool)
}

// ipCache is the set of interactions the auth manager performs with the IPCache
type ipCache interface {
GetNodeIP(uint16) string
GetNodeID(nodeIP net.IP) (nodeID uint16, exists bool)
}

// authHandler is responsible to handle authentication for a specific auth type
type authHandler interface {
authenticate(*authRequest) (*authResponse, error)
Expand All @@ -60,7 +54,7 @@ type authResponse struct {
expirationTime time.Time
}

func newAuthManager(logger logrus.FieldLogger, authHandlers []authHandler, authmap authMap, ipCache ipCache) (*authManager, error) {
func newAuthManager(logger logrus.FieldLogger, authHandlers []authHandler, authmap authMap, nodeIDHandler types.NodeIDHandler) (*authManager, error) {
ahs := map[policy.AuthType]authHandler{}
for _, ah := range authHandlers {
if ah == nil {
Expand All @@ -76,7 +70,7 @@ func newAuthManager(logger logrus.FieldLogger, authHandlers []authHandler, authm
logger: logger,
authHandlers: ahs,
authmap: authmap,
ipCache: ipCache,
nodeIDHandler: nodeIDHandler,
pending: make(map[authKey]struct{}),
handleAuthenticationFunc: handleAuthentication,
}, nil
Expand Down Expand Up @@ -188,7 +182,7 @@ func (a *authManager) authenticate(key authKey) error {
return fmt.Errorf("unknown requested auth type: %s", key.authType)
}

nodeIP := a.ipCache.GetNodeIP(key.remoteNodeID)
nodeIP := a.nodeIDHandler.GetNodeIP(key.remoteNodeID)
if nodeIP == "" {
return fmt.Errorf("remote node IP not available for node ID %d", key.remoteNodeID)
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/auth/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/stretchr/testify/assert"
"golang.org/x/exp/maps"

"github.com/cilium/cilium/api/v1/models"
"github.com/cilium/cilium/pkg/auth/certs"
"github.com/cilium/cilium/pkg/identity"
"github.com/cilium/cilium/pkg/policy"
Expand Down Expand Up @@ -175,6 +176,13 @@ type fakeIPCache struct {
nodeIdMappings map[uint16]string
}

func (r *fakeIPCache) DumpNodeIDs() []*models.NodeID {
return []*models.NodeID{}
}

func (r *fakeIPCache) RestoreNodeIDs() {
}

func newFakeIPCache(mappings map[uint16]string) *fakeIPCache {
return &fakeIPCache{
nodeIdMappings: mappings,
Expand Down
3 changes: 1 addition & 2 deletions pkg/datapath/cells.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/cilium/cilium/pkg/defaults"
"github.com/cilium/cilium/pkg/hive"
"github.com/cilium/cilium/pkg/hive/cell"
ipcache "github.com/cilium/cilium/pkg/ipcache/types"
"github.com/cilium/cilium/pkg/maps"
"github.com/cilium/cilium/pkg/maps/eventsmap"
"github.com/cilium/cilium/pkg/maps/nodemap"
Expand Down Expand Up @@ -69,7 +68,7 @@ var Cell = cell.Module(
// Gratuitous ARP event processor emits GARP packets on k8s pod creation events.
garp.Cell,

cell.Provide(func(dp types.Datapath) ipcache.NodeIDHandler {
cell.Provide(func(dp types.Datapath) types.NodeIDHandler {
return dp.NodeIDs()
}),
)
Expand Down
1 change: 1 addition & 0 deletions test/controlplane/suite/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func startCiliumAgent(t *testing.T, clientset k8sClient.Clientset, extraCell cel
cell.Provide(
func() k8sClient.Clientset { return clientset },
func() datapath.Datapath { return fdp },
func() datapath.NodeIDHandler { return fdp.NodeIDs() },
func() *option.DaemonConfig { return option.Config },
func() cnicell.CNIConfigManager { return &fakecni.FakeCNIConfigManager{} },
func() signalmap.Map { return fakesignalmap.NewFakeSignalMap([][]byte{}, time.Second) },
Expand Down

0 comments on commit 1602494

Please sign in to comment.