diff --git a/references/docgen/def-doc/trait/env.eg.md b/references/docgen/def-doc/trait/env.eg.md index a342401ee..b8aa94f5d 100644 --- a/references/docgen/def-doc/trait/env.eg.md +++ b/references/docgen/def-doc/trait/env.eg.md @@ -28,4 +28,30 @@ spec: env: key_for_nginx_first: value_first key_for_nginx_second: value_second +``` + +```yaml +apiVersion: core.oam.dev/v1beta1 +kind: Application +metadata: + name: busybox +spec: + components: + - name: busybox + type: webservice + properties: + image: busybox + cmd: ["sleep", "86400"] + traits: + - type: sidecar + properties: + name: sidecar-nginx + image: nginx + - type: env + properties: + # you can use env to control one container, if containerName not specified, it will patch on the first index container + containerName: busybox + env: + key_for_busybox_first: value_first + key_for_busybox_second: value_second ``` \ No newline at end of file diff --git a/references/docgen/markdown.go b/references/docgen/markdown.go index bcd17172e..9ba847ebf 100644 --- a/references/docgen/markdown.go +++ b/references/docgen/markdown.go @@ -24,19 +24,15 @@ import ( "sort" "strings" + "github.com/kubevela/workflow/pkg/cue/packages" + "github.com/pkg/errors" "golang.org/x/text/cases" "golang.org/x/text/language" - - "github.com/oam-dev/kubevela/pkg/oam/discoverymapper" - "k8s.io/klog/v2" - "github.com/pkg/errors" - - "github.com/kubevela/workflow/pkg/cue/packages" - "github.com/oam-dev/kubevela/apis/types" "github.com/oam-dev/kubevela/pkg/cue" + "github.com/oam-dev/kubevela/pkg/oam/discoverymapper" "github.com/oam-dev/kubevela/pkg/utils/common" ) diff --git a/references/docgen/parser.go b/references/docgen/parser.go index 244133d79..81c297f77 100644 --- a/references/docgen/parser.go +++ b/references/docgen/parser.go @@ -156,8 +156,66 @@ func (ref *ParseReference) prepareConsoleParameter(tableName string, parameterLi return ConsoleReference{TableName: tableName, TableObject: table} } -// parseParameters parses every parameter -// TODO(wonderflowe2e/plugin/plugin_test.go:122): refactor the code to reduce the complexity +func cueValue2Ident(val cue.Value) *ast.Ident { + var ident *ast.Ident + if source, ok := val.Source().(*ast.Ident); ok { + ident = source + } + if source, ok := val.Source().(*ast.Field); ok { + if v, ok := source.Value.(*ast.Ident); ok { + ident = v + } + } + return ident +} + +func getIndentName(val cue.Value) string { + ident := cueValue2Ident(val) + if ident != nil && len(ident.Name) != 0 { + return strings.TrimPrefix(ident.Name, "#") + } + return val.IncompleteKind().String() +} + +func getConcreteOrValueType(val cue.Value) string { + op, elements := val.Expr() + if op != cue.OrOp { + return val.IncompleteKind().String() + } + var printTypes []string + for _, ev := range elements { + incompleteKind := ev.IncompleteKind().String() + if !ev.IsConcrete() { + return incompleteKind + } + ident := cueValue2Ident(ev) + if ident != nil && len(ident.Name) != 0 { + printTypes = append(printTypes, strings.TrimPrefix(ident.Name, "#")) + } else { + // only convert string in `or` operator for now + opName, err := ev.String() + if err != nil { + return incompleteKind + } + opName = `"` + opName + `"` + printTypes = append(printTypes, opName) + } + } + return strings.Join(printTypes, " or ") +} + +func getSuffix(capName string, containSuffix bool) (string, string) { + var suffixTitle = " (" + capName + ")" + var suffixRef = "-" + strings.ToLower(capName) + if !containSuffix || capName == "" { + suffixTitle = "" + suffixRef = "" + } + return suffixTitle, suffixRef +} + +// parseParameters parses every parameter to docs +// TODO(wonderflow): refactor the code to reduce the complexity // nolint:staticcheck,gocyclo func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, paramKey string, depth int, containSuffix bool) (string, []ConsoleReference, error) { var doc string @@ -167,19 +225,15 @@ func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, if !paraValue.Exists() { return "", console, nil } + suffixTitle, suffixRef := getSuffix(capName, containSuffix) - var suffixTitle = " (" + capName + ")" - var suffixRef = "-" + strings.ToLower(capName) - if !containSuffix || capName == "" { - suffixTitle = "" - suffixRef = "" - } switch paraValue.Kind() { case cue.StructKind: arguments, err := paraValue.Struct() if err != nil { - return "", nil, fmt.Errorf("arguments not defined as struct %w", err) + return "", nil, fmt.Errorf("field %s not defined as struct %w", paramKey, err) } + if arguments.Len() == 0 { var param ReferenceParameter param.Name = "\\-" @@ -200,7 +254,7 @@ func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, continue } val := fi.Value - name := fi.Name + name := fi.Selector param.Name = name if def, ok := val.Default(); ok && def.IsConcrete() { param.Default = velacue.GetDefault(def) @@ -212,19 +266,18 @@ func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, case cue.StructKind: if subField, err := val.Struct(); err == nil && subField.Len() == 0 { // err cannot be not nil,so ignore it if mapValue, ok := val.Elem(); ok { - var ident *ast.Ident - if source, ok := mapValue.Source().(*ast.Ident); ok { - ident = source - } - if source, ok := mapValue.Source().(*ast.Field); ok { - if v, ok := source.Value.(*ast.Ident); ok { - ident = v + indentName := getIndentName(mapValue) + _, err := mapValue.Fields() + if err == nil { + subDoc, subConsole, err := ref.parseParameters(capName, mapValue, indentName, depth+1, containSuffix) + if err != nil { + return "", nil, err } - } - if ident != nil && len(ident.Name) != 0 { - param.PrintableType = fmt.Sprintf("map[string]:%s", ident.Name) + param.PrintableType = fmt.Sprintf("map[string]%s(#%s%s)", indentName, strings.ToLower(indentName), suffixRef) + doc += subDoc + console = append(console, subConsole...) } else { - param.PrintableType = fmt.Sprintf("map[string]:%s", mapValue.IncompleteKind().String()) + param.PrintableType = "map[string]" + mapValue.IncompleteKind().String() } } else { param.PrintableType = val.IncompleteKind().String() @@ -234,7 +287,10 @@ func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, if op == cue.OrOp { var printTypes []string for idx, ev := range elements { - opName := fmt.Sprintf("%s-option-%d", name, idx) + opName := getIndentName(ev) + if opName == "struct" { + opName = fmt.Sprintf("type-option-%d", idx+1) + } subDoc, subConsole, err := ref.parseParameters(capName, ev, opName, depth+1, containSuffix) if err != nil { return "", nil, err @@ -276,16 +332,35 @@ func (ref *ParseReference) parseParameters(capName string, paraValue cue.Value, param.PrintableType = fmt.Sprintf("[]%s", elem.IncompleteKind().String()) } default: - param.PrintableType = param.Type.String() + param.PrintableType = getConcreteOrValueType(val) } params = append(params, param) } default: var param ReferenceParameter - // TODO better composition type handling, see command trait. - param.Name = "--" - param.Usage = "Composition type" - param.PrintableType = extractTypeFromError(paraValue) + op, elements := paraValue.Expr() + if op == cue.OrOp { + var printTypes []string + for idx, ev := range elements { + opName := getIndentName(ev) + if opName == "struct" { + opName = fmt.Sprintf("type-option-%d", idx+1) + } + subDoc, subConsole, err := ref.parseParameters(capName, ev, opName, depth+1, containSuffix) + if err != nil { + return "", nil, err + } + printTypes = append(printTypes, fmt.Sprintf("[%s](#%s%s)", opName, strings.ToLower(opName), suffixRef)) + doc += subDoc + console = append(console, subConsole...) + } + param.PrintableType = strings.Join(printTypes, " or ") + } else { + // TODO more composition type to be handle here + param.Name = "--" + param.Usage = "Unsupported Composition Type" + param.PrintableType = extractTypeFromError(paraValue) + } params = append(params, param) } diff --git a/references/docgen/parser_test.go b/references/docgen/parser_test.go index 0426351f4..e516788f4 100644 --- a/references/docgen/parser_test.go +++ b/references/docgen/parser_test.go @@ -19,7 +19,6 @@ package docgen import ( "encoding/json" "fmt" - "io/ioutil" "os" "path/filepath" "reflect" @@ -562,34 +561,272 @@ func TestParseLocalFile(t *testing.T) { } func TestExtractParameter(t *testing.T) { - ref := &ConsoleReference{} - cueTemplate := ` -parameter: { + testcases := map[string]struct { + cueTemplate string + contains string + }{ + "normal-case": { + cueTemplate: `parameter: { + str: string + itr: int + btr: bool + ct: cts: string +}`, + contains: `### normal-case + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + str | | string | true | + itr | | int | true | + btr | | bool | true | + ct | | [ct](#ct) | true | + + +#### ct + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + cts | | string | true |`, + }, + "normal-map-string-string": { + cueTemplate: `parameter: { + envMappings: [string]: string +}`, + contains: `### normal-map-string-string + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + envMappings | | map[string]string | true |`, + }, + "normal-map-case": { + cueTemplate: `parameter: { // +usage=The mapping of environment variables to secret envMappings: [string]: #KeySecret } #KeySecret: { key?: string secret: string -} -` - oldStdout := os.Stdout - defer func() { - os.Stdout = oldStdout - }() +}`, + contains: `### normal-map-case - r, w, _ := os.Pipe() - os.Stdout = w - cueValue, _ := common.GetCUEParameterValue(cueTemplate, nil) - defaultDepth := 0 - defaultDisplay := "console" - ref.DisplayFormat = defaultDisplay - _, console, err := ref.parseParameters("", cueValue, Specification, defaultDepth, false) - assert.NoError(t, err) - assert.Equal(t, 1, len(console)) - console[0].TableObject.Render() - err = w.Close() - assert.NoError(t, err) - out, _ := ioutil.ReadAll(r) - assert.Contains(t, string(out), "map[string]:#KeySecret") + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + envMappings | The mapping of environment variables to secret. | map[string]KeySecret(#keysecret) | true | + + +#### KeySecret + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + key | | string | false | + secret | | string | true |`, + }, + "or-case-with-type": { + cueTemplate: ` parameter: { + orValue: #KeyConfig | #KeySecret + } + + #KeySecret: { + key: "abc" + secret: string + } + + #KeyConfig: { + key: "abc" + config: string + }`, + contains: `### or-case-with-type + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + orValue | | [KeyConfig](#keyconfig) or [KeySecret](#keysecret) | true | + + +#### KeyConfig + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + key | | string | true | + config | | string | true | + + +#### KeySecret + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + key | | string | true | + secret | | string | true | `, + }, + "or-type-with-const-str": { + cueTemplate: `parameter: { + type: *"configMap" | "secret" | "emptyDir" | "ephemeral" +}`, + contains: `### or-type-with-const-str + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + type | | "configMap" or "secret" or "emptyDir" or "ephemeral" | false | configMap`, + }, + "or-type-with-const-and-string": { + cueTemplate: `parameter: { + type: *"configMap" | "secret" | "emptyDir" | string +}`, + contains: `### or-type-with-const-and-string + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + type | | string | false | configMap`, + }, + "var-or-with-struct-var": { + cueTemplate: ` + parameter: { + orValue: KeyConfig | KeySecret + } + + KeySecret: { + key: "abc" + secret: string + } + + KeyConfig: { + key: "abc" + config: string + }`, + contains: `### var-or-with-struct-var + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + orValue | | [KeyConfig](#keyconfig) or [KeySecret](#keysecret) | true | + + +#### KeyConfig + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + key | | string | true | + config | | string | true | + + +#### KeySecret + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + key | | string | true | + secret | | string | true | `, + }, + } + + ref := &MarkdownReference{} + for key, ca := range testcases { + cueValue, _ := common.GetCUEParameterValue(ca.cueTemplate, nil) + out, _, err := ref.parseParameters("", cueValue, key, 0, false) + assert.NoError(t, err, key) + assert.Contains(t, out, ca.contains, key) + } +} + +func TestExtractParameterFromFiles(t *testing.T) { + ref := &MarkdownReference{} + testcases := map[string]struct { + path string + contains string + }{ + "env": { + path: "testdata/parameter/env.cue", + contains: `### env + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + | | [PatchParams](#patchparams) or [type-option-2](#type-option-2) | false | + + +#### PatchParams + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containerName | Specify the name of the target container, if not set, use the component name. | string | false | empty + replace | Specify if replacing the whole environment settings for the container. | bool | false | false + env | Specify the environment variables to merge, if key already existing, override its value. | map[string]string | true | + unset | Specify which existing environment variables to unset. | []string | true | + + +#### type-option-2 + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containers | Specify the environment variables for multiple containers. | [[]containers](#containers) | true | + + +##### containers + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containerName | Specify the name of the target container, if not set, use the component name. | string | false | empty + replace | Specify if replacing the whole environment settings for the container. | bool | false | false + env | Specify the environment variables to merge, if key already existing, override its value. | map[string]string | true | + unset | Specify which existing environment variables to unset. | []string | true |`, + }, + "command": { + path: "testdata/parameter/command.cue", + contains: `### command + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + | | [PatchParams](#patchparams) or [type-option-2](#type-option-2) | false | + + +#### PatchParams + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containerName | Specify the name of the target container, if not set, use the component name. | string | false | empty + command | Specify the command to use in the target container, if not set, it will not be changed. | null | true | + args | Specify the args to use in the target container, if set, it will override existing args. | null | true | + addArgs | Specify the args to add in the target container, existing args will be kept, cannot be used with args. | null | true | + delArgs | Specify the existing args to delete in the target container, cannot be used with args. | null | true | + + +#### type-option-2 + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containers | Specify the commands for multiple containers. | [[]containers](#containers) | true | + + +##### containers + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + containerName | Specify the name of the target container, if not set, use the component name. | string | false | empty + command | Specify the command to use in the target container, if not set, it will not be changed. | null | true | + args | Specify the args to use in the target container, if set, it will override existing args. | null | true | + addArgs | Specify the args to add in the target container, existing args will be kept, cannot be used with args. | null | true | + delArgs | Specify the existing args to delete in the target container, cannot be used with args. | null | true |`, + }, + "condition": { + path: "testdata/parameter/condition.cue", + contains: `### condition + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + volumes | | [[]volumes](#volumes) | true | + + +#### volumes + + Name | Description | Type | Required | Default + ---- | ----------- | ---- | -------- | ------- + name | | string | true | + defaultMode | only works when type equals configmap. | int | false | 420 + type | | "configMap" or "secret" or "emptyDir" or "ephemeral" | false | configMap`, + }, + } + for key, ca := range testcases { + content, err := os.ReadFile(ca.path) + assert.NoError(t, err, ca.path) + cueValue, _ := common.GetCUEParameterValue(string(content), nil) + out, _, err := ref.parseParameters("", cueValue, key, 0, false) + assert.NoError(t, err, key) + assert.Contains(t, out, ca.contains, key) + } } diff --git a/references/docgen/testdata/parameter/command.cue b/references/docgen/testdata/parameter/command.cue new file mode 100644 index 000000000..5f6dbfd9e --- /dev/null +++ b/references/docgen/testdata/parameter/command.cue @@ -0,0 +1,17 @@ +#PatchParams: { + // +usage=Specify the name of the target container, if not set, use the component name + containerName: *"" | string + // +usage=Specify the command to use in the target container, if not set, it will not be changed + command: *null | [...string] + // +usage=Specify the args to use in the target container, if set, it will override existing args + args: *null | [...string] + // +usage=Specify the args to add in the target container, existing args will be kept, cannot be used with args + addArgs: *null | [...string] + // +usage=Specify the existing args to delete in the target container, cannot be used with args + delArgs: *null | [...string] +} + +parameter: *#PatchParams | close({ + // +usage=Specify the commands for multiple containers + containers: [...#PatchParams] +}) diff --git a/references/docgen/testdata/parameter/condition.cue b/references/docgen/testdata/parameter/condition.cue new file mode 100644 index 000000000..36c94af8a --- /dev/null +++ b/references/docgen/testdata/parameter/condition.cue @@ -0,0 +1,10 @@ +parameter: { + volumes: [...{ + name: string + type: *"configMap" | "secret" | "emptyDir" | "ephemeral" + if type == "configMap" { + //+usage=only works when type equals configmap + defaultMode: *420 | int + }}, + ] +} diff --git a/references/docgen/testdata/parameter/env.cue b/references/docgen/testdata/parameter/env.cue new file mode 100644 index 000000000..f09d5b6d5 --- /dev/null +++ b/references/docgen/testdata/parameter/env.cue @@ -0,0 +1,14 @@ +#PatchParams: { + // +usage=Specify the name of the target container, if not set, use the component name + containerName: *"" | string + // +usage=Specify if replacing the whole environment settings for the container + replace: *false | bool + // +usage=Specify the environment variables to merge, if key already existing, override its value + env: [string]: string + // +usage=Specify which existing environment variables to unset + unset: *[] | [...string] +} +parameter: *#PatchParams | close({ + // +usage=Specify the environment variables for multiple containers + containers: [...#PatchParams] +})