Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDP-283086: POC validator with custom function #287

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions tools/ipa/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# A Self-Documenting Makefile: http://marmelab.com/blog/2016/02/29/auto-documented-makefile.html

SPECTRAL_VERSION=${SPECTRAL_VERSION:-6.11.1}
SPECTRAL_CORE_VERSION=${SPECTRAL_CORE_VERSION:-1.19.1} # If we need to use spectral core .js

.PHONY: deps
deps: ## Download go module dependencies
@echo "==> Installing npx..."
npm install -g npx
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you gonan use npm can you move to a package json instead of a make file, also postman has a package json, let's please have a single package json and not multiple ones

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, was copying the existing spectral makefile for now. Do you mean having the same package json for postman + ipa tool?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean having the same package json for postman + ipa tool?

Yes a single source for dependencies in the repo, ie a mono repo approach



.PHONY: devtools
devtools: ## Install dev tools
@echo "==> Installing dev tools..."
npm install -g @stoplight/spectral-cli@"${SPECTRAL_VERSION}"
npm install -g @stoplight/spectral-core@"${SPECTRAL_CORE_VERSION}"

.PHONY: setup
setup: deps devtools ## Set up dev env

.PHONY: linterr
linterr: ## Run spectral linter on foas
@echo "==> Running spectral linter"
npx -- @stoplight/spectral-cli@"${SPECTRAL_VERSION}" lint ../../openapi/v2.yaml --ruleset=ipa-spectral.yaml -v

.PHONY: lintwarn
lintwarn: ## Run spectral linter on foas
@echo "==> Running spectral linter"
npx -- @stoplight/spectral-cli@"${SPECTRAL_VERSION}" lint ../../openapi/v2.yaml --ruleset=ipa-spectral --fail-severity=warn -v

.PHONY: help
.DEFAULT_GOAL := help
help:
@grep -h -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
2 changes: 2 additions & 0 deletions tools/ipa/ipa-spectral.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
extends:
- ./rulesets/IPA-104.yaml
15 changes: 15 additions & 0 deletions tools/ipa/rulesets/IPA-104.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# IPA-104: Get
# http://go/ipa/104

functions:
- eachResourceHasGetMethod

rules:
xgen-IPA-104-resource-has-GET:
description: "APIs must provide a get method for resources. http://go/ipa/104"
message: "{{error}} http://go/ipa/117"
severity: error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning :P

given: "$.paths"
then:
field: "@key"
function: "eachResourceHasGetMethod"
95 changes: 95 additions & 0 deletions tools/ipa/rulesets/functions/eachResourceHasGetMethod.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
const ERROR_MESSAGE = "APIs must provide a get method for resources."

export default (input, _, context) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we document what version of ES can we use?

if (isChild(input) || isCustomMethod(input)) {
return
}

const oas = context.documentInventory.resolved

const resourcePaths = getResourcePaths(input, Object.keys(oas.paths))

if (isSingletonResource(resourcePaths)) {
// Singleton resource, may have custom methods
if (!hasGetMethod(oas.paths[resourcePaths[0]])) {
return [
{
message: ERROR_MESSAGE
}
]
}
} else if (isNormalResource(resourcePaths)) {
// Normal resource, may have custom methods
if (!hasGetMethod(oas.paths[resourcePaths[1]])) {
return [
{
message: ERROR_MESSAGE
}
]
}
}
}

function isChild(path) {
return path.endsWith("}")
}

function isCustomMethod(path) {
return path.includes(":")
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be defined in a separate file and imported here, I may answer one of @drinkbird questions

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I expect we'll have a bunch of common helper functions we can import for different custom functions


/**
* Get all paths for a resource based on the parent path
*
* @param parent the parent path string
* @param allPaths all paths as an array of strings
* @returns {*} a string array of all paths for a resource, including the parent
*/
function getResourcePaths(parent, allPaths) {
const childPathPattern = new RegExp(`^${parent}/{[a-zA-Z]+}$`);
const customMethodPattern = new RegExp(`^${parent}/{[a-zA-Z]+}:+[a-zA-Z]+$`);
return allPaths.filter(p => parent === p || childPathPattern.test(p) || customMethodPattern.test(p));
}

/**
* Checks if a resource is a singleton resource based on the paths for the
* resource. The resource may have custom methods.
*
* @param resourcePaths all paths for the resource as an array of strings
* @returns {boolean}
*/
function isSingletonResource(resourcePaths) {
if (resourcePaths.length === 1) {
return true
}
const additionalPaths = resourcePaths.slice(1)
return !additionalPaths.some((p) => !isCustomMethod(p))
}

/**
* Checks if a resource is a normal resource based on the paths for the
* resource. The resource may have custom methods.
*
* @param resourcePaths all paths for the resource as an array of strings
* @returns {boolean}
*/
function isNormalResource(resourcePaths) {
if (resourcePaths.length === 2 && isChild(resourcePaths[1])) {
return true
}
if (resourcePaths.length < 3 || !isChild(resourcePaths[1])) {
return false
}
const additionalPaths = resourcePaths.slice(2)
return !additionalPaths.some((p) => !isCustomMethod(p))
}

/**
* Checks if a path object has a GET method
*
* @param pathObject the path object to evaluate
* @returns {boolean}
*/
function hasGetMethod(pathObject) {
return Object.keys(pathObject).some((o) => o === "get")
}
Loading