Merge pull request #2130 from FabianKramm/main

refactor: make sure mappings are always correct

pkg/mappings/generic/mapper.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/loft-sh/vcluster/pkg/mappings"
+	"github.com/loft-sh/vcluster/pkg/mappings/store/verify"
 	"github.com/loft-sh/vcluster/pkg/scheme"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
 	"github.com/loft-sh/vcluster/pkg/util/translate"
@@ -81,6 +82,13 @@ func (n *mapper) VirtualToHost(ctx *synccontext.SyncContext, req types.Namespace
 }
 
 func (n *mapper) HostToVirtual(ctx *synccontext.SyncContext, req types.NamespacedName, pObj client.Object) (retName types.NamespacedName) {
+	if !verify.CheckHostObject(ctx, synccontext.Object{
+		GroupVersionKind: n.gvk,
+		NamespacedName:   req,
+	}) {
+		return types.NamespacedName{}
+	}
+
 	vName := TryToTranslateBackByAnnotations(ctx, req, pObj, n.gvk)
 	if vName.Name != "" {
 		return vName
@@ -272,5 +280,12 @@ func tryToFindHostNameShortInStore(ctx *synccontext.SyncContext, pName types.Nam
 }
 
 func (n *mapper) IsManaged(ctx *synccontext.SyncContext, pObj client.Object) (bool, error) {
+	if !verify.CheckHostObject(ctx, synccontext.Object{
+		GroupVersionKind: n.gvk,
+		NamespacedName:   client.ObjectKeyFromObject(pObj),
+	}) {
+		return false, nil
+	}
+
 	return translate.Default.IsManaged(ctx, pObj), nil
 }

pkg/mappings/generic/mapper_test.go

@@ -335,7 +335,7 @@ func TestTryToTranslateBackByName(t *testing.T) {
 	secretMapping := synccontext.NameMapping{
 		GroupVersionKind: gvk,
 		VirtualName:      types.NamespacedName{Name: "virtual-name", Namespace: "virtual-namespace"},
-		HostName:         types.NamespacedName{Name: "host-name", Namespace: "host-namespace"},
+		HostName:         types.NamespacedName{Name: "host-name", Namespace: targetNamespace},
 	}
 	syncContext.Context = synccontext.WithMapping(syncContext.Context, secretMapping)
 	assert.Equal(t, TryToTranslateBackByName(syncContext, secretMapping.HostName, gvk).String(), types.NamespacedName{}.String())
@@ -352,7 +352,7 @@ func TestTryToTranslateBackByName(t *testing.T) {
 	assert.NilError(t, err)
 	vConfig.Experimental.MultiNamespaceMode.Enabled = true
 	req := types.NamespacedName{
-		Namespace: "test",
+		Namespace: targetNamespace,
 		Name:      "test",
 	}
 	assert.Equal(t, TryToTranslateBackByName(syncContext, req, gvk).String(), req.String())

pkg/mappings/resources/namespaces_test.go

@@ -13,10 +13,97 @@ import (
 	"gotest.tools/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
+func TestNamespaceMapperHostToVirtual(t *testing.T) {
+	type testCase struct {
+		Name string
+
+		MultiNamespaceMode bool
+
+		Object client.Object
+
+		ExpectedMapping types.NamespacedName
+	}
+	var testCases = []testCase{
+		{
+			Name: "Simple multi-namespace",
+
+			MultiNamespaceMode: true,
+
+			Object: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "host-namespace-1",
+					Annotations: map[string]string{
+						translate.NameAnnotation: "virtual-namespace-1",
+						translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Namespace").String(),
+					},
+				},
+			},
+		},
+		{
+			Name: "Simple multi-namespace translated",
+
+			MultiNamespaceMode: true,
+
+			Object: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: translate.NewMultiNamespaceTranslator(testingutil.DefaultTestTargetNamespace).HostNamespace(nil, "virtual-namespace-1"),
+					Annotations: map[string]string{
+						translate.NameAnnotation: "virtual-namespace-1",
+						translate.KindAnnotation: corev1.SchemeGroupVersion.WithKind("Namespace").String(),
+					},
+				},
+			},
+
+			ExpectedMapping: types.NamespacedName{Name: "virtual-namespace-1"},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.Name, func(t *testing.T) {
+			storeBackend := store.NewMemoryBackend()
+			mappingsStore, err := store.NewStore(context.TODO(), nil, nil, storeBackend)
+			assert.NilError(t, err)
+
+			vConfig := testingutil.NewFakeConfig()
+			mappingsRegistry := mappings.NewMappingsRegistry(mappingsStore)
+			if testCase.MultiNamespaceMode {
+				translate.Default = translate.NewMultiNamespaceTranslator(testingutil.DefaultTestTargetNamespace)
+				vConfig.Experimental.MultiNamespaceMode.Enabled = true
+			} else {
+				translate.Default = translate.NewSingleNamespaceTranslator(testingutil.DefaultTestTargetNamespace)
+			}
+
+			// check recording
+			registerContext := &synccontext.RegisterContext{
+				Context:         context.TODO(),
+				Config:          vConfig,
+				Mappings:        mappingsRegistry,
+				PhysicalManager: testingutil.NewFakeManager(testingutil.NewFakeClient(scheme.Scheme)),
+				VirtualManager:  testingutil.NewFakeManager(testingutil.NewFakeClient(scheme.Scheme)),
+			}
+
+			// create namespace mapper
+			namespaceMapper, err := CreateNamespacesMapper(registerContext)
+			assert.NilError(t, err)
+			err = mappingsRegistry.AddMapper(namespaceMapper)
+			assert.NilError(t, err)
+
+			// create objects
+			err = registerContext.PhysicalManager.GetClient().Create(registerContext, testCase.Object)
+			assert.NilError(t, err)
+
+			// migrate
+			vName := namespaceMapper.HostToVirtual(registerContext.ToSyncContext("my-log"), client.ObjectKeyFromObject(testCase.Object), testCase.Object)
+			assert.Equal(t, vName.String(), testCase.ExpectedMapping.String())
+		})
+	}
+}
+
 func TestNamespaceMapperMigrate(t *testing.T) {
 	type testCase struct {
 		Name string

pkg/mappings/store/store.go

@@ -22,10 +22,18 @@ import (
 
 const GarbageCollectionInterval = time.Minute * 3
 
+type VerifyMapping func(mapping synccontext.NameMapping) bool
+
 func NewStore(ctx context.Context, cachedVirtualClient, cachedHostClient client.Client, backend Backend) (synccontext.MappingsStore, error) {
+	return NewStoreWithVerifyMapping(ctx, cachedVirtualClient, cachedHostClient, backend, nil)
+}
+
+func NewStoreWithVerifyMapping(ctx context.Context, cachedVirtualClient, cachedHostClient client.Client, backend Backend, verifyMapping VerifyMapping) (synccontext.MappingsStore, error) {
 	store := &Store{
 		backend: backend,
 
+		verifyMapping: verifyMapping,
+
 		sender: uuid.NewString(),
 
 		cachedVirtualClient: cachedVirtualClient,
@@ -51,6 +59,8 @@ func NewStore(ctx context.Context, cachedVirtualClient, cachedHostClient client.
 type Store struct {
 	m sync.RWMutex
 
+	verifyMapping VerifyMapping
+
 	sender string
 
 	cachedVirtualClient client.Client
@@ -192,6 +202,11 @@ func (s *Store) start(ctx context.Context) error {
 	}
 
 	for _, mapping := range mappings {
+		// verify mapping if needed
+		if s.verifyMapping != nil && !s.verifyMapping(mapping.NameMapping) {
+			continue
+		}
+
 		oldMapping, ok := s.mappings[mapping.NameMapping]
 		if ok {
 			s.removeMapping(oldMapping)
@@ -229,6 +244,11 @@ func (s *Store) handleEvent(ctx context.Context, watchEvent BackendWatchResponse
 			continue
 		}
 
+		// verify mapping if needed
+		if s.verifyMapping != nil && !s.verifyMapping(event.Mapping.NameMapping) {
+			continue
+		}
+
 		klog.FromContext(ctx).V(1).Info("mapping store received event", "type", event.Type, "mapping", event.Mapping.String())
 
 		// remove mapping in any case
@@ -344,6 +364,11 @@ func (s *Store) AddReference(ctx context.Context, nameMapping, belongsTo synccon
 		return err
 	}
 
+	// verify mapping if needed
+	if s.verifyMapping != nil && !s.verifyMapping(nameMapping) {
+		return nil
+	}
+
 	// check if there is already a mapping
 	mapping, ok := s.findMapping(belongsTo)
 	if !ok {

pkg/mappings/store/verify/verify.go

@@ -0,0 +1,28 @@
+package verify
+
+import (
+	"github.com/loft-sh/vcluster/pkg/mappings/store"
+	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
+	"github.com/loft-sh/vcluster/pkg/util/translate"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func NewVerifyMapping(ctx *synccontext.SyncContext) store.VerifyMapping {
+	return func(mapping synccontext.NameMapping) bool {
+		return CheckHostObject(ctx, mapping.Host())
+	}
+}
+
+func CheckHostObject(ctx *synccontext.SyncContext, hostObject synccontext.Object) bool {
+	// we don't allow mappings that are not within targeted namespaces
+	if hostObject.Namespace != "" && !translate.Default.IsTargetedNamespace(ctx, hostObject.Namespace) {
+		return false
+	}
+
+	// we don't allow namespace mappings that are not within targeted namespaces
+	if hostObject.GroupVersionKind.String() == corev1.SchemeGroupVersion.WithKind("Namespace").String() && !translate.Default.IsTargetedNamespace(ctx, hostObject.Name) {
+		return false
+	}
+
+	return true
+}

pkg/setup/controller_context.go

@@ -11,13 +11,13 @@ import (
 	"github.com/loft-sh/vcluster/pkg/etcd"
 	"github.com/loft-sh/vcluster/pkg/mappings"
 	"github.com/loft-sh/vcluster/pkg/mappings/store"
+	"github.com/loft-sh/vcluster/pkg/mappings/store/verify"
 	"github.com/loft-sh/vcluster/pkg/plugin"
 	"github.com/loft-sh/vcluster/pkg/pro"
 	"github.com/loft-sh/vcluster/pkg/scheme"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
 	"github.com/loft-sh/vcluster/pkg/telemetry"
 	"github.com/loft-sh/vcluster/pkg/util/blockingcacheclient"
-	translatepro "github.com/loft-sh/vcluster/pkg/util/translate/pro"
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -111,8 +111,6 @@ func getLocalCacheOptions(options *config.VirtualClusterConfig) cache.Options {
 	defaultNamespaces := make(map[string]cache.Config)
 	if !options.Experimental.MultiNamespaceMode.Enabled {
 		defaultNamespaces[options.WorkloadTargetNamespace] = cache.Config{}
-
-		translatepro.AddMappingsToCache(defaultNamespaces)
 	}
 	// do we need access to another namespace to export the kubeconfig ?
 	// we will need access to all the objects that the vcluster usually has access to
@@ -328,12 +326,7 @@ func initControllerContext(
 		return nil, fmt.Errorf("create etcd client: %w", err)
 	}
 
-	mappingStore, err := store.NewStore(ctx, virtualManager.GetClient(), localManager.GetClient(), store.NewEtcdBackend(etcdClient))
-	if err != nil {
-		return nil, fmt.Errorf("start mapping store: %w", err)
-	}
-
-	return &synccontext.ControllerContext{
+	controllerContext := &synccontext.ControllerContext{
 		Context:               ctx,
 		LocalManager:          localManager,
 		VirtualManager:        virtualManager,
@@ -342,11 +335,17 @@ func initControllerContext(
 
 		WorkloadNamespaceClient: currentNamespaceClient,
 
-		Mappings: mappings.NewMappingsRegistry(mappingStore),
-
 		StopChan: stopChan,
 		Config:   vClusterOptions,
-	}, nil
+	}
+
+	mappingStore, err := store.NewStoreWithVerifyMapping(ctx, virtualManager.GetClient(), localManager.GetClient(), store.NewEtcdBackend(etcdClient), verify.NewVerifyMapping(controllerContext.ToRegisterContext().ToSyncContext("verify-mapping")))
+	if err != nil {
+		return nil, fmt.Errorf("start mapping store: %w", err)
+	}
+
+	controllerContext.Mappings = mappings.NewMappingsRegistry(mappingStore)
+	return controllerContext, nil
 }
 
 func newCurrentNamespaceClient(ctx context.Context, localManager ctrl.Manager, options *config.VirtualClusterConfig) (client.Client, error) {

pkg/syncer/testing/context.go

@@ -8,6 +8,7 @@ import (
 	"github.com/loft-sh/vcluster/pkg/mappings"
 	"github.com/loft-sh/vcluster/pkg/mappings/resources"
 	"github.com/loft-sh/vcluster/pkg/mappings/store"
+	"github.com/loft-sh/vcluster/pkg/mappings/store/verify"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
 	syncer "github.com/loft-sh/vcluster/pkg/syncer/types"
 	"github.com/loft-sh/vcluster/pkg/util"
@@ -50,7 +51,6 @@ func FakeStartSyncer(t *testing.T, ctx *synccontext.RegisterContext, create func
 
 func NewFakeRegisterContext(vConfig *config.VirtualClusterConfig, pClient *testingutil.FakeIndexClient, vClient *testingutil.FakeIndexClient) *synccontext.RegisterContext {
 	ctx := context.Background()
-	mappingsStore, _ := store.NewStore(ctx, vClient, pClient, store.NewMemoryBackend())
 
 	// create register context
 	translate.Default = translate.NewSingleNamespaceTranslator(testingutil.DefaultTestTargetNamespace)
@@ -61,9 +61,12 @@ func NewFakeRegisterContext(vConfig *config.VirtualClusterConfig, pClient *testi
 		CurrentNamespaceClient: pClient,
 		VirtualManager:         testingutil.NewFakeManager(vClient),
 		PhysicalManager:        testingutil.NewFakeManager(pClient),
-		Mappings:               mappings.NewMappingsRegistry(mappingsStore),
 	}
 
+	// create new store
+	mappingsStore, _ := store.NewStoreWithVerifyMapping(ctx, vClient, pClient, store.NewMemoryBackend(), verify.NewVerifyMapping(registerCtx.ToSyncContext("verify-mapping")))
+	registerCtx.Mappings = mappings.NewMappingsRegistry(mappingsStore)
+
 	// make sure we do not ensure any CRDs
 	util.EnsureCRD = func(_ context.Context, _ *rest.Config, _ []byte, _ schema.GroupVersionKind) error {
 		return nil

pkg/util/translate/multi_namespace.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/loft-sh/vcluster/pkg/scheme"
 	"github.com/loft-sh/vcluster/pkg/syncer/synccontext"
-	"github.com/loft-sh/vcluster/pkg/util/translate/pro"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -88,27 +87,19 @@ func (s *multiNamespace) LabelsToTranslate() map[string]bool {
 	}
 }
 
-func (s *multiNamespace) IsTargetedNamespace(ctx *synccontext.SyncContext, pNamespace string) bool {
-	if _, ok := pro.HostNamespaceMatchesMapping(ctx, pNamespace); ok {
-		return true
-	}
-
+func (s *multiNamespace) IsTargetedNamespace(_ *synccontext.SyncContext, pNamespace string) bool {
 	return strings.HasPrefix(pNamespace, s.getNamespacePrefix()) && strings.HasSuffix(pNamespace, getNamespaceSuffix(s.currentNamespace, VClusterName))
 }
 
 func (s *multiNamespace) getNamespacePrefix() string {
 	return "vcluster"
 }
 
-func (s *multiNamespace) HostNamespace(ctx *synccontext.SyncContext, vNamespace string) string {
+func (s *multiNamespace) HostNamespace(_ *synccontext.SyncContext, vNamespace string) string {
 	if vNamespace == "" {
 		return ""
 	}
 
-	if pNamespace, ok := pro.VirtualNamespaceMatchesMapping(ctx, vNamespace); ok {
-		return pNamespace
-	}
-
 	return hostNamespace(s.currentNamespace, vNamespace, s.getNamespacePrefix(), VClusterName)
 }