Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to filter errors in packages based on codes #1467

Merged
merged 24 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions cmd/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import (

"github.com/spf13/cobra"

"github.com/elastic/package-spec/v2/code/go/pkg/validator"

"github.com/elastic/elastic-package/internal/cobraext"
"github.com/elastic/elastic-package/internal/docs"
"github.com/elastic/elastic-package/internal/logger"
"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/validation"
)

const lintLongDescription = `Use this command to validate the contents of a package using the package specification (see: https://github.com/elastic/package-spec).
Expand Down Expand Up @@ -69,10 +69,12 @@ func validateSourceCommandAction(cmd *cobra.Command, args []string) error {
if err != nil {
return fmt.Errorf("locating package root failed: %w", err)
}
err = validator.ValidateFromPath(packageRootPath)
if err != nil {
return fmt.Errorf("linting package failed: %w", err)
errs, skipped := validation.ValidateAndFilterFromPath(packageRootPath)
if skipped != nil {
logger.Infof("skipped errors: %v", skipped)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log these at the warning level instead?

mrodm marked this conversation as resolved.
Show resolved Hide resolved
}
if errs != nil {
return fmt.Errorf("linting package failed: %w", errs)
}

return nil
}
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,5 @@ require (
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
sigs.k8s.io/yaml v1.3.0 // indirect
)

replace github.com/elastic/package-spec/v2 => github.com/mrodm/package-spec/v2 v2.0.0-20230926171924-1b0b6af1b3eb
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ github.com/elastic/go-ucfg v0.8.6 h1:stUeyh2goTgGX+/wb9gzKvTv0YB0231LTpKUgCKj4U0
github.com/elastic/go-ucfg v0.8.6/go.mod h1:4E8mPOLSUV9hQ7sgLEJ4bvt0KhMuDJa8joDT2QGAEKA=
github.com/elastic/gojsonschema v1.2.1 h1:cUMbgsz0wyEB4x7xf3zUEvUVDl6WCz2RKcQPul8OsQc=
github.com/elastic/gojsonschema v1.2.1/go.mod h1:biw5eBS2Z4T02wjATMRSfecfjCmwaDPvuaqf844gLrg=
github.com/elastic/package-spec/v2 v2.12.0 h1:mIiOhFjVrWCSGTGYVu5nnaTJq+9GnP2vVz7ttMgi28c=
github.com/elastic/package-spec/v2 v2.12.0/go.mod h1:hKQ0du9JO310TiTP1UZkjngMAFr+dYr1MdVJcTvvC4Q=
github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcejNsXKSkQ6lcIaNec2nyfOdlTBR2lU=
github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM=
github.com/emicklei/go-restful/v3 v3.10.1 h1:rc42Y5YTp7Am7CS630D7JmhRjq4UlEUuEKfrDac4bSQ=
Expand Down Expand Up @@ -307,6 +305,8 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0=
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4=
github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc=
github.com/mrodm/package-spec/v2 v2.0.0-20230926171924-1b0b6af1b3eb h1:ehFz+TxgJoSCVHMk2xa1Jjt6Q0BoG00Lhkey/MwdiqU=
github.com/mrodm/package-spec/v2 v2.0.0-20230926171924-1b0b6af1b3eb/go.mod h1:hKQ0du9JO310TiTP1UZkjngMAFr+dYr1MdVJcTvvC4Q=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
Expand Down
21 changes: 13 additions & 8 deletions internal/builder/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@ import (

"github.com/magefile/mage/sh"

"github.com/elastic/package-spec/v2/code/go/pkg/validator"

"github.com/elastic/elastic-package/internal/environment"
"github.com/elastic/elastic-package/internal/files"
"github.com/elastic/elastic-package/internal/logger"
"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/validation"
)

const builtPackagesFolder = "packages"
Expand Down Expand Up @@ -195,9 +194,12 @@ func BuildPackage(options BuildOptions) (string, error) {
}

logger.Debugf("Validating built package (path: %s)", destinationDir)
err = validator.ValidateFromPath(destinationDir)
if err != nil {
return "", fmt.Errorf("invalid content found in built package: %w", err)
errs, skipped := validation.ValidateAndFilterFromPath(destinationDir)
if skipped != nil {
logger.Infof("skipped errors: %v", skipped)
mrodm marked this conversation as resolved.
Show resolved Hide resolved
}
if errs != nil {
return "", fmt.Errorf("invalid content found in built package: %w", errs)
}
return destinationDir, nil
}
Expand All @@ -218,9 +220,12 @@ func buildZippedPackage(options BuildOptions, destinationDir string) (string, er
logger.Debug("Skip validation of the built .zip package")
} else {
logger.Debugf("Validating built .zip package (path: %s)", zippedPackagePath)
err = validator.ValidateFromZip(zippedPackagePath)
if err != nil {
return "", fmt.Errorf("invalid content found in built zip package: %w", err)
errs, skipped := validation.ValidateAndFilterFromZip(zippedPackagePath)
if skipped != nil {
logger.Infof("skipped errors: %v", skipped)
mrodm marked this conversation as resolved.
Show resolved Hide resolved
}
if errs != nil {
return "", fmt.Errorf("invalid content found in built zip package: %w", errs)
}
}

Expand Down
5 changes: 2 additions & 3 deletions internal/packages/archetype/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import (
"path/filepath"
"testing"

"github.com/elastic/package-spec/v2/code/go/pkg/validator"

"github.com/stretchr/testify/require"

"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/validation"
)

func TestPackage(t *testing.T) {
Expand Down Expand Up @@ -83,7 +82,7 @@ func createPackageDescriptorForTest(packageType, kibanaVersion string) PackageDe
}

func checkPackage(packageRoot string) error {
err := validator.ValidateFromPath(packageRoot)
err := validation.ValidateFromPath(packageRoot)
if err != nil {
return fmt.Errorf("linting package failed: %w", err)
}
Expand Down
12 changes: 7 additions & 5 deletions internal/packages/installer/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ import (

"github.com/Masterminds/semver/v3"

"github.com/elastic/package-spec/v2/code/go/pkg/validator"

"github.com/elastic/elastic-package/internal/builder"
"github.com/elastic/elastic-package/internal/kibana"
"github.com/elastic/elastic-package/internal/logger"
"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/validation"
)

var semver8_7_0 = semver.MustParse("8.7.0")
Expand Down Expand Up @@ -61,9 +60,12 @@ func NewForPackage(options Options) (Installer, error) {
}
if !options.SkipValidation {
logger.Debugf("Validating built .zip package (path: %s)", options.ZipPath)
err := validator.ValidateFromZip(options.ZipPath)
if err != nil {
return nil, fmt.Errorf("invalid content found in built zip package: %w", err)
errs, skipped := validation.ValidateAndFilterFromZip(options.ZipPath)
if skipped != nil {
logger.Infof("skipped errors: %v", skipped)
mrodm marked this conversation as resolved.
Show resolved Hide resolved
}
if errs != nil {
return nil, fmt.Errorf("invalid content found in built zip package: %w", errs)
}
}
logger.Debug("Skip validation of the built .zip package")
Expand Down
108 changes: 108 additions & 0 deletions internal/validation/validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

package validation

import (
"archive/zip"
"fmt"
"io/fs"
"os"

"github.com/elastic/package-spec/v2/code/go/pkg/specerrors"
"github.com/elastic/package-spec/v2/code/go/pkg/specerrors/processors"
"github.com/elastic/package-spec/v2/code/go/pkg/validator"

"github.com/elastic/elastic-package/internal/logger"
)

const validationConfigPath = "validation.yml"

func ValidateFromPath(rootPath string) error {
return validator.ValidateFromPath(rootPath)
}

func ValidateFromZip(packagePath string) error {
return validator.ValidateFromPath(packagePath)
mrodm marked this conversation as resolved.
Show resolved Hide resolved
}

func ValidateAndFilterFromPath(rootPath string) (error, error) {
allErrors := validator.ValidateFromPath(rootPath)
if allErrors == nil {
return nil, nil
}

fsys := os.DirFS(rootPath)
errors, skipped, err := filterErrors(allErrors, fsys, validationConfigPath)
if err != nil {
return err, nil
}
return errors, skipped
}

func ValidateAndFilterFromZip(packagePath string) (error, error) {
allErrors := validator.ValidateFromZip(packagePath)
if allErrors == nil {
return nil, nil
}

fsys, err := zip.OpenReader(packagePath)
if err != nil {
return fmt.Errorf("failed to open zip file (%s): %w", packagePath, err), nil
}
defer fsys.Close()

fsZip, err := fsFromPackageZip(fsys)
if err != nil {
return fmt.Errorf("failed to extract filesystem from zip file (%s): %w", packagePath, err), nil
}

errors, skipped, err := filterErrors(allErrors, fsZip, validationConfigPath)
if err != nil {
return err, nil
}
return errors, skipped
}

func fsFromPackageZip(fsys fs.FS) (fs.FS, error) {
dirs, err := fs.ReadDir(fsys, ".")
if err != nil {
return nil, fmt.Errorf("failed to read root directory in zip file fs: %w", err)
}
if len(dirs) != 1 {
return nil, fmt.Errorf("a single directory is expected in zip file, %d found", len(dirs))
}

subDir, err := fs.Sub(fsys, dirs[0].Name())
if err != nil {
return nil, err
}
return subDir, nil
}

func filterErrors(allErrors error, fsys fs.FS, configPath string) (error, error, error) {
errs, ok := allErrors.(specerrors.ValidationErrors)
if !ok {
return allErrors, nil, nil
}

_, err := fs.Stat(fsys, configPath)
if err != nil {
logger.Debugf("file not found: %s", configPath)
return allErrors, nil, nil
}

config, err := processors.LoadConfigFilter(fsys, configPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still see this "processors" name as a too generic name, specially when used externally, but lets continue with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just changed all the code in processors to be in the same specerrors.

if err != nil {
return nil, nil, fmt.Errorf("failed to read config filter: %w", err)
}

filter := processors.NewFilter(config)

filteredErrors, skippedErrors, err := filter.Run(errs)
if err != nil {
return nil, nil, fmt.Errorf("failed to filter errors: %w", err)
}
return filteredErrors, skippedErrors, nil
}
93 changes: 93 additions & 0 deletions test/packages/other/invalid_spec_filtered/LICENSE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
Elastic License 2.0

URL: https://www.elastic.co/licensing/elastic-license

## Acceptance

By using the software, you agree to all of the terms and conditions below.

## Copyright License

The licensor grants you a non-exclusive, royalty-free, worldwide,
non-sublicensable, non-transferable license to use, copy, distribute, make
available, and prepare derivative works of the software, in each case subject to
the limitations and conditions below.

## Limitations

You may not provide the software to third parties as a hosted or managed
service, where the service provides users with access to any substantial set of
the features or functionality of the software.

You may not move, change, disable, or circumvent the license key functionality
in the software, and you may not remove or obscure any functionality in the
software that is protected by the license key.

You may not alter, remove, or obscure any licensing, copyright, or other notices
of the licensor in the software. Any use of the licensor’s trademarks is subject
to applicable law.

## Patents

The licensor grants you a license, under any patent claims the licensor can
license, or becomes able to license, to make, have made, use, sell, offer for
sale, import and have imported the software, in each case subject to the
limitations and conditions in this license. This license does not cover any
patent claims that you cause to be infringed by modifications or additions to
the software. If you or your company make any written claim that the software
infringes or contributes to infringement of any patent, your patent license for
the software granted under these terms ends immediately. If your company makes
such a claim, your patent license ends immediately for work on behalf of your
company.

## Notices

You must ensure that anyone who gets a copy of any part of the software from you
also gets a copy of these terms.

If you modify the software, you must include in any modified copies of the
software prominent notices stating that you have modified the software.

## No Other Rights

These terms do not imply any licenses other than those expressly granted in
these terms.

## Termination

If you use the software in violation of these terms, such use is not licensed,
and your licenses will automatically terminate. If the licensor provides you
with a notice of your violation, and you cease all violation of this license no
later than 30 days after you receive that notice, your licenses will be
reinstated retroactively. However, if you violate these terms after such
reinstatement, any additional violation of these terms will cause your licenses
to terminate automatically and permanently.

## No Liability

*As far as the law allows, the software comes as is, without any warranty or
condition, and the licensor will not be liable to you for any damages arising
out of these terms or the use or nature of the software, under any kind of
legal claim.*

## Definitions

The **licensor** is the entity offering these terms, and the **software** is the
software the licensor makes available under these terms, including any portion
of it.

**you** refers to the individual or entity agreeing to these terms.

**your company** is any legal entity, sole proprietorship, or other kind of
organization that you work for, plus all organizations that have control over,
are under the control of, or are under common control with that
organization. **control** means ownership of substantially all the assets of an
entity, or the power to direct its management and policies by vote, contract, or
otherwise. Control can be direct or indirect.

**your licenses** are all the licenses granted to you for the software under
these terms.

**use** means anything you do with the software requiring one of your licenses.

**trademark** means trademarks, service marks, and similar rights.
6 changes: 6 additions & 0 deletions test/packages/other/invalid_spec_filtered/changelog.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# newer versions go on top
- version: "0.0.1"
changes:
- description: Initial draft of the package
type: enhancement
link: https://github.com/elastic/integrations/pull/1 # FIXME Replace with the real PR link
Loading