Skip to content

Commit

Permalink
Validate schema when creating setter
Browse files Browse the repository at this point in the history
  • Loading branch information
@mortent
mortent committed Jul 29, 2020
1 parent 6f63cf7 commit 52083c6
Show file tree
Hide file tree
Showing 2 changed files with 301 additions and 1 deletion.
79 changes: 78 additions & 1 deletion cmd/config/internal/commands/cmdcreatesetter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
package commands

import (
"encoding/json"
"io/ioutil"
"strings"

"github.com/go-openapi/spec"
"github.com/spf13/cobra"
"sigs.k8s.io/kustomize/cmd/config/ext"
Expand Down Expand Up @@ -52,7 +56,7 @@ func NewCreateSetterRunner(parent string) *CreateSetterRunner {
"use this version of the setter format")
set.Flags().BoolVar(&r.CreateSetter.Required, "required", false,
"indicates that this setter must be set by package consumer before live apply/preview")
set.Flags().StringVar(&r.CreateSetter.SchemaPath, "schema-path", "",
set.Flags().StringVar(&r.SchemaPath, "schema-path", "",
`openAPI schema file path for setter constraints -- file content `+
`e.g. {"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]}`)
set.Flags().MarkHidden("version")
Expand All @@ -70,6 +74,7 @@ type CreateSetterRunner struct {
Set setters.CreateSetter
CreateSetter settersutil.SetterCreator
OpenAPIFile string
SchemaPath string
}

func (r *CreateSetterRunner) runE(c *cobra.Command, args []string) error {
Expand Down Expand Up @@ -129,6 +134,11 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error {
r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy
r.CreateSetter.Type = r.Set.SetPartialField.Type

err = r.processSchema()
if err != nil {
return err
}

if r.CreateSetter.Type == "array" {
if !c.Flag("field").Changed {
return errors.Errorf("field flag must be set for array type setters")
Expand All @@ -138,6 +148,51 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error {
return nil
}

func (r *CreateSetterRunner) processSchema() error {
sc, err := schemaFromFile(r.SchemaPath)
if err != nil {
return err
}

flagType := r.CreateSetter.Type
var schemaType string
switch {
// json schema allows more than one type to be specified, but openapi
// only allows one. So we follow the openapi convention.
case len(sc.Type) > 1:
return errors.Errorf("only one type is supported: %s",
strings.Join(sc.Type, ", "))
case len(sc.Type) == 1:
schemaType = sc.Type[0]
}

// Since type can be set both through the schema file and through the
// --type flag, we make sure the same value is set in both places. If they
// are both set with different values, we return an error.
switch {
case flagType == "" && schemaType != "":
r.CreateSetter.Type = schemaType
case flagType != "" && schemaType == "":
sc.Type = []string{flagType}
case flagType != "" && schemaType != "":
if flagType != schemaType {
return errors.Errorf("type provided in type flag (%s) and in schema (%s) doesn't match",
r.CreateSetter.Type, sc.Type[0])
}
}

// Only marshal the properties in SchemaProps. This means any fields in
// the schema file that isn't recognized will be dropped.
// TODO: Consider if we should return an error here instead of just dropping
// the unknown fields.
b, err := json.Marshal(sc.SchemaProps)
if err != nil {
return errors.Errorf("error marshalling schema: %v", err)
}
r.CreateSetter.Schema = string(b)
return nil
}

func (r *CreateSetterRunner) set(c *cobra.Command, args []string) error {
if setterVersion == "v2" {
return r.CreateSetter.Create(r.OpenAPIFile, args[0])
Expand All @@ -153,3 +208,25 @@ func (r *CreateSetterRunner) set(c *cobra.Command, args []string) error {
}
return nil
}

// schemaFromFile reads the contents from schemaPath and returns schema
func schemaFromFile(schemaPath string) (*spec.Schema, error) {
sc := &spec.Schema{}
if schemaPath == "" {
return sc, nil
}
sch, err := ioutil.ReadFile(schemaPath)
if err != nil {
return sc, err
}

if len(sch) == 0 {
return sc, nil
}

err = sc.UnmarshalJSON(sch)
if err != nil {
return sc, errors.Errorf("unable to parse schema: %v", err)
}
return sc, nil
}
223 changes: 223 additions & 0 deletions cmd/config/internal/commands/cmdcreatesetter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,229 @@ metadata:
spec:
profile: asm # {"type":"string","x-kustomize":{"setter":{"name":"profilesetter","value":"asm"}}}
hub: my-hub # {"type":"","x-kustomize":{"setter":{"name":"hubsetter","value":"my-hub"}}}
`,
},
{
name: "provide different type values in schema and with flag",
args: []string{"replicas", "3", "--description", "hello world", "--type", "string"},
schema: `{"type": "integer"}`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
err: `type provided in type flag (string) and in schema (integer) doesn't match`,
},
{
name: "invalid json in schema",
args: []string{"replicas", "3"},
schema: `{"foo": bar`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
err: "unable to parse schema: invalid character 'b' looking for beginning of value",
},
{
name: "unknown fields in schema are dropped",
args: []string{"replicas", "3"},
schema: `{"foo": "bar"}`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
openAPI:
definitions:
io.k8s.cli.setters.replicas:
x-k8s-cli:
setter:
name: replicas
value: "3"
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3 # {"$openapi":"replicas"}
`,
},
{
name: "unknown types are not accepted",
args: []string{"replicas", "3"},
schema: `{"type": "int"}`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
err: `invalid schema: type "int" is not supported. Must be one of: object, array, string, integer, number, boolean, file, null`,
},
{
name: "unknown types are not accepted in nested structures",
args: []string{"replicas", "3", "--field", "replicas"},
schema: `{"maxItems": 3, "type": "array", "items": {"type": "foo"}}`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
err: `invalid schema: type "foo" is not supported. Must be one of: object, array, string, integer, number, boolean, file, null`,
},
{
name: "unknown types are not accepted in --type flag",
args: []string{"replicas", "3", "--type", "bar"},
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
err: `invalid schema: type "bar" is not supported. Must be one of: object, array, string, integer, number, boolean, file, null`,
},
{
name: "unknown properties in schema are dropped",
args: []string{"replicas", "3", "--type", "integer"},
schema: `{"maximum": 3, "unknown": 42}`,
input: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3
`,
inputOpenAPI: `
apiVersion: v1alpha1
kind: Example
`,
expectedOpenAPI: `
apiVersion: v1alpha1
kind: Example
openAPI:
definitions:
io.k8s.cli.setters.replicas:
maximum: 3
type: integer
x-k8s-cli:
setter:
name: replicas
value: "3"
`,
expectedResources: `
apiVersion: apps/v1
kind: Deployment
metadata:
name: nginx-deployment
spec:
replicas: 3 # {"$openapi":"replicas"}
`,
},
}
Expand Down

0 comments on commit 52083c6

Please sign in to comment.