Replace gogoprotobuf with golang protobuf (#8949)

* Use google.golang.org/protobuf github.com/gogo/protobuf is deprecated [1] so use the official protobuf. [1] https://twitter.com/awalterschulze/status/1584553056100057088 * updated ID types for mock dataplane * convert proto type IDs to be valid map keys * solved copylocks warning by passing proto type reference * make all processor unit tests passed * convert proto types for app-policy * convert proto types for mock dataplane * addressed hjiawei's comments * make all TestCalculationGraph unit tests passed * fixed linter copylocks issues part 1 * fixed linter copylocks issue part 2 * replace deprecated grpc.Dial by grpc.NewClient * resovled conflict issues due to rebase * set default resolver scheme passthrough before grpc.NewClient for UNIX socket * Re-generate protobuf and cleanup go.mod * fixed syncserver_test and policysync_test issues * fixed policysync_test nsID issue * addressed hjiawei's comments * addressed hjiawei's comments: updated file names * fixed policysync_test which tried to compare two proto objs by using Expect/Equal * fixed static checks ineffectual assignment for equal * fixed policysync_test index value issue * fixed the panic issue when proto.RouteUpdate try to access Type * addressed hjiawei's comment: should use Eventrually for async assertion * Use one line Expect googleproto equal to be true * Add Expect assertion * Move grpc resolver SetDefaultScheme to init func * Fix app-policy checker unit tests * Fix felix calc state unit tests * Add protobuf target for pod2daemon and remove SKIP_PROTOBUF flag * Fix go import format * Use make fix-changed to reformat generated go files * Revert back to make fix for api * Update go mod * Fix felix untracked policy unit tests --------- Co-authored-by: Jiawei Huang <[email protected]>
projectcalico · Dec 23, 2024 · dd34212 · dd34212
1 parent 061a11c
commit dd34212
Show file tree

Hide file tree

Showing 99 changed files with 10,084 additions and 23,232 deletions.
diff --git a/.semaphore/semaphore-scheduled-builds.yml b/.semaphore/semaphore-scheduled-builds.yml
diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml
diff --git a/.semaphore/semaphore.yml.d/blocks/20-felix.yml b/.semaphore/semaphore.yml.d/blocks/20-felix.yml
@@ -55,9 +55,8 @@
               - s390x
         commands:
           # Only building the code, not the image here because the felix image is now only used for FV tests, which
-          # only run on AMD64 at the moment. Skip building protofbufs because the build fails on ARM due to missing
-          # image. We know they are up-to-date because an earlier build job checks already.
-          - ../.semaphore/run-and-monitor build-$ARCH.log make build ARCH=$ARCH SKIP_PROTOBUF=true
+          # only run on AMD64 at the moment.
+          - ../.semaphore/run-and-monitor build-$ARCH.log make build ARCH=$ARCH
 - name: "Felix: Build - native arm64 runner"
   run:
     when: "${FORCE_RUN} or change_in(['/*', '/api/', '/libcalico-go/', '/typha/', '/felix/'], {exclude: ['/**/.gitignore', '/**/README.md', '/**/LICENSE']})"
@@ -75,8 +74,7 @@
     jobs:
       - name: Build binary
         commands:
-          # Skipping protobuf build because it fails on ARM (but the pre-flight check ensures it's up-to-date).
-          - ../.semaphore/run-and-monitor build-arm64.log make build ARCH=arm64 SKIP_PROTOBUF=true
+          - ../.semaphore/run-and-monitor build-arm64.log make build ARCH=arm64
 - name: "Felix: Build Windows binaries"
   run:
     when: "${FORCE_RUN} or change_in(['/*', '/api/', '/libcalico-go/', '/typha/', '/felix/'], {exclude: ['/**/.gitignore', '/**/README.md', '/**/LICENSE']})"

diff --git a/app-policy/Makefile b/app-policy/Makefile
@@ -112,24 +112,12 @@ endif
 ../felix/proto/felixbackend.pb.go: ../felix/proto/felixbackend.proto
 	$(MAKE) --directory ../felix protobuf
 
-# We use gogofast for protobuf compilation.  Regular gogo is incompatible with
-# gRPC, since gRPC uses golang/protobuf for marshalling/unmarshalling in that
-# case.  See https://github.com/gogo/protobuf/issues/386 for more details.
-# Note that we cannot seem to use gogofaster because of incompatibility with
-# Envoy's validation library.
-# When importing, we must use gogo versions of google/protobuf and
-# google/rpc (aka googleapis).
-PROTOC_IMPORTS =  -I proto\
-		  -I ./
-
 protobuf: $(GENERATED_FILES)
 
 proto/healthz.pb.go: proto/healthz.proto
-	$(DOCKER_RUN) -v $(CURDIR):/src:rw --user $(LOCAL_USER_ID):$(LOCAL_GROUP_ID) \
-		      $(PROTOC_CONTAINER) \
-		      $(PROTOC_IMPORTS) \
-		      proto/*.proto \
-		      --gogofast_out=plugins=grpc:proto
+	$(DOCKER_RUN) -v $(CURDIR)/proto:/proto:rw \
+		$(CALICO_BUILD) \
+			sh -c 'protoc --proto_path=/proto --go_out=/proto --go-grpc_out=. --go_opt=paths=source_relative healthz.proto'
 	$(MAKE) fix-changed
 
 

diff --git a/app-policy/checker/check.go b/app-policy/checker/check.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/projectcalico/calico/app-policy/policystore"
 	"github.com/projectcalico/calico/felix/proto"
+	"github.com/projectcalico/calico/felix/types"
 )
 
 var OK = int32(code.Code_OK)
@@ -89,7 +90,7 @@ func checkTiers(store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, req
 		action := NO_MATCH
 	Policy:
 		for i, name := range policies {
-			pID := proto.PolicyID{Tier: tier.GetName(), Name: name}
+			pID := types.PolicyID{Tier: tier.GetName(), Name: name}
 			policy := store.PolicyByID[pID]
 			action = checkPolicy(policy, reqCache)
 			log.Debugf("Policy checked (ordinal=%d, profileId=%v, action=%v)", i, pID, action)
@@ -126,7 +127,7 @@ func checkTiers(store *policystore.PolicyStore, ep *proto.WorkloadEndpoint, req
 	// If we reach here, there were either no tiers, or a policy PASSed the request.
 	if len(ep.ProfileIds) > 0 {
 		for i, name := range ep.ProfileIds {
-			pID := proto.ProfileID{Name: name}
+			pID := types.ProfileID{Name: name}
 			profile := store.ProfileByID[pID]
 			action := checkProfile(profile, reqCache)
 			log.Debugf("Profile checked (ordinal=%d, profileId=%v, action=%v)", i, pID, action)

diff --git a/app-policy/checker/check_test.go b/app-policy/checker/check_test.go
@@ -18,11 +18,11 @@ import (
 	"testing"
 
 	authz "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
-	"github.com/gogo/googleapis/google/rpc"
 	. "github.com/onsi/gomega"
 
 	"github.com/projectcalico/calico/app-policy/policystore"
 	"github.com/projectcalico/calico/felix/proto"
+	"github.com/projectcalico/calico/felix/types"
 )
 
 // actionFromString should parse strings in case-insensitive mode.
@@ -123,21 +123,21 @@ func TestCheckNoIngressPolicyRulesInTier(t *testing.T) {
 		},
 		ProfileIds: []string{"profile1"},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		OutboundRules: []*proto.Rule{
 			{
 				Action: "allow",
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
 		OutboundRules: []*proto.Rule{
 			{
 				Action: "allow",
 			},
 		},
 	}
-	store.ProfileByID[proto.ProfileID{Name: "profile1"}] = &proto.Profile{
+	store.ProfileByID[types.ProfileID{Name: "profile1"}] = &proto.Profile{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "allow",
@@ -158,9 +158,7 @@ func TestCheckNoIngressPolicyRulesInTier(t *testing.T) {
 	}}
 
 	status := checkTiers(store, store.Endpoint, req)
-	expectedStatus := rpc.Status{Code: OK}
-	Expect(status.Code).To(Equal(expectedStatus.Code))
-	Expect(status.Message).To(Equal(expectedStatus.Message))
+	Expect(status.Code).To(Equal(OK))
 	Expect(status.Details).To(BeNil())
 }
 
@@ -220,15 +218,15 @@ func TestCheckStorePolicyMatch(t *testing.T) {
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "deny",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"HEAD"}},
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "allow",
@@ -268,15 +266,15 @@ func TestCheckStoreProfileOnly(t *testing.T) {
 		Tiers:      []*proto.TierInfo{},
 		ProfileIds: []string{"profile1", "profile2"},
 	}
-	store.ProfileByID[proto.ProfileID{Name: "profile1"}] = &proto.Profile{
+	store.ProfileByID[types.ProfileID{Name: "profile1"}] = &proto.Profile{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "Deny",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"HEAD"}},
 			},
 		},
 	}
-	store.ProfileByID[proto.ProfileID{Name: "profile2"}] = &proto.Profile{
+	store.ProfileByID[types.ProfileID{Name: "profile2"}] = &proto.Profile{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "allow",
@@ -321,15 +319,15 @@ func TestCheckStorePolicyDefaultDeny(t *testing.T) {
 		},
 		ProfileIds: []string{"profile1"},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "deny",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"HEAD"}},
 			},
 		},
 	}
-	store.ProfileByID[proto.ProfileID{Name: "profile1"}] = &proto.Profile{
+	store.ProfileByID[types.ProfileID{Name: "profile1"}] = &proto.Profile{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "allow",
@@ -368,15 +366,15 @@ func TestCheckStorePass(t *testing.T) {
 	}
 
 	// Policy1 matches and has action PASS, which means policy2 is not evaluated.
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "next-tier",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"GET"}},
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "deny",
@@ -386,7 +384,7 @@ func TestCheckStorePass(t *testing.T) {
 	}
 
 	// Profile1 matches and allows the traffic.
-	store.ProfileByID[proto.ProfileID{Name: "profile1"}] = &proto.Profile{
+	store.ProfileByID[types.ProfileID{Name: "profile1"}] = &proto.Profile{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "allow",
@@ -448,7 +446,7 @@ func TestCheckStoreWithInvalidData(t *testing.T) {
 		}},
 		ProfileIds: []string{"profile1"},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{InboundRules: []*proto.Rule{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{InboundRules: []*proto.Rule{
 		{
 			Action: "allow",
 			HttpMatch: &proto.HTTPMatch{
@@ -498,15 +496,15 @@ func TestCheckStorePolicyMultiTierMatch(t *testing.T) {
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "next-tier",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"GET", "HEAD"}},
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier2", Name: "policy2"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier2", Name: "policy2"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action: "deny",
@@ -516,7 +514,7 @@ func TestCheckStorePolicyMultiTierMatch(t *testing.T) {
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier2", Name: "policy3"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier2", Name: "policy3"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action: "allow",
@@ -526,7 +524,7 @@ func TestCheckStorePolicyMultiTierMatch(t *testing.T) {
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier3", Name: "policy4"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier3", Name: "policy4"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action: "allow",
@@ -584,23 +582,23 @@ func TestCheckStorePolicyMultiTierDiffTierMatch(t *testing.T) {
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy1"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "deny",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"HEAD"}},
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier1", Name: "policy2"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action:    "next-tier",
 				HttpMatch: &proto.HTTPMatch{Methods: []string{"GET"}},
 			},
 		},
 	}
-	store.PolicyByID[proto.PolicyID{Tier: "tier2", Name: "policy3"}] = &proto.Policy{
+	store.PolicyByID[types.PolicyID{Tier: "tier2", Name: "policy3"}] = &proto.Policy{
 		InboundRules: []*proto.Rule{
 			{
 				Action: "allow",

diff --git a/app-policy/checker/match_test.go b/app-policy/checker/match_test.go
@@ -23,6 +23,7 @@ import (
 
 	"github.com/projectcalico/calico/app-policy/policystore"
 	"github.com/projectcalico/calico/felix/proto"
+	"github.com/projectcalico/calico/felix/types"
 )
 
 var (
@@ -351,10 +352,12 @@ func TestMatchRuleNamespaceSelectors(t *testing.T) {
 	}}
 
 	store := policystore.NewPolicyStore()
-	id := proto.NamespaceID{Name: "src"}
-	store.NamespaceByID[id] = &proto.NamespaceUpdate{Id: &id, Labels: map[string]string{"place": "src"}}
-	id = proto.NamespaceID{Name: "dst"}
-	store.NamespaceByID[id] = &proto.NamespaceUpdate{Id: &id, Labels: map[string]string{"place": "dst"}}
+	id := types.NamespaceID{Name: "src"}
+	protoID := types.NamespaceIDToProto(id)
+	store.NamespaceByID[id] = &proto.NamespaceUpdate{Id: protoID, Labels: map[string]string{"place": "src"}}
+	id = types.NamespaceID{Name: "dst"}
+	protoID = types.NamespaceIDToProto(id)
+	store.NamespaceByID[id] = &proto.NamespaceUpdate{Id: protoID, Labels: map[string]string{"place": "dst"}}
 	reqCache, err := NewRequestCache(store, req)
 	Expect(err).To(Succeed())
 	Expect(match(rule, reqCache, "")).To(BeTrue())

diff --git a/app-policy/checker/requestcache.go b/app-policy/checker/requestcache.go
@@ -23,7 +23,7 @@ import (
 	log "github.com/sirupsen/logrus"
 
 	"github.com/projectcalico/calico/app-policy/policystore"
-	"github.com/projectcalico/calico/felix/proto"
+	"github.com/projectcalico/calico/felix/types"
 )
 
 // requestCache contains the CheckRequest and cached copies of computed information about the request
@@ -119,7 +119,7 @@ func (r *requestCache) initPeer(aPeer *authz.AttributeContext_Peer) (*peer, erro
 	}
 
 	// If the service account is in the store, copy labels over.
-	id := proto.ServiceAccountID{Name: peer.Name, Namespace: peer.Namespace}
+	id := types.ServiceAccountID{Name: peer.Name, Namespace: peer.Namespace}
 	msg, ok := r.store.ServiceAccountByID[id]
 	if ok {
 		for k, v := range msg.GetLabels() {
@@ -132,7 +132,7 @@ func (r *requestCache) initPeer(aPeer *authz.AttributeContext_Peer) (*peer, erro
 func (r *requestCache) initNamespace(name string) *namespace {
 	ns := &namespace{Name: name}
 	// If the namespace is in the store, copy labels over.
-	id := proto.NamespaceID{Name: name}
+	id := types.NamespaceID{Name: name}
 	msg, ok := r.store.NamespaceByID[id]
 	if ok {
 		ns.Labels = make(map[string]string)