policy: Inline selectorPolicy.L4Policy

There is no reason selectorPolicy.L4Policy needs to be a pointer as we
can initialize an empty L4Policy for it.

Refactor by inlining L4Policy instead of pointing to it, as this
simplifies code by removing the need for pointer nil checks.

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

pkg/endpoint/api.go

@@ -386,7 +386,7 @@ func (e *Endpoint) GetPolicyModel() *models.EndpointPolicyStatus {
 		realizedL4Policy *policy.L4Policy
 	)
 	if e.realizedPolicy != nil {
-		realizedL4Policy = e.realizedPolicy.L4Policy
+		realizedL4Policy = &e.realizedPolicy.L4Policy
 	}
 
 	mdl := &models.EndpointPolicy{
@@ -406,7 +406,7 @@ func (e *Endpoint) GetPolicyModel() *models.EndpointPolicyStatus {
 		desiredL4Policy *policy.L4Policy
 	)
 	if e.desiredPolicy != nil {
-		desiredL4Policy = e.desiredPolicy.L4Policy
+		desiredL4Policy = &e.desiredPolicy.L4Policy
 	}
 
 	desiredMdl := &models.EndpointPolicy{

pkg/endpoint/policy.go

@@ -110,8 +110,8 @@ func (e *Endpoint) updateNetworkPolicy(proxyWaitGroup *completion.WaitGroup) (re
 		return nil, nil
 	}
 
-	// If desired L4Policy is nil then no policy change is needed.
-	if e.desiredPolicy == nil || e.desiredPolicy.L4Policy == nil {
+	// If desired policy is nil then no policy change is needed.
+	if e.desiredPolicy == nil {
 		return nil, nil
 	}
 
@@ -120,7 +120,7 @@ func (e *Endpoint) updateNetworkPolicy(proxyWaitGroup *completion.WaitGroup) (re
 	}
 
 	// Publish the updated policy to L7 proxies.
-	return e.proxy.UpdateNetworkPolicy(e, e.visibilityPolicy, e.desiredPolicy.L4Policy, e.desiredPolicy.IngressPolicyEnabled, e.desiredPolicy.EgressPolicyEnabled, proxyWaitGroup)
+	return e.proxy.UpdateNetworkPolicy(e, e.visibilityPolicy, &e.desiredPolicy.L4Policy, e.desiredPolicy.IngressPolicyEnabled, e.desiredPolicy.EgressPolicyEnabled, proxyWaitGroup)
 }
 
 // setNextPolicyRevision updates the desired policy revision field

pkg/policy/distillery.go

@@ -165,9 +165,6 @@ func (cache *PolicyCache) GetAuthTypes(localID, remoteID identityPkg.NumericIden
 
 	// SelectorPolicy is const after it has been created, so no locking needed to access it
 	selPolicy := cip.getPolicy()
-	if selPolicy.L4Policy == nil {
-		return nil // Local identity added, but policy not computed yet
-	}
 
 	var resTypes AuthTypes
 	for cs, authTypes := range selPolicy.L4Policy.AuthMap {
@@ -206,7 +203,7 @@ func newCachedSelectorPolicy(identity *identityPkg.Identity, selectorCache *Sele
 	cip := &cachedSelectorPolicy{
 		identity: identity,
 	}
-	cip.setPolicy(newSelectorPolicy(0, selectorCache))
+	cip.setPolicy(newSelectorPolicy(selectorCache))
 	return cip
 }
 

pkg/policy/l4.go

@@ -1180,8 +1180,8 @@ type L4Policy struct {
 }
 
 // NewL4Policy creates a new L4Policy
-func NewL4Policy(revision uint64) *L4Policy {
-	return &L4Policy{
+func NewL4Policy(revision uint64) L4Policy {
+	return L4Policy{
 		Ingress:  newL4DirectionPolicy(),
 		Egress:   newL4DirectionPolicy(),
 		Revision: revision,

pkg/policy/resolve.go

@@ -22,7 +22,7 @@ type selectorPolicy struct {
 	SelectorCache *SelectorCache
 
 	// L4Policy contains the computed L4 and L7 policy.
-	L4Policy *L4Policy
+	L4Policy L4Policy
 
 	// IngressPolicyEnabled specifies whether this policy contains any policy
 	// at ingress.
@@ -34,9 +34,7 @@ type selectorPolicy struct {
 }
 
 func (p *selectorPolicy) Attach(ctx PolicyContext) {
-	if p.L4Policy != nil {
-		p.L4Policy.Attach(ctx)
-	}
+	p.L4Policy.Attach(ctx)
 }
 
 // EndpointPolicy is a structure which contains the resolved policy across all
@@ -72,36 +70,31 @@ type PolicyOwner interface {
 }
 
 // newSelectorPolicy returns an empty selectorPolicy stub.
-func newSelectorPolicy(revision uint64, selectorCache *SelectorCache) *selectorPolicy {
+func newSelectorPolicy(selectorCache *SelectorCache) *selectorPolicy {
 	return &selectorPolicy{
-		Revision:      revision,
+		Revision:      0,
 		SelectorCache: selectorCache,
+		L4Policy:      NewL4Policy(0),
 	}
 }
 
 // insertUser adds a user to the L4Policy so that incremental
 // updates of the L4Policy may be fowarded.
 func (p *selectorPolicy) insertUser(user *EndpointPolicy) {
-	if p.L4Policy != nil {
-		p.L4Policy.insertUser(user)
-	}
+	p.L4Policy.insertUser(user)
 }
 
 // removeUser removes a user from the L4Policy so the EndpointPolicy
 // can be freed when not needed any more
 func (p *selectorPolicy) removeUser(user *EndpointPolicy) {
-	if p.L4Policy != nil {
-		p.L4Policy.removeUser(user)
-	}
+	p.L4Policy.removeUser(user)
 }
 
 // Detach releases resources held by a selectorPolicy to enable
 // successful eventual GC.  Note that the selectorPolicy itself if not
 // modified in any way, so that it can be used concurrently.
 func (p *selectorPolicy) Detach() {
-	if p.L4Policy != nil {
-		p.L4Policy.Detach(p.SelectorCache)
-	}
+	p.L4Policy.Detach(p.SelectorCache)
 }
 
 // DistillPolicy filters down the specified selectorPolicy (which acts
@@ -158,9 +151,6 @@ func (p *EndpointPolicy) Detach() {
 // the datapath-friendly format inside EndpointPolicy.PolicyMapState.
 // Called with selectorcache locked for reading
 func (p *EndpointPolicy) toMapState() {
-	if p.L4Policy == nil {
-		return
-	}
 	p.L4Policy.Ingress.toMapState(p)
 	p.L4Policy.Egress.toMapState(p)
 }
@@ -200,11 +190,6 @@ type getProxyPortFunc func(*L4Filter) (proxyPort uint16, ok bool)
 // function to obtain a proxy port number to use. Changes to 'p.PolicyMapState' are collected in
 // 'adds' and 'updated' so that they can be reverted when needed.
 func (p *EndpointPolicy) UpdateRedirects(ingress bool, identities Identities, getProxyPort getProxyPortFunc, adds Keys, updated MapState) {
-	// Nothing to do if L4Policy does not exist
-	if p.L4Policy == nil {
-		return
-	}
-
 	l4policy := &p.L4Policy.Ingress
 	if ingress {
 		l4policy = &p.L4Policy.Egress
@@ -247,10 +232,7 @@ func (l4policy L4DirectionPolicy) updateRedirects(p *EndpointPolicy, identities
 func (p *EndpointPolicy) ConsumeMapChanges() (adds, deletes Keys) {
 	p.selectorPolicy.SelectorCache.mutex.Lock()
 	defer p.selectorPolicy.SelectorCache.mutex.Unlock()
-	features := allFeatures
-	if p.selectorPolicy.L4Policy != nil {
-		features = p.selectorPolicy.L4Policy.Ingress.features | p.selectorPolicy.L4Policy.Egress.features
-	}
+	features := p.selectorPolicy.L4Policy.Ingress.features | p.selectorPolicy.L4Policy.Egress.features
 	return p.policyMapChanges.consumeMapChanges(p.PolicyMapState, features, p.SelectorCache)
 }
 
@@ -288,6 +270,6 @@ func (p *EndpointPolicy) AllowsIdentity(identity identity.NumericIdentity) (ingr
 // NewEndpointPolicy returns an empty EndpointPolicy stub.
 func NewEndpointPolicy(repo *Repository) *EndpointPolicy {
 	return &EndpointPolicy{
-		selectorPolicy: newSelectorPolicy(0, repo.GetSelectorCache()),
+		selectorPolicy: newSelectorPolicy(repo.GetSelectorCache()),
 	}
 }

pkg/policy/resolve_deny_test.go

@@ -143,7 +143,7 @@ func (ds *PolicyTestSuite) TestL3WithIngressDenyWildcard(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -228,7 +228,7 @@ func (ds *PolicyTestSuite) TestL3WithLocalHostWildcardd(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -312,7 +312,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressDenyWildcard(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -452,7 +452,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressDeny(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {

pkg/policy/resolve_test.go

@@ -289,7 +289,6 @@ func (ds *PolicyTestSuite) TestL7WithIngressWildcard(c *C) {
 	defer repo.Mutex.RUnlock()
 	selPolicy, err := repo.resolvePolicyLocked(fooIdentity)
 	c.Assert(err, IsNil)
-	c.Assert(selPolicy.L4Policy, NotNil)
 	c.Assert(selPolicy.L4Policy.redirectTypes, Equals, redirectTypeEnvoy)
 
 	policy := selPolicy.DistillPolicy(DummyOwner{}, false)
@@ -298,7 +297,7 @@ func (ds *PolicyTestSuite) TestL7WithIngressWildcard(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -394,7 +393,7 @@ func (ds *PolicyTestSuite) TestL7WithLocalHostWildcardd(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -486,7 +485,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngressWildcard(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {
@@ -628,7 +627,7 @@ func (ds *PolicyTestSuite) TestMapStateWithIngress(c *C) {
 		selectorPolicy: &selectorPolicy{
 			Revision:      repo.GetRevision(),
 			SelectorCache: repo.GetSelectorCache(),
-			L4Policy: &L4Policy{
+			L4Policy: L4Policy{
 				Revision: repo.GetRevision(),
 				Ingress: L4DirectionPolicy{PortRules: L4PolicyMap{
 					"80/TCP": {

pkg/policy/rule_test.go

@@ -118,7 +118,7 @@ func (ds *PolicyTestSuite) TestL4Policy(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(res.Egress, Not(IsNil))
 
-	c.Assert(res, checker.DeepEquals, expected)
+	c.Assert(&res, checker.Equals, &expected)
 	c.Assert(ingressState.selectedRules, Equals, 1)
 	c.Assert(ingressState.matchedRules, Equals, 1)
 
@@ -244,7 +244,7 @@ func (ds *PolicyTestSuite) TestL4Policy(c *C) {
 	c.Assert(res.Egress, Not(IsNil))
 
 	c.Assert(len(res.Ingress.PortRules), Equals, 1)
-	c.Assert(res, checker.DeepEquals, expected)
+	c.Assert(&res, checker.Equals, &expected)
 	c.Assert(ingressState.selectedRules, Equals, 1)
 	c.Assert(ingressState.matchedRules, Equals, 1)
 
@@ -1137,7 +1137,7 @@ func (ds *PolicyTestSuite) TestICMPPolicy(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(res.Egress, Not(IsNil))
 
-	c.Assert(res, checker.Equals, expected)
+	c.Assert(&res, checker.Equals, &expected)
 	c.Assert(ingressState.selectedRules, Equals, 1)
 	c.Assert(ingressState.matchedRules, Equals, 1)
 	c.Assert(egressState.selectedRules, Equals, 1)
@@ -1198,7 +1198,7 @@ func (ds *PolicyTestSuite) TestICMPPolicy(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(res.Ingress, Not(IsNil))
 
-	c.Assert(res, checker.Equals, expected)
+	c.Assert(&res, checker.Equals, &expected)
 	c.Assert(ingressState.selectedRules, Equals, 1)
 	c.Assert(ingressState.matchedRules, Equals, 1)
 
@@ -1242,7 +1242,7 @@ func (ds *PolicyTestSuite) TestICMPPolicy(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(res.Ingress, Not(IsNil))
 
-	c.Assert(res, checker.Equals, expected)
+	c.Assert(&res, checker.Equals, &expected)
 	c.Assert(ingressState.selectedRules, Equals, 1)
 	c.Assert(ingressState.matchedRules, Equals, 1)
 }