From 17ec9ca2f24a15753d607c36325c44a5879a62bd Mon Sep 17 00:00:00 2001 From: nicolaferraro Date: Mon, 2 Nov 2020 16:25:12 +0100 Subject: [PATCH] Fix #751: add guard to global operator --- e2e/builder/global_test.go | 19 ++++++++++- pkg/cmd/operator/operator.go | 3 +- pkg/controller/build/build_controller.go | 8 +++++ .../integration/integration_controller.go | 9 +++++ .../integrationkit_controller.go | 8 +++++ .../integrationplatform_controller.go | 9 +++++ pkg/controller/kamelet/kamelet_controller.go | 9 +++++ .../kamelet_binding_controller.go | 9 +++++ pkg/platform/operator.go | 33 +++++++++++++++++++ 9 files changed, 105 insertions(+), 2 deletions(-) diff --git a/e2e/builder/global_test.go b/e2e/builder/global_test.go index 8d38d85cd2..f257149e23 100644 --- 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,7 +47,7 @@ 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()) @@ -54,6 +55,22 @@ func TestRunGlobalInstall(t *testing.T) { 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 index 9eec07ef0a..b11c021fa4 100644 --- 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 index d9e771f1e1..30a0e04dd1 100644 --- 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 index 8170a37b89..3d94b87316 100644 --- 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 index d16def6ea8..aaa87fe5c3 100644 --- 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 index 6412ba76eb..9a0d99d2d9 100644 --- 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 index 0522a59fbb..22d754889b 100644 --- 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 index dbebfc35a3..bbc84cbc89 100644 --- 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 index 82ed950c15..d0ff38bdab 100644 --- 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 +}