From a316d3d13e82ac5f8d2eae42a734f8a885cddb15 Mon Sep 17 00:00:00 2001 From: Utku Ozdemir Date: Thu, 22 Feb 2024 21:00:00 +0100 Subject: [PATCH] feat: add support for multiple licenses Allow specifying multiple policy documents of `type: commit` in `.conform.yaml`. Join these multiple policy declarations into a single license check policy. In the license check spec, introduce a new field, `root`, to allow specifying in which directory that license check should run. With this change, a single repository will be able to have different license rules for different paths. Signed-off-by: Utku Ozdemir --- internal/enforcer/enforcer.go | 81 +++++++++++++------ internal/policy/license/license.go | 71 +++++++++------- internal/policy/license/license_test.go | 72 ++++++++++++++--- .../policy/license/testdata/subdir1/data.txt | 3 + .../license/testdata/subdir1/subdir2/data.txt | 5 ++ 5 files changed, 169 insertions(+), 63 deletions(-) create mode 100644 internal/policy/license/testdata/subdir1/data.txt create mode 100644 internal/policy/license/testdata/subdir1/subdir2/data.txt diff --git a/internal/enforcer/enforcer.go b/internal/enforcer/enforcer.go index e19ec04a..3c606422 100644 --- a/internal/enforcer/enforcer.go +++ b/internal/enforcer/enforcer.go @@ -38,13 +38,6 @@ type PolicyDeclaration struct { Spec interface{} `yaml:"spec"` } -// policyMap defines the set of policies allowed within Conform. -var policyMap = map[string]policy.Policy{ - "commit": &commit.Commit{}, - "license": &license.License{}, - // "version": &version.Version{}, -} - // New loads the conform.yaml file and unmarshals it into a Conform struct. func New(r string) (*Conform, error) { c := &Conform{} @@ -84,8 +77,13 @@ func (c *Conform) Enforce(setters ...policy.Option) error { pass := true - for _, p := range c.Policies { - report, err := c.enforce(p, opts) + policiesWithTypes, err := c.convertDeclarations() + if err != nil { + return fmt.Errorf("failed to convert declarations: %w", err) + } + + for _, p := range policiesWithTypes { + report, err := p.policy.Compliance(opts) if err != nil { log.Fatal(err) } @@ -120,30 +118,61 @@ func (c *Conform) Enforce(setters ...policy.Option) error { return nil } -func (c *Conform) enforce(declaration *PolicyDeclaration, opts *policy.Options) (*policy.Report, error) { - if _, ok := policyMap[declaration.Type]; !ok { - return nil, errors.Errorf("Policy %q is not defined", declaration.Type) - } +type policyWithType struct { + policy policy.Policy + Type string +} + +func (c *Conform) convertDeclarations() ([]policyWithType, error) { + const typeLicense = "license" + + var ( + policies = make([]policyWithType, 0, len(c.Policies)) + licenses = make(license.Licenses, 0, len(c.Policies)) + ) - p := policyMap[declaration.Type] + for _, p := range c.Policies { + switch p.Type { + case typeLicense: + var lcs license.License + + if err := mapstructure.Decode(p.Spec, &lcs); err != nil { + return nil, fmt.Errorf("failed to convert license policy: %w", err) + } - // backwards compatibility, convert `gpg: bool` into `gpg: required: bool` - if declaration.Type == "commit" { - if spec, ok := declaration.Spec.(map[interface{}]interface{}); ok { - if gpg, ok := spec["gpg"]; ok { - if val, ok := gpg.(bool); ok { - spec["gpg"] = map[string]interface{}{ - "required": val, + licenses = append(licenses, lcs) + + case "commit": + // backwards compatibility, convert `gpg: bool` into `gpg: required: bool` + if spec, ok := p.Spec.(map[interface{}]interface{}); ok { + if gpg, ok := spec["gpg"]; ok { + if val, ok := gpg.(bool); ok { + spec["gpg"] = map[string]interface{}{ + "required": val, + } } } } + + var cmt commit.Commit + + if err := mapstructure.Decode(p.Spec, &cmt); err != nil { + return nil, fmt.Errorf("failed to convert commit policy: %w", err) + } + + policies = append(policies, policyWithType{ + Type: p.Type, + policy: &cmt, + }) + default: + return nil, fmt.Errorf("invalid policy type: %s", p.Type) } } - err := mapstructure.Decode(declaration.Spec, p) - if err != nil { - return nil, errors.Errorf("Internal error: %v", err) - } + policies = append(policies, policyWithType{ + Type: typeLicense, + policy: &licenses, + }) - return p.Compliance(opts) + return policies, nil } diff --git a/internal/policy/license/license.go b/internal/policy/license/license.go index fbf36dbb..b1003ec9 100644 --- a/internal/policy/license/license.go +++ b/internal/policy/license/license.go @@ -19,11 +19,14 @@ import ( "github.com/siderolabs/conform/internal/policy" ) -// License implements the policy.Policy interface and enforces source code -// license headers. +// Licenses implement the policy.Policy interface and enforces source code license headers. +type Licenses []License + +// License represents a single license policy. // //nolint:govet type License struct { + Root string `mapstructure:"root"` // SkipPaths applies gitignore-style patterns to file paths to skip completely // parts of the tree which shouldn't be scanned (e.g. .git/) SkipPaths []string `mapstructure:"skipPaths"` @@ -42,17 +45,17 @@ type License struct { } // Compliance implements the policy.Policy.Compliance function. -func (l *License) Compliance(_ *policy.Options) (*policy.Report, error) { +func (l *Licenses) Compliance(_ *policy.Options) (*policy.Report, error) { report := &policy.Report{} - report.AddCheck(l.ValidateLicenseHeader()) + report.AddCheck(l.ValidateLicenseHeaders()) return report, nil } // HeaderCheck enforces a license header on source code files. type HeaderCheck struct { - errors []error + licenseErrors []error } // Name returns the name of the check. @@ -62,8 +65,8 @@ func (l HeaderCheck) Name() string { // Message returns to check message. func (l HeaderCheck) Message() string { - if len(l.errors) != 0 { - return fmt.Sprintf("Found %d files without license header", len(l.errors)) + if len(l.licenseErrors) != 0 { + return fmt.Sprintf("Found %d files without license header", len(l.licenseErrors)) } return "All files have a valid license header" @@ -71,35 +74,49 @@ func (l HeaderCheck) Message() string { // Errors returns any violations of the check. func (l HeaderCheck) Errors() []error { - return l.errors + return l.licenseErrors +} + +// ValidateLicenseHeaders checks the header of a file and ensures it contains the provided value. +func (l Licenses) ValidateLicenseHeaders() policy.Check { //nolint:ireturn + check := HeaderCheck{} + + for _, license := range l { + if license.Root == "" { + license.Root = "." + } + + check.licenseErrors = append(check.licenseErrors, validateLicenseHeader(license)...) + } + + return check } -// ValidateLicenseHeader checks the header of a file and ensures it contains the -// provided value. -func (l License) ValidateLicenseHeader() policy.Check { //nolint:gocognit,ireturn +//nolint:gocognit +func validateLicenseHeader(license License) []error { + var errs []error + var buf bytes.Buffer - for _, pattern := range l.SkipPaths { + for _, pattern := range license.SkipPaths { fmt.Fprintf(&buf, "%s\n", pattern) } - check := HeaderCheck{} - - patternmatcher := gitignore.New(&buf, ".", func(e gitignore.Error) bool { - check.errors = append(check.errors, e.Underlying()) + patternmatcher := gitignore.New(&buf, license.Root, func(e gitignore.Error) bool { + errs = append(errs, e.Underlying()) return true }) - if l.Header == "" { - check.errors = append(check.errors, errors.New("Header is not defined")) + if license.Header == "" { + errs = append(errs, errors.New("Header is not defined")) - return check + return errs } - value := []byte(strings.TrimSpace(l.Header)) + value := []byte(strings.TrimSpace(license.Header)) - err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(license.Root, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -117,23 +134,23 @@ func (l License) ValidateLicenseHeader() policy.Check { //nolint:gocognit,iretur if info.Mode().IsRegular() { // Skip excluded suffixes. - for _, suffix := range l.ExcludeSuffixes { + for _, suffix := range license.ExcludeSuffixes { if strings.HasSuffix(info.Name(), suffix) { return nil } } // Check files matching the included suffixes. - for _, suffix := range l.IncludeSuffixes { + for _, suffix := range license.IncludeSuffixes { if strings.HasSuffix(info.Name(), suffix) { - if l.AllowPrecedingComments { + if license.AllowPrecedingComments { err = validateFileWithPrecedingComments(path, value) } else { err = validateFile(path, value) } if err != nil { - check.errors = append(check.errors, err) + errs = append(errs, err) } } } @@ -142,10 +159,10 @@ func (l License) ValidateLicenseHeader() policy.Check { //nolint:gocognit,iretur return nil }) if err != nil { - check.errors = append(check.errors, errors.Errorf("Failed to walk directory: %v", err)) + errs = append(errs, errors.Errorf("Failed to walk directory: %v", err)) } - return check + return errs } func validateFile(path string, value []byte) error { diff --git a/internal/policy/license/license_test.go b/internal/policy/license/license_test.go index 7a54e4ef..3fa05311 100644 --- a/internal/policy/license/license_test.go +++ b/internal/policy/license/license_test.go @@ -21,23 +21,75 @@ func TestLicense(t *testing.T) { // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/.` + const otherHeader = "// some-other-header" + t.Run("Default", func(t *testing.T) { - l := license.License{ - IncludeSuffixes: []string{".txt"}, - AllowPrecedingComments: false, - Header: header, + l := license.Licenses{ + { + SkipPaths: []string{"subdir1/"}, + IncludeSuffixes: []string{".txt"}, + AllowPrecedingComments: false, + Header: header, + }, } - check := l.ValidateLicenseHeader() + check := l.ValidateLicenseHeaders() assert.Equal(t, "Found 1 files without license header", check.Message()) }) t.Run("AllowPrecedingComments", func(t *testing.T) { - l := license.License{ - IncludeSuffixes: []string{".txt"}, - AllowPrecedingComments: true, - Header: header, + l := license.Licenses{ + { + SkipPaths: []string{"subdir1/"}, + IncludeSuffixes: []string{".txt"}, + AllowPrecedingComments: true, + Header: header, + }, + } + check := l.ValidateLicenseHeaders() + assert.Equal(t, "All files have a valid license header", check.Message()) + }) + + // File "testdata/subdir1/subdir2/data.txt" is valid for the root license, but "testdata/subdir1/" is skipped. + // It is invalid for the additional license, but that license skips "subdir2/" relative to itself. + // The check should pass. + t.Run("AdditionalValid", func(t *testing.T) { + l := license.Licenses{ + { + IncludeSuffixes: []string{".txt"}, + SkipPaths: []string{"testdata/subdir1/"}, + AllowPrecedingComments: true, + Header: header, + }, + { + Root: "testdata/subdir1/", + SkipPaths: []string{"subdir2/"}, + IncludeSuffixes: []string{".txt"}, + Header: otherHeader, + }, } - check := l.ValidateLicenseHeader() + check := l.ValidateLicenseHeaders() assert.Equal(t, "All files have a valid license header", check.Message()) }) + + // File "testdata/subdir1/subdir2/data.txt" is valid for the root license, but "testdata/subdir1/" is skipped. + // However, it is invalid for the additional license. + // The check should fail. + t.Run("AdditionalInvalid", func(t *testing.T) { + l := license.Licenses{ + { + IncludeSuffixes: []string{".txt"}, + SkipPaths: []string{"testdata/subdir1/"}, + AllowPrecedingComments: true, + Header: header, + }, + + { + Root: "testdata/subdir1/", + IncludeSuffixes: []string{".txt"}, + Header: otherHeader, + }, + } + check := l.ValidateLicenseHeaders() + assert.Equal(t, "Found 1 files without license header", check.Message()) + }) } diff --git a/internal/policy/license/testdata/subdir1/data.txt b/internal/policy/license/testdata/subdir1/data.txt new file mode 100644 index 00000000..fbf01726 --- /dev/null +++ b/internal/policy/license/testdata/subdir1/data.txt @@ -0,0 +1,3 @@ +// some-other-header + +content diff --git a/internal/policy/license/testdata/subdir1/subdir2/data.txt b/internal/policy/license/testdata/subdir1/subdir2/data.txt new file mode 100644 index 00000000..2d11fa28 --- /dev/null +++ b/internal/policy/license/testdata/subdir1/subdir2/data.txt @@ -0,0 +1,5 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +content