fix(kuma-cp) create default mesh resources when default mesh is skipp…

…ed (#1178)

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>

pkg/core/managers/apis/mesh/mesh_manager.go

@@ -80,7 +80,7 @@ func (m *meshManager) Create(ctx context.Context, resource core_model.Resource,
 	if err := m.store.Create(ctx, mesh, append(fs, core_store.CreatedAt(time.Now()))...); err != nil {
 		return err
 	}
-	if err := defaults_mesh.CreateDefaultMeshResources(m.otherManagers, opts.Name); err != nil {
+	if err := defaults_mesh.EnsureDefaultMeshResources(m.otherManagers, opts.Name); err != nil {
 		return err
 	}
 	return nil

pkg/defaults/mesh/mesh.go

@@ -5,23 +5,42 @@ import (
 
 	"github.com/kumahq/kuma/pkg/core"
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
+	"github.com/kumahq/kuma/pkg/tokens/builtin/issuer"
 )
 
 var log = core.Log.WithName("defaults").WithName("mesh")
 
-func CreateDefaultMeshResources(resManager manager.ResourceManager, meshName string) error {
-	log.Info("creating default resources for mesh", "mesh", meshName)
-	if err := createDefaultTrafficPermission(resManager, meshName); err != nil {
-		return errors.Wrap(err, "could not create default traffic permission")
+func EnsureDefaultMeshResources(resManager manager.ResourceManager, meshName string) error {
+	log.Info("ensuring default resources for Mesh exist", "mesh", meshName)
+
+	err, created := ensureDefaultTrafficPermission(resManager, meshName)
+	if err != nil {
+		return errors.Wrap(err, "could not create default TrafficPermission")
+	}
+	if created {
+		log.Info("default TrafficPermission created", "mesh", meshName, "name", defaultTrafficPermissionKey(meshName).Name)
+	} else {
+		log.Info("default TrafficPermission already exist", "mesh", meshName, "name", defaultTrafficPermissionKey(meshName).Name)
+	}
+
+	err, created = ensureDefaultTrafficRoute(resManager, meshName)
+	if err != nil {
+		return errors.Wrap(err, "could not create default TrafficRoute")
+	}
+	if created {
+		log.Info("default TrafficRoute created", "mesh", meshName, "name", defaultTrafficRouteKey(meshName).Name)
+	} else {
+		log.Info("default TrafficRoute already exist", "mesh", meshName, "name", defaultTrafficRouteKey(meshName).Name)
 	}
-	if err := createDefaultTrafficRoute(resManager, meshName); err != nil {
-		return errors.Wrap(err, "could not create default traffic permission")
+
+	created, err = ensureSigningKey(resManager, meshName)
+	if err != nil {
+		return errors.Wrap(err, "could not create default Signing Key")
 	}
-	log.Info("default TrafficPermission created", "mesh", meshName, "name", defaultTrafficPermissionName(meshName))
-	log.Info("creating Signing Key for mesh", "mesh", meshName)
-	if err := createSigningKey(resManager, meshName); err != nil {
-		return errors.Wrap(err, "could not create default signing key")
+	if created {
+		log.Info("default Signing Key created", "mesh", meshName, "name", issuer.SigningKeyResourceKey(meshName).Name)
+	} else {
+		log.Info("default Signing Key already exist", "mesh", meshName, "name", issuer.SigningKeyResourceKey(meshName).Name)
 	}
-	log.Info("Signing Key created", "mesh", meshName)
 	return nil
 }

pkg/defaults/mesh/mesh_suite_test.go

@@ -0,0 +1,13 @@
+package mesh_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestMesh(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Defaults Mesh")
+}

pkg/defaults/mesh/mesh_test.go

@@ -0,0 +1,68 @@
+package mesh_test
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
+	"github.com/kumahq/kuma/pkg/core/resources/apis/system"
+	"github.com/kumahq/kuma/pkg/core/resources/manager"
+	"github.com/kumahq/kuma/pkg/core/resources/model"
+	core_store "github.com/kumahq/kuma/pkg/core/resources/store"
+	"github.com/kumahq/kuma/pkg/defaults/mesh"
+	"github.com/kumahq/kuma/pkg/plugins/resources/memory"
+	"github.com/kumahq/kuma/pkg/tokens/builtin/issuer"
+)
+
+var _ = Describe("EnsureDefaultMeshResources", func() {
+
+	var resManager manager.ResourceManager
+
+	BeforeEach(func() {
+		store := memory.NewStore()
+		resManager = manager.NewResourceManager(store)
+
+		err := resManager.Create(context.Background(), &core_mesh.MeshResource{}, core_store.CreateByKey(model.DefaultMesh, model.NoMesh))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("should create default resources", func() {
+		// when
+		err := mesh.EnsureDefaultMeshResources(resManager, model.DefaultMesh)
+		Expect(err).ToNot(HaveOccurred())
+
+		// then default TrafficPermission for the mesh exist
+		err = resManager.Get(context.Background(), &core_mesh.TrafficPermissionResource{}, core_store.GetByKey("allow-all-default", model.DefaultMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		// and default TrafficRoute for the mesh exists
+		err = resManager.Get(context.Background(), &core_mesh.TrafficRouteResource{}, core_store.GetByKey("route-all-default", model.DefaultMesh))
+		Expect(err).ToNot(HaveOccurred())
+
+		// and Signing Key for the mesh exists
+		err = resManager.Get(context.Background(), &system.SecretResource{}, core_store.GetBy(issuer.SigningKeyResourceKey(model.DefaultMesh)))
+		Expect(err).ToNot(HaveOccurred())
+	})
+
+	It("should ignore subsequent calls to EnsureDefaultMeshResources", func() {
+		// given already ensured default resources
+		err := mesh.EnsureDefaultMeshResources(resManager, model.DefaultMesh)
+		Expect(err).ToNot(HaveOccurred())
+
+		// when ensuring again
+		err = mesh.EnsureDefaultMeshResources(resManager, model.DefaultMesh)
+
+		// then
+		Expect(err).ToNot(HaveOccurred())
+
+		// and all resources are in place
+		err = resManager.Get(context.Background(), &core_mesh.TrafficPermissionResource{}, core_store.GetByKey("allow-all-default", model.DefaultMesh))
+		Expect(err).ToNot(HaveOccurred())
+		err = resManager.Get(context.Background(), &core_mesh.TrafficRouteResource{}, core_store.GetByKey("route-all-default", model.DefaultMesh))
+		Expect(err).ToNot(HaveOccurred())
+		err = resManager.Get(context.Background(), &system.SecretResource{}, core_store.GetBy(issuer.SigningKeyResourceKey(model.DefaultMesh)))
+		Expect(err).ToNot(HaveOccurred())
+	})
+})

pkg/defaults/mesh/signing_key.go

@@ -3,18 +3,28 @@ package mesh
 import (
 	"context"
 
+	"github.com/pkg/errors"
+
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
 	core_store "github.com/kumahq/kuma/pkg/core/resources/store"
 	"github.com/kumahq/kuma/pkg/tokens/builtin/issuer"
 )
 
-func createSigningKey(resManager manager.ResourceManager, meshName string) error {
-	key, err := issuer.CreateSigningKey()
+func ensureSigningKey(resManager manager.ResourceManager, meshName string) (created bool, err error) {
+	signingKey, err := issuer.CreateSigningKey()
 	if err != nil {
-		return err
+		return false, errors.Wrap(err, "could not create a signing key")
+	}
+	key := issuer.SigningKeyResourceKey(meshName)
+	err = resManager.Get(context.Background(), &signingKey, core_store.GetBy(key))
+	if err == nil {
+		return false, nil
+	}
+	if !core_store.IsResourceNotFound(err) {
+		return false, errors.Wrap(err, "could not retrieve a resource")
 	}
-	if err := resManager.Create(context.Background(), &key, core_store.CreateBy(issuer.SigningKeyResourceKey(meshName))); err != nil {
-		return err
+	if err := resManager.Create(context.Background(), &signingKey, core_store.CreateBy(key)); err != nil {
+		return false, errors.Wrap(err, "could not create a resource")
 	}
-	return nil
+	return true, nil
 }

pkg/defaults/mesh/traffic_permission.go

@@ -4,9 +4,12 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pkg/errors"
+
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
 	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
+	"github.com/kumahq/kuma/pkg/core/resources/model"
 	"github.com/kumahq/kuma/pkg/core/resources/store"
 )
 
@@ -28,13 +31,27 @@ var defaultTrafficPermission = mesh_proto.TrafficPermission{
 }
 
 // TrafficPermission needs to contain mesh name inside it. Otherwise if the name is the same (ex. "allow-all") creating new mesh would fail because there is already resource of name "allow-all" which is unique key on K8S
-func defaultTrafficPermissionName(meshName string) string {
-	return fmt.Sprintf("allow-all-%s", meshName)
+func defaultTrafficPermissionKey(meshName string) model.ResourceKey {
+	return model.ResourceKey{
+		Mesh: meshName,
+		Name: fmt.Sprintf("allow-all-%s", meshName),
+	}
 }
 
-func createDefaultTrafficPermission(resManager manager.ResourceManager, meshName string) error {
+func ensureDefaultTrafficPermission(resManager manager.ResourceManager, meshName string) (err error, created bool) {
+	key := defaultTrafficPermissionKey(meshName)
 	tp := &core_mesh.TrafficPermissionResource{
 		Spec: defaultTrafficPermission,
 	}
-	return resManager.Create(context.Background(), tp, store.CreateByKey(defaultTrafficPermissionName(meshName), meshName))
+	err = resManager.Get(context.Background(), tp, store.GetBy(key))
+	if err == nil {
+		return nil, false
+	}
+	if !store.IsResourceNotFound(err) {
+		return errors.Wrap(err, "could not retrieve a resource"), false
+	}
+	if err := resManager.Create(context.Background(), tp, store.CreateBy(key)); err != nil {
+		return errors.Wrap(err, "could not create a resource"), false
+	}
+	return nil, true
 }

pkg/defaults/mesh/traffic_route.go

@@ -4,7 +4,10 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pkg/errors"
+
 	mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
+	"github.com/kumahq/kuma/pkg/core/resources/model"
 
 	core_mesh "github.com/kumahq/kuma/pkg/core/resources/apis/mesh"
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
@@ -29,13 +32,27 @@ var (
 )
 
 // TrafficRoute needs to contain mesh name inside it. Otherwise if the name is the same (ex. "allow-all") creating new mesh would fail because there is already resource of name "allow-all" which is unique key on K8S
-func defaultTrafficRouteName(meshName string) string {
-	return fmt.Sprintf("route-all-%s", meshName)
+func defaultTrafficRouteKey(meshName string) model.ResourceKey {
+	return model.ResourceKey{
+		Mesh: meshName,
+		Name: fmt.Sprintf("route-all-%s", meshName),
+	}
 }
 
-func createDefaultTrafficRoute(resManager manager.ResourceManager, meshName string) error {
-	tp := &core_mesh.TrafficRouteResource{
+func ensureDefaultTrafficRoute(resManager manager.ResourceManager, meshName string) (err error, created bool) {
+	tr := &core_mesh.TrafficRouteResource{
 		Spec: defaultTrafficRoute,
 	}
-	return resManager.Create(context.Background(), tp, store.CreateByKey(defaultTrafficRouteName(meshName), meshName))
+	key := defaultTrafficRouteKey(meshName)
+	err = resManager.Get(context.Background(), tr, store.GetBy(key))
+	if err == nil {
+		return nil, false
+	}
+	if !store.IsResourceNotFound(err) {
+		return errors.Wrap(err, "could not retrieve a resource"), false
+	}
+	if err := resManager.Create(context.Background(), tr, store.CreateBy(key)); err != nil {
+		return errors.Wrap(err, "could not create a resource"), false
+	}
+	return nil, true
 }

pkg/plugins/runtime/k8s/controllers/mesh_defaults_controller.go

@@ -9,23 +9,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"github.com/kumahq/kuma/pkg/core/resources/manager"
-	"github.com/kumahq/kuma/pkg/core/resources/model"
 	defaults_mesh "github.com/kumahq/kuma/pkg/defaults/mesh"
 	mesh_k8s "github.com/kumahq/kuma/pkg/plugins/resources/k8s/native/api/v1alpha1"
 )
 
-// MeshDefaultsReconciler creates default resources for a created Mesh
+// MeshDefaultsReconciler creates default resources for created Mesh
 type MeshDefaultsReconciler struct {
 	ResourceManager manager.ResourceManager
 }
 
 func (r *MeshDefaultsReconciler) Reconcile(req kube_ctrl.Request) (kube_ctrl.Result, error) {
-	// we skip default name Mesh because pkg/defaults/mesh.go uses ResourceManager that executes CreateDefaultMeshResources already
-	if req.Name == model.DefaultMesh {
-		return kube_ctrl.Result{}, nil
-	}
-
-	if err := defaults_mesh.CreateDefaultMeshResources(r.ResourceManager, req.Name); err != nil {
+	if err := defaults_mesh.EnsureDefaultMeshResources(r.ResourceManager, req.Name); err != nil {
 		return kube_ctrl.Result{}, errors.Wrap(err, "could not create default mesh resources")
 	}
 	return kube_ctrl.Result{}, nil
@@ -43,6 +37,7 @@ func (r *MeshDefaultsReconciler) SetupWithManager(mgr kube_ctrl.Manager) error {
 		Complete(r)
 }
 
+// we only want to react on Create events. User may want to delete default resources, we don't want to add them again when they update the Mesh
 var onlyCreate = predicate.Funcs{
 	CreateFunc: func(event event.CreateEvent) bool {
 		return true

test/e2e/kuma_helm_deploy_multi_apps_test.go

@@ -30,6 +30,13 @@ metadata:
 `, namespace)
 	}
 
+	defaultMesh := `
+apiVersion: kuma.io/v1alpha1
+kind: Mesh
+metadata:
+  name: default
+`
+
 	var cluster Cluster
 	var deployOptsFuncs []DeployOptionsFunc
 
@@ -48,12 +55,14 @@ metadata:
 		deployOptsFuncs = []DeployOptionsFunc{
 			WithInstallationMode(HelmInstallationMode),
 			WithHelmReleaseName(releaseName),
+			WithSkipDefaultMesh(true), // it's common case for HELM deployments that Mesh is also managed by HELM therefore it's not created by default
 			WithCNI(),
 		}
 
 		err = NewClusterSetup().
 			Install(Kuma(core.Standalone, deployOptsFuncs...)).
 			Install(KumaDNS()).
+			Install(YamlK8s(defaultMesh)).
 			Setup(cluster)
 		Expect(err).ToNot(HaveOccurred())
 

test/framework/interface.go

@@ -21,6 +21,7 @@ type deployOptions struct {
 	// cp specific
 	globalAddress    string
 	installationMode InstallationMode
+	skipDefaultMesh  bool
 	helmReleaseName  string
 	helmOpts         map[string]string
 	ctlOpts          map[string]string
@@ -43,6 +44,12 @@ func WithGlobalAddress(address string) DeployOptionsFunc {
 	}
 }
 
+func WithSkipDefaultMesh(skip bool) DeployOptionsFunc {
+	return func(o *deployOptions) {
+		o.skipDefaultMesh = skip
+	}
+}
+
 func WithInstallationMode(mode InstallationMode) DeployOptionsFunc {
 	return func(o *deployOptions) {
 		o.installationMode = mode