From 26141e972bafb483880c497f5e55f8c46a8c03f7 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Mon, 9 Aug 2021 19:53:42 +0200 Subject: [PATCH 1/4] Implement concurrent manifest validation against Hub in CLI --- cmd/cli/cmd/manifest/manifest.go | 17 ++ cmd/cli/cmd/manifest/validate.go | 52 ++++ cmd/cli/cmd/root.go | 3 +- cmd/cli/cmd/validate.go | 68 ----- cmd/cli/docs/capact.md | 2 +- cmd/cli/docs/capact_manifest.md | 25 ++ cmd/cli/docs/capact_manifest_validate.md | 48 ++++ internal/cli/alpha/manifestgen/interface.go | 3 +- internal/cli/alpha/manifestgen/metadata.go | 26 ++ internal/cli/alpha/manifestgen/terraform.go | 3 +- internal/cli/client/hub.go | 1 + internal/cli/schema/schema.go | 12 +- internal/cli/validate/validate.go | 234 ++++++++++++++++++ internal/cli/validate/validate_test.go | 51 ++++ ocf-spec/0.0.1/examples/examples_test.go | 2 +- pkg/hub/api/graphql/public/models.extended.go | 27 ++ pkg/hub/client/client.go | 1 + pkg/hub/client/public/client.go | 58 +++++ pkg/sdk/apis/0.0.1/types/types.extend.go | 31 ++- pkg/sdk/dbpopulator/loader.go | 3 +- pkg/sdk/manifest/fsvalidator.go | 74 ++++++ pkg/sdk/manifest/fsvalidator_test.go | 184 ++++++++++++++ pkg/sdk/manifest/json_ocfschema.go | 148 +++++++++++ pkg/sdk/manifest/json_remote_helper.go | 36 +++ .../manifest/json_remote_implementation.go | 116 +++++++++ pkg/sdk/manifest/json_remote_interface.go | 74 ++++++ pkg/sdk/manifest/json_remote_type.go | 48 ++++ pkg/sdk/manifest/json_type.go | 49 ++++ pkg/sdk/manifest/metadata.go | 27 +- pkg/sdk/manifest/options.go | 15 ++ ...ation.yaml => invalid-implementation.yaml} | 6 +- .../manifest/testdata/invalid-interface.yaml | 30 +++ .../manifest/testdata/invalid-manifest.yaml | 1 + pkg/sdk/manifest/testdata/invalid-type.yaml | 46 ++++ .../testdata/invalid-type_json-schema.yaml | 95 +++++++ .../testdata/valid-implementation.yaml | 87 +++++++ .../manifest/testdata/valid-interface.yaml | 30 +++ pkg/sdk/manifest/testdata/valid-type.yaml | 56 +++++ .../testdata/wrong_implementation.yaml | 33 --- pkg/sdk/manifest/types.go | 36 +++ pkg/sdk/manifest/validation.go | 189 -------------- pkg/sdk/manifest/validation_test.go | 41 --- 42 files changed, 1725 insertions(+), 363 deletions(-) create mode 100644 cmd/cli/cmd/manifest/manifest.go create mode 100644 cmd/cli/cmd/manifest/validate.go delete mode 100644 cmd/cli/cmd/validate.go create mode 100644 cmd/cli/docs/capact_manifest.md create mode 100644 cmd/cli/docs/capact_manifest_validate.md create mode 100644 internal/cli/alpha/manifestgen/metadata.go create mode 100644 internal/cli/validate/validate.go create mode 100644 internal/cli/validate/validate_test.go create mode 100644 pkg/hub/api/graphql/public/models.extended.go create mode 100644 pkg/sdk/manifest/fsvalidator.go create mode 100644 pkg/sdk/manifest/fsvalidator_test.go create mode 100644 pkg/sdk/manifest/json_ocfschema.go create mode 100644 pkg/sdk/manifest/json_remote_helper.go create mode 100644 pkg/sdk/manifest/json_remote_implementation.go create mode 100644 pkg/sdk/manifest/json_remote_interface.go create mode 100644 pkg/sdk/manifest/json_remote_type.go create mode 100644 pkg/sdk/manifest/json_type.go create mode 100644 pkg/sdk/manifest/options.go rename pkg/sdk/manifest/testdata/{correct_implementation.yaml => invalid-implementation.yaml} (87%) create mode 100644 pkg/sdk/manifest/testdata/invalid-interface.yaml create mode 100644 pkg/sdk/manifest/testdata/invalid-manifest.yaml create mode 100644 pkg/sdk/manifest/testdata/invalid-type.yaml create mode 100644 pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml create mode 100644 pkg/sdk/manifest/testdata/valid-implementation.yaml create mode 100644 pkg/sdk/manifest/testdata/valid-interface.yaml create mode 100644 pkg/sdk/manifest/testdata/valid-type.yaml delete mode 100644 pkg/sdk/manifest/testdata/wrong_implementation.yaml create mode 100644 pkg/sdk/manifest/types.go delete mode 100644 pkg/sdk/manifest/validation.go delete mode 100644 pkg/sdk/manifest/validation_test.go diff --git a/cmd/cli/cmd/manifest/manifest.go b/cmd/cli/cmd/manifest/manifest.go new file mode 100644 index 000000000..d3bdc67df --- /dev/null +++ b/cmd/cli/cmd/manifest/manifest.go @@ -0,0 +1,17 @@ +package manifest + +import "github.com/spf13/cobra" + +// NewCmd returns a cobra.Command for manifest related operations. +func NewCmd() *cobra.Command { + root := &cobra.Command{ + Use: "manifest", + Aliases: []string{"mf"}, + Short: "This command consists of multiple subcommands to interact with OCF manifests", + } + + root.AddCommand( + NewValidate(), + ) + return root +} diff --git a/cmd/cli/cmd/manifest/validate.go b/cmd/cli/cmd/manifest/validate.go new file mode 100644 index 000000000..3beb72642 --- /dev/null +++ b/cmd/cli/cmd/manifest/validate.go @@ -0,0 +1,52 @@ +package manifest + +import ( + "os" + + "capact.io/capact/internal/cli/validate" + + "capact.io/capact/internal/cli" + "capact.io/capact/internal/cli/heredoc" + "github.com/spf13/cobra" +) + +const defaultMaxConcurrency int = 5 + +// NewValidate returns a cobra.Command for validating Hub Manifests. +func NewValidate() *cobra.Command { + var opts validate.Options + + cmd := &cobra.Command{ + Use: "validate", + Short: "Validate OCF manifests", + Example: heredoc.WithCLIName(` + # Validate interface-group.yaml file with OCF specification in default location + manifest validate ocf-spec/0.0.1/examples/interface-group.yaml + + # Validate multiple files inside test_manifests directory + manifest validate pkg/cli/test_manifests/*.yaml + + # Validate interface-group.yaml file with custom OCF specification location + manifest validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml + + # Validate all Hub manifests + manifest validate ./manifests/**/*.yaml`, cli.Name), + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + validation, err := validate.New(os.Stdout, opts) + if err != nil { + return err + } + + return validation.Run(cmd.Context(), args) + }, + } + + flags := cmd.Flags() + flags.StringVarP(&opts.SchemaLocation, "schemas", "s", "", "Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used.") + flags.BoolVarP(&opts.Verbose, "verbose", "v", false, "Prints more verbose output.") + flags.BoolVarP(&opts.ServerSide, "enable-remote-checks", "r", false, "Executes additional manifests checks against Capact Hub.") + flags.IntVar(&opts.MaxConcurrency, "concurrency", defaultMaxConcurrency, "Maximum number of concurrent workers.") + + return cmd +} diff --git a/cmd/cli/cmd/root.go b/cmd/cli/cmd/root.go index 259cdf1c4..f859a3749 100644 --- a/cmd/cli/cmd/root.go +++ b/cmd/cli/cmd/root.go @@ -6,6 +6,7 @@ import ( "capact.io/capact/cmd/cli/cmd/alpha" "capact.io/capact/cmd/cli/cmd/environment" + "capact.io/capact/cmd/cli/cmd/manifest" "capact.io/capact/cmd/cli/cmd/policy" "capact.io/capact/cmd/cli/cmd/action" @@ -76,7 +77,6 @@ func NewRoot() *cobra.Command { rootCmd.PersistentFlags().StringVarP(&configPath, "config", "c", "", "Path to the YAML config file") rootCmd.AddCommand( - NewValidate(), NewDocs(), NewLogin(), NewLogout(), @@ -84,6 +84,7 @@ func NewRoot() *cobra.Command { NewUpgrade(), NewCompletion(), NewVersion(), + manifest.NewCmd(), hub.NewCmd(), configcmd.NewCmd(), action.NewCmd(), diff --git a/cmd/cli/cmd/validate.go b/cmd/cli/cmd/validate.go deleted file mode 100644 index f14116ab2..000000000 --- a/cmd/cli/cmd/validate.go +++ /dev/null @@ -1,68 +0,0 @@ -package cmd - -import ( - "fmt" - "os" - - "capact.io/capact/internal/cli" - "capact.io/capact/internal/cli/heredoc" - "capact.io/capact/internal/cli/schema" - "capact.io/capact/pkg/sdk/manifest" - - "github.com/fatih/color" - "github.com/spf13/cobra" -) - -// NewValidate returns a cobra.Command for validating Hub Manifests. -func NewValidate() *cobra.Command { - schemaProvider := schema.Provider{} - - validateCmd := &cobra.Command{ - Use: "validate", - Short: "Validate OCF manifests", - Example: heredoc.WithCLIName(` - # Validate interface-group.yaml file with OCF specification in default location - validate ocf-spec/0.0.1/examples/interface-group.yaml - - # Validate multiple files inside test_manifests directory - validate pkg/cli/test_manifests/*.yaml - - # Validate interface-group.yaml file with custom OCF specification location - validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml - - # Validate all Hub manifests - validate ./manifests/**/*.yaml`, cli.Name), - Args: cobra.MinimumNArgs(1), - Run: func(cmd *cobra.Command, args []string) { - validator := manifest.NewFilesystemValidator(schemaProvider.FileSystem()) - - fmt.Println("Validating files...") - - shouldFail := false - - for _, filepath := range args { - result, err := validator.ValidateFile(filepath) - - if err == nil && result.Valid() { - color.Green("- %s: PASSED\n", filepath) - } else { - color.Red("- %s: FAILED\n", filepath) - for _, err := range append(result.Errors, err) { - color.Red(" %v", err) - } - - shouldFail = true - } - } - - if shouldFail { - fmt.Fprintf(os.Stderr, "Some files failed validation\n") - os.Exit(1) - } - }, - } - - schemaProvider.RegisterSchemaFlags(validateCmd.PersistentFlags()) - - return validateCmd -} diff --git a/cmd/cli/docs/capact.md b/cmd/cli/docs/capact.md index 7accb1dae..7d2a70b1c 100644 --- a/cmd/cli/docs/capact.md +++ b/cmd/cli/docs/capact.md @@ -66,9 +66,9 @@ capact [flags] * [capact install](capact_install.md) - install Capact into a given environment * [capact login](capact_login.md) - Login to a Hub (Gateway) server * [capact logout](capact_logout.md) - Logout from the Hub (Gateway) server +* [capact manifest](capact_manifest.md) - This command consists of multiple subcommands to interact with OCF manifests * [capact policy](capact_policy.md) - This command consists of multiple subcommands to interact with Policy * [capact typeinstance](capact_typeinstance.md) - This command consists of multiple subcommands to interact with target TypeInstances * [capact upgrade](capact_upgrade.md) - Upgrades Capact -* [capact validate](capact_validate.md) - Validate OCF manifests * [capact version](capact_version.md) - Show version information about this binary diff --git a/cmd/cli/docs/capact_manifest.md b/cmd/cli/docs/capact_manifest.md new file mode 100644 index 000000000..6108a94f9 --- /dev/null +++ b/cmd/cli/docs/capact_manifest.md @@ -0,0 +1,25 @@ +--- +title: capact manifest +--- + +## capact manifest + +This command consists of multiple subcommands to interact with OCF manifests + +### Options + +``` + -h, --help help for manifest +``` + +### Options inherited from parent commands + +``` + -c, --config string Path to the YAML config file +``` + +### SEE ALSO + +* [capact](capact.md) - Collective Capability Manager CLI +* [capact manifest validate](capact_manifest_validate.md) - Validate OCF manifests + diff --git a/cmd/cli/docs/capact_manifest_validate.md b/cmd/cli/docs/capact_manifest_validate.md new file mode 100644 index 000000000..b67c39d3e --- /dev/null +++ b/cmd/cli/docs/capact_manifest_validate.md @@ -0,0 +1,48 @@ +--- +title: capact manifest validate +--- + +## capact manifest validate + +Validate OCF manifests + +``` +capact manifest validate [flags] +``` + +### Examples + +``` +# Validate interface-group.yaml file with OCF specification in default location +capact manifest validate ocf-spec/0.0.1/examples/interface-group.yaml + +# Validate multiple files inside test_manifests directory +capact manifest validate pkg/cli/test_manifests/*.yaml + +# Validate interface-group.yaml file with custom OCF specification location +capact manifest validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml + +# Validate all Hub manifests +capact manifest validate ./manifests/**/*.yaml +``` + +### Options + +``` + --concurrency int Maximum number of concurrent workers. (default 5) + -r, --enable-remote-checks Executes additional manifests checks against Capact Hub. + -h, --help help for validate + -s, --schemas string Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used. + -v, --verbose Prints more verbose output. +``` + +### Options inherited from parent commands + +``` + -c, --config string Path to the YAML config file +``` + +### SEE ALSO + +* [capact manifest](capact_manifest.md) - This command consists of multiple subcommands to interact with OCF manifests + diff --git a/internal/cli/alpha/manifestgen/interface.go b/internal/cli/alpha/manifestgen/interface.go index 8b352d4e9..0ba8d7a44 100644 --- a/internal/cli/alpha/manifestgen/interface.go +++ b/internal/cli/alpha/manifestgen/interface.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "capact.io/capact/pkg/sdk/manifest" "github.com/pkg/errors" ) @@ -65,7 +64,7 @@ func GenerateInterfaceManifests(cfg *InterfaceConfig) (map[string]string, error) result := make(map[string]string, len(generated)) for _, m := range generated { - metadata, err := manifest.GetMetadata([]byte(m)) + metadata, err := unmarshalMetadata([]byte(m)) if err != nil { return nil, errors.Wrap(err, "while getting metadata for manifest") } diff --git a/internal/cli/alpha/manifestgen/metadata.go b/internal/cli/alpha/manifestgen/metadata.go new file mode 100644 index 000000000..62528d6ef --- /dev/null +++ b/internal/cli/alpha/manifestgen/metadata.go @@ -0,0 +1,26 @@ +package manifestgen + +import ( + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "sigs.k8s.io/yaml" +) + +// Metadata holds generic metadata information for Capact manifests. +type Metadata struct { + OCFVersion types.OcfVersion `yaml:"ocfVersion"` + Kind types.ManifestKind `yaml:"kind"` + Metadata struct { + Name string `yaml:"name"` + Prefix string `yaml:"prefix"` + } `yaml:"metadata"` +} + +// unmarshalMetadata reads the manifest metadata from a bytes lice of a Capact manifest. +func unmarshalMetadata(yamlBytes []byte) (Metadata, error) { + mm := Metadata{} + err := yaml.Unmarshal(yamlBytes, &mm) + if err != nil { + return mm, err + } + return mm, nil +} diff --git a/internal/cli/alpha/manifestgen/terraform.go b/internal/cli/alpha/manifestgen/terraform.go index e98c710d8..a569c8cff 100644 --- a/internal/cli/alpha/manifestgen/terraform.go +++ b/internal/cli/alpha/manifestgen/terraform.go @@ -5,7 +5,6 @@ import ( "sort" "strings" - "capact.io/capact/pkg/sdk/manifest" "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/pkg/errors" ) @@ -56,7 +55,7 @@ func GenerateTerraformManifests(cfg *TerraformConfig) (map[string]string, error) result := make(map[string]string, len(generated)) for _, m := range generated { - metadata, err := manifest.GetMetadata([]byte(m)) + metadata, err := unmarshalMetadata([]byte(m)) if err != nil { return nil, errors.Wrap(err, "while getting metadata for manifest") } diff --git a/internal/cli/client/hub.go b/internal/cli/client/hub.go index 92342e431..97a9dd224 100644 --- a/internal/cli/client/hub.go +++ b/internal/cli/client/hub.go @@ -21,6 +21,7 @@ type Hub interface { CreateTypeInstances(ctx context.Context, in *gqllocalapi.CreateTypeInstancesInput) ([]gqllocalapi.CreateTypeInstanceOutput, error) UpdateTypeInstances(ctx context.Context, in []gqllocalapi.UpdateTypeInstancesInput) ([]gqllocalapi.TypeInstance, error) DeleteTypeInstance(ctx context.Context, id string) error + CheckManifestRevisionsExist(ctx context.Context, manifestRefs []gqlpublicapi.ManifestReference) (map[gqlpublicapi.ManifestReference]bool, error) } // NewHub returns client for Capact Hub configured with saved credentials for a given server URL. diff --git a/internal/cli/schema/schema.go b/internal/cli/schema/schema.go index 244f38be9..a8e2a40b4 100644 --- a/internal/cli/schema/schema.go +++ b/internal/cli/schema/schema.go @@ -4,8 +4,6 @@ import ( "net/http" "os" "path/filepath" - - "github.com/spf13/pflag" ) //go:generate go run embed_schema.go @@ -15,6 +13,11 @@ type Provider struct { localSchemaRootDir string } +// NewProvider instantiates new Provider. +func NewProvider(localSchemaRootDir string) *Provider { + return &Provider{localSchemaRootDir: localSchemaRootDir} +} + // FileSystem returns file system implementation and root dir for schema directory. func (s *Provider) FileSystem() (http.FileSystem, string) { if len(s.localSchemaRootDir) != 0 { @@ -24,11 +27,6 @@ func (s *Provider) FileSystem() (http.FileSystem, string) { return Static, "." } -// RegisterSchemaFlags registers schema related flags -func (s *Provider) RegisterSchemaFlags(flagSet *pflag.FlagSet) { - flagSet.StringVarP(&s.localSchemaRootDir, "schemas", "s", "", "Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used.") -} - // LocalFileSystem fulfils the http.FileSystem interface and provides functionality to open files from local file system. type LocalFileSystem struct{} diff --git a/internal/cli/validate/validate.go b/internal/cli/validate/validate.go new file mode 100644 index 000000000..e9de06850 --- /dev/null +++ b/internal/cli/validate/validate.go @@ -0,0 +1,234 @@ +package validate + +import ( + "context" + "fmt" + "io" + "os" + "os/signal" + "strings" + "sync" + "syscall" + + "github.com/fatih/color" + + "capact.io/capact/internal/cli/client" + "capact.io/capact/internal/cli/config" + "capact.io/capact/internal/cli/schema" + "capact.io/capact/pkg/sdk/manifest" + "github.com/pkg/errors" +) + +// Options struct defines validation options for OCF manifest validation. +type Options struct { + SchemaLocation string + ServerSide bool + Verbose bool + MaxConcurrency int +} + +// ValidationResult defines a validation error. +type ValidationResult struct { + Path string + Errors []error +} + +// IsSuccess returns if there were any validation errors. +func (r *ValidationResult) IsSuccess() bool { + return len(r.Errors) == 0 +} + +// Error returns error message based on the ValidationResult data. +func (r *ValidationResult) Error() string { + if r == nil || len(r.Errors) == 0 { + return "" + } + + var errMsgs []string + for _, err := range r.Errors { + errMsgs = append(errMsgs, err.Error()) + } + + return fmt.Sprintf("%q:\n * %s\n", r.Path, strings.Join(errMsgs, "\n * ")) +} + +// Validation defines OCF manifest validation operation. +type Validation struct { + hubCli client.Hub + writer io.Writer + verbose bool + maxWorkers int + validatorFn func() manifest.FileSystemValidator +} + +// New creates new Validation. +func New(writer io.Writer, opts Options) (*Validation, error) { + server := config.GetDefaultContext() + fs, ocfSchemaRootPath := schema.NewProvider(opts.SchemaLocation).FileSystem() + + var ( + hubCli client.Hub + err error + validatorOpts []manifest.ValidatorOption + ) + + if opts.ServerSide { + hubCli, err = client.NewHub(server) + if err != nil { + return nil, errors.Wrap(err, "while creating Hub client") + } + + validatorOpts = append(validatorOpts, manifest.WithRemoteChecks(hubCli)) + } + + if opts.MaxConcurrency < 1 { + return nil, errors.New("concurrency parameter cannot be less than 1") + } + + return &Validation{ + // TODO: To improve: Share a single validator for all workers. + // Current implementation makes OCF JSON schemas caching separated per worker. + // That enforces thread-safe JSON validator implementations. OCF Schema validator is not thread safe. + validatorFn: func() manifest.FileSystemValidator { + return manifest.NewDefaultFilesystemValidator(fs, ocfSchemaRootPath, validatorOpts...) + }, + hubCli: hubCli, + writer: writer, + verbose: opts.Verbose, + maxWorkers: opts.MaxConcurrency, + }, nil +} + +// Run runs validation across all JSON validators. +func (v *Validation) Run(ctx context.Context, filePaths []string) error { + var workersCount = v.maxWorkers + if len(filePaths) < workersCount { + workersCount = len(filePaths) + } + + v.printIntroMessage(filePaths, workersCount) + + ctxWithCancel, cancelCtxOnSignalFn := v.makeCancellableContext(ctx) + go cancelCtxOnSignalFn() + + jobsCh := make(chan string, len(filePaths)) + resultsCh := make(chan ValidationResult, len(filePaths)) + + var wg sync.WaitGroup + for i := 0; i < workersCount; i++ { + wg.Add(1) + wrker := newWorker(&wg, v.validatorFn()) + go wrker.Do(ctxWithCancel, jobsCh, resultsCh) + } + + for _, filepath := range filePaths { + jobsCh <- filepath + } + close(jobsCh) + + go func() { + wg.Wait() + close(resultsCh) + }() + + var processedFilesCount, errsCount int + for res := range resultsCh { + processedFilesCount++ + errsCount += len(res.Errors) + v.printPartialResult(res) + } + + return v.outputResultSummary(processedFilesCount, errsCount) +} + +func (v *Validation) makeCancellableContext(ctx context.Context) (context.Context, func()) { + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) + + ctxWithCancel, cancelFn := context.WithCancel(ctx) + + return ctxWithCancel, func() { + select { + case <-sigCh: + cancelFn() + case <-ctx.Done(): + } + } +} + +func (v *Validation) printIntroMessage(filePaths []string, workersCount int) { + fileNoun := properNounFor("file", len(filePaths)) + fmt.Fprintf(v.writer, "Validating %s in %d concurrent %s...\n", fileNoun, workersCount, properNounFor("job", workersCount)) +} + +func (v *Validation) outputResultSummary(processedFilesCount int, errsCount int) error { + fileNoun := properNounFor("file", processedFilesCount) + fmt.Fprintf(v.writer, "\nValidated %d %s in total.\n", processedFilesCount, fileNoun) + + if errsCount > 0 { + errNoun := properNounFor("error", errsCount) + return fmt.Errorf("detected %d validation %s", errsCount, errNoun) + } + + fmt.Fprintf(v.writer, "🚀 No errors detected.\n") + return nil +} + +func (v *Validation) printPartialResult(res ValidationResult) { + if !res.IsSuccess() { + var prefix string + if v.verbose { + prefix = fmt.Sprintf("%s ", color.RedString("✗")) + } + fmt.Fprintf(v.writer, "- %s%s\n", prefix, res.Error()) + return + } + + // Print successes only in verbose mode + if !v.verbose { + return + } + fmt.Fprintf(v.writer, "- %s %q\n", color.GreenString("✓"), res.Path) +} + +type worker struct { + wg *sync.WaitGroup + validator manifest.FileSystemValidator +} + +func newWorker(wg *sync.WaitGroup, validator manifest.FileSystemValidator) *worker { + return &worker{wg: wg, validator: validator} +} + +// Do executes the worker logic. +func (w *worker) Do(ctx context.Context, jobCh <-chan string, resultCh chan<- ValidationResult) { + defer w.wg.Done() + for { + select { + case <-ctx.Done(): + return + case filePath, ok := <-jobCh: + if !ok { + return + } + res, err := w.validator.Do(ctx, filePath) + resultErrs := res.Errors + if err != nil { + resultErrs = append(resultErrs, err) + } + + resultCh <- ValidationResult{ + Path: filePath, + Errors: resultErrs, + } + } + } +} + +func properNounFor(str string, numberOfItems int) string { + if numberOfItems == 1 { + return str + } + + return str + "s" +} diff --git a/internal/cli/validate/validate_test.go b/internal/cli/validate/validate_test.go new file mode 100644 index 000000000..3360a70b3 --- /dev/null +++ b/internal/cli/validate/validate_test.go @@ -0,0 +1,51 @@ +package validate_test + +import ( + "context" + "io/ioutil" + "path" + "strings" + "testing" + + "capact.io/capact/internal/cli/validate" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidation_Run(t *testing.T) { + validation, err := validate.New(ioutil.Discard, validate.Options{MaxConcurrency: 5}) + require.NoError(t, err) + + pathToExamples := "../../../ocf-spec/0.0.1/examples" + files, err := ioutil.ReadDir(pathToExamples) + require.NoError(t, err) + + var filePaths []string + for _, file := range files { + if file.IsDir() || !strings.HasSuffix(file.Name(), ".yaml") { + continue + } + + filePaths = append(filePaths, path.Join(pathToExamples, file.Name())) + } + + require.True(t, len(filePaths) > 0) + t.Log(filePaths) + + err = validation.Run(context.Background(), filePaths) + assert.NoError(t, err) +} + +func TestValidation_NoFiles(t *testing.T) { + validation, err := validate.New(ioutil.Discard, validate.Options{MaxConcurrency: 5}) + require.NoError(t, err) + + filePaths := []string{"/this/file/doesnt/exist", "/same/here"} + + require.True(t, len(filePaths) > 0) + t.Log(filePaths) + + err = validation.Run(context.Background(), filePaths) + assert.Error(t, err) + assert.EqualError(t, err, "detected 2 validation errors") +} diff --git a/ocf-spec/0.0.1/examples/examples_test.go b/ocf-spec/0.0.1/examples/examples_test.go index ba478f985..ae383f3bd 100644 --- a/ocf-spec/0.0.1/examples/examples_test.go +++ b/ocf-spec/0.0.1/examples/examples_test.go @@ -45,7 +45,7 @@ func TestManifestsValid(t *testing.T) { // given // when - result, err := validator.ValidateFile(tc.manifestPath) + result, err := validator.Do(tc.manifestPath) // then require.Nil(t, err, "returned error: %v", err) diff --git a/pkg/hub/api/graphql/public/models.extended.go b/pkg/hub/api/graphql/public/models.extended.go new file mode 100644 index 000000000..86a6e06c6 --- /dev/null +++ b/pkg/hub/api/graphql/public/models.extended.go @@ -0,0 +1,27 @@ +package graphql + +import ( + "fmt" + "strings" +) + +// ManifestReference holds Capact OCF manifest identification details. +type ManifestReference struct { + Path string `json:"path"` + Revision string `json:"revision"` +} + +// GQLQueryName returns name of GraphQL query needed to get details of the manifest. +// TODO: Very naive implementation. To refactor for later once it is more widely used. +func (r ManifestReference) GQLQueryName() (string, error) { + parts := strings.Split(r.Path, ".") + if len(parts) < 3 { + return "", fmt.Errorf("path parts for %q cannot be less than 3", r.Path) + } + + if parts[1] == "core" { + return parts[2], nil + } + + return parts[1], nil +} diff --git a/pkg/hub/client/client.go b/pkg/hub/client/client.go index 984b0945d..f699996a7 100644 --- a/pkg/hub/client/client.go +++ b/pkg/hub/client/client.go @@ -39,6 +39,7 @@ type Public interface { ListImplementationRevisionsForInterface(ctx context.Context, ref hubpublicgraphql.InterfaceReference, opts ...public.GetImplementationOption) ([]hubpublicgraphql.ImplementationRevision, error) ListInterfacesWithLatestRevision(ctx context.Context, filter hubpublicgraphql.InterfaceFilter) ([]*hubpublicgraphql.Interface, error) ListImplementationRevisions(ctx context.Context, filter *hubpublicgraphql.ImplementationRevisionFilter) ([]*hubpublicgraphql.ImplementationRevision, error) + CheckManifestRevisionsExist(ctx context.Context, manifestRefs []hubpublicgraphql.ManifestReference) (map[hubpublicgraphql.ManifestReference]bool, error) } // New returns a new Client to interact with the Capact Local and Public Hub. diff --git a/pkg/hub/client/public/client.go b/pkg/hub/client/public/client.go index 868c6d702..d58ad7b08 100644 --- a/pkg/hub/client/public/client.go +++ b/pkg/hub/client/public/client.go @@ -203,6 +203,64 @@ func (c *Client) ListImplementationRevisionsForInterface(ctx context.Context, re return result, nil } +// CheckManifestRevisionsExist checks if manifests with provided manifest references exist. +func (c *Client) CheckManifestRevisionsExist(ctx context.Context, manifestRefs []gqlpublicapi.ManifestReference) (map[gqlpublicapi.ManifestReference]bool, error) { + if len(manifestRefs) == 0 { + return map[gqlpublicapi.ManifestReference]bool{}, nil + } + + getAlias := func(i int) string { + return fmt.Sprintf("partial%d", i) + } + + strBuilder := strings.Builder{} + for i, manifestRef := range manifestRefs { + alias := getAlias(i) + queryName, err := manifestRef.GQLQueryName() + if err != nil { + return nil, errors.Wrap(err, "while getting GraphQL query name for a given manifest") + } + + partialQuery := fmt.Sprintf(` + %s: %s(path:"%s") { + revision(revision:"%s") { + revision + } + } + `, alias, queryName, manifestRef.Path, manifestRef.Revision) + strBuilder.WriteString(partialQuery) + } + + req := graphql.NewRequest(fmt.Sprintf(` + query CheckManifestRevisionsExist { + %s + }`, + strBuilder.String(), + )) + + var resp map[string]struct { + Revision struct { + Revision *string `json:"revision"` + } `json:"revision"` + } + + err := retry.Do(func() error { + return c.client.Run(ctx, req, &resp) + }, retry.Attempts(retryAttempts)) + + if err != nil { + return nil, errors.Wrap(err, "while executing query to check Type Revisions exist") + } + + result := map[gqlpublicapi.ManifestReference]bool{} + for i, manifestRef := range manifestRefs { + alias := getAlias(i) + result[manifestRef] = resp[alias].Revision.Revision != nil + } + + return result, nil +} + var key = regexp.MustCompile(`\$(\w+):`) // Args is used to store arguments to GraphQL queries. diff --git a/pkg/sdk/apis/0.0.1/types/types.extend.go b/pkg/sdk/apis/0.0.1/types/types.extend.go index 40d1a4d59..34acfd2f1 100644 --- a/pkg/sdk/apis/0.0.1/types/types.extend.go +++ b/pkg/sdk/apis/0.0.1/types/types.extend.go @@ -27,10 +27,39 @@ type InputTypeInstanceRef struct { ID string `json:"id"` } -// TypeRefWithOptRevision specify type by path and optional revision. +// TypeRefWithOptRevision specifies type by path and optional revision. type TypeRefWithOptRevision struct { // Path of a given Type. Path string `json:"path"` // Version of the manifest content in the SemVer format. Revision *string `json:"revision"` } + +// ManifestKind specifies OCF manifest kind. +type ManifestKind string + +const ( + // RepoMetadataManifestKind specifies RepoMetadata kind. + RepoMetadataManifestKind ManifestKind = "RepoMetadata" + // TypeManifestKind specifies Type kind. + TypeManifestKind ManifestKind = "Type" + // AttributeManifestKind specifies Attribute kind. + AttributeManifestKind ManifestKind = "Attribute" + // InterfaceManifestKind specifies Interface kind. + InterfaceManifestKind ManifestKind = "Interface" + // ImplementationManifestKind specifies Implementation kind. + ImplementationManifestKind ManifestKind = "Implementation" + // InterfaceGroupManifestKind specifies InterfaceGroup kind. + InterfaceGroupManifestKind ManifestKind = "InterfaceGroup" + // VendorManifestKind specifies Vendor kind. + VendorManifestKind ManifestKind = "Vendor" +) + +// OCFVersion specifies the OCF version. +type OCFVersion string + +// ManifestMetadata specifies the essential, common OCF manifest metadata. +type ManifestMetadata struct { + OCFVersion OCFVersion `yaml:"ocfVersion"` + Kind ManifestKind `yaml:"kind"` +} diff --git a/pkg/sdk/dbpopulator/loader.go b/pkg/sdk/dbpopulator/loader.go index 2a5a3d971..1419859ab 100644 --- a/pkg/sdk/dbpopulator/loader.go +++ b/pkg/sdk/dbpopulator/loader.go @@ -6,6 +6,7 @@ import ( "path/filepath" "capact.io/capact/pkg/sdk/manifest" + "github.com/pkg/errors" ) @@ -32,7 +33,7 @@ func Group(paths []string) (map[string][]string, error) { if err != nil { return manifests, errors.Wrapf(err, "while reading file from path %s", path) } - metadata, err := manifest.GetMetadata(content) + metadata, err := manifest.UnmarshalManifestMetadata(content) if err != nil { return manifests, errors.Wrapf(err, "while unmarshaling manifest content from path %s", path) } diff --git a/pkg/sdk/manifest/fsvalidator.go b/pkg/sdk/manifest/fsvalidator.go new file mode 100644 index 000000000..9f54de259 --- /dev/null +++ b/pkg/sdk/manifest/fsvalidator.go @@ -0,0 +1,74 @@ +package manifest + +import ( + "context" + "io/ioutil" + "net/http" + "path/filepath" + + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" + "sigs.k8s.io/yaml" +) + +// FSValidator validates manifests using a OCF specification, which is read from a filesystem. +type FSValidator struct { + commonValidators []JSONValidator + kindValidators map[types.ManifestKind][]JSONValidator +} + +// NewDefaultFilesystemValidator returns a new FSValidator. +func NewDefaultFilesystemValidator(fs http.FileSystem, ocfSchemaRootPath string, opts ...ValidatorOption) FileSystemValidator { + validator := &FSValidator{ + commonValidators: []JSONValidator{ + NewOCFSchemaValidator(fs, ocfSchemaRootPath), + }, + kindValidators: map[types.ManifestKind][]JSONValidator{ + types.TypeManifestKind: { + NewTypeValidator(), + }, + }, + } + + for _, opt := range opts { + opt(validator) + } + + return validator +} + +// Do validates a manifest. +func (v *FSValidator) Do(ctx context.Context, path string) (ValidationResult, error) { + yamlBytes, err := ioutil.ReadFile(filepath.Clean(path)) + if err != nil { + return newValidationResult(), err + } + + metadata, err := UnmarshalManifestMetadata(yamlBytes) + if err != nil { + return newValidationResult(errors.Wrap(err, "failed to read manifest metadata")), nil + } + + jsonBytes, err := yaml.YAMLToJSON(yamlBytes) + if err != nil { + return newValidationResult(errors.Wrap(err, "cannot convert YAML manifest to JSON")), nil + } + + validators := append(v.commonValidators, v.kindValidators[metadata.Kind]...) + + var validationErrs []error + for _, validator := range validators { + res, err := validator.Do(ctx, metadata, jsonBytes) + if err != nil { + validationErrs = append(validationErrs, errors.Wrapf(err, "while running validator %s", validator.Name())) + } + + var prefixedResErrs []error + for _, resErr := range res.Errors { + prefixedResErrs = append(prefixedResErrs, errors.Wrap(resErr, validator.Name())) + } + validationErrs = append(validationErrs, prefixedResErrs...) + } + + return newValidationResult(validationErrs...), nil +} diff --git a/pkg/sdk/manifest/fsvalidator_test.go b/pkg/sdk/manifest/fsvalidator_test.go new file mode 100644 index 000000000..88a57e8e8 --- /dev/null +++ b/pkg/sdk/manifest/fsvalidator_test.go @@ -0,0 +1,184 @@ +package manifest_test + +import ( + "context" + "testing" + + graphql "capact.io/capact/pkg/hub/api/graphql/public" + "github.com/pkg/errors" + + "capact.io/capact/internal/cli/schema" + "github.com/stretchr/testify/assert" + + "capact.io/capact/pkg/sdk/manifest" + + "github.com/stretchr/testify/require" +) + +func TestFilesystemValidator_ValidateFile(t *testing.T) { + // given + sampleAttr := manifestRef("cap.core.sample.attr") + tests := map[string]struct { + manifestPath string + expectedValidationErrorMsgs []string + expectedGeneralErrMsg string + hubCli manifest.Hub + }{ + "Valid Implementation": { + manifestPath: "testdata/valid-implementation.yaml", + expectedValidationErrorMsgs: []string{}, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + manifestRef("cap.sample.attribute"): true, + manifestRef("cap.type.mattermost.helm.install-input"): true, + manifestRef("cap.type.database.postgresql.config"): true, + manifestRef("cap.interface.productivity.mattermost.install"): true, + manifestRef("cap.core.type.platform.kubernetes"): true, + manifestRef("cap.interface.runner.helm.install"): true, + manifestRef("cap.interface.runner.argo.run"): true, + manifestRef("cap.interface.templating.jinja2.template"): true, + manifestRef("cap.interface.database.postgresql.install"): true, + manifestRef("cap.interface.database.postgresql.create-db"): true, + manifestRef("cap.interface.database.postgresql.create-user"): true, + }, nil), + }, + "Valid Interface": { + manifestPath: "testdata/valid-interface.yaml", + expectedValidationErrorMsgs: []string{}, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + manifestRef("cap.type.productivity.mattermost.config"): true, + manifestRef("cap.type.productivity.mattermost.install-input"): true, + }, nil), + }, + "Valid Type": { + manifestPath: "testdata/valid-type.yaml", + expectedValidationErrorMsgs: []string{}, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{sampleAttr: true}, nil), + }, + "Invalid Implementation": { + manifestPath: "testdata/invalid-implementation.yaml", + expectedValidationErrorMsgs: []string{ + "OCFSchemaValidator: spec: appVersion is required", + "RemoteImplementationValidator: manifest revision 'cap.interface.cms.wordpress:0.1.0' doesn't exist in Hub", + }, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + manifestRef("cap.interface.cms.wordpress"): false, + }, nil), + }, + "Invalid Interface": { + manifestPath: "testdata/invalid-interface.yaml", + expectedValidationErrorMsgs: []string{ + "RemoteInterfaceValidator: manifest revision 'cap.type.productivity.mattermost.install-input:0.1.0' doesn't exist in Hub", + }, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + manifestRef("cap.type.productivity.mattermost.install-input"): false, + manifestRef("cap.type.productivity.mattermost.config"): true, + }, nil), + }, + "Invalid JSON Schema in Type": { + manifestPath: "testdata/invalid-type_json-schema.yaml", + expectedValidationErrorMsgs: []string{ + "TypeValidator: type: Must validate at least one schema (anyOf)", + `TypeValidator: type: type must be one of the following: "array", "boolean", "integer", "null", "number", "object", "string"`, + }, + }, + "Invalid Type": { + manifestPath: "testdata/invalid-type.yaml", + expectedValidationErrorMsgs: []string{ + "TypeValidator: invalid character '}' looking for beginning of object key string", + "RemoteTypeValidator: manifest revision 'cap.core.sample.attr:0.1.0' doesn't exist in Hub", + }, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + sampleAttr: false, + }, nil), + }, + "Error from Hub": { + manifestPath: "testdata/valid-interface.yaml", + expectedValidationErrorMsgs: []string{ + "while running validator RemoteInterfaceValidator: while checking if manifest revisions exist: test error", + }, + hubCli: fixHub(t, map[graphql.ManifestReference]bool{ + manifestRef("cap.type.productivity.mattermost.config"): true, + manifestRef("cap.type.productivity.mattermost.install-input"): true, + }, errors.New("test error")), + }, + "Cannot load file": { + manifestPath: "testdata/no-file.yaml", + expectedGeneralErrMsg: "open testdata/no-file.yaml: no such file or directory", + }, + "Invalid manifest": { + manifestPath: "testdata/invalid-manifest.yaml", + expectedValidationErrorMsgs: []string{ + "failed to read manifest metadata: OCFVersion and Kind must not be empty", + }, + }, + } + + for tn, tc := range tests { + t.Run(tn, func(t *testing.T) { + var opts []manifest.ValidatorOption + if tc.hubCli != nil { + opts = append(opts, manifest.WithRemoteChecks(tc.hubCli)) + } + + validator := manifest.NewDefaultFilesystemValidator( + &schema.LocalFileSystem{}, + "../../../ocf-spec", + opts..., + ) + + // when + result, err := validator.Do(context.Background(), tc.manifestPath) + + // then + if tc.expectedGeneralErrMsg != "" { + require.Error(t, err) + assert.EqualError(t, err, tc.expectedGeneralErrMsg) + } else { + require.Nil(t, err) + } + + require.Len(t, result.Errors, len(tc.expectedValidationErrorMsgs)) + + if len(result.Errors) > 0 { + var errMsgs []string + for _, err := range result.Errors { + errMsgs = append(errMsgs, err.Error()) + } + assert.ElementsMatch(t, tc.expectedValidationErrorMsgs, errMsgs) + } + }) + } +} + +type fakeHub struct { + fn func(ctx context.Context, manifestRefs []graphql.ManifestReference) (map[graphql.ManifestReference]bool, error) +} + +func (h *fakeHub) CheckManifestRevisionsExist(ctx context.Context, manifestRefs []graphql.ManifestReference) (map[graphql.ManifestReference]bool, error) { + return h.fn(ctx, manifestRefs) +} + +func fixHub(t *testing.T, result map[graphql.ManifestReference]bool, err error) *fakeHub { + hub := &fakeHub{ + fn: func(ctx context.Context, manifestRefs []graphql.ManifestReference) (map[graphql.ManifestReference]bool, error) { + var resultManifestRefs []graphql.ManifestReference + for key := range result { + resultManifestRefs = append(resultManifestRefs, key) + } + ok := assert.ElementsMatch(t, manifestRefs, resultManifestRefs) + if !ok { + return nil, errors.New("manifest references don't match") + } + + return result, err + }, + } + return hub +} + +func manifestRef(path string) graphql.ManifestReference { + return graphql.ManifestReference{ + Path: path, + Revision: "0.1.0", + } +} diff --git a/pkg/sdk/manifest/json_ocfschema.go b/pkg/sdk/manifest/json_ocfschema.go new file mode 100644 index 000000000..400549063 --- /dev/null +++ b/pkg/sdk/manifest/json_ocfschema.go @@ -0,0 +1,148 @@ +package manifest + +import ( + "context" + "fmt" + "net/http" + "os" + "sort" + + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/iancoleman/strcase" + "github.com/pkg/errors" + "github.com/xeipuuv/gojsonschema" +) + +type loadedOCFSchema struct { + common *gojsonschema.SchemaLoader + kind map[types.ManifestKind]*gojsonschema.Schema +} + +// OCFSchemaValidator validates manifests using a OCF specification, which is read from a filesystem. +type OCFSchemaValidator struct { + fs http.FileSystem + + schemaRootPath string + cachedSchemas map[types.OCFVersion]*loadedOCFSchema +} + +// NewOCFSchemaValidator returns a new OCFSchemaValidator. +func NewOCFSchemaValidator(fs http.FileSystem, schemaRootPath string) *OCFSchemaValidator { + return &OCFSchemaValidator{ + schemaRootPath: schemaRootPath, + fs: fs, + cachedSchemas: map[types.OCFVersion]*loadedOCFSchema{}, + } +} + +// Do validates a manifest. +func (v *OCFSchemaValidator) Do(_ context.Context, metadata types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { + schema, err := v.getManifestSchema(metadata) + if err != nil { + return newValidationResult(), errors.Wrap(err, "while getting manifest JSON schema") + } + + manifestLoader := gojsonschema.NewBytesLoader(jsonBytes) + + jsonschemaResult, err := schema.Validate(manifestLoader) + if err != nil { + return newValidationResult(err), nil + } + + result := newValidationResult() + + for _, err := range jsonschemaResult.Errors() { + result.Errors = append(result.Errors, fmt.Errorf("%v", err.String())) + } + + return result, err +} + +// Name returns validator name. +func (v *OCFSchemaValidator) Name() string { + return "OCFSchemaValidator" +} + +func (v *OCFSchemaValidator) getManifestSchema(metadata types.ManifestMetadata) (*gojsonschema.Schema, error) { + var ok bool + var cachedSchema *loadedOCFSchema + + if cachedSchema, ok = v.cachedSchemas[metadata.OCFVersion]; !ok { + cachedSchema = &loadedOCFSchema{ + common: nil, + kind: map[types.ManifestKind]*gojsonschema.Schema{}, + } + v.cachedSchemas[metadata.OCFVersion] = cachedSchema + } + + if schema, ok := cachedSchema.kind[metadata.Kind]; ok { + return schema, nil + } + + rootLoader := v.getRootSchemaJSONLoader(metadata) + + if cachedSchema.common == nil { + sl, err := v.getCommonSchemaLoader(metadata.OCFVersion) + if err != nil { + return nil, errors.Wrap(err, "failed to get common schema loader") + } + cachedSchema.common = sl + } + + schema, err := cachedSchema.common.Compile(rootLoader) + if err != nil { + return nil, errors.Wrapf(err, "failed to compile schema for %s/%s", metadata.OCFVersion, metadata.Kind) + } + + cachedSchema.kind[metadata.Kind] = schema + + return schema, nil +} + +func (v *OCFSchemaValidator) getRootSchemaJSONLoader(metadata types.ManifestMetadata) gojsonschema.JSONLoader { + filename := strcase.ToKebab(string(metadata.Kind)) + path := fmt.Sprintf("file://%s/%s/schema/%s.json", v.schemaRootPath, metadata.OCFVersion, filename) + return gojsonschema.NewReferenceLoaderFileSystem(path, v.fs) +} + +func (v *OCFSchemaValidator) getCommonSchemaLoader(ocfVersion types.OCFVersion) (*gojsonschema.SchemaLoader, error) { + commonDir := fmt.Sprintf("%s/%s/schema/common", v.schemaRootPath, ocfVersion) + + sl := gojsonschema.NewSchemaLoader() + + files, err := v.readDir(commonDir) + if err != nil { + return nil, errors.Wrap(err, "failed to list common schemas directory") + } + + for _, file := range files { + if file.IsDir() { + continue + } + + path := fmt.Sprintf("file://%s/%s", commonDir, file.Name()) + if err := sl.AddSchemas(gojsonschema.NewReferenceLoaderFileSystem(path, v.fs)); err != nil { + return nil, errors.Wrapf(err, "cannot load common schema %s", path) + } + } + + return sl, nil +} + +// readDir reads the directory named by dirname and returns +// a list of directory entries sorted by filename. +func (v *OCFSchemaValidator) readDir(dirname string) ([]os.FileInfo, error) { + f, err := v.fs.Open(dirname) + if err != nil { + return nil, err + } + list, err := f.Readdir(-1) + if err != nil { + return nil, err + } + if err := f.Close(); err != nil { + return nil, err + } + sort.Slice(list, func(i, j int) bool { return list[i].Name() < list[j].Name() }) + return list, nil +} diff --git a/pkg/sdk/manifest/json_remote_helper.go b/pkg/sdk/manifest/json_remote_helper.go new file mode 100644 index 000000000..e364c03ed --- /dev/null +++ b/pkg/sdk/manifest/json_remote_helper.go @@ -0,0 +1,36 @@ +package manifest + +import ( + "context" + "fmt" + + hubpublicgraphql "capact.io/capact/pkg/hub/api/graphql/public" + "github.com/pkg/errors" +) + +// Hub is an interface for Hub GraphQL client methods needed for the remote validation. +type Hub interface { + CheckManifestRevisionsExist(ctx context.Context, manifestRefs []hubpublicgraphql.ManifestReference) (map[hubpublicgraphql.ManifestReference]bool, error) +} + +func checkManifestRevisionsExist(ctx context.Context, hub Hub, manifestRefsToCheck []hubpublicgraphql.ManifestReference) (ValidationResult, error) { + if len(manifestRefsToCheck) == 0 { + return ValidationResult{}, nil + } + + res, err := hub.CheckManifestRevisionsExist(ctx, manifestRefsToCheck) + if err != nil { + return ValidationResult{}, errors.Wrap(err, "while checking if manifest revisions exist") + } + + var validationErrs []error + for typeRef, exists := range res { + if exists { + continue + } + + validationErrs = append(validationErrs, fmt.Errorf("manifest revision '%s:%s' doesn't exist in Hub", typeRef.Path, typeRef.Revision)) + } + + return ValidationResult{Errors: validationErrs}, nil +} diff --git a/pkg/sdk/manifest/json_remote_implementation.go b/pkg/sdk/manifest/json_remote_implementation.go new file mode 100644 index 000000000..244143bd6 --- /dev/null +++ b/pkg/sdk/manifest/json_remote_implementation.go @@ -0,0 +1,116 @@ +package manifest + +import ( + "context" + "encoding/json" + "strings" + + hubpublicgraphql "capact.io/capact/pkg/hub/api/graphql/public" + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" +) + +// RemoteImplementationValidator is a validator for Implementation manifest, which calls Hub in order to do validation checks. +type RemoteImplementationValidator struct { + hub Hub +} + +// NewRemoteImplementationValidator creates new RemoteImplementationValidator. +func NewRemoteImplementationValidator(hub Hub) *RemoteImplementationValidator { + return &RemoteImplementationValidator{ + hub: hub, + } +} + +// Do is a method which triggers the validation. +func (v *RemoteImplementationValidator) Do(ctx context.Context, _ types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { + var entity types.Implementation + err := json.Unmarshal(jsonBytes, &entity) + if err != nil { + return ValidationResult{}, errors.Wrap(err, "while unmarshalling JSON into Implementation type") + } + + var manifestRefsToCheck []hubpublicgraphql.ManifestReference + + // Attributes + for path, attr := range entity.Metadata.Attributes { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: path, + Revision: attr.Revision, + }) + } + + // AdditionalInput + if entity.Spec.AdditionalInput != nil { + // Parameters + additionalInputParams := entity.Spec.AdditionalInput.Parameters + if additionalInputParams != nil && additionalInputParams.TypeRef != nil { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: additionalInputParams.TypeRef.Path, + Revision: additionalInputParams.TypeRef.Revision, + }) + } + + // TypeInstances + for _, ti := range entity.Spec.AdditionalInput.TypeInstances { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: ti.TypeRef.Path, + Revision: ti.TypeRef.Revision, + }) + } + } + + // AdditionalOutput + if entity.Spec.AdditionalOutput != nil { + for _, ti := range entity.Spec.AdditionalOutput.TypeInstances { + if ti.TypeRef == nil { + continue + } + + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: ti.TypeRef.Path, + Revision: ti.TypeRef.Revision, + }) + } + } + + // Implements + for _, implementsItem := range entity.Spec.Implements { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: implementsItem.Path, + Revision: implementsItem.Revision, + }) + } + + // Requires + for requiresKey, requiresValue := range entity.Spec.Requires { + var itemsToCheck []types.RequireEntity + itemsToCheck = append(itemsToCheck, requiresValue.OneOf...) + itemsToCheck = append(itemsToCheck, requiresValue.AllOf...) + itemsToCheck = append(itemsToCheck, requiresValue.AnyOf...) + + for _, requiresSubItem := range itemsToCheck { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: strings.Join([]string{requiresKey, requiresSubItem.Name}, "."), + Revision: requiresSubItem.Revision, + }) + } + } + + // Imports + for _, importsItem := range entity.Spec.Imports { + for _, method := range importsItem.Methods { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: strings.Join([]string{importsItem.InterfaceGroupPath, method.Name}, "."), + Revision: method.Revision, + }) + } + } + + return checkManifestRevisionsExist(ctx, v.hub, manifestRefsToCheck) +} + +// Name returns the validator name. +func (v *RemoteImplementationValidator) Name() string { + return "RemoteImplementationValidator" +} diff --git a/pkg/sdk/manifest/json_remote_interface.go b/pkg/sdk/manifest/json_remote_interface.go new file mode 100644 index 000000000..90981eb84 --- /dev/null +++ b/pkg/sdk/manifest/json_remote_interface.go @@ -0,0 +1,74 @@ +package manifest + +import ( + "context" + "encoding/json" + + hubpublicgraphql "capact.io/capact/pkg/hub/api/graphql/public" + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" +) + +// RemoteInterfaceValidator is a validator for Interface manifest, which calls Hub in order to do validation checks. +type RemoteInterfaceValidator struct { + hub Hub +} + +// NewRemoteInterfaceValidator creates new RemoteImplementationValidator. +func NewRemoteInterfaceValidator(hub Hub) *RemoteInterfaceValidator { + return &RemoteInterfaceValidator{ + hub: hub, + } +} + +// Do is a method which triggers the validation. +func (v *RemoteInterfaceValidator) Do(ctx context.Context, _ types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { + var entity types.Interface + err := json.Unmarshal(jsonBytes, &entity) + if err != nil { + return ValidationResult{}, errors.Wrap(err, "while unmarshalling JSON into Interface type") + } + + var manifestRefsToCheck []hubpublicgraphql.ManifestReference + + // Input Parameters + if entity.Spec.Input.Parameters != nil { + for _, param := range entity.Spec.Input.Parameters.ParameterMap { + if param.TypeRef == nil { + continue + } + + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: param.TypeRef.Path, + Revision: param.TypeRef.Revision, + }) + } + } + + // Input TypeInstances + for _, ti := range entity.Spec.Input.TypeInstances { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: ti.TypeRef.Path, + Revision: ti.TypeRef.Revision, + }) + } + + // Output TypeInstances + for _, ti := range entity.Spec.Output.TypeInstances { + if ti.TypeRef == nil { + continue + } + + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: ti.TypeRef.Path, + Revision: ti.TypeRef.Revision, + }) + } + + return checkManifestRevisionsExist(ctx, v.hub, manifestRefsToCheck) +} + +// Name returns the validator name. +func (v *RemoteInterfaceValidator) Name() string { + return "RemoteInterfaceValidator" +} diff --git a/pkg/sdk/manifest/json_remote_type.go b/pkg/sdk/manifest/json_remote_type.go new file mode 100644 index 000000000..9f5c007ca --- /dev/null +++ b/pkg/sdk/manifest/json_remote_type.go @@ -0,0 +1,48 @@ +package manifest + +import ( + "context" + "encoding/json" + + hubpublicgraphql "capact.io/capact/pkg/hub/api/graphql/public" + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" +) + +// RemoteTypeValidator is a validator for Type manifest, which calls Hub in order to do validation checks. +type RemoteTypeValidator struct { + hub Hub +} + +// NewRemoteTypeValidator creates new RemoteTypeValidator. +func NewRemoteTypeValidator(hub Hub) *RemoteTypeValidator { + return &RemoteTypeValidator{ + hub: hub, + } +} + +// Do is a method which triggers the validation. +func (v *RemoteTypeValidator) Do(ctx context.Context, _ types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { + var entity types.Type + err := json.Unmarshal(jsonBytes, &entity) + if err != nil { + return ValidationResult{}, errors.Wrap(err, "while unmarshalling JSON into Type type") + } + + var manifestRefsToCheck []hubpublicgraphql.ManifestReference + + // Attributes + for path, attr := range entity.Metadata.Attributes { + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ + Path: path, + Revision: attr.Revision, + }) + } + + return checkManifestRevisionsExist(ctx, v.hub, manifestRefsToCheck) +} + +// Name returns the validator name. +func (v *RemoteTypeValidator) Name() string { + return "RemoteTypeValidator" +} diff --git a/pkg/sdk/manifest/json_type.go b/pkg/sdk/manifest/json_type.go new file mode 100644 index 000000000..ba8c14400 --- /dev/null +++ b/pkg/sdk/manifest/json_type.go @@ -0,0 +1,49 @@ +package manifest + +import ( + "context" + "encoding/json" + "fmt" + + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" + "github.com/xeipuuv/gojsonschema" +) + +// TypeValidator is a validator for Type manifest. +type TypeValidator struct{} + +// NewTypeValidator creates new TypeValidator. +func NewTypeValidator() *TypeValidator { + return &TypeValidator{} +} + +// Do is a method which triggers the validation. +func (v *TypeValidator) Do(_ context.Context, _ types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { + var typeEntity types.Type + err := json.Unmarshal(jsonBytes, &typeEntity) + if err != nil { + return ValidationResult{}, errors.Wrap(err, "while unmarshalling JSON into Type type") + } + + jsonSchemaStr := typeEntity.Spec.JSONSchema.Value + schemaLoader := gojsonschema.NewReferenceLoader("http://json-schema.org/draft-07/schema") + manifestLoader := gojsonschema.NewStringLoader(jsonSchemaStr) + + jsonSchemaValidationResult, err := gojsonschema.Validate(schemaLoader, manifestLoader) + if err != nil { + return newValidationResult(err), nil + } + + result := ValidationResult{} + for _, err := range jsonSchemaValidationResult.Errors() { + result.Errors = append(result.Errors, fmt.Errorf("%v", err.String())) + } + + return result, nil +} + +// Name returns the validator name. +func (v *TypeValidator) Name() string { + return "TypeValidator" +} diff --git a/pkg/sdk/manifest/metadata.go b/pkg/sdk/manifest/metadata.go index 347eb4943..cbb4e40f9 100644 --- a/pkg/sdk/manifest/metadata.go +++ b/pkg/sdk/manifest/metadata.go @@ -1,23 +1,22 @@ package manifest -import "sigs.k8s.io/yaml" +import ( + "capact.io/capact/pkg/sdk/apis/0.0.1/types" + "github.com/pkg/errors" + "sigs.k8s.io/yaml" +) -// Metadata holds generic metadata information for Capact manifests -type Metadata struct { - OCFVersion ocfVersion `yaml:"ocfVersion"` - Kind kind `yaml:"kind"` - Metadata struct { - Name string `yaml:"name"` - Prefix string `yaml:"prefix"` - } `yaml:"metadata"` -} - -// GetMetadata reads the manifest metadata from a byteslice of a Capact manifest -func GetMetadata(yamlBytes []byte) (Metadata, error) { - mm := Metadata{} +// UnmarshalManifestMetadata loads essential manifest metadata (kind and OCF version) from YAML bytes. +func UnmarshalManifestMetadata(yamlBytes []byte) (types.ManifestMetadata, error) { + mm := types.ManifestMetadata{} err := yaml.Unmarshal(yamlBytes, &mm) if err != nil { return mm, err } + + if mm.OCFVersion == "" || mm.Kind == "" { + return mm, errors.New("OCFVersion and Kind must not be empty") + } + return mm, nil } diff --git a/pkg/sdk/manifest/options.go b/pkg/sdk/manifest/options.go new file mode 100644 index 000000000..279ecd1c3 --- /dev/null +++ b/pkg/sdk/manifest/options.go @@ -0,0 +1,15 @@ +package manifest + +import "capact.io/capact/pkg/sdk/apis/0.0.1/types" + +// ValidatorOption is used to provide additional configuration options for the validation. +type ValidatorOption func(validator *FSValidator) + +// WithRemoteChecks enables validation checks for manifests against Capact Hub. +func WithRemoteChecks(hubCli Hub) ValidatorOption { + return func(r *FSValidator) { + r.kindValidators[types.TypeManifestKind] = append(r.kindValidators[types.TypeManifestKind], NewRemoteTypeValidator(hubCli)) + r.kindValidators[types.InterfaceManifestKind] = append(r.kindValidators[types.InterfaceManifestKind], NewRemoteInterfaceValidator(hubCli)) + r.kindValidators[types.ImplementationManifestKind] = append(r.kindValidators[types.ImplementationManifestKind], NewRemoteImplementationValidator(hubCli)) + } +} diff --git a/pkg/sdk/manifest/testdata/correct_implementation.yaml b/pkg/sdk/manifest/testdata/invalid-implementation.yaml similarity index 87% rename from pkg/sdk/manifest/testdata/correct_implementation.yaml rename to pkg/sdk/manifest/testdata/invalid-implementation.yaml index bd692f9f1..699fc9919 100644 --- a/pkg/sdk/manifest/testdata/correct_implementation.yaml +++ b/pkg/sdk/manifest/testdata/invalid-implementation.yaml @@ -14,11 +14,13 @@ metadata: name: Foo Bar url: https://examples.com/foo/bar spec: - appVersion: "5.4.x - 5.5.x, 5.6.0-alpha0" + + # MISSING SECTION + # appVersion: "5.4.x - 5.5.x, 5.6.0-alpha0" implements: - path: cap.interface.cms.wordpress - revision: "1.0.0" + revision: "0.1.0" outputTypeInstanceRelations: {} diff --git a/pkg/sdk/manifest/testdata/invalid-interface.yaml b/pkg/sdk/manifest/testdata/invalid-interface.yaml new file mode 100644 index 000000000..14d0e71b4 --- /dev/null +++ b/pkg/sdk/manifest/testdata/invalid-interface.yaml @@ -0,0 +1,30 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Interface +metadata: + prefix: cap.interface.productivity.mattermost + name: install + displayName: "Install Mattermost Team Edition" + description: "Install action for Mattermost Team Edition" + documentationURL: https://docs.mattermost.com/ + supportURL: https://docs.mattermost.com/ + iconURL: https://docs.mattermost.com/_static/images/Mattermost-Logo-Blue.svg + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + +spec: + input: + parameters: + input-parameters: + typeRef: + path: cap.type.productivity.mattermost.install-input + revision: 0.1.0 + + output: + typeInstances: + mattermost-config: + typeRef: + path: cap.type.productivity.mattermost.config + revision: 0.1.0 diff --git a/pkg/sdk/manifest/testdata/invalid-manifest.yaml b/pkg/sdk/manifest/testdata/invalid-manifest.yaml new file mode 100644 index 000000000..611962cf5 --- /dev/null +++ b/pkg/sdk/manifest/testdata/invalid-manifest.yaml @@ -0,0 +1 @@ +hello: there diff --git a/pkg/sdk/manifest/testdata/invalid-type.yaml b/pkg/sdk/manifest/testdata/invalid-type.yaml new file mode 100644 index 000000000..df5c0862c --- /dev/null +++ b/pkg/sdk/manifest/testdata/invalid-type.yaml @@ -0,0 +1,46 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Type +metadata: + name: config + prefix: cap.type.database.postgresql + displayName: PostgreSQL config + description: Defines configuration for PostgreSQL + documentationURL: https://capact.io + supportURL: https://capact.io + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + attributes: + cap.core.sample.attr: + revision: 0.1.0 +spec: + jsonSchema: + # Invalid JSON + value: |- + { + "$schema": "http://json-schema.org/draft-07/schema", + "type": "objectdsds", + "title": "The schema for PostgreSQL configuration", + "examples": [ + { + "superuser": { + "defaultDBName": "default_db", + "host": "host", + "port": 9000, + } + ], + "required": [ + "superuser", + "defaultDBName", + "host", + "port" + + "title": "Port", + "minimum": 0, + "maximum": 65535 + } + }, + "additionalProperties": false + } diff --git a/pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml b/pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml new file mode 100644 index 000000000..0fde279fe --- /dev/null +++ b/pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml @@ -0,0 +1,95 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Type +metadata: + name: config + prefix: cap.type.database.postgresql + displayName: PostgreSQL config + description: Defines configuration for PostgreSQL + documentationURL: https://capact.io + supportURL: https://capact.io + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + attributes: + cap.core.attribute.workload.stateful: + revision: 0.1.0 +spec: + jsonSchema: + # + # Invalid type of root object + # + value: |- + { + "$schema":"http://json-schema.org/draft-07/schema", + "type":"objectdas", + "title":"The schema for PostgreSQL configuration", + "examples":[ + { + "superuser":{ + "username":"username", + "password":"password" + }, + "defaultDBName":"default_db", + "host":"host", + "port":9000 + } + ], + "required":[ + "superuser", + "defaultDBName", + "host", + "port" + ], + "definitions":{ + "hostname":{ + "type":"string", + "format":"hostname", + "title":"Hostname" + }, + "port":{ + "type":"integer", + "title":"Port", + "minimum":0, + "maximum":65535 + } + }, + "properties":{ + "superuser":{ + "$id":"#/properties/superuser", + "type":"object", + "title":"Defines superuser details", + "required":[ + "username", + "password" + ], + "properties":{ + "username":{ + "$id":"#/properties/superuser/properties/username", + "type":"string", + "title":"Create the specified user with superuser power and a database with the same name.", + "default":"postgres" + }, + "password":{ + "$id":"#/properties/superuser/properties/password", + "type":"string", + "title":"Sets the superuser password for PostgreSQL" + } + }, + "additionalProperties":false + }, + "defaultDBName":{ + "$id":"#/properties/defaultDBName", + "type":"string", + "title":"Defines a different name for the default database that is created" + }, + "host":{ + "$ref":"#/definitions/hostname" + }, + "port":{ + "$ref":"#/definitions/port" + } + }, + "additionalProperties":false + } diff --git a/pkg/sdk/manifest/testdata/valid-implementation.yaml b/pkg/sdk/manifest/testdata/valid-implementation.yaml new file mode 100644 index 000000000..e7849ac6f --- /dev/null +++ b/pkg/sdk/manifest/testdata/valid-implementation.yaml @@ -0,0 +1,87 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Implementation +metadata: + prefix: cap.implementation.mattermost.mattermost-team-edition + name: install + displayName: Install Mattermost Team Edition + description: Action which installs Mattermost Team Edition via Helm chart + documentationURL: https://docs.mattermost.com/ + supportURL: https://docs.mattermost.com/ + license: + name: "Apache 2.0" + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + attributes: + cap.sample.attribute: + revision: 0.1.0 + +spec: + appVersion: "10,11,12,13" + + outputTypeInstanceRelations: + mattermost-config: + uses: + - mattermost-helm-release + - postgresql + - database + - database-user + + additionalInput: + typeInstances: + postgresql: + typeRef: + path: cap.type.database.postgresql.config + revision: 0.1.0 + verbs: ["get"] + parameters: + typeRef: + path: cap.type.mattermost.helm.install-input + revision: 0.1.0 + + implements: + - path: cap.interface.productivity.mattermost.install + revision: 0.1.0 + + requires: + cap.core.type.platform: + oneOf: + - name: kubernetes + revision: 0.1.0 + + imports: + - interfaceGroupPath: cap.interface.runner.helm + alias: helm + methods: + - name: install + revision: 0.1.0 + - interfaceGroupPath: cap.interface.runner.argo + alias: argo + methods: + - name: run + revision: 0.1.0 + - interfaceGroupPath: cap.interface.templating.jinja2 + alias: jinja2 + methods: + - name: template + revision: 0.1.0 + - interfaceGroupPath: cap.interface.database.postgresql + alias: postgresql + methods: + - name: install + revision: 0.1.0 + - name: create-db + revision: 0.1.0 + - name: create-user + revision: 0.1.0 + + action: + runnerInterface: argo.run + args: + workflow: + # Invalid workflow as it is not validated + entrypoint: mattermost-install + templates: + - name: mattermost-install diff --git a/pkg/sdk/manifest/testdata/valid-interface.yaml b/pkg/sdk/manifest/testdata/valid-interface.yaml new file mode 100644 index 000000000..14d0e71b4 --- /dev/null +++ b/pkg/sdk/manifest/testdata/valid-interface.yaml @@ -0,0 +1,30 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Interface +metadata: + prefix: cap.interface.productivity.mattermost + name: install + displayName: "Install Mattermost Team Edition" + description: "Install action for Mattermost Team Edition" + documentationURL: https://docs.mattermost.com/ + supportURL: https://docs.mattermost.com/ + iconURL: https://docs.mattermost.com/_static/images/Mattermost-Logo-Blue.svg + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + +spec: + input: + parameters: + input-parameters: + typeRef: + path: cap.type.productivity.mattermost.install-input + revision: 0.1.0 + + output: + typeInstances: + mattermost-config: + typeRef: + path: cap.type.productivity.mattermost.config + revision: 0.1.0 diff --git a/pkg/sdk/manifest/testdata/valid-type.yaml b/pkg/sdk/manifest/testdata/valid-type.yaml new file mode 100644 index 000000000..3c07ae227 --- /dev/null +++ b/pkg/sdk/manifest/testdata/valid-type.yaml @@ -0,0 +1,56 @@ +ocfVersion: 0.0.1 +revision: 0.1.0 +kind: Type +metadata: + name: config + prefix: cap.type.productivity.mattermost + displayName: Mattermost config + description: Defines configuration for Mattermost instance + documentationURL: https://docs.mattermost.com/ + supportURL: https://docs.mattermost.com/ + iconURL: https://docs.mattermost.com/_static/images/Mattermost-Logo-Blue.svg + maintainers: + - email: team-dev@capact.io + name: Capact Dev Team + url: https://capact.io + attributes: + cap.core.sample.attr: + revision: 0.1.0 + +spec: + jsonSchema: + value: |- + { + "$schema": "http://json-schema.org/draft-07/schema", + "type": "object", + "title": "The schema for Mattermost configuration", + "required": [ + "version" + ], + "definitions": { + "semVer": { + "type": "string", + "minLength": 5, + "pattern": "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$", + "title": "Semantic Versioning version", + "examples": [ + "1.19.0", + "2.0.1-alpha1" + ] + }, + "hostname": { + "type": "string", + "format": "hostname", + "title": "Hostname" + } + }, + "properties": { + "version": { + "$ref": "#/definitions/semVer" + }, + "host": { + "$ref": "#/definitions/hostname" + } + }, + "additionalProperties": true + } diff --git a/pkg/sdk/manifest/testdata/wrong_implementation.yaml b/pkg/sdk/manifest/testdata/wrong_implementation.yaml deleted file mode 100644 index 8c58e3b30..000000000 --- a/pkg/sdk/manifest/testdata/wrong_implementation.yaml +++ /dev/null @@ -1,33 +0,0 @@ -ocfVersion: 0.0.1 -revision: 0.0.1 -kind: Implementation -metadata: - name: install - description: WordPress installation - license: - name: "BSD-3-Clause" - maintainers: - - email: foo@example.com - name: Foo Bar - url: https://examples.com/foo/bar - - email: foo@example.com - name: Foo Bar - url: https://examples.com/foo/bar -spec: - appVersion: "5.4.x - 5.5.x, 5.6.0-alpha0" - - # MISSING SECTION - #implements: - # - path: cap.interface.cms.wordpress - # revision: "1.0.0" - - outputTypeInstanceRelations: {} - - action: - runnerInterface: argo.run - args: - workflow: - steps: - - name: mysql-install - - name: mysql-create-db - - name: wp-install diff --git a/pkg/sdk/manifest/types.go b/pkg/sdk/manifest/types.go new file mode 100644 index 000000000..61e6eba6f --- /dev/null +++ b/pkg/sdk/manifest/types.go @@ -0,0 +1,36 @@ +package manifest + +import ( + "context" + + "capact.io/capact/pkg/sdk/apis/0.0.1/types" +) + +// FileSystemValidator is an interface, with the Do method. +// Do validates the manifest in filepath and return a ValidationResult. +// If other, not manifest related errors occur, it will return an error. +type FileSystemValidator interface { + Do(ctx context.Context, filepath string) (ValidationResult, error) +} + +// ValidationResult hold the result of the manifest validation. +type ValidationResult struct { + Errors []error +} + +// Valid returns true, if the manifest contains no errors. +func (r *ValidationResult) Valid() bool { + return len(r.Errors) == 0 +} + +func newValidationResult(errs ...error) ValidationResult { + return ValidationResult{ + Errors: errs, + } +} + +// JSONValidator is an interface of validator which takes JSON bytes as input. +type JSONValidator interface { + Do(ctx context.Context, metadata types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) + Name() string +} diff --git a/pkg/sdk/manifest/validation.go b/pkg/sdk/manifest/validation.go deleted file mode 100644 index 564ea3b9e..000000000 --- a/pkg/sdk/manifest/validation.go +++ /dev/null @@ -1,189 +0,0 @@ -package manifest - -import ( - "fmt" - "io/ioutil" - "net/http" - "os" - "path/filepath" - "sort" - - "github.com/iancoleman/strcase" - "github.com/pkg/errors" - "github.com/xeipuuv/gojsonschema" - "sigs.k8s.io/yaml" -) - -// Validator is a interface, with the ValidateFile method. -// ValidateFile validates the manifest in filepath and return a ValidationResult. -// If other, not manifest related errors occur, it will return an error. -type Validator interface { - ValidateFile(filepath string) (ValidationResult, error) -} - -// ValidationResult hold the result of the manifest validation. -type ValidationResult struct { - Errors []error -} - -// Valid returns true, if the manifest contains no errors. -func (r *ValidationResult) Valid() bool { - return len(r.Errors) == 0 -} - -func newValidationResult(errors ...error) ValidationResult { - return ValidationResult{ - Errors: errors, - } -} - -// FilesystemManifestValidator validates manifests using a OCF specification, which is read from a filesystem. -type FilesystemManifestValidator struct { - schemaRootPath string - cachedSchemas map[ocfVersion]*loadedOCFSchema - fs http.FileSystem -} - -type ocfVersion string - -type kind string - -type loadedOCFSchema struct { - common *gojsonschema.SchemaLoader - kind map[kind]*gojsonschema.Schema -} - -// NewFilesystemValidator returns a new FilesystemManifestValidator. -func NewFilesystemValidator(fs http.FileSystem, schemaRootPath string) Validator { - return &FilesystemManifestValidator{ - schemaRootPath: schemaRootPath, - fs: fs, - cachedSchemas: map[ocfVersion]*loadedOCFSchema{}, - } -} - -// ValidateFile validates a manifest. -func (v *FilesystemManifestValidator) ValidateFile(path string) (ValidationResult, error) { - data, err := ioutil.ReadFile(filepath.Clean(path)) - if err != nil { - return newValidationResult(), err - } - - return v.validateYamlBytes(data) -} - -func (v *FilesystemManifestValidator) validateYamlBytes(yamlBytes []byte) (ValidationResult, error) { - metadata, err := GetMetadata(yamlBytes) - if err != nil { - return newValidationResult(errors.Wrap(err, "failed to read manifest metadata")), err - } - - schema, err := v.getManifestSchema(metadata) - if err != nil { - return newValidationResult(), errors.Wrap(err, "failed to get JSON schema") - } - - jsonBytes, err := yaml.YAMLToJSON(yamlBytes) - if err != nil { - return newValidationResult(errors.Wrap(err, "cannot convert YAML manifest to JSON")), err - } - - manifestLoader := gojsonschema.NewBytesLoader(jsonBytes) - - jsonschemaResult, err := schema.Validate(manifestLoader) - if err != nil { - return newValidationResult(errors.Wrap(err, "error occurred during JSON schema validation")), err - } - - result := newValidationResult() - - for _, err := range jsonschemaResult.Errors() { - result.Errors = append(result.Errors, fmt.Errorf("%v", err.String())) - } - - return result, err -} - -func (v *FilesystemManifestValidator) getManifestSchema(metadata Metadata) (*gojsonschema.Schema, error) { - var ok bool - var cachedSchema *loadedOCFSchema - - if cachedSchema, ok = v.cachedSchemas[metadata.OCFVersion]; !ok { - cachedSchema = &loadedOCFSchema{ - common: nil, - kind: map[kind]*gojsonschema.Schema{}, - } - v.cachedSchemas[metadata.OCFVersion] = cachedSchema - } - - if schema, ok := cachedSchema.kind[metadata.Kind]; ok { - return schema, nil - } - - rootLoader := v.getRootSchemaJSONLoader(metadata) - - if cachedSchema.common == nil { - sl, err := v.getCommonSchemaLoader(metadata.OCFVersion) - if err != nil { - return nil, errors.Wrap(err, "failed to get common schema loader") - } - cachedSchema.common = sl - } - - schema, err := cachedSchema.common.Compile(rootLoader) - if err != nil { - return nil, errors.Wrapf(err, "failed to compile schema for %s/%s", metadata.OCFVersion, metadata.Kind) - } - - cachedSchema.kind[metadata.Kind] = schema - - return schema, nil -} - -func (v *FilesystemManifestValidator) getRootSchemaJSONLoader(metadata Metadata) gojsonschema.JSONLoader { - filename := strcase.ToKebab(string(metadata.Kind)) - path := fmt.Sprintf("file://%s/%s/schema/%s.json", v.schemaRootPath, metadata.OCFVersion, filename) - return gojsonschema.NewReferenceLoaderFileSystem(path, v.fs) -} - -func (v *FilesystemManifestValidator) getCommonSchemaLoader(ocfVersion ocfVersion) (*gojsonschema.SchemaLoader, error) { - commonDir := fmt.Sprintf("%s/%s/schema/common", v.schemaRootPath, ocfVersion) - - sl := gojsonschema.NewSchemaLoader() - - files, err := v.ReadDir(commonDir) - if err != nil { - return nil, errors.Wrap(err, "failed to list common schemas directory") - } - - for _, file := range files { - if file.IsDir() { - continue - } - - path := fmt.Sprintf("file://%s/%s", commonDir, file.Name()) - if err := sl.AddSchemas(gojsonschema.NewReferenceLoaderFileSystem(path, v.fs)); err != nil { - return nil, errors.Wrapf(err, "cannot load common schema %s", path) - } - } - - return sl, nil -} - -// ReadDir reads the directory named by dirname and returns -// a list of directory entries sorted by filename. -func (v *FilesystemManifestValidator) ReadDir(dirname string) ([]os.FileInfo, error) { - f, err := v.fs.Open(dirname) - if err != nil { - return nil, err - } - list, err := f.Readdir(-1) - if err != nil { - return nil, err - } - if err := f.Close(); err != nil { - return nil, err - } - sort.Slice(list, func(i, j int) bool { return list[i].Name() < list[j].Name() }) - return list, nil -} diff --git a/pkg/sdk/manifest/validation_test.go b/pkg/sdk/manifest/validation_test.go deleted file mode 100644 index fbfae56be..000000000 --- a/pkg/sdk/manifest/validation_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package manifest_test - -import ( - "testing" - - "capact.io/capact/internal/cli/schema" - "capact.io/capact/pkg/sdk/manifest" - - "github.com/stretchr/testify/require" -) - -func TestFilesystemValidator_ValidateFile(t *testing.T) { - validator := manifest.NewFilesystemValidator(&schema.LocalFileSystem{}, "../../../ocf-spec") - - tests := map[string]struct { - manifestPath string - result bool - }{ - "Implementation should be invalid": { - manifestPath: "testdata/wrong_implementation.yaml", - result: false, - }, - "Implementation should be valid": { - manifestPath: "testdata/correct_implementation.yaml", - result: true, - }, - } - - for tn, tc := range tests { - t.Run(tn, func(t *testing.T) { - // given - - // when - result, err := validator.ValidateFile(tc.manifestPath) - - // then - require.Nil(t, err, "failed to read file: %v", err) - require.Equal(t, tc.result, result.Valid(), result.Errors) - }) - } -} From eb5df6d05aab8b73bf39d7aa5ca122b3c0b78e5c Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 11 Aug 2021 12:15:34 +0200 Subject: [PATCH 2/4] Improve code after review --- cmd/cli/cmd/manifest/validate.go | 16 ++--- cmd/cli/docs/capact_manifest_validate.md | 20 +++--- cmd/cli/main.go | 24 ++++++- internal/cli/validate/validate.go | 71 ++++++++----------- internal/cli/validate/validate_test.go | 10 ++- pkg/sdk/manifest/fsvalidator.go | 4 +- pkg/sdk/manifest/fsvalidator_test.go | 2 +- pkg/sdk/manifest/json_ocfschema.go | 9 ++- .../manifest/json_remote_implementation.go | 20 ++---- pkg/sdk/manifest/json_type.go | 2 +- 10 files changed, 91 insertions(+), 87 deletions(-) diff --git a/cmd/cli/cmd/manifest/validate.go b/cmd/cli/cmd/manifest/validate.go index 3beb72642..62ae11bf2 100644 --- a/cmd/cli/cmd/manifest/validate.go +++ b/cmd/cli/cmd/manifest/validate.go @@ -22,15 +22,15 @@ func NewValidate() *cobra.Command { Example: heredoc.WithCLIName(` # Validate interface-group.yaml file with OCF specification in default location manifest validate ocf-spec/0.0.1/examples/interface-group.yaml - - # Validate multiple files inside test_manifests directory - manifest validate pkg/cli/test_manifests/*.yaml + + # Validate multiple files inside test_manifests directory with additional server-side checks + manifest validate --server-side pkg/cli/test_manifests/*.yaml + + # Validate all Hub manifests with additional server-side checks + manifest validate --server-side ./manifests/**/*.yaml # Validate interface-group.yaml file with custom OCF specification location - manifest validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml - - # Validate all Hub manifests - manifest validate ./manifests/**/*.yaml`, cli.Name), + manifest validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml`, cli.Name), Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { validation, err := validate.New(os.Stdout, opts) @@ -45,7 +45,7 @@ func NewValidate() *cobra.Command { flags := cmd.Flags() flags.StringVarP(&opts.SchemaLocation, "schemas", "s", "", "Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used.") flags.BoolVarP(&opts.Verbose, "verbose", "v", false, "Prints more verbose output.") - flags.BoolVarP(&opts.ServerSide, "enable-remote-checks", "r", false, "Executes additional manifests checks against Capact Hub.") + flags.BoolVar(&opts.ServerSide, "server-side", false, "Executes additional manifests checks against Capact Hub.") flags.IntVar(&opts.MaxConcurrency, "concurrency", defaultMaxConcurrency, "Maximum number of concurrent workers.") return cmd diff --git a/cmd/cli/docs/capact_manifest_validate.md b/cmd/cli/docs/capact_manifest_validate.md index b67c39d3e..620c6a304 100644 --- a/cmd/cli/docs/capact_manifest_validate.md +++ b/cmd/cli/docs/capact_manifest_validate.md @@ -16,24 +16,24 @@ capact manifest validate [flags] # Validate interface-group.yaml file with OCF specification in default location capact manifest validate ocf-spec/0.0.1/examples/interface-group.yaml -# Validate multiple files inside test_manifests directory -capact manifest validate pkg/cli/test_manifests/*.yaml +# Validate multiple files inside test_manifests directory with additional server-side checks +capact manifest validate --server-side pkg/cli/test_manifests/*.yaml + +# Validate all Hub manifests with additional server-side checks +capact manifest validate --server-side ./manifests/**/*.yaml # Validate interface-group.yaml file with custom OCF specification location capact manifest validate -s my/ocf/spec/directory ocf-spec/0.0.1/examples/interface-group.yaml - -# Validate all Hub manifests -capact manifest validate ./manifests/**/*.yaml ``` ### Options ``` - --concurrency int Maximum number of concurrent workers. (default 5) - -r, --enable-remote-checks Executes additional manifests checks against Capact Hub. - -h, --help help for validate - -s, --schemas string Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used. - -v, --verbose Prints more verbose output. + --concurrency int Maximum number of concurrent workers. (default 5) + -h, --help help for validate + -s, --schemas string Path to the local directory with OCF JSONSchemas. If not provided, built-in JSONSchemas are used. + --server-side Executes additional manifests checks against Capact Hub. + -v, --verbose Prints more verbose output. ``` ### Options inherited from parent commands diff --git a/cmd/cli/main.go b/cmd/cli/main.go index ca54aa11e..aa68a2bb8 100644 --- a/cmd/cli/main.go +++ b/cmd/cli/main.go @@ -1,15 +1,37 @@ package main import ( + "context" "os" + "os/signal" + "syscall" "capact.io/capact/cmd/cli/cmd" ) func main() { rootCmd := cmd.NewRoot() + ctx, cancel := cancelableContext() + defer cancel() - if err := rootCmd.Execute(); err != nil { + if err := rootCmd.ExecuteContext(ctx); err != nil { os.Exit(1) } } + +// cancelableContext returns context that is canceled when stop signal is received +func cancelableContext() (context.Context, context.CancelFunc) { + ctx, cancel := context.WithCancel(context.Background()) + sigCh := make(chan os.Signal, 1) + signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) + + go func() { + select { + case <-ctx.Done(): + case <-sigCh: + cancel() + } + }() + + return ctx, cancel +} diff --git a/internal/cli/validate/validate.go b/internal/cli/validate/validate.go index e9de06850..13886957f 100644 --- a/internal/cli/validate/validate.go +++ b/internal/cli/validate/validate.go @@ -4,11 +4,8 @@ import ( "context" "fmt" "io" - "os" - "os/signal" "strings" "sync" - "syscall" "github.com/fatih/color" @@ -27,6 +24,15 @@ type Options struct { MaxConcurrency int } +// Validate validates the Options struct fields. +func (o *Options) Validate() error { + if o.MaxConcurrency < 1 { + return errors.New("concurrency parameter cannot be less than 1") + } + + return nil +} + // ValidationResult defines a validation error. type ValidationResult struct { Path string @@ -52,7 +58,7 @@ func (r *ValidationResult) Error() string { return fmt.Sprintf("%q:\n * %s\n", r.Path, strings.Join(errMsgs, "\n * ")) } -// Validation defines OCF manifest validation operation. +// Validation provides functionality to validate OCF manifests. type Validation struct { hubCli client.Hub writer io.Writer @@ -63,6 +69,10 @@ type Validation struct { // New creates new Validation. func New(writer io.Writer, opts Options) (*Validation, error) { + if err := opts.Validate(); err != nil { + return nil, err + } + server := config.GetDefaultContext() fs, ocfSchemaRootPath := schema.NewProvider(opts.SchemaLocation).FileSystem() @@ -81,14 +91,10 @@ func New(writer io.Writer, opts Options) (*Validation, error) { validatorOpts = append(validatorOpts, manifest.WithRemoteChecks(hubCli)) } - if opts.MaxConcurrency < 1 { - return nil, errors.New("concurrency parameter cannot be less than 1") - } - return &Validation{ // TODO: To improve: Share a single validator for all workers. - // Current implementation makes OCF JSON schemas caching separated per worker. - // That enforces thread-safe JSON validator implementations. OCF Schema validator is not thread safe. + // Current implementation makes OCF JSON schemas caching separated per validationWorker. + // That enforces thread-safe JSON validator implementations. OCF Schema validator is not thread-safe. validatorFn: func() manifest.FileSystemValidator { return manifest.NewDefaultFilesystemValidator(fs, ocfSchemaRootPath, validatorOpts...) }, @@ -108,17 +114,14 @@ func (v *Validation) Run(ctx context.Context, filePaths []string) error { v.printIntroMessage(filePaths, workersCount) - ctxWithCancel, cancelCtxOnSignalFn := v.makeCancellableContext(ctx) - go cancelCtxOnSignalFn() - jobsCh := make(chan string, len(filePaths)) resultsCh := make(chan ValidationResult, len(filePaths)) var wg sync.WaitGroup for i := 0; i < workersCount; i++ { wg.Add(1) - wrker := newWorker(&wg, v.validatorFn()) - go wrker.Do(ctxWithCancel, jobsCh, resultsCh) + worker := newValidationWorker(&wg, v.validatorFn()) + go worker.Do(ctx, jobsCh, resultsCh) } for _, filepath := range filePaths { @@ -141,21 +144,6 @@ func (v *Validation) Run(ctx context.Context, filePaths []string) error { return v.outputResultSummary(processedFilesCount, errsCount) } -func (v *Validation) makeCancellableContext(ctx context.Context) (context.Context, func()) { - sigCh := make(chan os.Signal, 1) - signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - - ctxWithCancel, cancelFn := context.WithCancel(ctx) - - return ctxWithCancel, func() { - select { - case <-sigCh: - cancelFn() - case <-ctx.Done(): - } - } -} - func (v *Validation) printIntroMessage(filePaths []string, workersCount int) { fileNoun := properNounFor("file", len(filePaths)) fmt.Fprintf(v.writer, "Validating %s in %d concurrent %s...\n", fileNoun, workersCount, properNounFor("job", workersCount)) @@ -176,11 +164,7 @@ func (v *Validation) outputResultSummary(processedFilesCount int, errsCount int) func (v *Validation) printPartialResult(res ValidationResult) { if !res.IsSuccess() { - var prefix string - if v.verbose { - prefix = fmt.Sprintf("%s ", color.RedString("✗")) - } - fmt.Fprintf(v.writer, "- %s%s\n", prefix, res.Error()) + fmt.Fprintf(v.writer, "- %s %s\n", color.RedString("✗"), res.Error()) return } @@ -191,17 +175,17 @@ func (v *Validation) printPartialResult(res ValidationResult) { fmt.Fprintf(v.writer, "- %s %q\n", color.GreenString("✓"), res.Path) } -type worker struct { +type validationWorker struct { wg *sync.WaitGroup validator manifest.FileSystemValidator } -func newWorker(wg *sync.WaitGroup, validator manifest.FileSystemValidator) *worker { - return &worker{wg: wg, validator: validator} +func newValidationWorker(wg *sync.WaitGroup, validator manifest.FileSystemValidator) *validationWorker { + return &validationWorker{wg: wg, validator: validator} } -// Do executes the worker logic. -func (w *worker) Do(ctx context.Context, jobCh <-chan string, resultCh chan<- ValidationResult) { +// Do executes the validationWorker logic. +func (w *validationWorker) Do(ctx context.Context, jobCh <-chan string, resultCh chan<- ValidationResult) { defer w.wg.Done() for { select { @@ -211,10 +195,13 @@ func (w *worker) Do(ctx context.Context, jobCh <-chan string, resultCh chan<- Va if !ok { return } + + var resultErrs []error res, err := w.validator.Do(ctx, filePath) - resultErrs := res.Errors if err != nil { - resultErrs = append(resultErrs, err) + resultErrs = append(resultErrs, errors.Wrap(err, "internal:")) + } else { + resultErrs = append(resultErrs, res.Errors...) } resultCh <- ValidationResult{ diff --git a/internal/cli/validate/validate_test.go b/internal/cli/validate/validate_test.go index 3360a70b3..9d9c440ce 100644 --- a/internal/cli/validate/validate_test.go +++ b/internal/cli/validate/validate_test.go @@ -12,7 +12,8 @@ import ( "github.com/stretchr/testify/require" ) -func TestValidation_Run(t *testing.T) { +func TestValidation_Run_SmokeTest(t *testing.T) { + // given validation, err := validate.New(ioutil.Discard, validate.Options{MaxConcurrency: 5}) require.NoError(t, err) @@ -32,11 +33,15 @@ func TestValidation_Run(t *testing.T) { require.True(t, len(filePaths) > 0) t.Log(filePaths) + // when err = validation.Run(context.Background(), filePaths) + + // then assert.NoError(t, err) } func TestValidation_NoFiles(t *testing.T) { + // given validation, err := validate.New(ioutil.Discard, validate.Options{MaxConcurrency: 5}) require.NoError(t, err) @@ -45,7 +50,10 @@ func TestValidation_NoFiles(t *testing.T) { require.True(t, len(filePaths) > 0) t.Log(filePaths) + // when err = validation.Run(context.Background(), filePaths) + + // then assert.Error(t, err) assert.EqualError(t, err, "detected 2 validation errors") } diff --git a/pkg/sdk/manifest/fsvalidator.go b/pkg/sdk/manifest/fsvalidator.go index 9f54de259..0745ac7a3 100644 --- a/pkg/sdk/manifest/fsvalidator.go +++ b/pkg/sdk/manifest/fsvalidator.go @@ -41,7 +41,7 @@ func NewDefaultFilesystemValidator(fs http.FileSystem, ocfSchemaRootPath string, func (v *FSValidator) Do(ctx context.Context, path string) (ValidationResult, error) { yamlBytes, err := ioutil.ReadFile(filepath.Clean(path)) if err != nil { - return newValidationResult(), err + return ValidationResult{}, err } metadata, err := UnmarshalManifestMetadata(yamlBytes) @@ -60,7 +60,7 @@ func (v *FSValidator) Do(ctx context.Context, path string) (ValidationResult, er for _, validator := range validators { res, err := validator.Do(ctx, metadata, jsonBytes) if err != nil { - validationErrs = append(validationErrs, errors.Wrapf(err, "while running validator %s", validator.Name())) + validationErrs = append(validationErrs, errors.Wrapf(err, "%s: internal", validator.Name())) } var prefixedResErrs []error diff --git a/pkg/sdk/manifest/fsvalidator_test.go b/pkg/sdk/manifest/fsvalidator_test.go index 88a57e8e8..7c0ff4b82 100644 --- a/pkg/sdk/manifest/fsvalidator_test.go +++ b/pkg/sdk/manifest/fsvalidator_test.go @@ -84,7 +84,7 @@ func TestFilesystemValidator_ValidateFile(t *testing.T) { "Invalid Type": { manifestPath: "testdata/invalid-type.yaml", expectedValidationErrorMsgs: []string{ - "TypeValidator: invalid character '}' looking for beginning of object key string", + "TypeValidator: spec.jsonSchema.value: invalid character '}' looking for beginning of object key string", "RemoteTypeValidator: manifest revision 'cap.core.sample.attr:0.1.0' doesn't exist in Hub", }, hubCli: fixHub(t, map[graphql.ManifestReference]bool{ diff --git a/pkg/sdk/manifest/json_ocfschema.go b/pkg/sdk/manifest/json_ocfschema.go index 400549063..cdf21fe9e 100644 --- a/pkg/sdk/manifest/json_ocfschema.go +++ b/pkg/sdk/manifest/json_ocfschema.go @@ -39,23 +39,22 @@ func NewOCFSchemaValidator(fs http.FileSystem, schemaRootPath string) *OCFSchema func (v *OCFSchemaValidator) Do(_ context.Context, metadata types.ManifestMetadata, jsonBytes []byte) (ValidationResult, error) { schema, err := v.getManifestSchema(metadata) if err != nil { - return newValidationResult(), errors.Wrap(err, "while getting manifest JSON schema") + return ValidationResult{}, errors.Wrap(err, "while getting manifest JSON schema") } manifestLoader := gojsonschema.NewBytesLoader(jsonBytes) jsonschemaResult, err := schema.Validate(manifestLoader) if err != nil { - return newValidationResult(err), nil + return ValidationResult{}, err } - result := newValidationResult() - + result := ValidationResult{} for _, err := range jsonschemaResult.Errors() { result.Errors = append(result.Errors, fmt.Errorf("%v", err.String())) } - return result, err + return result, nil } // Name returns validator name. diff --git a/pkg/sdk/manifest/json_remote_implementation.go b/pkg/sdk/manifest/json_remote_implementation.go index 244143bd6..f3495a2ba 100644 --- a/pkg/sdk/manifest/json_remote_implementation.go +++ b/pkg/sdk/manifest/json_remote_implementation.go @@ -45,18 +45,12 @@ func (v *RemoteImplementationValidator) Do(ctx context.Context, _ types.Manifest // Parameters additionalInputParams := entity.Spec.AdditionalInput.Parameters if additionalInputParams != nil && additionalInputParams.TypeRef != nil { - manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ - Path: additionalInputParams.TypeRef.Path, - Revision: additionalInputParams.TypeRef.Revision, - }) + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference(*additionalInputParams.TypeRef)) } // TypeInstances for _, ti := range entity.Spec.AdditionalInput.TypeInstances { - manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ - Path: ti.TypeRef.Path, - Revision: ti.TypeRef.Revision, - }) + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference(ti.TypeRef)) } } @@ -67,19 +61,13 @@ func (v *RemoteImplementationValidator) Do(ctx context.Context, _ types.Manifest continue } - manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ - Path: ti.TypeRef.Path, - Revision: ti.TypeRef.Revision, - }) + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference(*ti.TypeRef)) } } // Implements for _, implementsItem := range entity.Spec.Implements { - manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference{ - Path: implementsItem.Path, - Revision: implementsItem.Revision, - }) + manifestRefsToCheck = append(manifestRefsToCheck, hubpublicgraphql.ManifestReference(implementsItem)) } // Requires diff --git a/pkg/sdk/manifest/json_type.go b/pkg/sdk/manifest/json_type.go index ba8c14400..f2315cea8 100644 --- a/pkg/sdk/manifest/json_type.go +++ b/pkg/sdk/manifest/json_type.go @@ -32,7 +32,7 @@ func (v *TypeValidator) Do(_ context.Context, _ types.ManifestMetadata, jsonByte jsonSchemaValidationResult, err := gojsonschema.Validate(schemaLoader, manifestLoader) if err != nil { - return newValidationResult(err), nil + return newValidationResult(errors.Wrap(err, "spec.jsonSchema.value")), nil } result := ValidationResult{} From fa3516b2518b2469ac29a24772d129def218366d Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 11 Aug 2021 12:20:42 +0200 Subject: [PATCH 3/4] Move `sdk/manifest` validation-related logic to `sdk/validation/manifest` --- internal/cli/validate/validate.go | 3 ++- pkg/sdk/dbpopulator/loader.go | 2 +- pkg/sdk/manifest/metadata.go | 4 ++-- pkg/sdk/{ => validation}/manifest/fsvalidator.go | 4 +++- pkg/sdk/{ => validation}/manifest/fsvalidator_test.go | 4 ++-- pkg/sdk/{ => validation}/manifest/json_ocfschema.go | 0 pkg/sdk/{ => validation}/manifest/json_remote_helper.go | 0 .../{ => validation}/manifest/json_remote_implementation.go | 0 pkg/sdk/{ => validation}/manifest/json_remote_interface.go | 0 pkg/sdk/{ => validation}/manifest/json_remote_type.go | 0 pkg/sdk/{ => validation}/manifest/json_type.go | 0 pkg/sdk/{ => validation}/manifest/options.go | 0 .../manifest/testdata/invalid-implementation.yaml | 0 .../{ => validation}/manifest/testdata/invalid-interface.yaml | 0 .../{ => validation}/manifest/testdata/invalid-manifest.yaml | 0 pkg/sdk/{ => validation}/manifest/testdata/invalid-type.yaml | 0 .../manifest/testdata/invalid-type_json-schema.yaml | 0 .../manifest/testdata/valid-implementation.yaml | 0 .../{ => validation}/manifest/testdata/valid-interface.yaml | 0 pkg/sdk/{ => validation}/manifest/testdata/valid-type.yaml | 0 pkg/sdk/{ => validation}/manifest/types.go | 0 21 files changed, 10 insertions(+), 7 deletions(-) rename pkg/sdk/{ => validation}/manifest/fsvalidator.go (95%) rename pkg/sdk/{ => validation}/manifest/fsvalidator_test.go (99%) rename pkg/sdk/{ => validation}/manifest/json_ocfschema.go (100%) rename pkg/sdk/{ => validation}/manifest/json_remote_helper.go (100%) rename pkg/sdk/{ => validation}/manifest/json_remote_implementation.go (100%) rename pkg/sdk/{ => validation}/manifest/json_remote_interface.go (100%) rename pkg/sdk/{ => validation}/manifest/json_remote_type.go (100%) rename pkg/sdk/{ => validation}/manifest/json_type.go (100%) rename pkg/sdk/{ => validation}/manifest/options.go (100%) rename pkg/sdk/{ => validation}/manifest/testdata/invalid-implementation.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/invalid-interface.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/invalid-manifest.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/invalid-type.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/invalid-type_json-schema.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/valid-implementation.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/valid-interface.yaml (100%) rename pkg/sdk/{ => validation}/manifest/testdata/valid-type.yaml (100%) rename pkg/sdk/{ => validation}/manifest/types.go (100%) diff --git a/internal/cli/validate/validate.go b/internal/cli/validate/validate.go index 13886957f..d1dd6fdaf 100644 --- a/internal/cli/validate/validate.go +++ b/internal/cli/validate/validate.go @@ -7,12 +7,13 @@ import ( "strings" "sync" + "capact.io/capact/pkg/sdk/validation/manifest" + "github.com/fatih/color" "capact.io/capact/internal/cli/client" "capact.io/capact/internal/cli/config" "capact.io/capact/internal/cli/schema" - "capact.io/capact/pkg/sdk/manifest" "github.com/pkg/errors" ) diff --git a/pkg/sdk/dbpopulator/loader.go b/pkg/sdk/dbpopulator/loader.go index 1419859ab..d8a08bf29 100644 --- a/pkg/sdk/dbpopulator/loader.go +++ b/pkg/sdk/dbpopulator/loader.go @@ -33,7 +33,7 @@ func Group(paths []string) (map[string][]string, error) { if err != nil { return manifests, errors.Wrapf(err, "while reading file from path %s", path) } - metadata, err := manifest.UnmarshalManifestMetadata(content) + metadata, err := manifest.UnmarshalMetadata(content) if err != nil { return manifests, errors.Wrapf(err, "while unmarshaling manifest content from path %s", path) } diff --git a/pkg/sdk/manifest/metadata.go b/pkg/sdk/manifest/metadata.go index cbb4e40f9..431b39b61 100644 --- a/pkg/sdk/manifest/metadata.go +++ b/pkg/sdk/manifest/metadata.go @@ -6,8 +6,8 @@ import ( "sigs.k8s.io/yaml" ) -// UnmarshalManifestMetadata loads essential manifest metadata (kind and OCF version) from YAML bytes. -func UnmarshalManifestMetadata(yamlBytes []byte) (types.ManifestMetadata, error) { +// UnmarshalMetadata loads essential manifest metadata (kind and OCF version) from YAML bytes. +func UnmarshalMetadata(yamlBytes []byte) (types.ManifestMetadata, error) { mm := types.ManifestMetadata{} err := yaml.Unmarshal(yamlBytes, &mm) if err != nil { diff --git a/pkg/sdk/manifest/fsvalidator.go b/pkg/sdk/validation/manifest/fsvalidator.go similarity index 95% rename from pkg/sdk/manifest/fsvalidator.go rename to pkg/sdk/validation/manifest/fsvalidator.go index 0745ac7a3..18d4997d6 100644 --- a/pkg/sdk/manifest/fsvalidator.go +++ b/pkg/sdk/validation/manifest/fsvalidator.go @@ -6,6 +6,8 @@ import ( "net/http" "path/filepath" + "capact.io/capact/pkg/sdk/manifest" + "capact.io/capact/pkg/sdk/apis/0.0.1/types" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -44,7 +46,7 @@ func (v *FSValidator) Do(ctx context.Context, path string) (ValidationResult, er return ValidationResult{}, err } - metadata, err := UnmarshalManifestMetadata(yamlBytes) + metadata, err := manifest.UnmarshalMetadata(yamlBytes) if err != nil { return newValidationResult(errors.Wrap(err, "failed to read manifest metadata")), nil } diff --git a/pkg/sdk/manifest/fsvalidator_test.go b/pkg/sdk/validation/manifest/fsvalidator_test.go similarity index 99% rename from pkg/sdk/manifest/fsvalidator_test.go rename to pkg/sdk/validation/manifest/fsvalidator_test.go index 7c0ff4b82..3fc5ed29b 100644 --- a/pkg/sdk/manifest/fsvalidator_test.go +++ b/pkg/sdk/validation/manifest/fsvalidator_test.go @@ -4,14 +4,14 @@ import ( "context" "testing" + "capact.io/capact/pkg/sdk/validation/manifest" + graphql "capact.io/capact/pkg/hub/api/graphql/public" "github.com/pkg/errors" "capact.io/capact/internal/cli/schema" "github.com/stretchr/testify/assert" - "capact.io/capact/pkg/sdk/manifest" - "github.com/stretchr/testify/require" ) diff --git a/pkg/sdk/manifest/json_ocfschema.go b/pkg/sdk/validation/manifest/json_ocfschema.go similarity index 100% rename from pkg/sdk/manifest/json_ocfschema.go rename to pkg/sdk/validation/manifest/json_ocfschema.go diff --git a/pkg/sdk/manifest/json_remote_helper.go b/pkg/sdk/validation/manifest/json_remote_helper.go similarity index 100% rename from pkg/sdk/manifest/json_remote_helper.go rename to pkg/sdk/validation/manifest/json_remote_helper.go diff --git a/pkg/sdk/manifest/json_remote_implementation.go b/pkg/sdk/validation/manifest/json_remote_implementation.go similarity index 100% rename from pkg/sdk/manifest/json_remote_implementation.go rename to pkg/sdk/validation/manifest/json_remote_implementation.go diff --git a/pkg/sdk/manifest/json_remote_interface.go b/pkg/sdk/validation/manifest/json_remote_interface.go similarity index 100% rename from pkg/sdk/manifest/json_remote_interface.go rename to pkg/sdk/validation/manifest/json_remote_interface.go diff --git a/pkg/sdk/manifest/json_remote_type.go b/pkg/sdk/validation/manifest/json_remote_type.go similarity index 100% rename from pkg/sdk/manifest/json_remote_type.go rename to pkg/sdk/validation/manifest/json_remote_type.go diff --git a/pkg/sdk/manifest/json_type.go b/pkg/sdk/validation/manifest/json_type.go similarity index 100% rename from pkg/sdk/manifest/json_type.go rename to pkg/sdk/validation/manifest/json_type.go diff --git a/pkg/sdk/manifest/options.go b/pkg/sdk/validation/manifest/options.go similarity index 100% rename from pkg/sdk/manifest/options.go rename to pkg/sdk/validation/manifest/options.go diff --git a/pkg/sdk/manifest/testdata/invalid-implementation.yaml b/pkg/sdk/validation/manifest/testdata/invalid-implementation.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/invalid-implementation.yaml rename to pkg/sdk/validation/manifest/testdata/invalid-implementation.yaml diff --git a/pkg/sdk/manifest/testdata/invalid-interface.yaml b/pkg/sdk/validation/manifest/testdata/invalid-interface.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/invalid-interface.yaml rename to pkg/sdk/validation/manifest/testdata/invalid-interface.yaml diff --git a/pkg/sdk/manifest/testdata/invalid-manifest.yaml b/pkg/sdk/validation/manifest/testdata/invalid-manifest.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/invalid-manifest.yaml rename to pkg/sdk/validation/manifest/testdata/invalid-manifest.yaml diff --git a/pkg/sdk/manifest/testdata/invalid-type.yaml b/pkg/sdk/validation/manifest/testdata/invalid-type.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/invalid-type.yaml rename to pkg/sdk/validation/manifest/testdata/invalid-type.yaml diff --git a/pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml b/pkg/sdk/validation/manifest/testdata/invalid-type_json-schema.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/invalid-type_json-schema.yaml rename to pkg/sdk/validation/manifest/testdata/invalid-type_json-schema.yaml diff --git a/pkg/sdk/manifest/testdata/valid-implementation.yaml b/pkg/sdk/validation/manifest/testdata/valid-implementation.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/valid-implementation.yaml rename to pkg/sdk/validation/manifest/testdata/valid-implementation.yaml diff --git a/pkg/sdk/manifest/testdata/valid-interface.yaml b/pkg/sdk/validation/manifest/testdata/valid-interface.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/valid-interface.yaml rename to pkg/sdk/validation/manifest/testdata/valid-interface.yaml diff --git a/pkg/sdk/manifest/testdata/valid-type.yaml b/pkg/sdk/validation/manifest/testdata/valid-type.yaml similarity index 100% rename from pkg/sdk/manifest/testdata/valid-type.yaml rename to pkg/sdk/validation/manifest/testdata/valid-type.yaml diff --git a/pkg/sdk/manifest/types.go b/pkg/sdk/validation/manifest/types.go similarity index 100% rename from pkg/sdk/manifest/types.go rename to pkg/sdk/validation/manifest/types.go From f0b945d071830fb721387eb8e1329d0eb55e3d9f Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 11 Aug 2021 12:32:38 +0200 Subject: [PATCH 4/4] Fix tests after moving packages --- pkg/sdk/validation/manifest/fsvalidator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/sdk/validation/manifest/fsvalidator_test.go b/pkg/sdk/validation/manifest/fsvalidator_test.go index 3fc5ed29b..dddc2e9a6 100644 --- a/pkg/sdk/validation/manifest/fsvalidator_test.go +++ b/pkg/sdk/validation/manifest/fsvalidator_test.go @@ -94,7 +94,7 @@ func TestFilesystemValidator_ValidateFile(t *testing.T) { "Error from Hub": { manifestPath: "testdata/valid-interface.yaml", expectedValidationErrorMsgs: []string{ - "while running validator RemoteInterfaceValidator: while checking if manifest revisions exist: test error", + "RemoteInterfaceValidator: internal: while checking if manifest revisions exist: test error", }, hubCli: fixHub(t, map[graphql.ManifestReference]bool{ manifestRef("cap.type.productivity.mattermost.config"): true, @@ -122,7 +122,7 @@ func TestFilesystemValidator_ValidateFile(t *testing.T) { validator := manifest.NewDefaultFilesystemValidator( &schema.LocalFileSystem{}, - "../../../ocf-spec", + "../../../../ocf-spec", opts..., )