From 60a114d73851015be9244ad87e20f30ebc50a290 Mon Sep 17 00:00:00 2001 From: Oliver Nocon <33484802+OliverNocon@users.noreply.github.com> Date: Mon, 11 Oct 2021 15:22:24 +0200 Subject: [PATCH] chore(docs): improve step documentation (#3162) * chore(docs): improve step documentation * chore: fix test * chore: add note box * use latest mkdocs imgage * add extensions and fix config --- .github/workflows/documentation.yaml | 2 +- documentation/mkdocs.yml | 9 +++- pkg/documentation/generator.go | 4 +- pkg/documentation/generator/description.go | 34 ++++++++++-- .../generator/description_test.go | 10 ++-- pkg/documentation/generator/main.go | 4 +- pkg/documentation/generator/parameters.go | 50 ++++++++++++----- .../generator/parameters_test.go | 53 +++++++++++++++++-- 8 files changed, 133 insertions(+), 33 deletions(-) diff --git a/.github/workflows/documentation.yaml b/.github/workflows/documentation.yaml index 391e4b7a7..f31f60c8d 100644 --- a/.github/workflows/documentation.yaml +++ b/.github/workflows/documentation.yaml @@ -59,7 +59,7 @@ jobs: docker run \ -u $(id -u):$(id -g) \ -v ${GITHUB_WORKSPACE}/documentation:/docs \ - squidfunk/mkdocs-material:3.0.4 build --clean --strict + squidfunk/mkdocs-material:latest build --clean --strict - name: Provide Docs Metadata run: | diff --git a/documentation/mkdocs.yml b/documentation/mkdocs.yml index 6274da592..e92d5d7d5 100644 --- a/documentation/mkdocs.yml +++ b/documentation/mkdocs.yml @@ -178,10 +178,15 @@ extra: code: 'Ubuntu Mono' markdown_extensions: - admonition - - codehilite(guess_lang=false) - - toc(permalink=true) + - attr_list + - codehilite: + guess_lang: false + - toc: + permalink: true - footnotes - pymdownx.superfences + - pymdownx.tabbed + - pymdownx.details extra_css: - 'css/extra.css' edit_uri: edit/master/documentation/docs diff --git a/pkg/documentation/generator.go b/pkg/documentation/generator.go index 3f0a9dfa3..6182e5548 100644 --- a/pkg/documentation/generator.go +++ b/pkg/documentation/generator.go @@ -31,11 +31,13 @@ func main() { var docTemplatePath string var customLibraryStepFile string var customDefaultFiles sliceFlags + var includeAzure bool flag.StringVar(&metadataPath, "metadataDir", "./resources/metadata", "The directory containing the step metadata. Default points to \\'resources/metadata\\'.") flag.StringVar(&docTemplatePath, "docuDir", "./documentation/docs/steps/", "The directory containing the docu stubs. Default points to \\'documentation/docs/steps/\\'.") flag.StringVar(&customLibraryStepFile, "customLibraryStepFile", "", "") flag.Var(&customDefaultFiles, "customDefaultFile", "Path to a custom default configuration file.") + flag.BoolVar(&includeAzure, "includeAzure", false, "Include Azure-specifics in step documentation.") flag.Parse() @@ -59,7 +61,7 @@ func main() { OpenDocTemplateFile: openDocTemplateFile, DocFileWriter: writeFile, OpenFile: openFile, - }) + }, includeAzure) checkError(err) } diff --git a/pkg/documentation/generator/description.go b/pkg/documentation/generator/description.go index df734b989..51f10bc8f 100644 --- a/pkg/documentation/generator/description.go +++ b/pkg/documentation/generator/description.go @@ -6,13 +6,15 @@ import ( "github.com/SAP/jenkins-library/pkg/config" ) -const configRecommendation = "We recommend to define values of [step parameters](#parameters) via [config.yml file](../configuration.md). In this case, calling the step is reduced to one simple line.
Calling the step can be done either via the Jenkins library step or on the [command line](../cli/index.md)." +const configRecommendation = "We recommend to define values of [step parameters](#parameters) via [.pipeline/config.yml file](../configuration.md).
In this case, calling the step is essentially reduced to defining the step name.
Calling the step can be done either in an orchestrator specific way (e.g. via a Jenkins library step) or on the command line." const ( headlineDescription = "## Description\n\n" headlineUsage = "## Usage\n\n" - headlineJenkinsPipeline = "### Jenkins Pipeline\n\n" - headlineCommandLine = "### Command Line\n\n" + headlineJenkinsPipeline = " === Jenkins\n\n" + headlineCommandLine = " === Command Line\n\n" + headlineAzure = " === Azure\n\n" + spacingTabBox = " " ) // defaultBinaryName is the name of the local binary that is used for sample generation. @@ -45,10 +47,32 @@ func createDescriptionSection(stepData *config.StepData) string { description += headlineDescription + stepData.Metadata.LongDescription + "\n\n" description += headlineUsage description += configRecommendation + "\n\n" + description += `!!! tip ""` + "\n\n" + // add Jenkins-specific information description += headlineJenkinsPipeline - description += fmt.Sprintf("```groovy\nlibrary('%s')\n\n%v script: this\n```\n\n", libraryName, stepData.Metadata.Name) + description += fmt.Sprintf("%v```groovy\n", spacingTabBox) + description += fmt.Sprintf("%vlibrary('%s')\n\n", spacingTabBox, libraryName) + description += fmt.Sprintf("%v%v script: this\n", spacingTabBox, stepData.Metadata.Name) + description += fmt.Sprintf("%v```\n\n", spacingTabBox) + + // add Azure-specific information if activated + if includeAzure { + description += headlineAzure + description += fmt.Sprintf("%v```\n", spacingTabBox) + description += fmt.Sprintf("%vsteps:\n", spacingTabBox) + description += fmt.Sprintf("%v - task: piper@1\n", spacingTabBox) + description += fmt.Sprintf("%v name: %v\n", spacingTabBox, stepData.Metadata.Name) + description += fmt.Sprintf("%v inputs:\n", spacingTabBox) + description += fmt.Sprintf("%v stepName: %v\n", spacingTabBox, stepData.Metadata.Name) + description += fmt.Sprintf("%v```\n\n", spacingTabBox) + } + + // add command line information description += headlineCommandLine - description += fmt.Sprintf("```sh\n%s %v\n```\n\n", binaryName, stepData.Metadata.Name) + description += fmt.Sprintf("%v```sh\n", spacingTabBox) + description += fmt.Sprintf("%v%s %v\n", spacingTabBox, binaryName, stepData.Metadata.Name) + description += fmt.Sprintf("%v```\n\n", spacingTabBox) + description += stepOutputs(stepData) return description } diff --git a/pkg/documentation/generator/description_test.go b/pkg/documentation/generator/description_test.go index c38ad0076..4539b2199 100644 --- a/pkg/documentation/generator/description_test.go +++ b/pkg/documentation/generator/description_test.go @@ -48,8 +48,9 @@ func TestCreateDescriptionSection(t *testing.T) { }, want: headlineDescription + "TestDescription" + "\n\n" + headlineUsage + configRecommendation + "\n\n" + - headlineJenkinsPipeline + "```groovy\nlibrary('piper-lib-os')\n\nteststep script: this\n```" + "\n\n" + - headlineCommandLine + "```sh\npiper teststep\n```" + "\n\n", + "!!! tip \"\"" + "\n\n" + + headlineJenkinsPipeline + " ```groovy\n library('piper-lib-os')\n\n teststep script: this\n ```" + "\n\n" + + headlineCommandLine + " ```sh\n piper teststep\n ```" + "\n\n", }, { name: "custom step description section", @@ -58,8 +59,9 @@ func TestCreateDescriptionSection(t *testing.T) { }, want: headlineDescription + "TestDescription" + "\n\n" + headlineUsage + configRecommendation + "\n\n" + - headlineJenkinsPipeline + "```groovy\nlibrary('myLibrary')\n\nmyCustomStep script: this\n```" + "\n\n" + - headlineCommandLine + "```sh\nmyBinary myCustomStep\n```" + "\n\n", + "!!! tip \"\"" + "\n\n" + + headlineJenkinsPipeline + " ```groovy\n library('myLibrary')\n\n myCustomStep script: this\n ```" + "\n\n" + + headlineCommandLine + " ```sh\n myBinary myCustomStep\n ```" + "\n\n", }, } for _, testcase := range tests { diff --git a/pkg/documentation/generator/main.go b/pkg/documentation/generator/main.go index 6bc1f1b4e..16f5bddb2 100644 --- a/pkg/documentation/generator/main.go +++ b/pkg/documentation/generator/main.go @@ -19,6 +19,7 @@ type DocuHelperData struct { } var stepParameterNames []string +var includeAzure bool func readStepConfiguration(stepMetadata config.StepData, customDefaultFiles []string, docuHelperData DocuHelperData) config.StepConfig { filters := stepMetadata.GetParameterFilters() @@ -53,7 +54,8 @@ func readStepConfiguration(stepMetadata config.StepData, customDefaultFiles []st } // GenerateStepDocumentation generates step coding based on step configuration provided in yaml files -func GenerateStepDocumentation(metadataFiles []string, customDefaultFiles []string, docuHelperData DocuHelperData) error { +func GenerateStepDocumentation(metadataFiles []string, customDefaultFiles []string, docuHelperData DocuHelperData, azure bool) error { + includeAzure = azure for key := range metadataFiles { stepMetadata := readStepMetadata(metadataFiles[key], docuHelperData) diff --git a/pkg/documentation/generator/parameters.go b/pkg/documentation/generator/parameters.go index aa28497c2..f3fa807ef 100644 --- a/pkg/documentation/generator/parameters.go +++ b/pkg/documentation/generator/parameters.go @@ -16,8 +16,12 @@ func createParametersSection(stepData *config.StepData) string { // sort parameters alphabetically with mandatory parameters first sortStepParameters(stepData, true) - parameters += "### Overview\n\n" - parameters += createParameterOverview(stepData) + parameters += "### Overview - Step\n\n" + parameters += createParameterOverview(stepData, false) + + parameters += "### Overview - Execution Environment\n\n" + parameters += "!!! note \"Orchestrator-specific only\"\n\n These parameters are relevant for orchestrator usage and not considered when using the command line option.\n\n" + parameters += createParameterOverview(stepData, true) // sort parameters alphabetically sortStepParameters(stepData, false) @@ -27,12 +31,15 @@ func createParametersSection(stepData *config.StepData) string { return parameters } -func createParameterOverview(stepData *config.StepData) string { +func createParameterOverview(stepData *config.StepData, executionEnvironment bool) string { var table = "| Name | Mandatory | Additional information |\n" table += "| ---- | --------- | ---------------------- |\n" for _, param := range stepData.Spec.Inputs.Parameters { - table += fmt.Sprintf("| [%v](#%v) | %v | %v |\n", param.Name, strings.ToLower(param.Name), ifThenElse(param.Mandatory, "**yes**", "no"), parameterFurtherInfo(param.Name, stepData)) + furtherInfo, err := parameterFurtherInfo(param.Name, stepData, executionEnvironment) + if err == nil { + table += fmt.Sprintf("| [%v](#%v) | %v | %v |\n", param.Name, strings.ToLower(param.Name), ifThenElse(param.Mandatory, "**yes**", "no"), furtherInfo) + } } table += "\n" @@ -40,29 +47,33 @@ func createParameterOverview(stepData *config.StepData) string { return table } -func parameterFurtherInfo(paramName string, stepData *config.StepData) string { +func parameterFurtherInfo(paramName string, stepData *config.StepData, executionEnvironment bool) (string, error) { // handle general parameters // ToDo: add special handling once we have more than one general parameter to consider if paramName == "verbose" { - return "activates debug output" + return checkParameterInfo("activates debug output", true, executionEnvironment) } if paramName == "script" { - return "[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#) reference to Jenkins main pipeline script" + return checkParameterInfo("[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#) reference to Jenkins main pipeline script", true, executionEnvironment) } - // handle Jenkins-specific parameters + // handle non-step parameters (e.g. Jenkins-specific parameters as well as execution environment parameters) + jenkinsParams := []string{"containerCommand", "containerName", "containerShell", "dockerVolumeBind", "dockerWorkspace", "sidecarReadyCommand", "sidecarWorkspace", "stashContent"} if !contains(stepParameterNames, paramName) { for _, secret := range stepData.Spec.Inputs.Secrets { if paramName == secret.Name && secret.Type == "jenkins" { - return "[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#) id of credentials ([using credentials](https://www.jenkins.io/doc/book/using/using-credentials/))" + return checkParameterInfo("[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#) id of credentials ([using credentials](https://www.jenkins.io/doc/book/using/using-credentials/))", true, executionEnvironment) } } - return "[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#)" + if contains(jenkinsParams, paramName) { + return checkParameterInfo("[![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#)", false, executionEnvironment) + } + return checkParameterInfo("", false, executionEnvironment) } - // handle Secrets + // handle step-parameters (incl. secrets) for _, param := range stepData.Spec.Inputs.Parameters { if paramName == param.Name { if param.Secret { @@ -76,12 +87,23 @@ func parameterFurtherInfo(paramName string, stepData *config.StepData) string { secretInfo += fmt.Sprintf(" ([`%v`](#%v))", res.Name, strings.ToLower(res.Name)) } } - return secretInfo + return checkParameterInfo(secretInfo, true, executionEnvironment) } - return "" + return checkParameterInfo("", true, executionEnvironment) } } - return "" + return checkParameterInfo("", true, executionEnvironment) +} + +func checkParameterInfo(furtherInfo string, stepParam bool, executionEnvironment bool) (string, error) { + if stepParam && !executionEnvironment || !stepParam && executionEnvironment { + return furtherInfo, nil + } + + if executionEnvironment { + return "", fmt.Errorf("step parameter not relevant as execution environment parameter") + } + return "", fmt.Errorf("execution environment parameter not relevant as step parameter") } func createParameterDetails(stepData *config.StepData) string { diff --git a/pkg/documentation/generator/parameters_test.go b/pkg/documentation/generator/parameters_test.go index fa1ec9f83..2ba00a8ac 100644 --- a/pkg/documentation/generator/parameters_test.go +++ b/pkg/documentation/generator/parameters_test.go @@ -9,6 +9,7 @@ import ( ) func TestCreateParameterOverview(t *testing.T) { + stepData := config.StepData{ Spec: config.StepSpec{ Inputs: config.StepInputs{ @@ -17,20 +18,35 @@ func TestCreateParameterOverview(t *testing.T) { }, Parameters: []config.StepParameters{ {Name: "param1"}, + {Name: "dockerImage", Default: "testImage"}, {Name: "stashContent", Default: "testStash"}, }, }, }, } + stepParameterNames = []string{"param1"} - expected := `| Name | Mandatory | Additional information | + t.Run("Test Step Section", func(t *testing.T) { + + expected := `| Name | Mandatory | Additional information | | ---- | --------- | ---------------------- | | [param1](#param1) | no | | + +` + + assert.Equal(t, expected, createParameterOverview(&stepData, false)) + }) + + t.Run("Test Execution Section", func(t *testing.T) { + expected := `| Name | Mandatory | Additional information | +| ---- | --------- | ---------------------- | +| [dockerImage](#dockerimage) | no | | | [stashContent](#stashcontent) | no | [![Jenkins only](https://img.shields.io/badge/-Jenkins%20only-yellowgreen)](#) | ` - stepParameterNames = []string{"param1"} - assert.Equal(t, expected, createParameterOverview(&stepData)) + assert.Equal(t, expected, createParameterOverview(&stepData, true)) + }) + stepParameterNames = []string{} } @@ -44,7 +60,6 @@ func TestParameterFurtherInfo(t *testing.T) { }{ {paramName: "verbose", stepParams: []string{}, stepData: nil, contains: "activates debug output"}, {paramName: "script", stepParams: []string{}, stepData: nil, contains: "reference to Jenkins main pipeline script"}, - {paramName: "contextTest", stepParams: []string{}, stepData: &config.StepData{}, contains: "Jenkins only", notContains: []string{"pipeline script", "id of credentials"}}, {paramName: "noop", stepParams: []string{"noop"}, stepData: &config.StepData{Spec: config.StepSpec{Inputs: config.StepInputs{Parameters: []config.StepParameters{}}}}, contains: ""}, { paramName: "testCredentialId", @@ -89,7 +104,8 @@ func TestParameterFurtherInfo(t *testing.T) { for _, test := range tt { stepParameterNames = test.stepParams - res := parameterFurtherInfo(test.paramName, test.stepData) + res, err := parameterFurtherInfo(test.paramName, test.stepData, false) + assert.NoError(t, err) stepParameterNames = []string{} if len(test.contains) == 0 { assert.Equalf(t, test.contains, res, fmt.Sprintf("param %v", test.paramName)) @@ -102,6 +118,33 @@ func TestParameterFurtherInfo(t *testing.T) { } } +func TestCheckParameterInfo(t *testing.T) { + t.Parallel() + tt := []struct { + info string + stepParam bool + executionEnvironment bool + expected string + expectedErr error + }{ + {info: "step param", stepParam: true, executionEnvironment: false, expected: "step param", expectedErr: nil}, + {info: "execution param", stepParam: false, executionEnvironment: true, expected: "execution param", expectedErr: nil}, + {info: "step param in execution", stepParam: true, executionEnvironment: true, expected: "", expectedErr: fmt.Errorf("step parameter not relevant as execution environment parameter")}, + {info: "execution param in step", stepParam: false, executionEnvironment: false, expected: "", expectedErr: fmt.Errorf("execution environment parameter not relevant as step parameter")}, + } + + for _, test := range tt { + result, err := checkParameterInfo(test.info, test.stepParam, test.executionEnvironment) + if test.expectedErr == nil { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, fmt.Sprint(test.expectedErr)) + } + assert.Equal(t, test.expected, result) + } + +} + func TestCreateParameterDetails(t *testing.T) { stepData := config.StepData{ Spec: config.StepSpec{