From e494516593bc98c2dfbd796c6ceab926fbe306ea Mon Sep 17 00:00:00 2001 From: Kinso <529724975@qq.com> Date: Tue, 15 Jun 2021 13:36:23 +0800 Subject: [PATCH] fix(openapi): only parse the parameter field (#1790) Co-authored-by: kinsolee --- pkg/controller/utils/capability.go | 20 +++- pkg/controller/utils/capability_test.go | 111 +++++++++++------- pkg/utils/common/common.go | 31 ++++- pkg/utils/common/common_test.go | 51 ++++---- pkg/utils/common/testdata/emptyParameter.cue | 3 + pkg/utils/common/testdata/emptyParameter.json | 15 +++ pkg/utils/common/testdata/workload1.cue | 4 + 7 files changed, 162 insertions(+), 73 deletions(-) create mode 100644 pkg/utils/common/testdata/emptyParameter.cue create mode 100644 pkg/utils/common/testdata/emptyParameter.json diff --git a/pkg/controller/utils/capability.go b/pkg/controller/utils/capability.go index 32be7cd59..f2a2c3464 100644 --- a/pkg/controller/utils/capability.go +++ b/pkg/controller/utils/capability.go @@ -44,9 +44,6 @@ import ( "github.com/oam-dev/kubevela/pkg/utils/common" ) -// ErrNoSectionParameterInCue means there is not parameter section in Cue template of a workload -const ErrNoSectionParameterInCue = "capability %s doesn't contain section `parameter`" - // data types of parameter value const ( TerraformVariableString string = "string" @@ -58,6 +55,15 @@ const ( TerraformVariableObject string = "object" ) +// ErrNoSectionParameterInCue means there is not parameter section in Cue template of a workload +type ErrNoSectionParameterInCue struct { + capName string +} + +func (e ErrNoSectionParameterInCue) Error() string { + return fmt.Sprintf("capability %s doesn't contain section `parameter`", e.capName) +} + // CapabilityDefinitionInterface is the interface for Capability (WorkloadDefinition and TraitDefinition) type CapabilityDefinitionInterface interface { GetCapabilityObject(ctx context.Context, k8sClient client.Client, namespace, name string) (*types.Capability, error) @@ -369,6 +375,12 @@ func getOpenAPISchema(capability types.Capability, pd *packages.PackageDiscover) func generateOpenAPISchemaFromCapabilityParameter(capability types.Capability, pd *packages.PackageDiscover) ([]byte, error) { template, err := prepareParameterCue(capability.Name, capability.CueTemplate) if err != nil { + if errors.As(err, &ErrNoSectionParameterInCue{}) { + // return OpenAPI with empty object parameter, making it possible to generate ConfigMap + var r cue.Runtime + cueInst, _ := r.Compile("-", "") + return common.GenOpenAPI(cueInst) + } return nil, err } @@ -421,7 +433,7 @@ func prepareParameterCue(capabilityName, capabilityTemplate string) (string, err } if !withParameterFlag { - return "", fmt.Errorf(ErrNoSectionParameterInCue, capabilityName) + return "", ErrNoSectionParameterInCue{capName: capabilityName} } return template, nil } diff --git a/pkg/controller/utils/capability_test.go b/pkg/controller/utils/capability_test.go index ed2f198fc..b9aca07e9 100644 --- a/pkg/controller/utils/capability_test.go +++ b/pkg/controller/utils/capability_test.go @@ -18,9 +18,7 @@ package utils import ( - "fmt" "io/ioutil" - "os" "path/filepath" "strings" "testing" @@ -32,11 +30,9 @@ import ( "github.com/oam-dev/kubevela/apis/core.oam.dev/common" "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1" - "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/appfile" "github.com/oam-dev/kubevela/pkg/cue" "github.com/oam-dev/kubevela/pkg/oam/util" - "github.com/oam-dev/kubevela/pkg/utils/system" ) const TestDir = "testdata/definition" @@ -92,6 +88,54 @@ annotations: { `, want: want{data: "{\"type\":\"string\"}", err: nil}, }, + "parameter in cue is a list type,": { + reason: "Prepare a list parameter cue file", + name: "workload4", + data: ` +annotations: { + "test":parameter +} + + parameter:[...string] +`, + want: want{data: "{\"items\":{\"type\":\"string\"},\"type\":\"array\"}", err: nil}, + }, + "parameter in cue is an int type,": { + reason: "Prepare an int parameter cue file", + name: "workload5", + data: ` +annotations: { + "test":parameter +} + + parameter: int +`, + want: want{data: "{\"type\":\"integer\"}", err: nil}, + }, + "parameter in cue is a float type,": { + reason: "Prepare a float parameter cue file", + name: "workload6", + data: ` +annotations: { + "test":parameter +} + + parameter: float +`, + want: want{data: "{\"type\":\"number\"}", err: nil}, + }, + "parameter in cue is a bool type,": { + reason: "Prepare a bool parameter cue file", + name: "workload7", + data: ` +annotations: { + "test":parameter +} + + parameter: bool +`, + want: want{data: "{\"type\":\"boolean\"}", err: nil}, + }, "cue doesn't contain parameter section": { reason: "Prepare a cue file which doesn't contain `parameter` section", name: "invalidWorkload", @@ -104,7 +148,28 @@ noParameter: { min: int } `, - want: want{data: "", err: fmt.Errorf("capability invalidWorkload doesn't contain section `parameter`")}, + want: want{data: "{\"type\":\"object\"}", err: nil}, + }, + "cue doesn't parse other sections except parameter": { + reason: "Prepare a cue file which contains `context.appName` field", + name: "withContextField", + data: ` +patch: { + spec: template: metadata: labels: { + for k, v in parameter { + "\(k)": v + } + } + spec: template: metadata: annotations: { + "route-name.oam.dev": #routeName + } +} + +#routeName: "\(context.appName)-\(context.name)" + +parameter: [string]: string +`, + want: want{data: "{\"additionalProperties\":{\"type\":\"string\"},\"type\":\"object\"}", err: nil}, }, } @@ -154,42 +219,6 @@ func TestFixOpenAPISchema(t *testing.T) { } } -func TestGenerateOpenAPISchemaFromCapabilityParameter(t *testing.T) { - var invalidWorkloadName = "IAmAnInvalidWorkloadDefinition" - capabilityDir, _ := system.GetCapabilityDir() - if _, err := os.Stat(capabilityDir); err != nil && os.IsNotExist(err) { - os.MkdirAll(capabilityDir, 0755) - } - - type want struct { - data []byte - err error - } - - cases := map[string]struct { - reason string - capability types.Capability - want want - }{ - "GenerateOpenAPISchemaFromInvalidCapability": { - reason: "generate OpenAPI schema for an invalid Workload/Trait", - capability: types.Capability{Name: invalidWorkloadName}, - want: want{data: nil, err: fmt.Errorf("capability IAmAnInvalidWorkloadDefinition doesn't contain section `parameter`")}, - }, - } - for name, tc := range cases { - t.Run(name, func(t *testing.T) { - got, err := generateOpenAPISchemaFromCapabilityParameter(tc.capability, pd) - if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { - t.Errorf("\n%s\ngetDefinition(...): -want error, +got error:\n%s", tc.reason, diff) - } - if diff := cmp.Diff(tc.want.data, got); diff != "" { - t.Errorf("\n%s\ngetDefinition(...): -want, +got:\n%s", tc.reason, diff) - } - }) - } -} - func TestNewCapabilityComponentDef(t *testing.T) { terraform := &common.Terraform{ Configuration: "test", diff --git a/pkg/utils/common/common.go b/pkg/utils/common/common.go index a0eb057d4..5efe277a7 100644 --- a/pkg/utils/common/common.go +++ b/pkg/utils/common/common.go @@ -30,6 +30,7 @@ import ( "path/filepath" "cuelang.org/go/cue" + "cuelang.org/go/cue/format" "cuelang.org/go/encoding/openapi" "github.com/AlecAivazis/survey/v2" "github.com/ghodss/yaml" @@ -130,8 +131,12 @@ func GenOpenAPI(inst *cue.Instance) ([]byte, error) { if inst.Err != nil { return nil, inst.Err } + paramOnlyIns, err := RefineParameterInstance(inst) + if err != nil { + return nil, err + } defaultConfig := &openapi.Config{} - b, err := openapi.Gen(inst, defaultConfig) + b, err := openapi.Gen(paramOnlyIns, defaultConfig) if err != nil { return nil, err } @@ -140,6 +145,30 @@ func GenOpenAPI(inst *cue.Instance) ([]byte, error) { return out.Bytes(), nil } +// RefineParameterInstance refines cue instance to merely include `parameter` identifier +func RefineParameterInstance(inst *cue.Instance) (*cue.Instance, error) { + r := cue.Runtime{} + paramVal := inst.LookupDef(velacue.ParameterTag) + var paramOnlyStr string + switch k := paramVal.IncompleteKind(); k { + case cue.StructKind, cue.ListKind: + sysopts := []cue.Option{cue.All(), cue.DisallowCycles(true), cue.ResolveReferences(true), cue.Docs(true)} + paramSyntax, _ := format.Node(paramVal.Syntax(sysopts...)) + paramOnlyStr = fmt.Sprintf("#%s: %s\n", velacue.ParameterTag, string(paramSyntax)) + case cue.IntKind, cue.StringKind, cue.FloatKind, cue.BoolKind: + paramOnlyStr = fmt.Sprintf("#%s: %v", velacue.ParameterTag, paramVal) + case cue.BottomKind: + paramOnlyStr = fmt.Sprintf("#%s: {}", velacue.ParameterTag) + default: + return nil, fmt.Errorf("unsupport parameter kind: %s", k.String()) + } + paramOnlyIns, err := r.Compile("-", paramOnlyStr) + if err != nil { + return nil, err + } + return paramOnlyIns, nil +} + // RealtimePrintCommandOutput prints command output in real time // If logFile is "", it will prints the stdout, or it will write to local file func RealtimePrintCommandOutput(cmd *exec.Cmd, logFile string) error { diff --git a/pkg/utils/common/common_test.go b/pkg/utils/common/common_test.go index 7ee2293c7..fec026226 100644 --- a/pkg/utils/common/common_test.go +++ b/pkg/utils/common/common_test.go @@ -171,52 +171,49 @@ name func TestGenOpenAPI(t *testing.T) { type want struct { - data []byte - err error + targetSchemaFile string + err error } - var dir = "testdata" - var validCueFile = "workload1.cue" - var validTargetSchema = "workload1.json" - targetFile := filepath.Join(dir, validTargetSchema) - expect, _ := ioutil.ReadFile(targetFile) - - normalWant := want{ - data: expect, - err: nil, - } - - f := filepath.FromSlash(validCueFile) - - inst := cue.Build(load.Instances([]string{f}, &load.Config{ - Dir: dir, - }))[0] - cases := map[string]struct { reason string - fileDir string fileName string targetSchema string want want }{ "GenOpenAPI": { - reason: "generate OpenAPI schema", - fileDir: dir, - fileName: validCueFile, - targetSchema: validTargetSchema, - want: normalWant, + reason: "generate valid OpenAPI schema with context", + fileName: "workload1.cue", + want: want{ + targetSchemaFile: "workload1.json", + err: nil, + }, + }, + "EmptyOpenAPI": { + reason: "generate empty OpenAPI schema", + fileName: "emptyParameter.cue", + want: want{ + targetSchemaFile: "emptyParameter.json", + err: nil, + }, }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { + inst := cue.Build(load.Instances([]string{filepath.FromSlash(tc.fileName)}, &load.Config{ + Dir: "testdata", + }))[0] got, err := GenOpenAPI(inst) if tc.want.err != nil { if diff := cmp.Diff(tc.want.err, errors.New(err.Error()), test.EquateErrors()); diff != "" { t.Errorf("\n%s\nGenOpenAPIFromFile(...): -want error, +got error:\n%s", tc.reason, diff) } } - - if diff := cmp.Diff(tc.want.data, got); diff != "" { + if tc.want.targetSchemaFile == "" { + return + } + wantSchema, _ := ioutil.ReadFile(filepath.Join("testdata", tc.want.targetSchemaFile)) + if diff := cmp.Diff(wantSchema, got); diff != "" { t.Errorf("\n%s\nGenOpenAPIFromFile(...): -want, +got:\n%s", tc.reason, diff) } }) diff --git a/pkg/utils/common/testdata/emptyParameter.cue b/pkg/utils/common/testdata/emptyParameter.cue new file mode 100644 index 000000000..d432ff799 --- /dev/null +++ b/pkg/utils/common/testdata/emptyParameter.cue @@ -0,0 +1,3 @@ +#routeName: "\(context.appName)-\(context.name)" + +context: {} \ No newline at end of file diff --git a/pkg/utils/common/testdata/emptyParameter.json b/pkg/utils/common/testdata/emptyParameter.json new file mode 100644 index 000000000..58f4e1f09 --- /dev/null +++ b/pkg/utils/common/testdata/emptyParameter.json @@ -0,0 +1,15 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Generated by cue.", + "version": "no version" + }, + "paths": {}, + "components": { + "schemas": { + "parameter": { + "type": "object" + } + } + } +} \ No newline at end of file diff --git a/pkg/utils/common/testdata/workload1.cue b/pkg/utils/common/testdata/workload1.cue index d5a4bca78..cf85a573d 100644 --- a/pkg/utils/common/testdata/workload1.cue +++ b/pkg/utils/common/testdata/workload1.cue @@ -8,3 +8,7 @@ cpu?: string } + +#routeName: "\(context.appName)-\(context.name)" + +context: {} \ No newline at end of file