From 79c252c57ce35401a4506bca0f8ba3aa32986b75 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Wed, 22 Jan 2020 19:39:30 +1100 Subject: [PATCH] IgnorePathIssues config option to treat some issues as non-fatal --- README.md | 33 ++++ cmd/gon/item.go | 69 ++++--- cmd/gon/item_test.go | 174 ++++++++++++++++++ internal/config/config.go | 7 + internal/config/testdata/basic.hcl.golden | 3 +- internal/config/testdata/entitle.hcl.golden | 3 +- .../config/testdata/env_appleid.hcl.golden | 3 +- internal/config/testdata/notarize.hcl.golden | 3 +- .../testdata/notarize_multiple.hcl.golden | 3 +- 9 files changed, 270 insertions(+), 28 deletions(-) create mode 100644 cmd/gon/item_test.go diff --git a/README.md b/README.md index 4f40fca..fec2085 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ gon helps you automate the process of notarization. - [Prerequisite: Acquiring a Developer ID Certificate](#prerequisite-acquiring-a-developer-id-certificate) - [Configuration File](#configuration-file) - [Notarization-Only Configuration](#notarization-only-configuration) + - [Handling Notarization Issues](#handling-notarization-issues) - [Processing Time](#processing-time) - [Using within Automation](#using-within-automation) - [Machine-Readable Output](#machine-readable-output) @@ -310,6 +311,38 @@ apple_id { Note you may specify multiple `notarize` blocks to notarize multipel files concurrently. +### Handling Notarization Issues + +After notarization, Apple sends a log containing what actions it performed. The +log may contain a list of "issues", which may or may not be fatal. In some +circumstances, a successful notarization can still produce a bundle that does +not pass Gatekeeper and will fail to install. By default, `gon` will treat any +issues reported by Apple as fatal due to the likelihood of an non-installable +bundle. + +If you know that some issues reported by Apple that are non-fatal, such as an +executable included in your bundle that is not essential for the install, you +can treat these issues as non-fatal and `gon` will not fail. The +`ignorable_path_issues` configuration option allows you to supply a regular +expression that matches the `path` attached to the issues. The `path` reported +by Apple normally has the format ` ` so your regular +expression should take this into account. Multiple paths should be composed +into a single regular expression by broad matches or `|`. Supplying `".*"` will +treat all issues as non-fatal (caveat emptor!). + +```hcl +notarize { + path = "/path/to/terraform.pkg" +... +} + +apple_id { + ... +} + +ignorable_path_issues = "^terraform.pkg Contents/Payload/usr/lib/inconsequential/boop$" +``` + ### Processing Time The notarization process requires submitting your package(s) to Apple diff --git a/cmd/gon/item.go b/cmd/gon/item.go index fda3da0..dcea81b 100644 --- a/cmd/gon/item.go +++ b/cmd/gon/item.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "sync" "github.com/fatih/color" @@ -128,29 +129,7 @@ func (i *item) notarize(ctx context.Context, opts *processOptions) error { ) } - // If we have any issues then it is a failed notarization. Notarization - // can "succeed" with warnings, but when you attempt to use/open a file - // Gatekeeper rejects it. So we currently reject any and all issues. - if len(log.Issues) > 0 { - var err error - - lock.Lock() - color.New(color.FgRed).Fprintf(os.Stdout, - " %s%d issues during notarization:\n", - opts.Prefix, len(log.Issues)) - for idx, issue := range log.Issues { - color.New(color.FgRed).Fprintf(os.Stdout, - " %sIssue #%d (%s) for path %q: %s\n", - opts.Prefix, idx+1, issue.Severity, issue.Path, issue.Message) - - // Append the error so we can return it - err = multierror.Append(err, fmt.Errorf( - "%s for path %q: %s", - issue.Severity, issue.Path, issue.Message, - )) - } - lock.Unlock() - + if err := handleIssues(opts, log); err != nil { return err } } @@ -198,6 +177,50 @@ func (i *item) notarize(ctx context.Context, opts *processOptions) error { return nil } +func handleIssues(opts *processOptions, log *notarize.Log) error { + // If we have any issues then it is a failed notarization. Notarization + // can "succeed" with warnings, but when you attempt to use/open a file + // Gatekeeper rejects it. So we currently reject any and all issues. + + lock := opts.OutputLock + if len(log.Issues) > 0 { + var err multierror.Error + + lock.Lock() + color.New(color.FgRed).Fprintf(os.Stdout, + " %s%d issues during notarization:\n", + opts.Prefix, len(log.Issues)) + for idx, issue := range log.Issues { + color.New(color.FgRed).Fprintf(os.Stdout, + " %sIssue #%d (%s) for path %q: %s\n", + opts.Prefix, idx+1, issue.Severity, issue.Path, issue.Message) + + if opts.Config.IgnorePathIssues != nil { + matched, err := regexp.MatchString(*opts.Config.IgnorePathIssues, issue.Path) + if err != nil { + return err // poorly formatted regex? + } + if matched { + continue + } + } + // Append the error so we can return it + err = *multierror.Append(&err, fmt.Errorf( + "%s for path %q: %s", + issue.Severity, issue.Path, issue.Message, + )) + } + lock.Unlock() + + if len(err.WrappedErrors()) == 0 { + return nil + } + return &err + } + + return nil +} + // String implements Stringer func (i *item) String() string { result := i.Path diff --git a/cmd/gon/item_test.go b/cmd/gon/item_test.go new file mode 100644 index 0000000..f8b8424 --- /dev/null +++ b/cmd/gon/item_test.go @@ -0,0 +1,174 @@ +package main + +import ( + "sync" + "testing" + + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-multierror" + "github.com/mitchellh/gon/internal/config" + "github.com/mitchellh/gon/notarize" + "github.com/stretchr/testify/require" +) + +func TestHandleIssues_NoIssues(t *testing.T) { + // single issue + verifyHandleIssues(t, config.Config{}) +} + +func TestHandleIssues_DefaultIssues(t *testing.T) { + // single issue + verifyHandleIssues(t, config.Config{}, logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + expectedError: "borked for path \"pkg foo/bar/baz\": nope nope nope", + }) + + // two issues + verifyHandleIssues(t, + config.Config{}, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + expectedError: "borked for path \"pkg foo/bar/baz\": nope nope nope", + }, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "verysevere", + Path: "pkg foo/bar/bang", + Message: "no, just no", + }, + expectedError: "verysevere for path \"pkg foo/bar/bang\": no, just no", + }, + ) +} + +func TestHandleIssues_IgnoreAllIssues(t *testing.T) { + regex := "^pkg foo/bar/.*$" + + // single issue + verifyHandleIssues(t, + config.Config{IgnorePathIssues: ®ex}, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + }, + ) + + // two issues + verifyHandleIssues(t, + config.Config{IgnorePathIssues: ®ex}, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + }, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "verysevere", + Path: "pkg foo/bar/bang", + Message: "no, just no", + }, + }, + ) +} + +func TestHandleIssues_IgnoreSomeIssues(t *testing.T) { + regex := "^pkg foo/bar/baz$" + + // single issue + verifyHandleIssues(t, + config.Config{IgnorePathIssues: ®ex}, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + }, + ) + + // two issues + verifyHandleIssues(t, + config.Config{IgnorePathIssues: ®ex}, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "borked", + Path: "pkg foo/bar/baz", + Message: "nope nope nope", + }, + }, + logAndExpectedError{ + issue: notarize.LogIssue{ + Severity: "verysevere", + Path: "pkg foo/bar/bang", + Message: "no, just no", + }, + expectedError: "verysevere for path \"pkg foo/bar/bang\": no, just no", + }, + ) +} + +type logAndExpectedError struct { + issue notarize.LogIssue + expectedError string +} + +func expectedErrors(logAndExpectedErrors []logAndExpectedError) int { + var ee = 0 + for _, lee := range logAndExpectedErrors { + if lee.expectedError != "" { + ee++ + } + } + return ee +} + +func verifyHandleIssues(t *testing.T, cfg config.Config, logAndExpectedErrors ...logAndExpectedError) { + logger := hclog.L() + + issues := make([]notarize.LogIssue, len(logAndExpectedErrors)) + for idx, lee := range logAndExpectedErrors { + issues[idx] = lee.issue + } + + var lock sync.Mutex + log := notarize.Log{Issues: issues} + + opts := processOptions{ + Config: &cfg, + Logger: logger, + Prefix: "TestHandleIssues", + OutputLock: &lock, + } + + err := handleIssues(&opts, &log) + + if expectedErrors(logAndExpectedErrors) == 0 { + require.NoError(t, err) + } else { + require.Error(t, err) + + me, ok := err.(*multierror.Error) + require.True(t, ok) + require.Len(t, me.WrappedErrors(), expectedErrors(logAndExpectedErrors)) + idx := 0 + for _, lee := range logAndExpectedErrors { + if lee.expectedError != "" { + require.EqualError(t, me.WrappedErrors()[idx], lee.expectedError) + idx++ + } + } + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 8c8706e..bcf5bef 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,6 +28,13 @@ type Config struct { // Dmg, if present, creates a dmg file to package the signed `Source` files // into. Dmg files support stapling so this allows offline usage. Dmg *Dmg `hcl:"dmg,block"` + + // IgnorePathIssues, if present, will allow a notarization to succeed in the + // presence of issues reported by Apple. Supply a regular expression to match + // against the path(s) for which issues should be considered non-fatal, or + // ".*" to match all issues. + // Note that paths reported by Apple take the format: " ". + IgnorePathIssues *string `hcl:"ignorable_path_issues,optional"` } // AppleId are the authentication settings for Apple systems. diff --git a/internal/config/testdata/basic.hcl.golden b/internal/config/testdata/basic.hcl.golden index e31f737..c5e2481 100644 --- a/internal/config/testdata/basic.hcl.golden +++ b/internal/config/testdata/basic.hcl.golden @@ -14,5 +14,6 @@ Provider: (string) "" }), Zip: (*config.Zip)(), - Dmg: (*config.Dmg)() + Dmg: (*config.Dmg)(), + IgnorePathIssues: (*string)() }) diff --git a/internal/config/testdata/entitle.hcl.golden b/internal/config/testdata/entitle.hcl.golden index 1f10236..51fe468 100644 --- a/internal/config/testdata/entitle.hcl.golden +++ b/internal/config/testdata/entitle.hcl.golden @@ -14,5 +14,6 @@ Provider: (string) "" }), Zip: (*config.Zip)(), - Dmg: (*config.Dmg)() + Dmg: (*config.Dmg)(), + IgnorePathIssues: (*string)() }) diff --git a/internal/config/testdata/env_appleid.hcl.golden b/internal/config/testdata/env_appleid.hcl.golden index 70382c1..3cbda3c 100644 --- a/internal/config/testdata/env_appleid.hcl.golden +++ b/internal/config/testdata/env_appleid.hcl.golden @@ -10,5 +10,6 @@ }), AppleId: (*config.AppleId)(), Zip: (*config.Zip)(), - Dmg: (*config.Dmg)() + Dmg: (*config.Dmg)(), + IgnorePathIssues: (*string)() }) diff --git a/internal/config/testdata/notarize.hcl.golden b/internal/config/testdata/notarize.hcl.golden index 8566ee1..8e8dfe4 100644 --- a/internal/config/testdata/notarize.hcl.golden +++ b/internal/config/testdata/notarize.hcl.golden @@ -16,5 +16,6 @@ Provider: (string) "" }), Zip: (*config.Zip)(), - Dmg: (*config.Dmg)() + Dmg: (*config.Dmg)(), + IgnorePathIssues: (*string)() }) diff --git a/internal/config/testdata/notarize_multiple.hcl.golden b/internal/config/testdata/notarize_multiple.hcl.golden index b667f6d..5612424 100644 --- a/internal/config/testdata/notarize_multiple.hcl.golden +++ b/internal/config/testdata/notarize_multiple.hcl.golden @@ -21,5 +21,6 @@ Provider: (string) "" }), Zip: (*config.Zip)(), - Dmg: (*config.Dmg)() + Dmg: (*config.Dmg)(), + IgnorePathIssues: (*string)() })