Repository: apigee/registry
Pull request #387 (2021-11-08 21:48:22)

Implement a sample linter for OpenAPI that enforces rules on descriptions
## Summary
The following pull request creates a sample OpenAPI linter which serves as a proof-of-concept for custom linter creation in the API Registry. It lints two rules:
- `description-less-than-1000-chars`: Ensures that description fields in the OpenAPI spec are less than 1000 characters.
- `description-contains-no-tags`: Ensures that description fields in the OpenAPI spec contain no tags (such as HTML tags).

## Test
- Have registry running on `localhost:8080`.
- Place [the following OpenAPI Sample spec](https://gist.github.com/muhammadharis/13f1681ad65bf6a59366f3068748c0d8) into ~/petstore.yaml 
- Run `LOCAL.sh`
- Ensure that the following style guide is located under `~/styleguide.yaml`:
```yaml
id: openapistyleguide
mime_types:
  - application/x.openapi+gzip;version=2
  - application/x.openapi+gzip;version=3
guidelines:
  - id: Description
    display_name: Govern Description Properties
    description: This guideline governs properties for ref fields on specs.
    rules:
      - id: DescriptionLessThan1000Chars
        description: Description fields should be less than 1000 chars
        linter: openapi-sample
        linter_rulename: description-less-than-1000-chars
        severity: ERROR

      - id: DescriptionContainsNoTags
        description: Description fields should not contain tags
        linter: openapi-sample
        linter_rulename: description-contains-no-tags
        severity: ERROR
    status: ACTIVE
linters:
  - name: openapi-sample
```


Run the following series of commands:

```bash
apg admin delete-project --name projects/example

apg admin create-project --project_id example --insecure --address localhost:8080

apg registry create-api --api_id petstore --parent=projects/example/locations/global

apg registry create-api-version --api_version_id v1 --parent=projects/example/locations/global/apis/petstore

registry upload spec --version=projects/example/locations/global/apis/petstore/versions/v1 --style=openapi ~/petstore.yaml

registry upload styleguide ~/styleguide.yaml --project-id=example

registry compute conformance projects/test/locations/global/apis/petstore/versions/v1/specs/petstore.yaml

registry get projects/example/locations/global/apis/petstore/versions/v1/specs/petstore.yaml/artifacts/conformance-openapistyleguide --contents
```

Ensure that the output matches [the following gist](https://gist.github.com/muhammadharis/7ed03bf240d6b34cbcf97a89f6c2718e).

## Running Unit Tests
`go test -timeout 30s github.com/apigee/registry/cmd/registry/plugins/registry-lint-openapi-sample`

muhammadharis: @timburks Thanks so much for the review. I've addressed your comments, and here is a summary of what I addressed:
> It's not clear to me why this is a separate function and not just part of Run()

You are right. I was trying to keep consistent with the Spectral and API Linter plugin in which we were stubbing out the calls to the linter in unit tests through the `executer` since they were path-dependent (would only work if the linter was installed on the path, which might not be the case in unit tests). I have removed additional Impl function.

>This could just be a function type - the struct seems to only exist to define this function - when needed, we could pass a function pointer instead of a struct instance.

Great point, I like this idea. Although since I've removed the executer altogether in this specific PR, I think this doesn't need to be done anymore. However with that being said this method can be used to simplify the logic for the Spectral and API Linter plugin. I will open up a subsequent PR implementing this in Spectral and API linter.




Patch:
diff --git a/cmd/registry/plugins/registry-lint-openapi-sample/README.md b/cmd/registry/plugins/registry-lint-openapi-sample/README.md
new file mode 100644
index 0000000..9a6242b
--- /dev/null
+++ b/cmd/registry/plugins/registry-lint-openapi-sample/README.md
@@ -0,0 +1,4 @@
+The sample OpenAPI linter in this directory enforces the following rules:
+
+`description-less-than-1000-chars`: Ensures that description fields in the OpenAPI spec are less than 1000 characters.
+`description-contains-no-tags`: Ensures that description fields in the OpenAPI spec contain no tags (such as HTML tags).
diff --git a/cmd/registry/plugins/registry-lint-openapi-sample/main.go b/cmd/registry/plugins/registry-lint-openapi-sample/main.go
new file mode 100644
index 0000000..b272449
--- /dev/null
+++ b/cmd/registry/plugins/registry-lint-openapi-sample/main.go
@@ -0,0 +1,211 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"github.com/apex/log"
+	lint "github.com/apigee/registry/cmd/registry/plugins/linter"
+	"github.com/apigee/registry/rpc"
+	"gopkg.in/yaml.v3"
+)
+
+type DescriptionField struct {
+	StartLineNumber   int
+	StartColumnNumber int
+	EndLineNumber     int
+	EndColumnNumber   int
+	Description       string
+}
+
+// sampleOpenApiLinterRunner implements the LinterRunner interface for the sample OpenAPI linter.
+type sampleOpenApiLinterRunner struct{}
+
+const descriptionLessThan1000CharsRuleId = "description-less-than-1000-chars"
+const descriptionContainsNoTagsRuleId = "description-contains-no-tags"
+
+func (linter *sampleOpenApiLinterRunner) Run(req *rpc.LinterRequest) (*rpc.LinterResponse, error) {
+	lintFiles := make([]*rpc.LintFile, 0)
+
+	// Traverse the files in the directory
+	err := filepath.Walk(req.GetSpecDirectory(), func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// Async API and Open API specs are YAML files
+		if !strings.HasSuffix(path, ".yaml") && !strings.HasSuffix(path, ".yml") {
+			return nil
+		}
+
+		// Execute the linter.
+		problems, err := lintFile(path, req.GetRuleIds())
+		if err != nil {
+			return err
+		}
+
+		// Formulate the response.
+		lintFiles = append(lintFiles, &rpc.LintFile{
+			FilePath: path,
+			Problems: problems,
+		})
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return &rpc.LinterResponse{
+		Lint: &rpc.Lint{
+			Name:  "registry-lint-openapi-sample",
+			Files: lintFiles,
+		},
+	}, nil
+}
+
+func lintFile(specPath string, ruleIds []string) ([]*rpc.LintProblem, error) {
+	specFile, err := ioutil.ReadFile(specPath)
+	if err != nil {
+		return nil, err
+	}
+
+	var parsedNode yaml.Node
+	err = yaml.Unmarshal(specFile, &parsedNode)
+	if err != nil {
+		log.Fatalf("Unmarshal node: %v", err)
+	}
+
+	problems := make([]*rpc.LintProblem, 0)
+	for _, ruleId := range ruleIds {
+		lintProblems, err := lintWithRule(&parsedNode, ruleId)
+		if err != nil {
+			log.Errorf("Error while linting %s", err)
+			continue
+		}
+
+		problems = append(problems, lintProblems...)
+	}
+
+	return problems, nil
+}
+
+func getDescriptionsFromSpec(node *yaml.Node) []*DescriptionField {
+	results := make([]*DescriptionField, 0)
+	getDescriptionsFromSpecHelper(node, &results)
+	return results
+}
+
+func getDescriptionsFromSpecHelper(node *yaml.Node, results *[]*DescriptionField) {
+	contentLength := len(node.Content)
+	for i := 0; i < contentLength; i++ {
+		child := node.Content[i]
+		if i-1 >= 0 && node.Content[i-1].Kind == yaml.ScalarNode && node.Content[i-1].Value == "description" {
+			endLineNumber := child.Line + 1
+			endLineColumn := 0
+			if i+1 < contentLength {
+				endLineNumber = node.Content[i+1].Line
+				endLineColumn = node.Content[i+1].Column
+			}
+
+			*results = append(*results, &DescriptionField{
+				StartLineNumber:   child.Line,
+				StartColumnNumber: child.Column,
+				EndLineNumber:     endLineNumber,
+				EndColumnNumber:   endLineColumn,
+				Description:       child.Value,
+			})
+		}
+
+		if child.Kind != yaml.ScalarNode {
+			getDescriptionsFromSpecHelper(child, results)
+		}
+	}
+}
+
+func enforceDescriptionLessThan1000Chars(descriptions *[]*DescriptionField) []*rpc.LintProblem {
+	problems := make([]*rpc.LintProblem, 0)
+	for _, description := range *descriptions {
+		if len(description.Description) >= 1000 {
+			problems = append(problems, &rpc.LintProblem{
+				Message:    "Description field should be less than 1000 chars.",
+				RuleId:     descriptionLessThan1000CharsRuleId,
+				Suggestion: "Ensure that your description field is less than 1000 chars in length.",
+				Location: &rpc.LintLocation{
+					StartPosition: &rpc.LintPosition{
+						LineNumber:   int32(description.StartLineNumber),
+						ColumnNumber: int32(description.StartColumnNumber),
+					},
+					EndPosition: &rpc.LintPosition{
+						LineNumber:   int32(description.EndLineNumber),
+						ColumnNumber: int32(description.EndColumnNumber),
+					},
+				},
+			})
+		}
+	}
+	return problems
+}
+
+func enforceDescriptionContainsNoTagsRuleId(descriptions *[]*DescriptionField) []*rpc.LintProblem {
+	problems := make([]*rpc.LintProblem, 0)
+	for _, description := range *descriptions {
+		r, err := regexp.Compile(".*<[^>]*>.*")
+		if err != nil {
+			continue
+		}
+		if r.MatchString(description.Description) {
+			problems = append(problems, &rpc.LintProblem{
+				Message:    "Description field should not contain any tags.",
+				RuleId:     descriptionContainsNoTagsRuleId,
+				Suggestion: "Ensure that your description field does not contain any tags (regex <[^>]*>)",
+				Location: &rpc.LintLocation{
+					StartPosition: &rpc.LintPosition{
+						LineNumber:   int32(description.StartLineNumber),
+						ColumnNumber: int32(description.StartColumnNumber),
+					},
+					EndPosition: &rpc.LintPosition{
+						LineNumber:   int32(description.EndLineNumber),
+						ColumnNumber: int32(description.EndColumnNumber),
+					},
+				},
+			})
+		}
+	}
+	return problems
+}
+
+func lintWithRule(node *yaml.Node, ruleId string) ([]*rpc.LintProblem, error) {
+	descriptions := getDescriptionsFromSpec(node)
+
+	if ruleId == descriptionLessThan1000CharsRuleId {
+		return enforceDescriptionLessThan1000Chars(&descriptions), nil
+	} else if ruleId == descriptionContainsNoTagsRuleId {
+		return enforceDescriptionContainsNoTagsRuleId(&descriptions), nil
+	}
+
+	return nil, fmt.Errorf("unsupported rule id %s", ruleId)
+}
+
+func main() {
+	lint.Main(&sampleOpenApiLinterRunner{})
+}


Test patch:
diff --git a/cmd/registry/plugins/registry-lint-openapi-sample/main_test.go b/cmd/registry/plugins/registry-lint-openapi-sample/main_test.go
new file mode 100644
index 0000000..12ed03b
--- /dev/null
+++ b/cmd/registry/plugins/registry-lint-openapi-sample/main_test.go
@@ -0,0 +1,169 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/apigee/registry/rpc"
+	"github.com/stretchr/testify/assert"
+)
+
+func setupFakeSpec(contents string) (dirPath, specFilePath string, err error) {
+	tempDir, err := ioutil.TempDir("", "")
+	if err != nil {
+		return "", "", err
+	}
+
+	f, err := ioutil.TempFile(tempDir, "*.yaml")
+	if err != nil {
+		return "", "", err
+	}
+
+	err = ioutil.WriteFile(f.Name(), []byte(contents), 0644)
+	if err != nil {
+		return "", "", err
+	}
+
+	return tempDir, f.Name(), nil
+}
+
+func TestRunDescriptionContainsNoTagsRule(t *testing.T) {
+	contents := `
+        openapi: "3.0.2"
+        info:
+            title: "Swagger Petstore <script></script> with eval("
+        servers:
+            - url: http://petstore.swagger.io/v1
+        paths:
+            /pets:
+                get:
+                description: Gets a <list> of all pets
+    `
+	specDirectory, specFilePath, err := setupFakeSpec(contents)
+	defer os.RemoveAll(specDirectory)
+	assert.Equal(t, err, nil)
+	linter := &sampleOpenApiLinterRunner{}
+	request := &rpc.LinterRequest{
+		SpecDirectory: specDirectory,
+		RuleIds:       []string{"description-contains-no-tags"},
+	}
+	expectedResponse := &rpc.LinterResponse{
+		Lint: &rpc.Lint{
+			Name: "registry-lint-openapi-sample",
+			Files: []*rpc.LintFile{
+				{
+					FilePath: specFilePath,
+					Problems: []*rpc.LintProblem{
+						{
+							Message:    "Description field should not contain any tags.",
+							RuleId:     "description-contains-no-tags",
+							Suggestion: "Ensure that your description field does not contain any tags (regex <[^>]*>)",
+							Location: &rpc.LintLocation{
+								StartPosition: &rpc.LintPosition{
+									LineNumber:   10,
+									ColumnNumber: 30,
+								},
+								EndPosition: &rpc.LintPosition{
+									LineNumber:   11,
+									ColumnNumber: 0,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	response, err := linter.Run(request)
+	assert.Equal(t, nil, err)
+	assert.EqualValues(t, expectedResponse, response)
+}
+
+func TestRunDescriptionLessThan1000CharsRule(t *testing.T) {
+	contents := `
+        openapi: "3.0.2"
+        info:
+            title: "Swagger Petstore <script></script> with eval("
+        servers:
+            - url: http://petstore.swagger.io/v1
+        paths:
+            /pets:
+                get:
+                description: >
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+                    eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee
+    `
+	specDirectory, specFilePath, err := setupFakeSpec(contents)
+	defer os.RemoveAll(specDirectory)
+	assert.Equal(t, err, nil)
+	linter := &sampleOpenApiLinterRunner{}
+	request := &rpc.LinterRequest{
+		SpecDirectory: specDirectory,
+		RuleIds:       []string{"description-less-than-1000-chars"},
+	}
+	expectedResponse := &rpc.LinterResponse{
+		Lint: &rpc.Lint{
+			Name: "registry-lint-openapi-sample",
+			Files: []*rpc.LintFile{
+				{
+					FilePath: specFilePath,
+					Problems: []*rpc.LintProblem{
+						{
+							Message:    "Description field should be less than 1000 chars.",
+							RuleId:     "description-less-than-1000-chars",
+							Suggestion: "Ensure that your description field is less than 1000 chars in length.",
+							Location: &rpc.LintLocation{
+								StartPosition: &rpc.LintPosition{
+									LineNumber:   10,
+									ColumnNumber: 30,
+								},
+								EndPosition: &rpc.LintPosition{
+									LineNumber:   11,
+									ColumnNumber: 0,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	response, err := linter.Run(request)
+	assert.Equal(t, nil, err)
+	assert.EqualValues(t, expectedResponse, response)
+}