Skip to content

Commit

Permalink
daemon,test: remove enableRemoteNodeIdentity flag
Browse files Browse the repository at this point in the history
The enable-remote-node-identity agent flag was marked as deprecated for
1.15 in commit cf472ef ("daemon: Deprecate
EnableRemoteNodeIdentity").

Signed-off-by: Donia Chaiehloudj <donia.cld@isovalent.com>
  • Loading branch information
doniacld committed Mar 21, 2024
1 parent 133a206 commit 88e7a15
Show file tree
Hide file tree
Showing 17 changed files with 34 additions and 170 deletions.
1 change: 0 additions & 1 deletion .github/actions/ginkgo/main-focus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ include:
###
# K8sAgentPolicyTest Multi-node policy test validates fromEntities policies Validates fromEntities all policy
# K8sAgentPolicyTest Multi-node policy test validates fromEntities policies Validates fromEntities cluster policy
# K8sAgentPolicyTest Multi-node policy test validates fromEntities policies with remote-node identity disabled Allows from all hosts with cnp fromEntities host policy
# K8sAgentPolicyTest Multi-node policy test validates fromEntities policies with remote-node identity enabled Validates fromEntities remote-node policy
# K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath
- focus: "f04-agent-policy-multi-node-1"
Expand Down
6 changes: 1 addition & 5 deletions daemon/cmd/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,8 +642,7 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup, params *daemonParams
})
}

treatRemoteNodeAsHost := option.Config.AlwaysAllowLocalhost() && !option.Config.EnableRemoteNodeIdentity
policyAPI.InitEntities(params.ClusterInfo.Name, treatRemoteNodeAsHost)
policyAPI.InitEntities(params.ClusterInfo.Name)

bootstrapStats.restore.Start()
// fetch old endpoints before k8s is configured.
Expand Down Expand Up @@ -750,9 +749,6 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup, params *daemonParams
case !option.Config.EnableNodePort:
err = fmt.Errorf("BPF masquerade requires NodePort (--%s=\"true\")",
option.EnableNodePort)
case !option.Config.EnableRemoteNodeIdentity:
err = fmt.Errorf("BPF masquerade requires remote node identities (--%s=\"true\")",
option.EnableRemoteNodeIdentity)
case len(option.Config.MasqueradeInterfaces) > 0:
err = fmt.Errorf("BPF masquerade does not allow to specify devices via --%s (use --%s instead)",
option.MasqueradeInterfaces, option.Devices)
Expand Down
8 changes: 0 additions & 8 deletions daemon/cmd/daemon_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,6 @@ func InitGlobalFlags(cmd *cobra.Command, vp *viper.Viper) {
flags.String(option.IPv6MCastDevice, "", "Device that joins a Solicited-Node multicast group for IPv6")
option.BindEnv(vp, option.IPv6MCastDevice)

flags.Bool(option.EnableRemoteNodeIdentity, defaults.EnableRemoteNodeIdentity, "Enable use of remote node identity")
flags.MarkDeprecated(option.EnableRemoteNodeIdentity, "Remote Node Identity is needed for various other features to work or work fully, including EgressGateway and Policies. There is no benefit to having it turned off. It will be removed in v1.16.")
option.BindEnv(vp, option.EnableRemoteNodeIdentity)

flags.String(option.EncryptInterface, "", "Transparent encryption interface")
option.BindEnv(vp, option.EncryptInterface)

Expand Down Expand Up @@ -1378,10 +1374,6 @@ func initEnv(vp *viper.Viper) {
if option.Config.EnableIPSec {
log.Fatal("IPSec cannot be used with the host firewall.")
}
if option.Config.EnableEndpointRoutes && !option.Config.EnableRemoteNodeIdentity {
log.Fatalf("The host firewall requires remote-node identities (%s) when running with %s",
option.EnableRemoteNodeIdentity, option.EnableEndpointRoutes)
}
}

if option.Config.EnableIPv6Masquerade && option.Config.EnableBPFMasquerade && option.Config.EnableHostFirewall {
Expand Down
7 changes: 3 additions & 4 deletions daemon/cmd/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ var (
nm manager.NodeManager
mtuConfig = mtu.NewConfiguration(0, false, false, false, false, 0, nil)
fakeConfig = &option.DaemonConfig{
RoutingMode: option.RoutingModeTunnel,
EnableRemoteNodeIdentity: true,
EnableIPSec: true,
EncryptNode: true,
RoutingMode: option.RoutingModeTunnel,
EnableIPSec: true,
EncryptNode: true,
}
)

Expand Down
3 changes: 0 additions & 3 deletions pkg/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,6 @@ const (
// specified in the L7 policies.
CertsDirectory = RuntimePath + "/certs"

// EnableRemoteNodeIdentity is the default value for option.EnableRemoteNodeIdentity
EnableRemoteNodeIdentity = true

// IPAMExpiration is the timeout after which an IP subject to expiratio
// is being released again if no endpoint is being created in time.
IPAMExpiration = 10 * time.Minute
Expand Down
6 changes: 0 additions & 6 deletions pkg/egressgateway/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,6 @@ func NewEgressGatewayManager(p Params) (out struct {
return out, fmt.Errorf("egress gateway requires --%s=\"true\" and --%s=\"true\"", option.EnableIPv4Masquerade, option.EnableBPFMasquerade)
}

if !dcfg.EnableRemoteNodeIdentity {
// datapath code depends on remote node identities to distinguish between
// cluster-local and cluster-egress traffic.
return out, fmt.Errorf("egress gateway requires remote node identities (--%s=\"true\")", option.EnableRemoteNodeIdentity)
}

if dcfg.EnableL7Proxy {
log.WithField(logfields.URL, "https://github.com/cilium/cilium/issues/19642").
Warningf("both egress gateway and L7 proxy (--%s) are enabled. This is currently not fully supported: "+
Expand Down
5 changes: 2 additions & 3 deletions pkg/ipcache/kvstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,12 @@ func (iw *IPIdentityWatcher) OnUpdate(k storepkg.Key) {
}

peerIdentity := ipIDPair.ID
if option.Config.EnableRemoteNodeIdentity && peerIdentity == identity.ReservedIdentityHost {
if peerIdentity == identity.ReservedIdentityHost {
// The only way we can discover IPs associated with the local host
// is directly via the NodeDiscovery package. If someone is informing
// this agent about IPs corresponding to the "host" via the kvstore,
// then they're sharing their own perspective on their own node IPs'
// identity. However, this node has remote-node enabled, so we should
// treat the peer as a "remote-node", not a "host".
// identity. We should treat the peer as a "remote-node", not a "host".
peerIdentity = identity.ReservedIdentityRemoteNode
}

Expand Down
47 changes: 22 additions & 25 deletions pkg/node/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,37 +421,34 @@ func (m *manager) endpointEncryptionKey(n *nodeTypes.Node) ipcacheTypes.EncryptK

func (m *manager) nodeIdentityLabels(n nodeTypes.Node) (nodeLabels labels.Labels, hasOverride bool) {
nodeLabels = labels.NewFrom(labels.LabelRemoteNode)
if m.conf.RemoteNodeIdentitiesEnabled() {
if n.IsLocal() {
nodeLabels = labels.NewFrom(labels.LabelHost)
if m.conf.PolicyCIDRMatchesNodes() {
for _, address := range n.IPAddresses {
addr, ok := ip.AddrFromIP(address.IP)
if ok {
bitLen := addr.BitLen()
if m.conf.EnableIPv4 && bitLen == net.IPv4len*8 ||
m.conf.EnableIPv6 && bitLen == net.IPv6len*8 {
prefix, err := addr.Prefix(bitLen)
if err == nil {
cidrLabels := labels.GetCIDRLabels(prefix)
nodeLabels.MergeLabels(cidrLabels)
}
if n.IsLocal() {
nodeLabels = labels.NewFrom(labels.LabelHost)
if m.conf.PolicyCIDRMatchesNodes() {
for _, address := range n.IPAddresses {
addr, ok := ip.AddrFromIP(address.IP)
if ok {
bitLen := addr.BitLen()
if m.conf.EnableIPv4 && bitLen == net.IPv4len*8 ||
m.conf.EnableIPv6 && bitLen == net.IPv6len*8 {
prefix, err := addr.Prefix(bitLen)
if err == nil {
cidrLabels := labels.GetCIDRLabels(prefix)
nodeLabels.MergeLabels(cidrLabels)
}
}
}
}
} else if !identity.NumericIdentity(n.NodeIdentity).IsReservedIdentity() {
// This needs to match clustermesh-apiserver's VMManager.AllocateNodeIdentity
nodeLabels = labels.Map2Labels(n.Labels, labels.LabelSourceK8s)
hasOverride = true
} else if !n.IsLocal() && option.Config.PerNodeLabelsEnabled() {
lbls := labels.Map2Labels(n.Labels, labels.LabelSourceNode)
filteredLbls, _ := labelsfilter.FilterNodeLabels(lbls)
nodeLabels.MergeLabels(filteredLbls)
}
} else {
nodeLabels = labels.NewFrom(labels.LabelHost)
} else if !identity.NumericIdentity(n.NodeIdentity).IsReservedIdentity() {
// This needs to match clustermesh-apiserver's VMManager.AllocateNodeIdentity
nodeLabels = labels.Map2Labels(n.Labels, labels.LabelSourceK8s)
hasOverride = true
} else if !n.IsLocal() && option.Config.PerNodeLabelsEnabled() {
lbls := labels.Map2Labels(n.Labels, labels.LabelSourceNode)
filteredLbls, _ := labelsfilter.FilterNodeLabels(lbls)
nodeLabels.MergeLabels(filteredLbls)
}

return nodeLabels, hasOverride
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/node/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,10 +481,10 @@ func (s *managerTestSuite) TestBackgroundSync(c *check.C) {
allNodeValidateCallsReceived.Wait()
}

func (s *managerTestSuite) TestIPCache(c *check.C) {
func (s *managerTestSuite) TestIpcache(c *check.C) {
ipcacheMock := newIPcacheMock()
dp := newSignalNodeHandler()
mngr, err := New(&option.DaemonConfig{EnableRemoteNodeIdentity: true}, ipcacheMock, newIPSetMock(), NewNodeMetrics(), cell.TestScope())
mngr, err := New(&option.DaemonConfig{}, ipcacheMock, newIPSetMock(), NewNodeMetrics(), cell.TestScope())
c.Assert(err, check.IsNil)
mngr.Subscribe(dp)
defer mngr.Stop(context.TODO())
Expand Down
13 changes: 0 additions & 13 deletions pkg/option/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -896,9 +896,6 @@ const (
// control plane, e.g. when using the managed etcd feature
EnableWellKnownIdentities = "enable-well-known-identities"

// EnableRemoteNodeIdentity enables use of the remote-node identity
EnableRemoteNodeIdentity = "enable-remote-node-identity"

// PolicyAuditModeArg argument enables policy audit mode.
PolicyAuditModeArg = "policy-audit-mode"

Expand Down Expand Up @@ -2011,9 +2008,6 @@ type DaemonConfig struct {
// control plane, e.g. when using the managed etcd feature
EnableWellKnownIdentities bool

// EnableRemoteNodeIdentity enables use of the remote-node identity
EnableRemoteNodeIdentity bool

// Azure options

// PolicyAuditMode enables non-drop mode for installed policies. In
Expand Down Expand Up @@ -2520,12 +2514,6 @@ func (c *DaemonConfig) NodeIpsetNeeded() bool {
return !c.TunnelingEnabled() && c.IptablesMasqueradingEnabled()
}

// RemoteNodeIdentitiesEnabled returns true if the remote-node identity feature
// is enabled
func (c *DaemonConfig) RemoteNodeIdentitiesEnabled() bool {
return c.EnableRemoteNodeIdentity
}

// NodeEncryptionEnabled returns true if node encryption is enabled
func (c *DaemonConfig) NodeEncryptionEnabled() bool {
return c.EncryptNode
Expand Down Expand Up @@ -2918,7 +2906,6 @@ func (c *DaemonConfig) Populate(vp *viper.Viper) {
c.BPFSocketLBHostnsOnly = vp.GetBool(BPFSocketLBHostnsOnly)
c.EnableSocketLB = vp.GetBool(EnableSocketLB)
c.EnableSocketLBTracing = vp.GetBool(EnableSocketLBTracing)
c.EnableRemoteNodeIdentity = vp.GetBool(EnableRemoteNodeIdentity)
c.EnableBPFTProxy = vp.GetBool(EnableBPFTProxy)
c.EnableXTSocketFallback = vp.GetBool(EnableXTSocketFallbackName)
c.EnableAutoDirectRouting = vp.GetBool(EnableAutoDirectRoutingName)
Expand Down
9 changes: 1 addition & 8 deletions pkg/policy/api/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (s EntitySlice) GetAsEndpointSelectors() EndpointSelectorSlice {
}

// InitEntities is called to initialize the policy API layer
func InitEntities(clusterName string, treatRemoteNodeAsHost bool) {
func InitEntities(clusterName string) {
EntitySelectorMapping[EntityCluster] = EndpointSelectorSlice{
endpointSelectorHost,
endpointSelectorRemoteNode,
Expand All @@ -141,11 +141,4 @@ func InitEntities(clusterName string, treatRemoteNodeAsHost bool) {
endpointSelectorKubeAPIServer,
NewESFromLabels(labels.NewLabel(k8sapi.PolicyLabelCluster, clusterName, labels.LabelSourceK8s)),
}

hostSelectors := make(EndpointSelectorSlice, 0, 2)
hostSelectors = append(hostSelectors, endpointSelectorHost)
if treatRemoteNodeAsHost {
hostSelectors = append(hostSelectors, endpointSelectorRemoteNode)
}
EntitySelectorMapping[EntityHost] = hostSelectors
}
33 changes: 2 additions & 31 deletions pkg/policy/api/entity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (s EntitySlice) matches(ctx labels.LabelArray) bool {
}

func (s *PolicyAPITestSuite) TestEntityMatches(c *C) {
InitEntities("cluster1", false)
InitEntities("cluster1")

c.Assert(EntityHost.matches(labels.ParseLabelArray("reserved:host")), Equals, true)
c.Assert(EntityHost.matches(labels.ParseLabelArray("reserved:host", "id:foo")), Equals, true)
Expand Down Expand Up @@ -71,7 +71,7 @@ func (s *PolicyAPITestSuite) TestEntityMatches(c *C) {
}

func (s *PolicyAPITestSuite) TestEntitySliceMatches(c *C) {
InitEntities("cluster1", false)
InitEntities("cluster1")

slice := EntitySlice{EntityHost, EntityWorld}
c.Assert(slice.matches(labels.ParseLabelArray("reserved:host")), Equals, true)
Expand All @@ -89,32 +89,3 @@ func (s *PolicyAPITestSuite) TestEntitySliceMatches(c *C) {
c.Assert(slice.matches(labels.ParseLabelArray("reserved:none")), Equals, false)
c.Assert(slice.matches(labels.ParseLabelArray("id=foo")), Equals, false)
}

func (s *PolicyAPITestSuite) TestEntityHostAllowsRemoteNode(c *C) {
tests := []struct {
name string
treatRemoteNodeAsHost bool
expectedMatches labels.LabelArray
expectedNonMatches labels.LabelArray
}{
{
"host entity selects remote-node identity",
true,
labels.ParseLabelArray("reserved:remote-node"),
labels.ParseLabelArray("reserved:all"),
},
{
"host entity does not select remote-node identity",
false,
labels.ParseLabelArray("reserved:host"),
labels.ParseLabelArray("reserved:remote-node"),
},
}

for _, tt := range tests {
InitEntities("cluster1", tt.treatRemoteNodeAsHost)
hostSelector := EntitySelectorMapping[EntityHost]
c.Assert(hostSelector.Matches(tt.expectedMatches), Equals, true, Commentf("Test Name: %s", tt.name))
c.Assert(hostSelector.Matches(tt.expectedNonMatches), Equals, false, Commentf("Test Name: %s", tt.name))
}
}
6 changes: 1 addition & 5 deletions pkg/policy/l4_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,14 @@ func (p *testPolicyContextType) IsDeny() bool {
}

var (
testPolicyContext = &testPolicyContextType{}
cachedRemoteNodeIdentitySetting bool
testPolicyContext = &testPolicyContextType{}
)

func (ds *PolicyTestSuite) SetUpSuite(c *C) {
cachedRemoteNodeIdentitySetting = option.Config.EnableRemoteNodeIdentity
option.Config.EnableRemoteNodeIdentity = true
re.InitRegexCompileLRU(defaults.FQDNRegexCompileLRUSize)
}

func (ds *PolicyTestSuite) TearDownSuite(c *C) {
option.Config.EnableRemoteNodeIdentity = cachedRemoteNodeIdentitySetting
}

// Tests in this file:
Expand Down
17 changes: 0 additions & 17 deletions pkg/policy/mapstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ var (
Identity: identity.ReservedIdentityHost.Uint32(),
TrafficDirection: trafficdirection.Ingress.Uint8(),
}
// localRemoteNodeKey represents an ingress L3 allow from remote nodes.
localRemoteNodeKey = Key{
Identity: identity.ReservedIdentityRemoteNode.Uint32(),
TrafficDirection: trafficdirection.Ingress.Uint8(),
}
// allKey represents a key for unknown traffic, i.e., all traffic.
allKey = Key{
Identity: identity.IdentityUnknown.Uint32(),
Expand Down Expand Up @@ -1296,18 +1291,6 @@ func (ms *mapState) determineAllowLocalhostIngress() {
}
es := NewMapStateEntry(nil, derivedFrom, 0, "", 0, false, ExplicitAuthType, AuthTypeDisabled) // Authentication never required for local host ingress
ms.denyPreferredInsert(localHostKey, es, nil, allFeatures)
if !option.Config.EnableRemoteNodeIdentity {
var isHostDenied bool
v, ok := ms.Get(localHostKey)
isHostDenied = ok && v.IsDeny
derivedFrom := labels.LabelArrayList{
labels.LabelArray{
labels.NewLabel(LabelKeyPolicyDerivedFrom, LabelAllowRemoteHostIngress, labels.LabelSourceReserved),
},
}
es := NewMapStateEntry(nil, derivedFrom, 0, "", 0, isHostDenied, ExplicitAuthType, AuthTypeDisabled) // Authentication never required for remote node ingress
ms.denyPreferredInsert(localRemoteNodeKey, es, nil, allFeatures)
}
}
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/policy/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,6 @@ func mergeIngress(policyCtx PolicyContext, ctx *SearchContext, fromEndpoints api
hostWildcardL7 := make([]string, 0, 2)
if option.Config.AlwaysAllowLocalhost() {
hostWildcardL7 = append(hostWildcardL7, labels.IDNameHost)
if !option.Config.EnableRemoteNodeIdentity {
hostWildcardL7 = append(hostWildcardL7, labels.IDNameRemoteNode)
}
}

var (
Expand Down
11 changes: 0 additions & 11 deletions test/k8s/manifests/cnp-from-entities-host.yaml

This file was deleted.

0 comments on commit 88e7a15

Please sign in to comment.