[Backport release-1.5] Fix: allow to read definition from user's namespace when force delete (#4789)

* Fix: allow to read definition from user's namespace when force deleting app with configuration

Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
(cherry picked from commit 2f08c36132)

* Fix test

Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
(cherry picked from commit 981950a14d)

* Fix wrong test

Signed-off-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>
(cherry picked from commit 62863f1007)

Co-authored-by: Qiaozp <qiaozhongpei.qzp@alibaba-inc.com>

charts/vela-core/templates/kubevela-controller.yaml

@@ -33,7 +33,7 @@ kind: ClusterRole
 metadata:
   name: {{ include "kubevela.fullname" . }}:manager
 rules:
-  - apiGroups: ["core.oam.dev", "terraform.core.oam.dev", "prism.oam.dev"]
+  - apiGroups: ["core.oam.dev", "terraform.core.oam.dev", "prism.oam.dev", "standard.oam.dev"]
     resources: ["*"]
     verbs: ["*"]
   - apiGroups: ["cluster.open-cluster-management.io"]

charts/vela-minimal/templates/kubevela-controller.yaml

@@ -36,7 +36,7 @@ kind: ClusterRole
 metadata:
   name: {{ include "kubevela.fullname" . }}:manager
 rules:
-  - apiGroups: ["core.oam.dev", "terraform.core.oam.dev", "prism.oam.dev"]
+  - apiGroups: ["core.oam.dev", "terraform.core.oam.dev", "prism.oam.dev", "standard.oam.dev"]
     resources: ["*"]
     verbs: ["*"]
   - apiGroups: ["cluster.open-cluster-management.io"]

cmd/apiserver/main.go

@@ -35,6 +35,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/apiserver/config"
 	"github.com/oam-dev/kubevela/pkg/apiserver/utils/log"
 	"github.com/oam-dev/kubevela/pkg/features"
+	"github.com/oam-dev/kubevela/pkg/utils"
 	"github.com/oam-dev/kubevela/version"
 )
 
@@ -50,6 +51,7 @@ func main() {
 	flag.DurationVar(&s.serverConfig.LeaderConfig.Duration, "duration", time.Second*5, "the lease lock resource name")
 	flag.DurationVar(&s.serverConfig.AddonCacheTime, "addon-cache-duration", time.Minute*10, "how long between two addon cache operation")
 	flag.BoolVar(&s.serverConfig.DisableStatisticCronJob, "disable-statistic-cronJob", false, "close the system statistic info calculating cronJob")
+	flag.StringVar(&s.serverConfig.PprofAddr, "pprof-addr", "", "The address for pprof to use while exporting profiling results. The default value is empty which means do not expose it. Set it to address like :6666 to expose it.")
 	flag.Float64Var(&s.serverConfig.KubeQPS, "kube-api-qps", 100, "the qps for kube clients. Low qps may lead to low throughput. High qps may give stress to api-server.")
 	flag.IntVar(&s.serverConfig.KubeBurst, "kube-api-burst", 300, "the burst for kube clients. Recommend setting it qps*3.")
 	features.APIServerMutableFeatureGate.AddFlag(flag.CommandLine)
@@ -90,6 +92,11 @@ func main() {
 	errChan := make(chan error)
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
+
+	if s.serverConfig.PprofAddr != "" {
+		go utils.EnablePprof(s.serverConfig.PprofAddr, errChan)
+	}
+
 	go func() {
 		if err := s.run(ctx, errChan); err != nil {
 			errChan <- fmt.Errorf("failed to run apiserver: %w", err)

cmd/core/main.go

@@ -22,8 +22,6 @@ import (
 	goflag "flag"
 	"fmt"
 	"io"
-	"net/http"
-	"net/http/pprof"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -53,6 +51,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/resourcekeeper"
+	pkgutils "github.com/oam-dev/kubevela/pkg/utils"
 	"github.com/oam-dev/kubevela/pkg/utils/common"
 	"github.com/oam-dev/kubevela/pkg/utils/system"
 	"github.com/oam-dev/kubevela/pkg/utils/util"
@@ -159,36 +158,7 @@ func main() {
 
 	if pprofAddr != "" {
 		// Start pprof server if enabled
-		mux := http.NewServeMux()
-		mux.HandleFunc("/debug/pprof/", pprof.Index)
-		mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
-		mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
-		mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
-		mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
-		pprofServer := http.Server{
-			Addr:    pprofAddr,
-			Handler: mux,
-		}
-		klog.InfoS("Starting debug HTTP server", "addr", pprofServer.Addr)
-
-		go func() {
-			go func() {
-				ctx := context.Background()
-				<-ctx.Done()
-
-				ctx, cancelFunc := context.WithTimeout(context.Background(), 60*time.Minute)
-				defer cancelFunc()
-
-				if err := pprofServer.Shutdown(ctx); err != nil {
-					klog.Error(err, "Failed to shutdown debug HTTP server")
-				}
-			}()
-
-			if err := pprofServer.ListenAndServe(); !errors.Is(http.ErrServerClosed, err) {
-				klog.Error(err, "Failed to start debug HTTP server")
-				panic(err)
-			}
-		}()
+		go pkgutils.EnablePprof(pprofAddr, nil)
 	}
 
 	if logFilePath != "" {

pkg/apiserver/config/config.go

@@ -46,6 +46,9 @@ type Config struct {
 
 	// KubeQPS the QPS of kube client
 	KubeQPS float64
+
+	// PprofAddr the address for pprof to use while exporting profiling results.
+	PprofAddr string
 }
 
 type leaderConfig struct {

pkg/apiserver/event/sync/worker.go

@@ -18,11 +18,12 @@ package sync
 
 import (
 	"context"
-	"encoding/json"
 	"sync"
 
 	"github.com/fatih/color"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/dynamic"
 	dynamicInformer "k8s.io/client-go/dynamic/dynamicinformer"
 	"k8s.io/client-go/rest"
@@ -58,14 +59,17 @@ func (a *ApplicationSync) Start(ctx context.Context, errorChan chan error) {
 	factory := dynamicInformer.NewFilteredDynamicSharedInformerFactory(dynamicClient, 0, v1.NamespaceAll, nil)
 	informer := factory.ForResource(v1beta1.SchemeGroupVersion.WithResource("applications")).Informer()
 	getApp := func(obj interface{}) *v1beta1.Application {
-		app := &v1beta1.Application{}
-		bs, err := json.Marshal(obj)
-		if err != nil {
-			log.Logger.Errorf("decode the application failure %s", err.Error())
+		if app, ok := obj.(*v1beta1.Application); ok {
 			return app
 		}
-		_ = json.Unmarshal(bs, app)
-		return app
+		var app v1beta1.Application
+		if object, ok := obj.(*unstructured.Unstructured); ok {
+			if err := runtime.DefaultUnstructuredConverter.FromUnstructured(object.Object, &app); err != nil {
+				log.Logger.Errorf("decode the application failure %s", err.Error())
+				return &app
+			}
+		}
+		return &app
 	}
 	cu := &CR2UX{
 		ds:                 a.Store,
@@ -89,6 +93,7 @@ func (a *ApplicationSync) Start(ctx context.Context, errorChan chan error) {
 			if err := cu.AddOrUpdate(ctx, app.(*v1beta1.Application)); err != nil {
 				log.Logger.Errorf("fail to add or update application %s", err.Error())
 			}
+			a.Queue.Done(app)
 		}
 	}()
 

pkg/definition/definition.go

@@ -235,7 +235,7 @@ func (def *Definition) FromCUE(val *cue.Value, templateString string) error {
 	labels := map[string]string{}
 	for k, v := range def.GetLabels() {
 		if !strings.HasPrefix(k, UserPrefix) {
-			annotations[k] = v
+			labels[k] = v
 		}
 	}
 	spec, ok := def.Object["spec"].(map[string]interface{})

pkg/resourcekeeper/gc.go

@@ -165,10 +165,25 @@ func (h *gcHandler) monitor(stage string) func() {
 	}
 }
 
+func (h *gcHandler) regularizeResourceTracker(rts ...*v1beta1.ResourceTracker) {
+	for _, rt := range rts {
+		if rt == nil {
+			continue
+		}
+		for i, mr := range rt.Spec.ManagedResources {
+			if ok, err := utils.IsClusterScope(mr.GroupVersionKind(), h.Client.RESTMapper()); err == nil && ok {
+				rt.Spec.ManagedResources[i].Namespace = ""
+			}
+		}
+	}
+}
+
 func (h *gcHandler) Init() {
 	cb := h.monitor("init")
 	defer cb()
-	h.cache.registerResourceTrackers(append(h._historyRTs, h._currentRT, h._rootRT)...)
+	rts := append(h._historyRTs, h._currentRT, h._rootRT)
+	h.regularizeResourceTracker(rts...)
+	h.cache.registerResourceTrackers(rts...)
 }
 
 func (h *gcHandler) scan(ctx context.Context) (inactiveRTs []*v1beta1.ResourceTracker) {

pkg/resourcekeeper/gc_suite_test.go

@@ -27,6 +27,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -221,4 +222,46 @@ var _ = Describe("Test ResourceKeeper garbage collection", func() {
 		}, 5*time.Second).Should(Succeed())
 	})
 
+	It("Test gc same cluster-scoped resource but legacy resource recorded with namespace", func() {
+		ctx := context.Background()
+		cr := &unstructured.Unstructured{Object: map[string]interface{}{
+			"apiVersion": "rbac.authorization.k8s.io/v1",
+			"kind":       "ClusterRole",
+			"metadata": map[string]interface{}{
+				"name": "test-cluster-scoped-resource",
+				"labels": map[string]interface{}{
+					oam.LabelAppName:      "app",
+					oam.LabelAppNamespace: namespace,
+				},
+			},
+		}}
+		Expect(testClient.Create(ctx, cr)).Should(Succeed())
+		app := &v1beta1.Application{ObjectMeta: metav1.ObjectMeta{Name: "app", Namespace: namespace}}
+		keeper := &resourceKeeper{
+			Client:     testClient,
+			app:        app,
+			applicator: apply.NewAPIApplicator(testClient),
+			cache:      newResourceCache(testClient, app),
+		}
+		h := gcHandler{resourceKeeper: keeper, cfg: newGCConfig()}
+		h._currentRT = &v1beta1.ResourceTracker{ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-scoped-resource-v2"}}
+		Expect(testClient.Create(ctx, h._currentRT)).Should(Succeed())
+		h._historyRTs = []*v1beta1.ResourceTracker{{ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-scoped-resource-v1"}}}
+		t := metav1.Now()
+		h._historyRTs[0].SetDeletionTimestamp(&t)
+		h._historyRTs[0].SetFinalizers([]string{resourcetracker.Finalizer})
+		h._currentRT.AddManagedResource(cr, true, false, "")
+		_cr := cr.DeepCopy()
+		_cr.SetNamespace(namespace)
+		h._historyRTs[0].AddManagedResource(_cr, true, false, "")
+		h.Init()
+		Expect(h.Finalize(ctx)).Should(Succeed())
+		Expect(testClient.Get(ctx, client.ObjectKeyFromObject(cr), &rbacv1.ClusterRole{})).Should(Succeed())
+		h._currentRT.Spec.ManagedResources[0].Name = "not-equal"
+		keeper.cache = newResourceCache(testClient, app)
+		h.Init()
+		Expect(h.Finalize(ctx)).Should(Succeed())
+		Expect(testClient.Get(ctx, client.ObjectKeyFromObject(cr), &rbacv1.ClusterRole{})).Should(Satisfy(errors.IsNotFound))
+	})
+
 })

pkg/resourcekeeper/utils.go

@@ -17,18 +17,15 @@ limitations under the License.
 package resourcekeeper
 
 import (
-	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+
+	"github.com/oam-dev/kubevela/pkg/utils"
 )
 
 // ClearNamespaceForClusterScopedResources clear namespace for cluster scoped resources
 func (h *resourceKeeper) ClearNamespaceForClusterScopedResources(manifests []*unstructured.Unstructured) {
 	for _, manifest := range manifests {
-		mappings, err := h.Client.RESTMapper().RESTMappings(manifest.GroupVersionKind().GroupKind(), manifest.GroupVersionKind().Version)
-		if err != nil {
-			continue
-		}
-		if len(mappings) > 0 && mappings[0].Scope.Name() == meta.RESTScopeNameRoot {
+		if ok, err := utils.IsClusterScope(manifest.GroupVersionKind(), h.Client.RESTMapper()); err == nil && ok {
 			manifest.SetNamespace("")
 		}
 	}

pkg/utils/k8s.go

@@ -26,7 +26,9 @@ import (
 	authv1 "k8s.io/api/authentication/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -189,3 +191,10 @@ func CreateOrUpdate(ctx context.Context, cli client.Client, obj client.Object) (
 func EscapeResourceNameToLabelValue(resourceName string) string {
 	return strings.ReplaceAll(resourceName, ":", "_")
 }
+
+// IsClusterScope check if the gvk is cluster scoped
+func IsClusterScope(gvk schema.GroupVersionKind, mapper meta.RESTMapper) (bool, error) {
+	mappings, err := mapper.RESTMappings(gvk.GroupKind(), gvk.Version)
+	isClusterScope := len(mappings) > 0 && mappings[0].Scope.Name() == meta.RESTScopeNameRoot
+	return isClusterScope, err
+}

pkg/utils/k8s_test.go

@@ -22,6 +22,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	apierror "k8s.io/apimachinery/pkg/api/errors"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -162,4 +163,13 @@ var _ = Describe("Test Create Or Update Namespace functions", func() {
 			Expect(gotNS.Labels).Should(HaveKeyWithValue(k, v))
 		}
 	})
+
+	It("Test IsClusterScope", func() {
+		ok, err := IsClusterScope(v1.SchemeGroupVersion.WithKind("ConfigMap"), k8sClient.RESTMapper())
+		Expect(err).Should(Succeed())
+		Expect(ok).Should(BeFalse())
+		ok, err = IsClusterScope(rbacv1.SchemeGroupVersion.WithKind("ClusterRole"), k8sClient.RESTMapper())
+		Expect(err).Should(Succeed())
+		Expect(ok).Should(BeTrue())
+	})
 })

pkg/utils/pprof.go

@@ -0,0 +1,51 @@
+/*
+Copyright 2022 The KubeVela Authors.
+
+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 utils
+
+import (
+	"net/http"
+	"net/http/pprof"
+
+	"k8s.io/klog/v2"
+)
+
+// EnablePprof listen to the pprofAddr and export the profiling results
+// If the errChan is nil, this function will panic when the listening error occurred.
+func EnablePprof(pprofAddr string, errChan chan error) {
+	// Start pprof server if enabled
+	mux := http.NewServeMux()
+	mux.HandleFunc("/debug/pprof/", pprof.Index)
+	mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
+	mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
+	mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
+	mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
+	pprofServer := http.Server{
+		Addr:    pprofAddr,
+		Handler: mux,
+	}
+
+	klog.InfoS("Starting debug HTTP server", "addr", pprofServer.Addr)
+
+	if err := pprofServer.ListenAndServe(); err != nil {
+		klog.Error(err, "Failed to start debug HTTP server")
+		if errChan != nil {
+			errChan <- err
+		} else {
+			panic(err)
+		}
+	}
+}

references/common/application.go

@@ -230,14 +230,17 @@ func prepareToForceDeleteTerraformComponents(ctx context.Context, k8sClient clie
 	for _, c := range app.Spec.Components {
 		var def corev1beta1.ComponentDefinition
 		if err := k8sClient.Get(ctx, client.ObjectKey{Name: c.Type, Namespace: types.DefaultKubeVelaNS}, &def); err != nil {
-			return err
+			if !apierrors.IsNotFound(err) {
+				return err
+			}
+			if err := k8sClient.Get(ctx, client.ObjectKey{Name: c.Type, Namespace: namespace}, &def); err != nil {
+				return err
+			}
 		}
 		if def.Spec.Schematic != nil && def.Spec.Schematic.Terraform != nil {
 			var conf terraformapi.Configuration
 			if err := k8sClient.Get(ctx, client.ObjectKey{Name: c.Name, Namespace: namespace}, &conf); err != nil {
-				if !apierrors.IsNotFound(err) {
-					return err
-				}
+				return err
 			}
 			conf.Spec.ForceDelete = &forceDelete
 			if err := k8sClient.Update(ctx, &conf); err != nil {

references/common/application_test.go

@@ -19,7 +19,7 @@ import (
 	"context"
 	"testing"
 
-	terraformapi "github.com/oam-dev/terraform-controller/api/v1beta1"
+	terraformapi "github.com/oam-dev/terraform-controller/api/v1beta2"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -55,7 +55,7 @@ func TestPrepareToForceDeleteTerraformComponents(t *testing.T) {
 	def1 := &v1beta1.ComponentDefinition{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "ComponentDefinition",
-			APIVersion: "core.oam.dev/v1beta1",
+			APIVersion: "core.oam.dev/v1beta2",
 		},
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "d1",
@@ -75,6 +75,16 @@ func TestPrepareToForceDeleteTerraformComponents(t *testing.T) {
 			Namespace: "default",
 		},
 	}
+
+	userNamespace := "another-namespace"
+	def2 := def1.DeepCopy()
+	def2.SetNamespace(userNamespace)
+	app2 := app1.DeepCopy()
+	app2.SetNamespace(userNamespace)
+	app2.SetName("app2")
+	conf2 := conf1.DeepCopy()
+	conf2.SetNamespace(userNamespace)
+
 	k8sClient1 := fake.NewClientBuilder().WithScheme(s).WithObjects(app1, def1, conf1).Build()
 
 	k8sClient2 := fake.NewClientBuilder().Build()
@@ -83,6 +93,7 @@ func TestPrepareToForceDeleteTerraformComponents(t *testing.T) {
 
 	k8sClient4 := fake.NewClientBuilder().WithScheme(s).WithObjects(app1, def1).Build()
 
+	k8sClient5 := fake.NewClientBuilder().WithScheme(s).WithObjects(app2, def2, conf2).Build()
 	type args struct {
 		k8sClient client.Client
 		namespace string
@@ -141,16 +152,27 @@ func TestPrepareToForceDeleteTerraformComponents(t *testing.T) {
 				"app1",
 			},
 			want: want{
-				errMsg: "no kind is registered for the type",
+				errMsg: "configurations.terraform.core.oam.dev \"c1\" not found",
 			},
 		},
+		"can read definition from application namespace": {
+			args: args{
+				k8sClient5,
+				userNamespace,
+				"app2",
+			},
+			want: want{},
+		},
 	}
 
 	for name, tc := range testcases {
 		t.Run(name, func(t *testing.T) {
 			err := prepareToForceDeleteTerraformComponents(ctx, tc.args.k8sClient, tc.args.namespace, tc.args.name)
-			if err != nil || tc.want.errMsg != "" {
+			if err != nil {
+				assert.NotEmpty(t, tc.want.errMsg)
 				assert.Contains(t, err.Error(), tc.want.errMsg)
+			} else {
+				assert.Empty(t, tc.want.errMsg)
 			}
 		})
 	}