Fix: don't return err if subresource type is not found when listing application resources (#5311)

Signed-off-by: hnd4r7 <307365651@qq.com>
(cherry picked from commit 58a150d1d4)

Co-authored-by: hnd4r7 <307365651@qq.com>

apis/core.oam.dev/common/types.go

@@ -333,7 +333,8 @@ type WorkflowStatus struct {
 	Steps          []workflowv1alpha1.WorkflowStepStatus `json:"steps,omitempty"`
 
 	StartTime metav1.Time `json:"startTime,omitempty"`
-	EndTime   metav1.Time `json:"endTime,omitempty"`
+	// +nullable
+	EndTime metav1.Time `json:"endTime,omitempty"`
 }
 
 // DefinitionType describes the type of DefinitionRevision.

charts/vela-core/crds/core.oam.dev_applicationrevisions.yaml

@@ -844,6 +844,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -2759,6 +2760,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -4813,6 +4815,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

charts/vela-core/crds/core.oam.dev_applications.yaml

@@ -768,6 +768,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean
@@ -1530,6 +1531,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

charts/vela-minimal/crds/core.oam.dev_applicationrevisions.yaml

@@ -844,6 +844,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -2759,6 +2760,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -4813,6 +4815,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

charts/vela-minimal/crds/core.oam.dev_applications.yaml

@@ -768,6 +768,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean
@@ -1530,6 +1531,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

legacy/charts/vela-core-legacy/crds/core.oam.dev_applicationrevisions.yaml

@@ -844,6 +844,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -2759,6 +2760,7 @@ spec:
                             type: object
                           endTime:
                             format: date-time
+                            nullable: true
                             type: string
                           finished:
                             type: boolean
@@ -4813,6 +4815,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

legacy/charts/vela-core-legacy/crds/core.oam.dev_applications.yaml

@@ -769,6 +769,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean
@@ -1531,6 +1532,7 @@ spec:
                     type: object
                   endTime:
                     format: date-time
+                    nullable: true
                     type: string
                   finished:
                     type: boolean

pkg/addon/helper_test.go

@@ -17,9 +17,12 @@ limitations under the License.
 package addon
 
 import (
+	"bytes"
 	"context"
 	"errors"
+	"net/http"
 	"net/http/httptest"
+	"os"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
@@ -29,6 +32,38 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+func setupMockServer() *httptest.Server {
+	var listenURL string
+	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		fileList := []string{
+			"index.yaml",
+			"fluxcd-test-version-1.0.0.tgz",
+			"fluxcd-test-version-2.0.0.tgz",
+			"vela-workflow-v0.3.5.tgz",
+			"foo-v1.0.0.tgz",
+			"bar-v1.0.0.tgz",
+			"bar-v2.0.0.tgz",
+			"mock-be-dep-addon-v1.0.0.tgz",
+		}
+		for _, f := range fileList {
+			if strings.Contains(req.URL.Path, f) {
+				file, err := os.ReadFile("../../e2e/addon/mock/testrepo/helm-repo/" + f)
+				if err != nil {
+					_, _ = w.Write([]byte(err.Error()))
+				}
+				if f == "index.yaml" {
+					// in index.yaml, url is hardcoded to 127.0.0.1:9098,
+					// so we need to replace it with the real random listen url
+					file = bytes.ReplaceAll(file, []byte("http://127.0.0.1:9098"), []byte(listenURL))
+				}
+				_, _ = w.Write(file)
+			}
+		}
+	}))
+	listenURL = s.URL
+	return s
+}
+
 var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() {
 	Describe("when no registry is added, no matter what you do, it will just return error", func() {
 		Context("when empty addonNames and registryNames is supplied", func() {
@@ -50,12 +85,15 @@ var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() {
 	})
 
 	Describe("one versioned registry is added", func() {
+		var s *httptest.Server
+
 		BeforeEach(func() {
-			// Prepare KubeVela registry
+			s = setupMockServer()
+			// Prepare registry
 			reg := &Registry{
-				Name: "KubeVela",
+				Name: "addon_helper_test",
 				Helm: &HelmSource{
-					URL: "https://addons.kubevela.net",
+					URL: s.URL,
 				},
 			}
 			ds := NewRegistryDataStore(k8sClient)
@@ -63,38 +101,36 @@ var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() {
 		})
 
 		AfterEach(func() {
-			// Clean up KubeVela registry
+			// Clean up registry
 			ds := NewRegistryDataStore(k8sClient)
-			Expect(ds.DeleteRegistry(context.Background(), "KubeVela")).To(Succeed())
+			Expect(ds.DeleteRegistry(context.Background(), "addon_helper_test")).To(Succeed())
+			s.Close()
 		})
 
 		Context("when empty addonNames and registryNames is supplied", func() {
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{"KubeVela"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{}, []string{"addon_helper_test"})
 				Expect(err).To(HaveOccurred())
 			})
 			It("should return error, empty addonNames are not allowed", func() {
-				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"KubeVela"})
+				_, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, nil, []string{"addon_helper_test"})
 				Expect(err).To(HaveOccurred())
 			})
 		})
 
 		Context("one existing addon name provided", func() {
 			It("should return one valid result, matching all registries", func() {
-				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"foo"}, nil)
+
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
-				Expect(res[0].Name).To(Equal("velaux"))
-				Expect(res[0].InstallPackage).ToNot(BeNil())
-				Expect(res[0].APISchema).ToNot(BeNil())
+				Expect(res[0].Name).To(Equal("foo"))
 			})
 			It("should return one valid result, matching one registry", func() {
-				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux"}, []string{"KubeVela"})
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"foo"}, []string{"addon_helper_test"})
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
-				Expect(res[0].Name).To(Equal("velaux"))
-				Expect(res[0].InstallPackage).ToNot(BeNil())
-				Expect(res[0].APISchema).ToNot(BeNil())
+				Expect(res[0].Name).To(Equal("foo"))
 			})
 		})
 
@@ -108,26 +144,20 @@ var _ = Describe("test FindAddonPackagesDetailFromRegistry", func() {
 
 		Context("two existing addon names provided", func() {
 			It("should return two valid result", func() {
-				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "traefik"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"foo", "bar"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(2))
-				Expect(res[0].Name).To(Equal("velaux"))
-				Expect(res[0].InstallPackage).ToNot(BeNil())
-				Expect(res[0].APISchema).ToNot(BeNil())
-				Expect(res[1].Name).To(Equal("traefik"))
-				Expect(res[1].InstallPackage).ToNot(BeNil())
-				Expect(res[1].APISchema).ToNot(BeNil())
+				Expect(res[0].Name).To(Equal("foo"))
+				Expect(res[1].Name).To(Equal("bar"))
 			})
 		})
 
 		Context("one existing addon name and one non-existent addon name provided", func() {
 			It("should return only one valid result", func() {
-				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"velaux", "non-existent-addon"}, nil)
+				res, err := FindAddonPackagesDetailFromRegistry(context.Background(), k8sClient, []string{"foo", "non-existent-addon"}, nil)
 				Expect(err).To(Succeed())
 				Expect(res).To(HaveLen(1))
-				Expect(res[0].Name).To(Equal("velaux"))
-				Expect(res[0].InstallPackage).ToNot(BeNil())
-				Expect(res[0].APISchema).ToNot(BeNil())
+				Expect(res[0].Name).To(Equal("foo"))
 			})
 		})
 	})

pkg/apiserver/domain/model/workflow.go

@@ -50,7 +50,7 @@ type Workflow struct {
 
 // WorkflowStep defines how to execute a workflow step.
 type WorkflowStep struct {
-	WorkflowStepBase `json:",inline"`
+	WorkflowStepBase `json:",inline" bson:",inline"`
 	SubSteps         []WorkflowStepBase `json:"subSteps,omitempty"`
 }
 
@@ -123,7 +123,7 @@ type WorkflowRecord struct {
 
 // WorkflowStepStatus is the workflow step status database model
 type WorkflowStepStatus struct {
-	StepStatus     `json:",inline"`
+	StepStatus     `json:",inline" bson:",inline"`
 	SubStepsStatus []StepStatus `json:"subSteps,omitempty"`
 }
 

pkg/apiserver/domain/service/config.go

@@ -137,6 +137,14 @@ func (u *configServiceImpl) CreateConfig(ctx context.Context, project string, re
 			return nil, err
 		}
 	}
+	exist, err := u.Factory.IsExist(ctx, ns, req.Name)
+	if err != nil {
+		klog.Errorf("check config name is exist failure %s", err.Error())
+		return nil, bcode.ErrConfigExist
+	}
+	if exist {
+		return nil, bcode.ErrConfigExist
+	}
 	var properties = make(map[string]interface{})
 	if err := json.Unmarshal([]byte(req.Properties), &properties); err != nil {
 		return nil, err
@@ -156,9 +164,6 @@ func (u *configServiceImpl) CreateConfig(ctx context.Context, project string, re
 		return nil, err
 	}
 	if err := u.Factory.CreateOrUpdateConfig(ctx, configItem, ns); err != nil {
-		if errors.Is(err, config.ErrConfigExist) {
-			return nil, bcode.ErrConfigExist
-		}
 		return nil, err
 	}
 	return convertConfig(project, *configItem), nil
@@ -196,9 +201,12 @@ func (u *configServiceImpl) UpdateConfig(ctx context.Context, project string, na
 		return nil, err
 	}
 	if err := u.Factory.CreateOrUpdateConfig(ctx, configItem, ns); err != nil {
-		if errors.Is(err, config.ErrConfigExist) {
+		if errors.Is(err, config.ErrChangeTemplate) {
 			return nil, bcode.ErrChangeTemplate
 		}
+		if errors.Is(err, config.ErrChangeSecretType) {
+			return nil, bcode.ErrChangeSecretType
+		}
 		return nil, err
 	}
 	return convertConfig(project, *configItem), nil

pkg/apiserver/utils/bcode/016_config.go

@@ -37,4 +37,7 @@ var (
 
 	// ErrNotFoundDistribution means the distribution is not exist
 	ErrNotFoundDistribution = NewBcode(404, 16007, "the distribution is not exist")
+
+	// ErrChangeSecretType the secret type of the config can not be change
+	ErrChangeSecretType = NewBcode(400, 16008, "the secret type of the config can not be change")
 )

pkg/config/factory.go

@@ -88,6 +88,12 @@ var ErrConfigNotFound = errors.New("the config is not exist")
 // ErrTemplateNotFound means the template is not exist
 var ErrTemplateNotFound = errors.New("the template is not exist")
 
+// ErrChangeTemplate means the template of the config can not be change
+var ErrChangeTemplate = errors.New("the template of the config can not be change")
+
+// ErrChangeSecretType means the secret type of the config can not be change
+var ErrChangeSecretType = errors.New("the secret type of the config can not be change")
+
 // NamespacedName the namespace and name model
 type NamespacedName struct {
 	Name      string `json:"name"`
@@ -192,6 +198,7 @@ type Factory interface {
 	ListConfigs(ctx context.Context, namespace, template, scope string, withStatus bool) ([]*Config, error)
 	DeleteConfig(ctx context.Context, namespace, name string) error
 	CreateOrUpdateConfig(ctx context.Context, i *Config, ns string) error
+	IsExist(ctx context.Context, namespace, name string) (bool, error)
 
 	CreateOrUpdateDistribution(ctx context.Context, ns, name string, ads *CreateDistributionSpec) error
 	ListDistributions(ctx context.Context, ns string) ([]*Distribution, error)
@@ -549,7 +556,10 @@ func (k *kubeConfigFactory) CreateOrUpdateConfig(ctx context.Context, i *Config,
 	var secret v1.Secret
 	if err := k.cli.Get(ctx, pkgtypes.NamespacedName{Namespace: i.Namespace, Name: i.Name}, &secret); err == nil {
 		if secret.Labels[types.LabelConfigType] != i.Template.Name {
-			return ErrConfigExist
+			return ErrChangeTemplate
+		}
+		if secret.Type != i.Secret.Type {
+			return ErrChangeSecretType
 		}
 	}
 	if err := k.apiApply.Apply(ctx, i.Secret, apply.Quiet()); err != nil {
@@ -571,6 +581,17 @@ func (k *kubeConfigFactory) CreateOrUpdateConfig(ctx context.Context, i *Config,
 	return nil
 }
 
+func (k *kubeConfigFactory) IsExist(ctx context.Context, namespace, name string) (bool, error) {
+	var secret v1.Secret
+	if err := k.cli.Get(ctx, pkgtypes.NamespacedName{Namespace: namespace, Name: name}, &secret); err != nil {
+		if apierrors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	return true, nil
+}
+
 func (k *kubeConfigFactory) ListConfigs(ctx context.Context, namespace, template, scope string, withStatus bool) ([]*Config, error) {
 	var list = &v1.SecretList{}
 	requirement := fmt.Sprintf("%s=%s", types.LabelConfigCatalog, types.VelaCoreConfig)

pkg/config/factory_test.go

@@ -145,6 +145,12 @@ var _ = Describe("test config factory", func() {
 		Expect(len(config.Targets)).Should(Equal(1))
 	})
 
+	It("check if the config exist", func() {
+		exist, err := fac.IsExist(context.TODO(), "default", "db-config")
+		Expect(err).Should(BeNil())
+		Expect(exist).Should(BeTrue())
+	})
+
 	It("list the distributions", func() {
 		distributions, err := fac.ListDistributions(context.TODO(), "default")
 		Expect(err).Should(BeNil())

pkg/velaql/providers/query/tree.go

@@ -25,6 +25,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	v12 "k8s.io/api/core/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -908,7 +909,7 @@ func iterateListSubResources(ctx context.Context, cluster string, k8sClient clie
 			clusterCTX := multicluster.ContextWithClusterName(ctx, cluster)
 			items, err := listItemByRule(clusterCTX, k8sClient, resource, *parentObject, specifiedFunc, rule.DefaultGenListOptionFunc, rule.DisableFilterByOwnerReference)
 			if err != nil {
-				if meta.IsNoMatchError(err) || runtime.IsNotRegisteredError(err) {
+				if meta.IsNoMatchError(err) || runtime.IsNotRegisteredError(err) || kerrors.IsNotFound(err) {
 					klog.Warningf("ignore list resources: %s as %v", resource.Kind, err)
 					continue
 				}

references/cli/uninstall.go

@@ -80,7 +80,7 @@ func NewUnInstallCommand(c common.Args, order string, ioStreams util.IOStreams)
 					return errors.Wrapf(err, "cannot check installed addon")
 				}
 				if len(addons) != 0 {
-					return fmt.Errorf("these addons have been eanbled :%v, please guarantee there is no application using these addons and use `vela uninstall -f` uninstall include addon ", addons)
+					return fmt.Errorf("these addons have been enabled :%v, please guarantee there is no application using these addons and use `vela uninstall -f` uninstall include addon ", addons)
 				}
 			}
 

test/e2e-apiserver-test/config_test.go

@@ -153,7 +153,7 @@ var _ = Describe("Test the rest api about the config", func() {
 		Expect(config.Secret).Should(BeNil())
 		Expect(config.Properties["registry"]).Should(Equal("kubevela.test.com"))
 
-		By("the template is not exist")
+		By("the config name is exist")
 		req = v1.CreateConfigRequest{
 			Name:        "test-registry",
 			Alias:       "Test Registry",
@@ -162,6 +162,17 @@ var _ = Describe("Test the rest api about the config", func() {
 			Properties:  `{"registry": "kubevela.test.com"}`,
 		}
 		res = post("/configs", req)
+		Expect(res.StatusCode).Should(Equal(400))
+
+		By("the template is not exist")
+		req = v1.CreateConfigRequest{
+			Name:        "test-registry2",
+			Alias:       "Test Registry",
+			Description: "This is a demo config",
+			Template:    v1.NamespacedName{Name: templateName + "notfound"},
+			Properties:  `{"registry": "kubevela.test.com"}`,
+		}
+		res = post("/configs", req)
 		Expect(res.StatusCode).Should(Equal(404))
 
 		By("without the template")