diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index 809e222ea..de23da0fb 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -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" @@ -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") @@ -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 { @@ -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") @@ -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]) @@ -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 +} diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index d909ebd33..4845cfd84 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -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"} `, }, } diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 7f123e1c7..6197aff95 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -5,8 +5,10 @@ package settersutil import ( "fmt" - "io/ioutil" + "strings" + "github.com/go-openapi/spec" + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" @@ -25,7 +27,7 @@ type SetterCreator struct { Type string - SchemaPath string + Schema string // FieldName if set will add the OpenAPI reference to fields with this name or path // FieldName may be the full name of the field, full path to the field, or the path suffix. @@ -46,9 +48,9 @@ type SetterCreator struct { } func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { - schema, err := schemaFromFile(c.SchemaPath) + err := validateSchema(c.Schema) if err != nil { - return err + return errors.Errorf("invalid schema: %v", err) } // Update the resources with the setter reference @@ -74,7 +76,7 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { // Update the OpenAPI definitions to hace the setter sd := setters2.SetterDefinition{ Name: c.Name, Value: c.FieldValue, SetBy: c.SetBy, Description: c.Description, - Type: c.Type, Schema: schema, Required: c.Required, + Type: c.Type, Schema: c.Schema, Required: c.Required, } if err := sd.AddToFile(openAPIPath); err != nil { return err @@ -97,14 +99,57 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { return nil } -// schemaFromFile reads the contents from schemaPath and returns schema -func schemaFromFile(schemaPath string) (string, error) { - if schemaPath == "" { - return "", nil - } - sch, err := ioutil.ReadFile(schemaPath) - if err != nil { - return "", err - } - return string(sch), nil +// The types recognized by by the go openapi validation library: +// https://github.com/go-openapi/validate/blob/master/helpers.go#L35 +var validTypeValues = []string{ + "object", "array", "string", "integer", "number", "boolean", "file", "null", +} + +// validateSchema parses the provided schema and validates it. +func validateSchema(schema string) error { + var sc spec.Schema + err := sc.UnmarshalJSON([]byte(schema)) + if err != nil { + return errors.Errorf("unable to parse schema: %v", err) + } + return validateSchemaTypes(&sc) +} + +// validateSchemaTypes traverses the schema and checks that only valid types +// are used. +func validateSchemaTypes(sc *spec.Schema) error { + if len(sc.Type) > 1 { + return errors.Errorf("only one type is supported: %s", strings.Join(sc.Type, ", ")) + } + + if len(sc.Type) == 1 { + t := sc.Type[0] + var match bool + for _, validType := range validTypeValues { + if t == validType { + match = true + } + } + if !match { + return errors.Errorf("type %q is not supported. Must be one of: %s", + t, strings.Join(validTypeValues, ", ")) + } + } + + if items := sc.Items; items != nil { + if items.Schema != nil { + err := validateSchemaTypes(items.Schema) + if err != nil { + return err + } + } + for i := range items.Schemas { + schema := items.Schemas[i] + err := validateSchemaTypes(&schema) + if err != nil { + return err + } + } + } + return nil }