Fix apache#751: add guard to global operator

nicolaferraro · Nov 2, 2020 · 17ec9ca · 17ec9ca
1 parent b2562d3
commit 17ec9ca
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 2 deletions.
diff --git a/e2e/builder/global_test.go b/e2e/builder/global_test.go
@@ -26,6 +26,7 @@ import (
 	"testing"
 
 	. "github.com/apache/camel-k/e2e/support"
+	"github.com/apache/camel-k/pkg/platform"
 	"github.com/apache/camel-k/pkg/util/openshift"
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/assert"
@@ -46,14 +47,30 @@ func TestRunGlobalInstall(t *testing.T) {
 	WithNewTestNamespace(t, func(ns string) {
 		Expect(Kamel("install", "-n", ns, "--global").Execute()).Should(BeNil())
 
-		// NS2
+		// NS2: namespace without operator
 		WithNewTestNamespace(t, func(ns2 string) {
 			Expect(Kamel("install", "-n", ns2, "--skip-operator-setup", "--olm=false").Execute()).Should(BeNil())
 
 			Expect(Kamel("run", "-n", ns2, "files/Java.java").Execute()).Should(BeNil())
 			Eventually(IntegrationPodPhase(ns2, "java"), TestTimeoutMedium).Should(Equal(v1.PodRunning))
 			Eventually(IntegrationLogs(ns2, "java"), TestTimeoutShort).Should(ContainSubstring("Magicstring!"))
 			Expect(Kamel("delete", "--all", "-n", ns2).Execute()).Should(BeNil())
+
+			Expect(ConfigMap(ns2, platform.OperatorLockName)()).Should(BeNil(), "No locking configmap expected")
+		})
+
+		// NS3: namespace with its own operator
+		WithNewTestNamespace(t, func(ns3 string) {
+			Expect(Kamel("install", "-n", ns3, "--olm=false").Execute()).Should(BeNil())
+
+			Expect(Kamel("run", "-n", ns3, "files/Java.java").Execute()).Should(BeNil())
+			Eventually(IntegrationPodPhase(ns3, "java"), TestTimeoutMedium).Should(Equal(v1.PodRunning))
+			Eventually(IntegrationLogs(ns3, "java"), TestTimeoutShort).Should(ContainSubstring("Magicstring!"))
+			Expect(Kamel("delete", "--all", "-n", ns3).Execute()).Should(BeNil())
+
+			Expect(ConfigMap(ns3, platform.OperatorLockName)()).ShouldNot(BeNil(),
+				"OperatorSDK is expected to use configmaps for locking: if this changes (e.g. using Leases) we should update our guard logic",
+			)
 		})
 
 		Expect(Kamel("uninstall", "-n", ns, "--skip-crd", "--skip-cluster-roles").Execute()).Should(BeNil())

diff --git a/pkg/cmd/operator/operator.go b/pkg/cmd/operator/operator.go
@@ -26,6 +26,7 @@ import (
 	"runtime"
 	"time"
 
+	"github.com/apache/camel-k/pkg/platform"
 	corev1 "k8s.io/api/core/v1"
 	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/record"
@@ -91,7 +92,7 @@ func Run() {
 	}
 
 	// Become the leader before proceeding
-	err = leader.Become(context.TODO(), "camel-k-lock")
+	err = leader.Become(context.TODO(), platform.OperatorLockName)
 	if err != nil {
 		if err == leader.ErrNoNamespace {
 			log.Info("Local run detected, leader election is disabled")

diff --git a/pkg/controller/build/build_controller.go b/pkg/controller/build/build_controller.go
@@ -138,6 +138,14 @@ func (r *ReconcileBuild) Reconcile(request reconcile.Request) (reconcile.Result,
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	// Fetch the Build instance
 	var instance v1.Build
 

diff --git a/pkg/controller/integration/integration_controller.go b/pkg/controller/integration/integration_controller.go
@@ -21,6 +21,7 @@ import (
 	"context"
 
 	camelevent "github.com/apache/camel-k/pkg/event"
+	"github.com/apache/camel-k/pkg/platform"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/batch/v1beta1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -236,6 +237,14 @@ func (r *ReconcileIntegration) Reconcile(request reconcile.Request) (reconcile.R
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	// Fetch the Integration instance
 	var instance v1.Integration
 

diff --git a/pkg/controller/integrationkit/integrationkit_controller.go b/pkg/controller/integrationkit/integrationkit_controller.go
@@ -171,6 +171,14 @@ func (r *ReconcileIntegrationKit) Reconcile(request reconcile.Request) (reconcil
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	var instance v1.IntegrationKit
 
 	// Fetch the IntegrationKit instance

diff --git a/pkg/controller/integrationplatform/integrationplatform_controller.go b/pkg/controller/integrationplatform/integrationplatform_controller.go
@@ -22,6 +22,7 @@ import (
 	"time"
 
 	camelevent "github.com/apache/camel-k/pkg/event"
+	"github.com/apache/camel-k/pkg/platform"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -111,6 +112,14 @@ func (r *ReconcileIntegrationPlatform) Reconcile(request reconcile.Request) (rec
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	// Fetch the IntegrationPlatform instance
 	var instance v1.IntegrationPlatform
 

diff --git a/pkg/controller/kamelet/kamelet_controller.go b/pkg/controller/kamelet/kamelet_controller.go
@@ -24,6 +24,7 @@ import (
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
 	"github.com/apache/camel-k/pkg/client"
 	camelevent "github.com/apache/camel-k/pkg/event"
+	"github.com/apache/camel-k/pkg/platform"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -109,6 +110,14 @@ func (r *ReconcileKamelet) Reconcile(request reconcile.Request) (reconcile.Resul
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	// Fetch the Kamelet instance
 	var instance v1alpha1.Kamelet
 

diff --git a/pkg/controller/kameletbinding/kamelet_binding_controller.go b/pkg/controller/kameletbinding/kamelet_binding_controller.go
@@ -25,6 +25,7 @@ import (
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
 	"github.com/apache/camel-k/pkg/client"
 	camelevent "github.com/apache/camel-k/pkg/event"
+	"github.com/apache/camel-k/pkg/platform"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
@@ -119,6 +120,14 @@ func (r *ReconcileKameletBinding) Reconcile(request reconcile.Request) (reconcil
 
 	ctx := context.TODO()
 
+	// Make sure the operator is allowed to act on namespace
+	if ok, err := platform.IsOperatorAllowedOnNamespace(ctx, r.client, request.Namespace); err != nil {
+		return reconcile.Result{}, err
+	} else if !ok {
+		rlog.Info("Ignoring request because namespace is locked")
+		return reconcile.Result{}, nil
+	}
+
 	// Fetch the KameletBinding instance
 	var instance v1alpha1.KameletBinding
 

diff --git a/pkg/platform/operator.go b/pkg/platform/operator.go
@@ -32,6 +32,8 @@ const operatorWatchNamespaceEnvVariable = "WATCH_NAMESPACE"
 const operatorNamespaceEnvVariable = "NAMESPACE"
 const operatorPodNameEnvVariable = "POD_NAME"
 
+const OperatorLockName = "camel-k-lock"
+
 // GetCurrentOperatorImage returns the image currently used by the running operator if present (when running out of cluster, it may be absent).
 func GetCurrentOperatorImage(ctx context.Context, c client.Client) (string, error) {
 	podNamespace := GetOperatorNamespace()
@@ -80,3 +82,34 @@ func GetOperatorPodName() string {
 	}
 	return ""
 }
+
+// IsNamespaceLocked tells if the namespace contains a lock indicating that an operator owns it
+func IsNamespaceLocked(ctx context.Context, c client.Client, namespace string) (bool, error) {
+	if namespace == "" {
+		return false, nil
+	}
+
+	cm := v1.ConfigMap{}
+	key := client.ObjectKey{
+		Namespace: namespace,
+		Name:      OperatorLockName,
+	}
+	if err := c.Get(ctx, key, &cm); err != nil && k8serrors.IsNotFound(err) {
+		return false, nil
+	} else if err != nil {
+		return true, err
+	}
+	return true, nil
+}
+
+// IsOperatorAllowedOnNamespace returns true if the current operator is allowed to react on changes in the given namespace
+func IsOperatorAllowedOnNamespace(ctx context.Context, c client.Client, namespace string) (bool, error) {
+	if !IsCurrentOperatorGlobal() {
+		return true, nil
+	}
+	alreadyOwned, err := IsNamespaceLocked(ctx, c, namespace)
+	if err != nil {
+		return false, err
+	}
+	return !alreadyOwned, nil
+}