Skip to content

Commit

Permalink
convert proto type IDs to be valid map keys
Browse files Browse the repository at this point in the history
  • Loading branch information
henrydo authored and hjiawei committed Jun 28, 2024
1 parent 3fb7dd7 commit b49a82b
Show file tree
Hide file tree
Showing 47 changed files with 1,389 additions and 632 deletions.
5 changes: 3 additions & 2 deletions felix/calc/calc_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/projectcalico/calico/felix/labelindex"
"github.com/projectcalico/calico/felix/proto"
"github.com/projectcalico/calico/felix/serviceindex"
"github.com/projectcalico/calico/felix/types"
"github.com/projectcalico/calico/libcalico-go/lib/backend/api"
"github.com/projectcalico/calico/libcalico-go/lib/backend/model"
"github.com/projectcalico/calico/libcalico-go/lib/net"
Expand Down Expand Up @@ -78,9 +79,9 @@ type passthruCallbacks interface {
OnIPPoolUpdate(model.IPPoolKey, *model.IPPool)
OnIPPoolRemove(model.IPPoolKey)
OnServiceAccountUpdate(*proto.ServiceAccountUpdate)
OnServiceAccountRemove(proto.ServiceAccountID)
OnServiceAccountRemove(types.ServiceAccountID)
OnNamespaceUpdate(*proto.NamespaceUpdate)
OnNamespaceRemove(proto.NamespaceID)
OnNamespaceRemove(types.NamespaceID)
OnWireguardUpdate(string, *model.Wireguard)
OnWireguardRemove(string)
OnGlobalBGPConfigUpdate(*v3.BGPConfiguration)
Expand Down
5 changes: 3 additions & 2 deletions felix/calc/encapsulation_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/projectcalico/calico/felix/config"
"github.com/projectcalico/calico/felix/dispatcher"
"github.com/projectcalico/calico/felix/proto"
"github.com/projectcalico/calico/felix/types"

"github.com/projectcalico/calico/libcalico-go/lib/backend/api"
"github.com/projectcalico/calico/libcalico-go/lib/backend/encap"
Expand Down Expand Up @@ -495,15 +496,15 @@ func (e *encapResolverCallbackRecorder) OnServiceAccountUpdate(update *proto.Ser
Fail("ServiceAccountUpdate received")
}

func (e *encapResolverCallbackRecorder) OnServiceAccountRemove(id proto.ServiceAccountID) {
func (e *encapResolverCallbackRecorder) OnServiceAccountRemove(id types.ServiceAccountID) {
Fail("ServiceAccountRemove received")
}

func (e *encapResolverCallbackRecorder) OnNamespaceUpdate(update *proto.NamespaceUpdate) {
Fail("NamespaceUpdate received")
}

func (e *encapResolverCallbackRecorder) OnNamespaceRemove(id proto.NamespaceID) {
func (e *encapResolverCallbackRecorder) OnNamespaceRemove(id types.NamespaceID) {
Fail("NamespaceRemove received")
}

Expand Down
55 changes: 29 additions & 26 deletions felix/calc/event_sequencer.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/projectcalico/calico/felix/labelindex"
"github.com/projectcalico/calico/felix/multidict"
"github.com/projectcalico/calico/felix/proto"
"github.com/projectcalico/calico/felix/types"
"github.com/projectcalico/calico/libcalico-go/lib/backend/model"
"github.com/projectcalico/calico/libcalico-go/lib/net"
"github.com/projectcalico/calico/libcalico-go/lib/set"
Expand Down Expand Up @@ -70,10 +71,10 @@ type EventSequencer struct {
pendingNotReady bool
pendingGlobalConfig map[string]string
pendingHostConfig map[string]string
pendingServiceAccountUpdates map[proto.ServiceAccountID]*proto.ServiceAccountUpdate
pendingServiceAccountDeletes set.Set[proto.ServiceAccountID]
pendingNamespaceUpdates map[proto.NamespaceID]*proto.NamespaceUpdate
pendingNamespaceDeletes set.Set[proto.NamespaceID]
pendingServiceAccountUpdates map[types.ServiceAccountID]*proto.ServiceAccountUpdate
pendingServiceAccountDeletes set.Set[types.ServiceAccountID]
pendingNamespaceUpdates map[types.NamespaceID]*proto.NamespaceUpdate
pendingNamespaceDeletes set.Set[types.NamespaceID]
pendingRouteUpdates map[routeID]*proto.RouteUpdate
pendingRouteDeletes set.Set[routeID]
pendingVTEPUpdates map[string]*proto.VXLANTunnelEndpointUpdate
Expand All @@ -93,8 +94,8 @@ type EventSequencer struct {
sentHostIPv6s set.Set[string]
sentHosts set.Set[string]
sentIPPools set.Set[ip.CIDR]
sentServiceAccounts set.Set[proto.ServiceAccountID]
sentNamespaces set.Set[proto.NamespaceID]
sentServiceAccounts set.Set[types.ServiceAccountID]
sentNamespaces set.Set[types.NamespaceID]
sentRoutes set.Set[routeID]
sentVTEPs set.Set[string]
sentWireguard set.Set[string]
Expand Down Expand Up @@ -149,10 +150,10 @@ func NewEventSequencer(conf configInterface) *EventSequencer {
pendingHostMetadataDeletes: set.New[string](),
pendingIPPoolUpdates: map[ip.CIDR]*model.IPPool{},
pendingIPPoolDeletes: set.New[ip.CIDR](),
pendingServiceAccountUpdates: map[proto.ServiceAccountID]*proto.ServiceAccountUpdate{},
pendingServiceAccountDeletes: set.New[proto.ServiceAccountID](),
pendingNamespaceUpdates: map[proto.NamespaceID]*proto.NamespaceUpdate{},
pendingNamespaceDeletes: set.New[proto.NamespaceID](),
pendingServiceAccountUpdates: map[types.ServiceAccountID]*proto.ServiceAccountUpdate{},
pendingServiceAccountDeletes: set.New[types.ServiceAccountID](),
pendingNamespaceUpdates: map[types.NamespaceID]*proto.NamespaceUpdate{},
pendingNamespaceDeletes: set.New[types.NamespaceID](),
pendingRouteUpdates: map[routeID]*proto.RouteUpdate{},
pendingRouteDeletes: set.New[routeID](),
pendingVTEPUpdates: map[string]*proto.VXLANTunnelEndpointUpdate{},
Expand All @@ -171,8 +172,8 @@ func NewEventSequencer(conf configInterface) *EventSequencer {
sentHostIPv6s: set.New[string](),
sentHosts: set.New[string](),
sentIPPools: set.New[ip.CIDR](),
sentServiceAccounts: set.New[proto.ServiceAccountID](),
sentNamespaces: set.New[proto.NamespaceID](),
sentServiceAccounts: set.New[types.ServiceAccountID](),
sentNamespaces: set.New[types.NamespaceID](),
sentRoutes: set.New[routeID](),
sentVTEPs: set.New[string](),
sentWireguard: set.New[string](),
Expand Down Expand Up @@ -887,7 +888,7 @@ func (buf *EventSequencer) flushAddsOrRemoves(setID string) {

func (buf *EventSequencer) OnServiceAccountUpdate(update *proto.ServiceAccountUpdate) {
// We trust the caller not to send us an update with nil ID, so safe to dereference.
id := *update.Id
id := types.ProtoToServiceAccountID(update.Id)
log.WithFields(log.Fields{
"key": id,
"labels": update.GetLabels(),
Expand All @@ -896,7 +897,7 @@ func (buf *EventSequencer) OnServiceAccountUpdate(update *proto.ServiceAccountUp
buf.pendingServiceAccountUpdates[id] = update
}

func (buf *EventSequencer) OnServiceAccountRemove(id proto.ServiceAccountID) {
func (buf *EventSequencer) OnServiceAccountRemove(id types.ServiceAccountID) {
log.WithFields(log.Fields{
"key": id,
}).Debug("ServiceAccount removed")
Expand All @@ -908,27 +909,28 @@ func (buf *EventSequencer) OnServiceAccountRemove(id proto.ServiceAccountID) {

func (buf *EventSequencer) flushServiceAccounts() {
// Order doesn't matter, but send removes first to reduce max occupancy
buf.pendingServiceAccountDeletes.Iter(func(id proto.ServiceAccountID) error {
msg := proto.ServiceAccountRemove{Id: &id}
buf.pendingServiceAccountDeletes.Iter(func(id types.ServiceAccountID) error {
pid := types.ServiceAccountIDToProto(id)
msg := proto.ServiceAccountRemove{Id: pid}
buf.Callback(&msg)
buf.sentServiceAccounts.Discard(id)
return nil
})
buf.pendingServiceAccountDeletes.Clear()
for _, msg := range buf.pendingServiceAccountUpdates {
buf.Callback(msg)
id := msg.Id
id := types.ProtoToServiceAccountID(msg.GetId())
// We safely dereferenced the Id in OnServiceAccountUpdate before adding it to the pending updates map, so
// it is safe to do so here.
buf.sentServiceAccounts.Add(*id)
buf.sentServiceAccounts.Add(id)
}
buf.pendingServiceAccountUpdates = make(map[proto.ServiceAccountID]*proto.ServiceAccountUpdate)
buf.pendingServiceAccountUpdates = make(map[types.ServiceAccountID]*proto.ServiceAccountUpdate)
log.Debug("Done flushing Service Accounts")
}

func (buf *EventSequencer) OnNamespaceUpdate(update *proto.NamespaceUpdate) {
// We trust the caller not to send us an update with nil ID, so safe to dereference.
id := *update.Id
id := types.ProtoToNamespaceID(update.GetId())
log.WithFields(log.Fields{
"key": id,
"labels": update.GetLabels(),
Expand All @@ -937,7 +939,7 @@ func (buf *EventSequencer) OnNamespaceUpdate(update *proto.NamespaceUpdate) {
buf.pendingNamespaceUpdates[id] = update
}

func (buf *EventSequencer) OnNamespaceRemove(id proto.NamespaceID) {
func (buf *EventSequencer) OnNamespaceRemove(id types.NamespaceID) {
log.WithFields(log.Fields{
"key": id,
}).Debug("Namespace removed")
Expand Down Expand Up @@ -981,21 +983,22 @@ func (buf *EventSequencer) OnGlobalBGPConfigUpdate(cfg *v3.BGPConfiguration) {

func (buf *EventSequencer) flushNamespaces() {
// Order doesn't matter, but send removes first to reduce max occupancy
buf.pendingNamespaceDeletes.Iter(func(id proto.NamespaceID) error {
msg := proto.NamespaceRemove{Id: &id}
buf.pendingNamespaceDeletes.Iter(func(id types.NamespaceID) error {
pid := types.NamespaceIDToProto(id)
msg := proto.NamespaceRemove{Id: pid}
buf.Callback(&msg)
buf.sentNamespaces.Discard(id)
return nil
})
buf.pendingNamespaceDeletes.Clear()
for _, msg := range buf.pendingNamespaceUpdates {
buf.Callback(msg)
id := msg.Id
id := types.ProtoToNamespaceID(msg.GetId())
// We safely dereferenced the Id in OnNamespaceUpdate before adding it to the pending updates map, so
// it is safe to do so here.
buf.sentNamespaces.Add(*id)
buf.sentNamespaces.Add(id)
}
buf.pendingNamespaceUpdates = make(map[proto.NamespaceID]*proto.NamespaceUpdate)
buf.pendingNamespaceUpdates = make(map[types.NamespaceID]*proto.NamespaceUpdate)
log.Debug("Done flushing Namespaces")
}

Expand Down
15 changes: 9 additions & 6 deletions felix/calc/profile_decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/projectcalico/calico/felix/dispatcher"
"github.com/projectcalico/calico/felix/proto"
"github.com/projectcalico/calico/felix/types"
"github.com/projectcalico/calico/libcalico-go/lib/backend/api"
"github.com/projectcalico/calico/libcalico-go/lib/backend/k8s/conversion"
"github.com/projectcalico/calico/libcalico-go/lib/backend/model"
Expand Down Expand Up @@ -54,22 +55,24 @@ func (p *ProfileDecoder) OnUpdate(update api.Update) (filterOut bool) {
switch id := idInterface.(type) {
case nil:
log.WithField("key", key.String()).Debug("Ignoring Profile labels")
case proto.ServiceAccountID:
case types.ServiceAccountID:
pid := types.ServiceAccountIDToProto(id)
if update.Value == nil {
p.callbacks.OnServiceAccountRemove(id)
} else {
labels := update.Value.(*apiv3.Profile).Spec.LabelsToApply
msg := proto.ServiceAccountUpdate{
Id: &id, Labels: decodeLabels(conversion.ServiceAccountLabelPrefix, labels)}
Id: pid, Labels: decodeLabels(conversion.ServiceAccountLabelPrefix, labels)}
p.callbacks.OnServiceAccountUpdate(&msg)
}
case proto.NamespaceID:
case types.NamespaceID:
pid := types.NamespaceIDToProto(id)
if update.Value == nil {
p.callbacks.OnNamespaceRemove(id)
} else {
labels := update.Value.(*apiv3.Profile).Spec.LabelsToApply
msg := proto.NamespaceUpdate{
Id: &id, Labels: decodeLabels(conversion.NamespaceLabelPrefix, labels)}
Id: pid, Labels: decodeLabels(conversion.NamespaceLabelPrefix, labels)}
p.callbacks.OnNamespaceUpdate(&msg)
}
}
Expand All @@ -79,11 +82,11 @@ func (p *ProfileDecoder) OnUpdate(update api.Update) (filterOut bool) {
func (p *ProfileDecoder) classifyProfile(key model.ResourceKey) interface{} {
namespace, name, err := p.converter.ProfileNameToServiceAccount(key.Name)
if err == nil {
return proto.ServiceAccountID{Name: name, Namespace: namespace}
return types.ServiceAccountID{Name: name, Namespace: namespace}
}
name, err = p.converter.ProfileNameToNamespace(key.Name)
if err == nil {
return proto.NamespaceID{Name: name}
return types.NamespaceID{Name: name}
}
return nil
}
Expand Down
13 changes: 7 additions & 6 deletions felix/calc/profile_decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/projectcalico/calico/felix/calc"
"github.com/projectcalico/calico/felix/dispatcher"
"github.com/projectcalico/calico/felix/proto"
"github.com/projectcalico/calico/felix/types"
"github.com/projectcalico/calico/libcalico-go/lib/backend/api"
"github.com/projectcalico/calico/libcalico-go/lib/backend/k8s/conversion"
"github.com/projectcalico/calico/libcalico-go/lib/backend/model"
Expand Down Expand Up @@ -100,15 +101,15 @@ var _ = Describe("profileDecoder", func() {
It("should send k8s service account profile remove", func() {
update := removeUpdate(conversion.ServiceAccountProfileNamePrefix + "test_namespace.test_serviceaccount")
uut.OnUpdate(update)
Expect(callbacks.saRemoves).To(Equal([]proto.ServiceAccountID{
Expect(callbacks.saRemoves).To(Equal([]types.ServiceAccountID{
{Name: "test_serviceaccount", Namespace: "test_namespace"},
}))
})

It("should send k8s namespace remove", func() {
update := removeUpdate(conversion.NamespaceProfileNamePrefix + "test_namespace")
uut.OnUpdate(update)
Expect(callbacks.nsRemoves).To(Equal([]proto.NamespaceID{
Expect(callbacks.nsRemoves).To(Equal([]types.NamespaceID{
{Name: "test_namespace"},
}))
})
Expand All @@ -133,9 +134,9 @@ var _ = Describe("profileDecoder", func() {

type passthruCallbackRecorder struct {
saUpdates []*proto.ServiceAccountUpdate
saRemoves []proto.ServiceAccountID
saRemoves []types.ServiceAccountID
nsUpdates []*proto.NamespaceUpdate
nsRemoves []proto.NamespaceID
nsRemoves []types.NamespaceID
}

func (p *passthruCallbackRecorder) OnHostIPUpdate(hostname string, ip *net.IP) {
Expand Down Expand Up @@ -182,15 +183,15 @@ func (p *passthruCallbackRecorder) OnServiceAccountUpdate(update *proto.ServiceA
p.saUpdates = append(p.saUpdates, update)
}

func (p *passthruCallbackRecorder) OnServiceAccountRemove(id proto.ServiceAccountID) {
func (p *passthruCallbackRecorder) OnServiceAccountRemove(id types.ServiceAccountID) {
p.saRemoves = append(p.saRemoves, id)
}

func (p *passthruCallbackRecorder) OnNamespaceUpdate(update *proto.NamespaceUpdate) {
p.nsUpdates = append(p.nsUpdates, update)
}

func (p *passthruCallbackRecorder) OnNamespaceRemove(id proto.NamespaceID) {
func (p *passthruCallbackRecorder) OnNamespaceRemove(id types.NamespaceID) {
p.nsRemoves = append(p.nsRemoves, id)
}

Expand Down
3 changes: 2 additions & 1 deletion felix/calc/rule_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"encoding/base64"

log "github.com/sirupsen/logrus"
googleproto "google.golang.org/protobuf/proto"

"github.com/projectcalico/api/pkg/lib/numorstring"

Expand Down Expand Up @@ -63,7 +64,7 @@ func fillInRuleIDs(rules []*proto.Rule, ruleIDSeed string) {
// library.
// TODO(smc) Can we do better than hashing the protobuf?
rule.RuleId = ""
data, err := rule.Marshal()
data, err := googleproto.Marshal(rule)
if err != nil {
log.WithError(err).WithField("rule", rule).Panic("Failed to marshal rule")
}
Expand Down
Loading

0 comments on commit b49a82b

Please sign in to comment.