Skip to content

Commit

Permalink
identity: Remove dependency on pkg/option
Browse files Browse the repository at this point in the history
pkg/identity is being used by external dependencies, requiring
pkg/option to present certain leads to incorrect behavior. Require a
configuration interface to be implemented. This also helps unit testing.

Signed-off-by: Thomas Graf <thomas@cilium.io>
  • Loading branch information
tgraf authored and aanm committed Apr 15, 2020
1 parent 9a6f60a commit 1000f70
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 51 deletions.
2 changes: 1 addition & 1 deletion daemon/cmd/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func NewDaemon(ctx context.Context, dp datapath.Datapath) (*Daemon, *endpointRes
})
if option.Config.EnableWellKnownIdentities {
// Must be done before calling policy.NewPolicyRepository() below.
num := identity.InitWellKnownIdentities()
num := identity.InitWellKnownIdentities(option.Config)
metrics.IdentityCount.Add(float64(num))
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/clustermesh/clustermesh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/cilium/cilium/pkg/kvstore"
"github.com/cilium/cilium/pkg/kvstore/store"
"github.com/cilium/cilium/pkg/lock"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"
"github.com/cilium/cilium/pkg/testutils"
"github.com/cilium/cilium/pkg/testutils/allocator"

Expand Down Expand Up @@ -102,7 +103,7 @@ func (s *ClusterMeshTestSuite) TestClusterMesh(c *C) {
kvstore.SetupDummy("etcd")
defer kvstore.Client().Close()

identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
// The nils are only used by k8s CRD identities. We default to kvstore.
mgr := cache.NewCachingIdentityAllocator(&allocator.IdentityAllocatorOwnerMock{})
<-mgr.InitIdentityAllocator(nil, nil)
Expand Down
3 changes: 2 additions & 1 deletion pkg/clustermesh/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/cilium/cilium/pkg/kvstore"
"github.com/cilium/cilium/pkg/loadbalancer"
"github.com/cilium/cilium/pkg/lock"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"
"github.com/cilium/cilium/pkg/rand"
"github.com/cilium/cilium/pkg/testutils"
"github.com/cilium/cilium/pkg/testutils/allocator"
Expand Down Expand Up @@ -68,7 +69,7 @@ func (s *ClusterMeshServicesTestSuite) SetUpTest(c *C) {

kvstore.Client().DeletePrefix(context.TODO(), "cilium/state/services/v1/"+s.randomName)
s.svcCache = k8s.NewServiceCache(fakeDatapath.NewNodeAddressing())
identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})

mgr := cache.NewCachingIdentityAllocator(&allocator.IdentityAllocatorOwnerMock{})
// The nils are only used by k8s CRD identities. We default to kvstore.
Expand Down
23 changes: 23 additions & 0 deletions pkg/clustermesh/types/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2016-2020 Authors of Cilium
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package types

const (
// ClusterIDMin is the minimum value of the cluster ID
ClusterIDMin = 0

// ClusterIDMax is the maximum value of the cluster ID
ClusterIDMax = 255
)
3 changes: 2 additions & 1 deletion pkg/endpoint/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/cilium/cilium/pkg/lock"
monitorAPI "github.com/cilium/cilium/pkg/monitor/api"
"github.com/cilium/cilium/pkg/option"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"
"github.com/cilium/cilium/pkg/policy"
"github.com/cilium/cilium/pkg/policy/api"
"github.com/cilium/cilium/pkg/testutils/allocator"
Expand Down Expand Up @@ -103,7 +104,7 @@ func (s *EndpointSuite) Datapath() datapath.Datapath {
func (s *EndpointSuite) SetUpTest(c *C) {
/* Required to test endpoint CEP policy model */
kvstore.SetupDummy("etcd")
identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
// The nils are only used by k8s CRD identities. We default to kvstore.
mgr := cache.NewCachingIdentityAllocator(&allocator.IdentityAllocatorOwnerMock{})
<-mgr.InitIdentityAllocator(nil, nil)
Expand Down
3 changes: 2 additions & 1 deletion pkg/endpoint/redirect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/lock"
monitorAPI "github.com/cilium/cilium/pkg/monitor/api"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"
"github.com/cilium/cilium/pkg/policy"
"github.com/cilium/cilium/pkg/policy/trafficdirection"
"github.com/cilium/cilium/pkg/proxy/logger"
Expand Down Expand Up @@ -130,7 +131,7 @@ func (s *RedirectSuite) TestAddVisibilityRedirects(c *check.C) {
kvstore.SetupDummy("etcd")
defer kvstore.Client().Close()

identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
idAllocatorOwner := &DummyIdentityAllocatorOwner{}

mgr := cache.NewCachingIdentityAllocator(idAllocatorOwner)
Expand Down
7 changes: 4 additions & 3 deletions pkg/identity/cache/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cilium/cilium/pkg/kvstore"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/lock"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"

. "gopkg.in/check.v1"
)
Expand Down Expand Up @@ -225,7 +226,7 @@ func (ias *IdentityAllocatorSuite) TestEventWatcherBatching(c *C) {
}

func (ias *IdentityAllocatorSuite) TestGetIdentityCache(c *C) {
identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
// The nils are only used by k8s CRD identities. We default to kvstore.
mgr := NewCachingIdentityAllocator(newDummyOwner())
<-mgr.InitIdentityAllocator(nil, nil)
Expand All @@ -243,7 +244,7 @@ func (ias *IdentityAllocatorSuite) TestAllocator(c *C) {
lbls3 := labels.NewLabelsFromSortedList("id=bar;user=susan")

owner := newDummyOwner()
identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
// The nils are only used by k8s CRD identities. We default to kvstore.
mgr := NewCachingIdentityAllocator(owner)
<-mgr.InitIdentityAllocator(nil, nil)
Expand Down Expand Up @@ -328,7 +329,7 @@ func (ias *IdentityAllocatorSuite) TestLocalAllocation(c *C) {
lbls1 := labels.NewLabelsFromSortedList("cidr:192.0.2.3/32")

owner := newDummyOwner()
identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})
// The nils are only used by k8s CRD identities. We default to kvstore.
mgr := NewCachingIdentityAllocator(owner)
<-mgr.InitIdentityAllocator(nil, nil)
Expand Down
14 changes: 2 additions & 12 deletions pkg/identity/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (

"github.com/cilium/cilium/pkg/identity"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/option"
fakeConfig "github.com/cilium/cilium/pkg/option/fake"

. "gopkg.in/check.v1"
)
Expand All @@ -47,17 +47,7 @@ type IdentityCacheTestSuite struct{}

var _ = Suite(&IdentityCacheTestSuite{})

func (s *IdentityCacheTestSuite) SetUpTest(c *C) {
option.Config.K8sNamespace = "kube-system"
}

func (s *IdentityCacheTestSuite) TestLookupReservedIdentity(c *C) {
bak := option.Config.ClusterName
option.Config.ClusterName = "default"
defer func() {
option.Config.ClusterName = bak
}()

mgr := NewCachingIdentityAllocator(newDummyOwner())
<-mgr.InitIdentityAllocator(nil, nil)

Expand All @@ -75,7 +65,7 @@ func (s *IdentityCacheTestSuite) TestLookupReservedIdentity(c *C) {
c.Assert(id, Not(IsNil))
c.Assert(id.ID, Equals, worldID)

identity.InitWellKnownIdentities()
identity.InitWellKnownIdentities(&fakeConfig.Config{})

id = mgr.LookupIdentity(context.TODO(), kvstoreLabels)
c.Assert(id, Not(IsNil))
Expand Down
35 changes: 18 additions & 17 deletions pkg/identity/numericidentity.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

api "github.com/cilium/cilium/pkg/k8s/apis/cilium.io"
"github.com/cilium/cilium/pkg/labels"
"github.com/cilium/cilium/pkg/option"
)

const (
Expand Down Expand Up @@ -147,22 +146,24 @@ func (w wellKnownIdentities) lookupByNumericIdentity(identity NumericIdentity) *
return wki.identity
}

type Configuration interface {
LocalClusterName() string
CiliumNamespaceName() string
}

// InitWellKnownIdentities establishes all well-known identities. Returns the
// number of well-known identities initialized.
func InitWellKnownIdentities() int {
// Derive the namespace in which the Cilium components are running
namespace := option.Config.K8sNamespace

func InitWellKnownIdentities(c Configuration) int {
// etcd-operator labels
// k8s:io.cilium.k8s.policy.serviceaccount=cilium-etcd-sa
// k8s:io.kubernetes.pod.namespace=<NAMESPACE>
// k8s:io.cilium/app=etcd-operator
// k8s:io.cilium.k8s.policy.cluster=default
WellKnown.add(ReservedETCDOperator, []string{
"k8s:io.cilium/app=etcd-operator",
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, namespace),
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, c.CiliumNamespaceName()),
fmt.Sprintf("k8s:%s=cilium-etcd-sa", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// cilium-etcd labels
Expand All @@ -179,9 +180,9 @@ func InitWellKnownIdentities() int {
"k8s:app=etcd",
"k8s:etcd_cluster=cilium-etcd",
"k8s:io.cilium/app=etcd-operator",
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, namespace),
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, c.CiliumNamespaceName()),
fmt.Sprintf("k8s:%s=default", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// kube-dns labels
Expand All @@ -193,7 +194,7 @@ func InitWellKnownIdentities() int {
"k8s:k8s-app=kube-dns",
fmt.Sprintf("k8s:%s=kube-system", api.PodNamespaceLabel),
fmt.Sprintf("k8s:%s=kube-dns", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// kube-dns EKS labels
Expand All @@ -207,7 +208,7 @@ func InitWellKnownIdentities() int {
"k8s:eks.amazonaws.com/component=kube-dns",
fmt.Sprintf("k8s:%s=kube-system", api.PodNamespaceLabel),
fmt.Sprintf("k8s:%s=kube-dns", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// kube-dns EKS labels
Expand All @@ -221,7 +222,7 @@ func InitWellKnownIdentities() int {
"k8s:eks.amazonaws.com/component=coredns",
fmt.Sprintf("k8s:%s=kube-system", api.PodNamespaceLabel),
fmt.Sprintf("k8s:%s=coredns", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// CoreDNS labels
Expand All @@ -233,7 +234,7 @@ func InitWellKnownIdentities() int {
"k8s:k8s-app=kube-dns",
fmt.Sprintf("k8s:%s=kube-system", api.PodNamespaceLabel),
fmt.Sprintf("k8s:%s=coredns", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// CiliumOperator labels
Expand All @@ -245,9 +246,9 @@ func InitWellKnownIdentities() int {
WellKnown.add(ReservedCiliumOperator, []string{
"k8s:name=cilium-operator",
"k8s:io.cilium/app=operator",
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, namespace),
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, c.CiliumNamespaceName()),
fmt.Sprintf("k8s:%s=cilium-operator", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

// cilium-etcd-operator labels
Expand All @@ -259,9 +260,9 @@ func InitWellKnownIdentities() int {
WellKnown.add(ReservedCiliumEtcdOperator, []string{
"k8s:name=cilium-etcd-operator",
"k8s:io.cilium/app=etcd-operator",
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, namespace),
fmt.Sprintf("k8s:%s=%s", api.PodNamespaceLabel, c.CiliumNamespaceName()),
fmt.Sprintf("k8s:%s=cilium-etcd-operator", api.PolicyLabelServiceAccount),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, option.Config.ClusterName),
fmt.Sprintf("k8s:%s=%s", api.PolicyLabelCluster, c.LocalClusterName()),
})

return len(WellKnown)
Expand Down
12 changes: 6 additions & 6 deletions pkg/identity/numericidentity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package identity

import (
"github.com/cilium/cilium/pkg/option"
"github.com/cilium/cilium/pkg/clustermesh/types"

. "gopkg.in/check.v1"
)
Expand All @@ -26,7 +26,7 @@ func (s *IdentityTestSuite) TestLocalIdentity(c *C) {
localID := NumericIdentity(LocalIdentityFlag | 1)
c.Assert(localID.HasLocalScope(), Equals, true)

maxClusterID := NumericIdentity(option.ClusterIDMax | 1)
maxClusterID := NumericIdentity(types.ClusterIDMax | 1)
c.Assert(maxClusterID.HasLocalScope(), Equals, false)

c.Assert(ReservedIdentityWorld.HasLocalScope(), Equals, false)
Expand Down Expand Up @@ -54,12 +54,12 @@ func (s *IdentityTestSuite) TestClusterID(c *C) {
clusterID: 255,
},
{ // make sure we support min/max configuration values
identity: option.ClusterIDMin << 16,
clusterID: option.ClusterIDMin,
identity: types.ClusterIDMin << 16,
clusterID: types.ClusterIDMin,
},
{
identity: option.ClusterIDMax << 16,
clusterID: option.ClusterIDMax,
identity: types.ClusterIDMax << 16,
clusterID: types.ClusterIDMax,
},
}

Expand Down
22 changes: 14 additions & 8 deletions pkg/option/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/cilium/cilium/api/v1/models"
"github.com/cilium/cilium/common"
"github.com/cilium/cilium/pkg/cidr"
clustermeshTypes "github.com/cilium/cilium/pkg/clustermesh/types"
"github.com/cilium/cilium/pkg/defaults"
"github.com/cilium/cilium/pkg/ip"
"github.com/cilium/cilium/pkg/lock"
Expand Down Expand Up @@ -494,12 +495,6 @@ const (
// ClusterIDName is the name of the ClusterID option
ClusterIDName = "cluster-id"

// ClusterIDMin is the minimum value of the cluster ID
ClusterIDMin = 0

// ClusterIDMax is the maximum value of the cluster ID
ClusterIDMax = 255

// ClusterMeshConfigName is the name of the ClusterMeshConfig option
ClusterMeshConfigName = "clustermesh-config"

Expand Down Expand Up @@ -2099,6 +2094,17 @@ func (c *DaemonConfig) EndpointStatusIsEnabled(option string) bool {
return ok
}

// LocalClusterName returns the name of the cluster Cilium is deployed in
func (c *DaemonConfig) LocalClusterName() string {
return c.ClusterName
}

// CiliumNamespaceName returns the name of the namespace in which Cilium is
// deployed in
func (c *DaemonConfig) CiliumNamespaceName() string {
return c.K8sNamespace
}

func (c *DaemonConfig) validateIPv6ClusterAllocCIDR() error {
ip, cidr, err := net.ParseCIDR(c.IPv6ClusterAllocCIDR)
if err != nil {
Expand Down Expand Up @@ -2144,9 +2150,9 @@ func (c *DaemonConfig) Validate() error {
return fmt.Errorf("invalid tunnel mode '%s', valid modes = {%s}", c.Tunnel, GetTunnelModes())
}

if c.ClusterID < ClusterIDMin || c.ClusterID > ClusterIDMax {
if c.ClusterID < clustermeshTypes.ClusterIDMin || c.ClusterID > clustermeshTypes.ClusterIDMax {
return fmt.Errorf("invalid cluster id %d: must be in range %d..%d",
c.ClusterID, ClusterIDMin, ClusterIDMax)
c.ClusterID, clustermeshTypes.ClusterIDMin, clustermeshTypes.ClusterIDMax)
}

if c.ClusterID != 0 {
Expand Down
28 changes: 28 additions & 0 deletions pkg/option/fake/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2020 Authors of Cilium
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package fake

type Config struct{}

// LocalClusterName returns the name of the cluster Cilium is deployed in
func (f *Config) LocalClusterName() string {
return "default"
}

// CiliumNamespaceName returns the name of the namespace in which Cilium is
// deployed in
func (f *Config) CiliumNamespaceName() string {
return "kube-system"
}

0 comments on commit 1000f70

Please sign in to comment.