From e3612ac352ae5c941bcc049e93072f443fd8ad1f Mon Sep 17 00:00:00 2001 From: StevenLeiZhang Date: Wed, 20 Apr 2022 13:37:34 +0800 Subject: [PATCH] Fix: vela-core does not report error, when component depends on invalid component (#3636) Signed-off-by: StevenLeiZhang --- .../v1alpha2/application/generator.go | 22 +++- .../v1alpha2/application/generator_test.go | 114 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go index e9e0a73c9..45677169c 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator.go @@ -117,8 +117,16 @@ func convertStepProperties(step *v1beta1.WorkflowStep, app *v1beta1.Application) return err } + var componentNames []string + for _, c := range app.Spec.Components { + componentNames = append(componentNames, c.Name) + } + for _, c := range app.Spec.Components { if c.Name == o.Component { + if dcName, ok := checkDependsOnValidComponent(c.DependsOn, componentNames); !ok { + return errors.Errorf("component %s not found, which is depended by %s", dcName, c.Name) + } step.Inputs = append(step.Inputs, c.Inputs...) for index := range step.Inputs { parameterKey := strings.TrimSpace(step.Inputs[index].ParameterKey) @@ -135,11 +143,23 @@ func convertStepProperties(step *v1beta1.WorkflowStep, app *v1beta1.Application) step.Properties = util.Object2RawExtension(c) return nil } - } return errors.Errorf("component %s not found", o.Component) } +func checkDependsOnValidComponent(dependsOnComponentNames, allComponentNames []string) (string, bool) { + // does not depends on other components + if dependsOnComponentNames == nil { + return "", true + } + for _, dc := range dependsOnComponentNames { + if !utils.StringsContain(allComponentNames, dc) { + return dc, false + } + } + return "", true +} + func (h *AppHandler) renderComponentFunc(appParser *appfile.Parser, appRev *v1beta1.ApplicationRevision, af *appfile.Appfile) oamProvider.ComponentRender { return func(comp common.ApplicationComponent, patcher *value.Value, clusterName string, overrideNamespace string, env string) (*unstructured.Unstructured, []*unstructured.Unstructured, error) { ctx := multicluster.ContextWithClusterName(context.Background(), clusterName) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go index 893efbeab..b5a6bc82a 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/generator_test.go @@ -240,4 +240,118 @@ var _ = Describe("Test Application workflow generator", func() { _, _, err = renderFunc(comp, nil, "", "", "") Expect(err).Should(BeNil()) }) + + It("Test generate application workflow with dependsOn", func() { + app := &oamcore.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: "Application", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-input-output", + Namespace: namespaceName, + }, + Spec: oamcore.ApplicationSpec{ + Components: []common.ApplicationComponent{ + { + Name: "myweb1", + Type: "worker-with-health", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + }, + { + Name: "myweb2", + Type: "worker-with-health", + DependsOn: []string{"myweb1"}, + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","lives": "i am lives","enemies": "empty"}`)}, + }, + }, + }, + } + af, err := appParser.GenerateAppFile(ctx, app) + Expect(err).Should(BeNil()) + appRev := &oamcore.ApplicationRevision{} + + handler, err := NewAppHandler(ctx, reconciler, app, appParser) + Expect(err).Should(Succeed()) + + taskRunner, err := handler.GenerateApplicationSteps(ctx, app, appParser, af, appRev) + Expect(err).To(BeNil()) + Expect(len(taskRunner)).Should(BeEquivalentTo(2)) + Expect(taskRunner[0].Name()).Should(BeEquivalentTo("myweb1")) + Expect(taskRunner[1].Name()).Should(BeEquivalentTo("myweb2")) + }) + + It("Test generate application workflow with invalid dependsOn", func() { + app := &oamcore.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: "Application", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-input-output", + Namespace: namespaceName, + }, + Spec: oamcore.ApplicationSpec{ + Components: []common.ApplicationComponent{ + { + Name: "myweb1", + Type: "worker-with-health", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + }, + { + Name: "myweb2", + Type: "worker-with-health", + DependsOn: []string{"myweb0"}, + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","lives": "i am lives","enemies": "empty"}`)}, + }, + }, + }, + } + af, err := appParser.GenerateAppFile(ctx, app) + Expect(err).Should(BeNil()) + appRev := &oamcore.ApplicationRevision{} + + handler, err := NewAppHandler(ctx, reconciler, app, appParser) + Expect(err).Should(Succeed()) + + _, err = handler.GenerateApplicationSteps(ctx, app, appParser, af, appRev) + Expect(err).NotTo(BeNil()) + }) + + It("Test generate application workflow with multiple invalid dependsOn", func() { + app := &oamcore.Application{ + TypeMeta: metav1.TypeMeta{ + Kind: "Application", + APIVersion: "core.oam.dev/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-input-output", + Namespace: namespaceName, + }, + Spec: oamcore.ApplicationSpec{ + Components: []common.ApplicationComponent{ + { + Name: "myweb1", + Type: "worker-with-health", + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)}, + }, + { + Name: "myweb2", + Type: "worker-with-health", + DependsOn: []string{"myweb1", "myweb0", "myweb3"}, + Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","lives": "i am lives","enemies": "empty"}`)}, + }, + }, + }, + } + af, err := appParser.GenerateAppFile(ctx, app) + Expect(err).Should(BeNil()) + appRev := &oamcore.ApplicationRevision{} + + handler, err := NewAppHandler(ctx, reconciler, app, appParser) + Expect(err).Should(Succeed()) + + _, err = handler.GenerateApplicationSteps(ctx, app, appParser, af, appRev) + Expect(err).NotTo(BeNil()) + }) })