Skip to content

Commit

Permalink
feat: per-operation versioning and deprecation
Browse files Browse the repository at this point in the history
Fixes snyk#99, snyk#100.

This changes how operation (path + method) are resolved and merged
across spec versions within a stability level.

Operations are now cumulative. Unless a particular operation (path +
method) is redefined or explicitly deprecated, past releases are assumed
to carry over to newer versions.

BREAKING CHANGE: This changes the behavior of the compiler and resulting
output. It's also a breaking API change, requiring a major version bump.
We do not anticipate any breaking changes to OpenAPI consumers.
  • Loading branch information
cmars committed Jan 11, 2022
1 parent c8f1b4d commit b90d68c
Show file tree
Hide file tree
Showing 25 changed files with 3,033 additions and 251 deletions.
6 changes: 5 additions & 1 deletion cmd/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ func Resolve(ctx *cli.Context) error {
if err != nil {
return err
}
specVersion, err := specVersions.At(ctx.String("at"))
version, err := vervet.ParseVersion(ctx.String("at"))
if err != nil {
return err
}
specVersion, err := specVersions.At(*version)
if err != nil {
return err
}
Expand Down
62 changes: 28 additions & 34 deletions cmd/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"time"

"github.com/olekukonko/tablewriter"
"github.com/urfave/cli/v2"

"github.com/snyk/vervet"
"github.com/snyk/vervet/config"
"github.com/snyk/vervet/internal/compiler"
"github.com/snyk/vervet/internal/generator"
"github.com/urfave/cli/v2"
)

// VersionList is a command that lists all the versions of matching resources.
Expand Down Expand Up @@ -49,41 +48,36 @@ func VersionList(ctx *cli.Context) error {
if err != nil {
return err
}
specVersions, err := vervet.LoadSpecVersionsFileset(specFiles)
resources, err := vervet.LoadResourceVersionsFileset(specFiles)
if err != nil {
return err
}
for _, rc := range specVersions.Resources() {
if rcArg := ctx.Args().Get(1); rcArg != "" && rcArg != rc.Name() {
continue
for _, version := range resources.Versions() {
rc, err := resources.At(version.String())
if err != nil {
return err
}
for _, version := range rc.Versions() {
doc, err := rc.At(version.String())
if err != nil {
return err
var pathNames []string
for k := range rc.Paths {
pathNames = append(pathNames, k)
}
sort.Strings(pathNames)
for _, pathName := range pathNames {
pathSpec := rc.Paths[pathName]
if pathSpec.Get != nil {
table.Append([]string{apiName, rc.Name, version.String(), pathName, "GET", pathSpec.Get.OperationID})
}
if pathSpec.Post != nil {
table.Append([]string{apiName, rc.Name, version.String(), pathName, "POST", pathSpec.Post.OperationID})
}
if pathSpec.Put != nil {
table.Append([]string{apiName, rc.Name, version.String(), pathName, "PUT", pathSpec.Put.OperationID})
}
var pathNames []string
for k := range doc.Paths {
pathNames = append(pathNames, k)
if pathSpec.Patch != nil {
table.Append([]string{apiName, rc.Name, version.String(), pathName, "PATCH", pathSpec.Patch.OperationID})
}
sort.Strings(pathNames)
for _, pathName := range pathNames {
pathSpec := doc.Paths[pathName]
if pathSpec.Get != nil {
table.Append([]string{apiName, rc.Name(), version.String(), pathName, "GET", pathSpec.Get.OperationID})
}
if pathSpec.Post != nil {
table.Append([]string{apiName, rc.Name(), version.String(), pathName, "POST", pathSpec.Post.OperationID})
}
if pathSpec.Put != nil {
table.Append([]string{apiName, rc.Name(), version.String(), pathName, "PUT", pathSpec.Put.OperationID})
}
if pathSpec.Patch != nil {
table.Append([]string{apiName, rc.Name(), version.String(), pathName, "PATCH", pathSpec.Patch.OperationID})
}
if pathSpec.Delete != nil {
table.Append([]string{apiName, rc.Name(), version.String(), pathName, "DELETE", pathSpec.Delete.OperationID})
}
if pathSpec.Delete != nil {
table.Append([]string{apiName, rc.Name, version.String(), pathName, "DELETE", pathSpec.Delete.OperationID})
}
}
}
Expand Down Expand Up @@ -198,13 +192,13 @@ func VersionNew(ctx *cli.Context) error {
}
sort.Strings(apiNames)
return fmt.Errorf(`API %q not found. Choose an existing one (%s) or
`+"`%s api new %s <resource path>`"+` to start a new API`,
`+"`%s api new %s <resource path>`"+` to start a new API`,
apiName, strings.Join(apiNames, ", "), os.Args[0], apiName)
}
if len(api.Resources) == 0 {
return fmt.Errorf(`API %q does not seem to have a resource set defined.
Please add a `+"`resources:`"+` section to
%q and try again`, apiName, configFile)
Please add a `+"`resources:`"+` section to
%q and try again`, apiName, configFile)
}

versionTime, err := time.Parse("2006-01-02", ctx.String("version"))
Expand Down
20 changes: 11 additions & 9 deletions cmd/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ resources/_examples/hello-world/2021-06-01/spec.yaml
resources/_examples/hello-world/2021-06-07/spec.yaml
resources/_examples/hello-world/2021-06-13/spec.yaml
resources/projects/2021-06-04/spec.yaml
resources/projects/2021-08-20/spec.yaml
`[1:])
}

Expand All @@ -70,15 +71,16 @@ func TestVersionList(t *testing.T) {
out, err := ioutil.ReadFile(tmpFile)
c.Assert(err, qt.IsNil)
c.Assert(string(out), qt.Equals, `
+----------+-------------+-------------------------+----------------------------+--------+------------------+
| API | RESOURCE | VERSION | PATH | METHOD | OPERATION |
+----------+-------------+-------------------------+----------------------------+--------+------------------+
| testdata | hello-world | 2021-06-01~experimental | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | hello-world | 2021-06-07~experimental | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | hello-world | 2021-06-13~beta | /examples/hello-world | POST | helloWorldCreate |
| testdata | hello-world | 2021-06-13~beta | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | projects | 2021-06-04~experimental | /orgs/{orgId}/projects | GET | getOrgsProjects |
+----------+-------------+-------------------------+----------------------------+--------+------------------+
+----------+-------------+-------------------------+--------------------------------------+--------+-------------------+
| API | RESOURCE | VERSION | PATH | METHOD | OPERATION |
+----------+-------------+-------------------------+--------------------------------------+--------+-------------------+
| testdata | hello-world | 2021-06-01~experimental | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | projects | 2021-06-04~experimental | /orgs/{orgId}/projects | GET | getOrgsProjects |
| testdata | hello-world | 2021-06-07~experimental | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | hello-world | 2021-06-13~beta | /examples/hello-world | POST | helloWorldCreate |
| testdata | hello-world | 2021-06-13~beta | /examples/hello-world/{id} | GET | helloWorldGetOne |
| testdata | projects | 2021-08-20~experimental | /orgs/{org_id}/projects/{project_id} | DELETE | deleteOrgsProject |
+----------+-------------+-------------------------+--------------------------------------+--------+-------------------+
`[1:])
}

Expand Down
2 changes: 1 addition & 1 deletion internal/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (c *Compiler) Build(ctx context.Context, apiName string) error {
return buildErr(err)
}

spec, err := specVersions.At(version.String())
spec, err := specVersions.At(*version)
if err == vervet.ErrNoMatchingVersion {
continue
} else if err != nil {
Expand Down
46 changes: 31 additions & 15 deletions resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,10 @@ func LoadResourceVersions(epPath string) (*ResourceVersions, error) {
func LoadResourceVersionsFileset(specYamls []string) (*ResourceVersions, error) {
var resourceVersions ResourceVersions
var err error
pathReleases := map[string]VersionSlice{}
type operationKey struct {
path, operation string
}
opReleases := map[operationKey]VersionSlice{}

for i := range specYamls {
specYamls[i], err = filepath.Abs(specYamls[i])
Expand All @@ -188,30 +191,43 @@ func LoadResourceVersionsFileset(specYamls []string) (*ResourceVersions, error)
if err != nil {
return nil, err
}
resourceVersions.versions = append(resourceVersions.versions, rc)
// Map of release versions per path
for path := range rc.Paths {
pathReleases[path] = append(pathReleases[path], rc.Version)
// Map release versions per operation
for path, pathItem := range rc.Paths {
for _, opName := range operationNames {
op := getOperationByName(pathItem, opName)
if op != nil {
op.ExtensionProps.Extensions[ExtSnykApiVersion] = rc.Version.String()
opKey := operationKey{path, opName}
opReleases[opKey] = append(opReleases[opKey], rc.Version)
}
}
}
resourceVersions.versions = append(resourceVersions.versions, rc)
}
// Sort release versions per path
for _, releases := range pathReleases {
for _, releases := range opReleases {
sort.Sort(releases)
}
// Sort the resources themselves by version
sort.Sort(resourceVersionSlice(resourceVersions.versions))
// Annotate each path in each resource version with the other change
// versions affecting the path. This supports navigation across versions.
for _, rc := range resourceVersions.versions {
for path, pathInfo := range rc.Paths {
// Annotate path with other release versions available for this path
releases := pathReleases[path]
pathInfo.ExtensionProps.Extensions[ExtSnykApiReleases] = releases.Strings()
// Annotate path with deprecated-by and sunset information
if deprecatedBy, ok := releases.Deprecates(rc.Version); ok {
pathInfo.ExtensionProps.Extensions[ExtSnykDeprecatedBy] = deprecatedBy.String()
if sunset, ok := rc.Version.Sunset(deprecatedBy); ok {
pathInfo.ExtensionProps.Extensions[ExtSnykSunsetEligible] = sunset.Format("2006-01-02")
for path, pathItem := range rc.Paths {
for _, opName := range operationNames {
op := getOperationByName(pathItem, opName)
if op == nil {
continue
}
// Annotate operation with other release versions available for this path
releases := opReleases[operationKey{path, opName}]
op.ExtensionProps.Extensions[ExtSnykApiReleases] = releases.Strings()
// Annotate operation with deprecated-by and sunset information
if deprecatedBy, ok := releases.Deprecates(rc.Version); ok {
op.ExtensionProps.Extensions[ExtSnykDeprecatedBy] = deprecatedBy.String()
if sunset, ok := rc.Version.Sunset(deprecatedBy); ok {
op.ExtensionProps.Extensions[ExtSnykSunsetEligible] = sunset.Format("2006-01-02")
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestVersionRangesProjects(t *testing.T) {
c := qt.New(t)
eps, err := LoadResourceVersions(testdata.Path("resources/projects"))
c.Assert(err, qt.IsNil)
c.Assert(eps.Versions(), qt.HasLen, 1)
c.Assert(eps.Versions(), qt.HasLen, 2)
tests := []struct {
query, match, err string
}{{
Expand Down
81 changes: 81 additions & 0 deletions resource_versions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package vervet

import (
"fmt"
"sort"

"github.com/getkin/kin-openapi/openapi3"
)

type resourceVersionsSlice []*ResourceVersions

func (s resourceVersionsSlice) validate() error {
for _, v := range s.versions() {
resourcePaths := map[string]string{}
for _, eps := range s {
ep, err := eps.At(v.String())
if err == ErrNoMatchingVersion {
continue
} else if err != nil {
return fmt.Errorf("validation failed: %w", err)
}
for path := range ep.Paths {
if conflict, ok := resourcePaths[path]; ok {
return fmt.Errorf("conflict: %q %q", conflict, ep.sourcePrefix)
}
resourcePaths[path] = ep.sourcePrefix
}
}
}
return nil
}

func (s resourceVersionsSlice) versions() VersionSlice {
vset := map[Version]bool{}
for _, eps := range s {
for i := range eps.versions {
vset[eps.versions[i].Version] = true
}
}
var versions VersionSlice
for v := range vset {
versions = append(versions, v)
}
sort.Sort(versions)
return versions
}

func (s resourceVersionsSlice) at(v Version) (*openapi3.T, error) {
var result *openapi3.T
for _, eps := range s {
ep, err := eps.At(v.String())
if err == ErrNoMatchingVersion {
continue
} else if err != nil {
return nil, err
}
if result == nil {
// Assign a clean copy of the contents of the first resource to the
// resulting spec. Marshaling is used to ensure that references in
// the source resource are dropped from the result, which could be
// modified on subsequent merges.
buf, err := ep.T.MarshalJSON()
if err != nil {
return nil, err
}
result = &openapi3.T{}
err = result.UnmarshalJSON(buf)
if err != nil {
return nil, err
}
}
Merge(result, ep.T, false)
}
if result == nil {
return nil, ErrNoMatchingVersion
}
// Remove the API stability extension from the merged OpenAPI spec, this
// extension is only applicable to individual resource version specs.
delete(result.ExtensionProps.Extensions, ExtSnykApiStability)
return result, nil
}
Loading

0 comments on commit b90d68c

Please sign in to comment.