From d6acd622e62ad6f42197050471ad1dd697ece88b Mon Sep 17 00:00:00 2001
From: Alex Clemmer <clemmer.alexander@gmail.com>
Date: Tue, 16 Jan 2018 22:26:52 -0800
Subject: [PATCH] Validate app, parts, and registry schemas on deserialization
Dependencies in a ksonnet application are installed from collections of
packages called registries. In order to be precise about which packages
an app depends on, and where they are from, we have an API that
specifies each of these nouns.
Unfortunately, we never hooked up code to verify the versions of these
specifications as we deserialize them.
This commit will correct that.
Signed-off-by: Alex Clemmer <clemmer.alexander@gmail.com>
---
metadata/app/schema.go | 31 ++++++++++++++++
metadata/app/schema_test.go | 44 ++++++++++++++++++++++
metadata/manager.go | 6 +--
metadata/parts/schema.go | 36 +++++++++++++++++-
metadata/parts/schema_test.go | 64 ++++++++++++++++++++++++++++++++
metadata/registry.go | 11 ++----
metadata/registry/schema.go | 41 +++++++++++++++++++-
metadata/registry/schema_test.go | 64 ++++++++++++++++++++++++++++++++
metadata/registry_managers.go | 16 +++-----
metadata/registry_test.go | 2 +-
prototype/interface.go | 4 ++
prototype/prototype_test.go | 50 +++++++++++++++++++++++--
prototype/specification.go | 19 ++++++++++
13 files changed, 361 insertions(+), 27 deletions(-)
create mode 100644 metadata/parts/schema_test.go
create mode 100644 metadata/registry/schema_test.go
diff --git a/metadata/app/schema.go b/metadata/app/schema.go
index 570cd971..c3c720e9 100644
--- a/metadata/app/schema.go
+++ b/metadata/app/schema.go
@@ -18,7 +18,9 @@ package app
import (
"fmt"
+ "github.com/blang/semver"
"github.com/ghodss/yaml"
+ "github.com/pkg/errors"
)
const (
@@ -46,6 +48,20 @@ type Spec struct {
License string `json:"license,omitempty"`
}
+func Unmarshal(bytes []byte) (*Spec, error) {
+ schema := Spec{}
+ err := yaml.Unmarshal(bytes, &schema)
+ if err != nil {
+ return nil, err
+ }
+
+ if err = schema.validate(); err != nil {
+ return nil, err
+ }
+
+ return &schema, nil
+}
+
func (s *Spec) Marshal() ([]byte, error) {
return yaml.Marshal(s)
}
@@ -74,6 +90,21 @@ func (s *Spec) AddRegistryRef(registryRefSpec *RegistryRefSpec) error {
return nil
}
+func (s *Spec) validate() error {
+ compatVer, _ := semver.Make(DefaultAPIVersion)
+ ver, err := semver.Make(s.APIVersion)
+ if err != nil {
+ return errors.Wrap(err, "Failed to parse version in app spec")
+ } else if compatVer.Compare(ver) != 0 {
+ return fmt.Errorf(
+ "Current app uses unsupported spec version '%s' (this client only supports %s)",
+ s.APIVersion,
+ DefaultAPIVersion)
+ }
+
+ return nil
+}
+
type RepositorySpec struct {
Type string `json:"type"`
URI string `json:"uri"`
diff --git a/metadata/app/schema_test.go b/metadata/app/schema_test.go
index 9e458cef..c56112d4 100644
--- a/metadata/app/schema_test.go
+++ b/metadata/app/schema_test.go
@@ -18,6 +18,8 @@ package app
import (
"fmt"
"testing"
+
+ "github.com/blang/semver"
)
func makeSimpleRefSpec(name, protocol, uri, version string) *RegistryRefSpec {
@@ -32,6 +34,48 @@ func makeSimpleRefSpec(name, protocol, uri, version string) *RegistryRefSpec {
}
}
+func TestApiVersionValidate(t *testing.T) {
+ type spec struct {
+ spec string
+ err bool
+ }
+ tests := []spec{
+ // Versions that we accept.
+ {spec: "0.0.1", err: false},
+ {spec: "0.0.1+build.1", err: false},
+
+ // Other versions.
+ {spec: "0.0.0", err: true},
+ {spec: "0.1.0", err: true},
+ {spec: "1.0.0", err: true},
+
+ // Builds and pre-releases of current version.
+ {spec: "0.0.1-alpha", err: true},
+ {spec: "0.0.1-beta+build.2", err: true},
+
+ // Other versions.
+ {spec: "0.1.0-alpha", err: true},
+ {spec: "0.1.0+build.1", err: true},
+ {spec: "0.1.0-beta+build.2", err: true},
+ {spec: "1.0.0-alpha", err: true},
+ {spec: "1.0.0+build.1", err: true},
+ {spec: "1.0.0-beta+build.2", err: true},
+ }
+
+ for _, test := range tests {
+ _, err := semver.Make(test.spec)
+ if err != nil {
+ t.Errorf("Failed to parse version '%s':\n%v", test.spec, err)
+ }
+
+ spec := &Spec{APIVersion: test.spec}
+ err = spec.validate()
+ if (test.err && err == nil) || (!test.err && err != nil) {
+ t.Errorf("Expected error for version '%s'? %t. Value of error: '%v'", test.spec, test.err, err)
+ }
+ }
+}
+
func TestGetRegistryRefSuccess(t *testing.T) {
example1 := Spec{
Registries: RegistryRefSpecs{
diff --git a/metadata/manager.go b/metadata/manager.go
index 4aab3ffe..061b588d 100644
--- a/metadata/manager.go
+++ b/metadata/manager.go
@@ -21,7 +21,6 @@ import (
"path"
"path/filepath"
- "github.com/ghodss/yaml"
"github.com/ksonnet/ksonnet/metadata/app"
param "github.com/ksonnet/ksonnet/metadata/params"
"github.com/ksonnet/ksonnet/metadata/registry"
@@ -243,8 +242,7 @@ func (m *manager) AppSpec() (*app.Spec, error) {
return nil, err
}
- schema := app.Spec{}
- err = yaml.Unmarshal(bytes, &schema)
+ schema, err := app.Unmarshal(bytes)
if err != nil {
return nil, err
}
@@ -261,7 +259,7 @@ func (m *manager) AppSpec() (*app.Spec, error) {
schema.Libraries = app.LibraryRefSpecs{}
}
- return &schema, nil
+ return schema, nil
}
func (m *manager) createUserDirTree() error {
diff --git a/metadata/parts/schema.go b/metadata/parts/schema.go
index c7f14ac1..dde51272 100644
--- a/metadata/parts/schema.go
+++ b/metadata/parts/schema.go
@@ -16,11 +16,15 @@
package parts
import (
+ "fmt"
+
+ "github.com/blang/semver"
"github.com/ghodss/yaml"
+ "github.com/pkg/errors"
)
const (
- DefaultApiVersion = "0.1"
+ DefaultAPIVersion = "0.0.1"
DefaultKind = "ksonnet.io/parts"
)
@@ -41,10 +45,40 @@ type Spec struct {
License string `json:"license"`
}
+func Unmarshal(bytes []byte) (*Spec, error) {
+ schema := Spec{}
+ err := yaml.Unmarshal(bytes, &schema)
+ if err != nil {
+ return nil, err
+ }
+
+ if err = schema.validate(); err != nil {
+ return nil, err
+ }
+
+ return &schema, nil
+}
+
func (s *Spec) Marshal() ([]byte, error) {
return yaml.Marshal(s)
}
+func (s *Spec) validate() error {
+ compatVer, _ := semver.Make(DefaultAPIVersion)
+ ver, err := semver.Make(s.APIVersion)
+ if err != nil {
+ return errors.Wrap(err, "Failed to parse version in app spec")
+ } else if compatVer.Compare(ver) != 0 {
+ return fmt.Errorf(
+ "Library '%s' uses unsupported spec version '%s' (this client only supports %s)",
+ s.Name,
+ s.APIVersion,
+ DefaultAPIVersion)
+ }
+
+ return nil
+}
+
type ContributorSpec struct {
Name string `json:"name"`
Email string `json:"email"`
diff --git a/metadata/parts/schema_test.go b/metadata/parts/schema_test.go
new file mode 100644
index 00000000..11b55e1c
--- /dev/null
+++ b/metadata/parts/schema_test.go
@@ -0,0 +1,64 @@
+// Copyright 2017 The kubecfg authors
+//
+//
+// 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 parts
+
+import (
+ "testing"
+
+ "github.com/blang/semver"
+)
+
+func TestApiVersionValidate(t *testing.T) {
+ type spec struct {
+ spec string
+ err bool
+ }
+ tests := []spec{
+ // Versions that we accept.
+ {spec: "0.0.1", err: false},
+ {spec: "0.0.1+build.1", err: false},
+
+ // Other versions.
+ {spec: "0.0.0", err: true},
+ {spec: "0.1.0", err: true},
+ {spec: "1.0.0", err: true},
+
+ // Builds and pre-releases of current version.
+ {spec: "0.0.1-alpha", err: true},
+ {spec: "0.0.1-beta+build.2", err: true},
+
+ // Other versions.
+ {spec: "0.1.0-alpha", err: true},
+ {spec: "0.1.0+build.1", err: true},
+ {spec: "0.1.0-beta+build.2", err: true},
+ {spec: "1.0.0-alpha", err: true},
+ {spec: "1.0.0+build.1", err: true},
+ {spec: "1.0.0-beta+build.2", err: true},
+ }
+
+ for _, test := range tests {
+ _, err := semver.Make(test.spec)
+ if err != nil {
+ t.Errorf("Failed to parse version '%s':\n%v", test.spec, err)
+ }
+
+ spec := &Spec{APIVersion: test.spec}
+ err = spec.validate()
+ if (test.err && err == nil) || (!test.err && err != nil) {
+ t.Errorf("Expected error for version '%s'? %t. Value of error: '%v'", test.spec, test.err, err)
+ }
+ }
+}
diff --git a/metadata/registry.go b/metadata/registry.go
index 6eb5dac0..42e6a544 100644
--- a/metadata/registry.go
+++ b/metadata/registry.go
@@ -5,7 +5,6 @@ import (
"os"
"path/filepath"
- "github.com/ghodss/yaml"
"github.com/ksonnet/ksonnet/metadata/app"
"github.com/ksonnet/ksonnet/metadata/parts"
"github.com/ksonnet/ksonnet/metadata/registry"
@@ -125,8 +124,7 @@ func (m *manager) GetDependency(libName string) (*parts.Spec, error) {
return nil, err
}
- var partsSpec parts.Spec
- err = yaml.Unmarshal(partsBytes, &partsSpec)
+ partsSpec, err := parts.Unmarshal(partsBytes)
if err != nil {
return nil, err
}
@@ -140,7 +138,7 @@ func (m *manager) GetDependency(libName string) (*parts.Spec, error) {
partsSpec.Prototypes = append(partsSpec.Prototypes, protoSpec.Name)
}
- return &partsSpec, nil
+ return partsSpec, nil
}
func (m *manager) CacheDependency(registryName, libID, libName, libVersion string) (*parts.Spec, error) {
@@ -335,12 +333,11 @@ func (m *manager) registrySpecFromFile(path AbsPath) (*registry.Spec, bool, erro
return nil, false, err
}
- registrySpec := registry.Spec{}
- err = yaml.Unmarshal(registrySpecBytes, ®istrySpec)
+ registrySpec, err := registry.Unmarshal(registrySpecBytes)
if err != nil {
return nil, false, err
}
- return ®istrySpec, true, nil
+ return registrySpec, true, nil
}
return nil, false, nil
diff --git a/metadata/registry/schema.go b/metadata/registry/schema.go
index a4ce51ff..5c91de3b 100644
--- a/metadata/registry/schema.go
+++ b/metadata/registry/schema.go
@@ -16,12 +16,16 @@
package registry
import (
+ "fmt"
+
+ "github.com/blang/semver"
"github.com/ghodss/yaml"
"github.com/ksonnet/ksonnet/metadata/app"
+ "github.com/pkg/errors"
)
const (
- DefaultApiVersion = "0.1"
+ DefaultAPIVersion = "0.1.0"
DefaultKind = "ksonnet.io/registry"
)
@@ -32,10 +36,45 @@ type Spec struct {
Libraries LibraryRefSpecs `json:"libraries"`
}
+func Unmarshal(bytes []byte) (*Spec, error) {
+ schema := Spec{}
+ err := yaml.Unmarshal(bytes, &schema)
+ if err != nil {
+ return nil, err
+ }
+
+ if err = schema.validate(); err != nil {
+ return nil, err
+ }
+
+ return &schema, nil
+}
+
func (s *Spec) Marshal() ([]byte, error) {
return yaml.Marshal(s)
}
+func (s *Spec) validate() error {
+ // Originally, the default value for `apiVersion` was `0.1`. This is not a
+ // valid semver, so before we do anything, we need to convert it to one.
+ if s.APIVersion == "0.1" {
+ s.APIVersion = "0.1.0"
+ }
+
+ compatVer, _ := semver.Make(DefaultAPIVersion)
+ ver, err := semver.Make(s.APIVersion)
+ if err != nil {
+ return errors.Wrap(err, "Failed to parse version in app spec")
+ } else if compatVer.Compare(ver) != 0 {
+ return fmt.Errorf(
+ "Registry uses unsupported spec version '%s' (this client only supports %s)",
+ s.APIVersion,
+ DefaultAPIVersion)
+ }
+
+ return nil
+}
+
type Specs []*Spec
type LibraryRef struct {
diff --git a/metadata/registry/schema_test.go b/metadata/registry/schema_test.go
new file mode 100644
index 00000000..4ceb2bb6
--- /dev/null
+++ b/metadata/registry/schema_test.go
@@ -0,0 +1,64 @@
+// Copyright 2017 The kubecfg authors
+//
+//
+// 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 registry
+
+import (
+ "testing"
+
+ "github.com/blang/semver"
+)
+
+func TestApiVersionValidate(t *testing.T) {
+ type spec struct {
+ spec string
+ err bool
+ }
+ tests := []spec{
+ // Versions that we accept.
+ {spec: "0.1.0", err: false},
+ {spec: "0.1.0+build.1", err: false},
+
+ // Other versions.
+ {spec: "0.0.0", err: true},
+ {spec: "0.0.1", err: true},
+ {spec: "1.0.0", err: true},
+
+ // Builds and pre-releases of current version.
+ {spec: "0.1.0-alpha", err: true},
+ {spec: "0.1.0-beta+build.2", err: true},
+
+ // Other versions.
+ {spec: "0.0.1-alpha", err: true},
+ {spec: "0.0.1+build.1", err: true},
+ {spec: "0.0.1-beta+build.2", err: true},
+ {spec: "1.0.0-alpha", err: true},
+ {spec: "1.0.0+build.1", err: true},
+ {spec: "1.0.0-beta+build.2", err: true},
+ }
+
+ for _, test := range tests {
+ _, err := semver.Make(test.spec)
+ if err != nil {
+ t.Errorf("Failed to parse version '%s':\n%v", test.spec, err)
+ }
+
+ spec := &Spec{APIVersion: test.spec}
+ err = spec.validate()
+ if (test.err && err == nil) || (!test.err && err != nil) {
+ t.Errorf("Expected error for version '%s'? %t. Value of error: '%v'", test.spec, test.err, err)
+ }
+ }
+}
diff --git a/metadata/registry_managers.go b/metadata/registry_managers.go
index 97c48168..9a8d5ac4 100644
--- a/metadata/registry_managers.go
+++ b/metadata/registry_managers.go
@@ -9,7 +9,6 @@ import (
"path"
"strings"
- "github.com/ghodss/yaml"
"github.com/google/go-github/github"
"github.com/ksonnet/ksonnet/metadata/app"
"github.com/ksonnet/ksonnet/metadata/parts"
@@ -120,8 +119,7 @@ func (gh *gitHubRegistryManager) FetchRegistrySpec() (*registry.Spec, error) {
}
// Deserialize, return.
- registrySpec := registry.Spec{}
- err = yaml.Unmarshal([]byte(registrySpecText), ®istrySpec)
+ registrySpec, err := registry.Unmarshal([]byte(registrySpecText))
if err != nil {
return nil, err
}
@@ -131,7 +129,7 @@ func (gh *gitHubRegistryManager) FetchRegistrySpec() (*registry.Spec, error) {
CommitSHA: gh.GitVersion.CommitSHA,
}
- return ®istrySpec, nil
+ return registrySpec, nil
}
func (gh *gitHubRegistryManager) MakeRegistryRefSpec() *app.RegistryRefSpec {
@@ -164,13 +162,12 @@ func (gh *gitHubRegistryManager) ResolveLibrarySpec(libID, libRefSpec string) (*
return nil, err
}
- parts := parts.Spec{}
- err = yaml.Unmarshal([]byte(partsSpecText), &parts)
+ parts, err := parts.Unmarshal([]byte(partsSpecText))
if err != nil {
return nil, err
}
- return &parts, nil
+ return parts, nil
}
func (gh *gitHubRegistryManager) ResolveLibrary(libID, libAlias, libRefSpec string, onFile registry.ResolveFile, onDir registry.ResolveDirectory) (*parts.Spec, *app.LibraryRefSpec, error) {
@@ -213,8 +210,7 @@ func (gh *gitHubRegistryManager) ResolveLibrary(libID, libAlias, libRefSpec stri
return nil, nil, err
}
- parts := parts.Spec{}
- err = yaml.Unmarshal([]byte(partsSpecText), &parts)
+ parts, err := parts.Unmarshal([]byte(partsSpecText))
if err != nil {
return nil, nil, err
}
@@ -228,7 +224,7 @@ func (gh *gitHubRegistryManager) ResolveLibrary(libID, libAlias, libRefSpec stri
},
}
- return &parts, &refSpec, nil
+ return parts, &refSpec, nil
}
func (gh *gitHubRegistryManager) resolveDir(client *github.Client, libID, path, version string, onFile registry.ResolveFile, onDir registry.ResolveDirectory) error {
diff --git a/metadata/registry_test.go b/metadata/registry_test.go
index f69a31a4..5c892686 100644
--- a/metadata/registry_test.go
+++ b/metadata/registry_test.go
@@ -188,7 +188,7 @@ func (m *mockRegistryManager) RegistrySpecFilePath() string {
func (m *mockRegistryManager) FetchRegistrySpec() (*registry.Spec, error) {
registrySpec := registry.Spec{
- APIVersion: registry.DefaultApiVersion,
+ APIVersion: registry.DefaultAPIVersion,
Kind: registry.DefaultKind,
}
diff --git a/prototype/interface.go b/prototype/interface.go
index 782884b6..e4a7d00e 100644
--- a/prototype/interface.go
+++ b/prototype/interface.go
@@ -11,6 +11,10 @@ func Unmarshal(bytes []byte) (*SpecificationSchema, error) {
return nil, err
}
+ if err = p.validate(); err != nil {
+ return nil, err
+ }
+
return &p, nil
}
diff --git a/prototype/prototype_test.go b/prototype/prototype_test.go
index d187031d..62069630 100644
--- a/prototype/prototype_test.go
+++ b/prototype/prototype_test.go
@@ -3,6 +3,8 @@ package prototype
import (
"sort"
"testing"
+
+ "github.com/blang/semver"
)
const (
@@ -10,7 +12,7 @@ const (
)
var simpleService = `{
- "apiVersion": "0.1",
+ "apiVersion": "0.0.1",
"name": "io.some-vendor.pkg.simple-service",
"template": {
"description": "Generates a simple service with a port exposed",
@@ -28,7 +30,7 @@ var simpleService = `{
}`
var simpleDeployment = `{
- "apiVersion": "0.1",
+ "apiVersion": "0.0.1",
"name": "io.some-vendor.pkg.simple-deployment",
"template": {
"description": "Generates a simple service with a port exposed",
@@ -62,7 +64,7 @@ func assertProp(t *testing.T, name string, expected string, actual string) {
func TestSimpleUnmarshal(t *testing.T) {
p := unmarshal(t, []byte(simpleService))
- assertProp(t, "apiVersion", p.APIVersion, "0.1")
+ assertProp(t, "apiVersion", p.APIVersion, "0.0.1")
assertProp(t, "name", p.Name, "io.some-vendor.pkg.simple-service")
assertProp(t, "description", p.Template.Description, "Generates a simple service with a port exposed")
}
@@ -147,3 +149,45 @@ func TestSearch(t *testing.T) {
})
assertSearch(t, idx, Substring, "foo", []string{})
}
+
+func TestApiVersionValidate(t *testing.T) {
+ type spec struct {
+ spec string
+ err bool
+ }
+ tests := []spec{
+ // Versions that we accept.
+ {spec: "0.0.1", err: false},
+ {spec: "0.0.1+build.1", err: false},
+
+ // Other versions.
+ {spec: "0.0.0", err: true},
+ {spec: "0.1.0", err: true},
+ {spec: "1.0.0", err: true},
+
+ // Builds and pre-releases of current version.
+ {spec: "0.0.1-alpha", err: true},
+ {spec: "0.0.1-beta+build.2", err: true},
+
+ // Other versions.
+ {spec: "0.1.0-alpha", err: true},
+ {spec: "0.1.0+build.1", err: true},
+ {spec: "0.1.0-beta+build.2", err: true},
+ {spec: "1.0.0-alpha", err: true},
+ {spec: "1.0.0+build.1", err: true},
+ {spec: "1.0.0-beta+build.2", err: true},
+ }
+
+ for _, test := range tests {
+ _, err := semver.Make(test.spec)
+ if err != nil {
+ t.Errorf("Failed to parse version '%s':\n%v", test.spec, err)
+ }
+
+ spec := &SpecificationSchema{APIVersion: test.spec}
+ err = spec.validate()
+ if (test.err && err == nil) || (!test.err && err != nil) {
+ t.Errorf("Expected error for version '%s'? %t. Value of error: '%v'", test.spec, test.err, err)
+ }
+ }
+}
diff --git a/prototype/specification.go b/prototype/specification.go
index 165bdcf5..056ccbe1 100644
--- a/prototype/specification.go
+++ b/prototype/specification.go
@@ -7,7 +7,9 @@ import (
"strconv"
"strings"
+ "github.com/blang/semver"
"github.com/ksonnet/ksonnet/utils"
+ "github.com/pkg/errors"
log "github.com/sirupsen/logrus"
)
@@ -18,6 +20,8 @@ import (
//
const (
+ DefaultAPIVersion = "0.0.1"
+
apiVersionTag = "@apiVersion"
nameTag = "@name"
descriptionTag = "@description"
@@ -135,6 +139,21 @@ type SpecificationSchema struct {
Template SnippetSchema `json:"template"`
}
+func (s *SpecificationSchema) validate() error {
+ compatVer, _ := semver.Make(DefaultAPIVersion)
+ ver, err := semver.Make(s.APIVersion)
+ if err != nil {
+ return errors.Wrap(err, "Failed to parse version in app spec")
+ } else if compatVer.Compare(ver) != 0 {
+ return fmt.Errorf(
+ "Current app uses unsupported spec version '%s' (this client only supports %s)",
+ s.APIVersion,
+ DefaultAPIVersion)
+ }
+
+ return nil
+}
+
// SpecificationSchemas is a slice of pointer to `SpecificationSchema`.
type SpecificationSchemas []*SpecificationSchema
--
GitLab