From f7212b722012acfe9e72e5d6d24cc1c11b5d10a8 Mon Sep 17 00:00:00 2001 From: Andrew Phelps Date: Wed, 18 Sep 2024 15:48:02 -0400 Subject: [PATCH] a/snapasserts: add components to output of ValidationSetsValidationError.Error --- asserts/snapasserts/validation_sets.go | 139 ++++++++++++++------ asserts/snapasserts/validation_sets_test.go | 58 +++++++- 2 files changed, 149 insertions(+), 48 deletions(-) diff --git a/asserts/snapasserts/validation_sets.go b/asserts/snapasserts/validation_sets.go index 1b36c38a91e..98ed65c8ec6 100644 --- a/asserts/snapasserts/validation_sets.go +++ b/asserts/snapasserts/validation_sets.go @@ -23,6 +23,7 @@ import ( "bytes" "errors" "fmt" + "io" "sort" "strconv" "strings" @@ -175,64 +176,120 @@ func (b byRevision) Less(i, j int) bool { return b[i].N < b[j].N } func (e *ValidationSetsValidationError) Error() string { buf := bytes.NewBufferString("validation sets assertions are not met:") - printDetails := func(header string, details map[string][]string, - printSnap func(snapName string, keys []string) string) { - if len(details) == 0 { - return - } - fmt.Fprintf(buf, "\n- %s:", header) - for snapName, validationSetKeys := range details { - fmt.Fprintf(buf, "\n - %s", printSnap(snapName, validationSetKeys)) - } - } if len(e.MissingSnaps) > 0 { fmt.Fprintf(buf, "\n- missing required snaps:") - for snapName, revisions := range e.MissingSnaps { - revisionsSorted := make([]snap.Revision, 0, len(revisions)) - for rev := range revisions { - revisionsSorted = append(revisionsSorted, rev) - } - sort.Sort(byRevision(revisionsSorted)) - t := make([]string, 0, len(revisionsSorted)) - for _, rev := range revisionsSorted { - keys := revisions[rev] - if rev.Unset() { - t = append(t, fmt.Sprintf("at any revision by sets %s", strings.Join(keys, ","))) - } else { - t = append(t, fmt.Sprintf("at revision %s by sets %s", rev, strings.Join(keys, ","))) - } - } - fmt.Fprintf(buf, "\n - %s (required %s)", snapName, strings.Join(t, ", ")) + for name, revisions := range e.MissingSnaps { + writeMissingError(buf, name, revisions) } } - printDetails("invalid snaps", e.InvalidSnaps, func(snapName string, validationSetKeys []string) string { - return fmt.Sprintf("%s (invalid for sets %s)", snapName, strings.Join(validationSetKeys, ",")) - }) + if len(e.InvalidSnaps) > 0 { + fmt.Fprintf(buf, "\n- invalid snaps:") + for snapName, validationSetKeys := range e.InvalidSnaps { + fmt.Fprintf(buf, "\n - %s (invalid for sets %s)", snapName, strings.Join(validationSetKeys, ",")) + } + } if len(e.WrongRevisionSnaps) > 0 { fmt.Fprint(buf, "\n- snaps at wrong revisions:") for snapName, revisions := range e.WrongRevisionSnaps { - revisionsSorted := make([]snap.Revision, 0, len(revisions)) - for rev := range revisions { - revisionsSorted = append(revisionsSorted, rev) - } - sort.Sort(byRevision(revisionsSorted)) - t := make([]string, 0, len(revisionsSorted)) - for _, rev := range revisionsSorted { - keys := revisions[rev] - t = append(t, fmt.Sprintf("at revision %s by sets %s", rev, strings.Join(keys, ","))) - } - fmt.Fprintf(buf, "\n - %s (required %s)", snapName, strings.Join(t, ", ")) + writeWrongRevisionError(buf, snapName, revisions) + } + } + + // the data structure here isn't really conducive to creating a + // non-hierarchical error message, maybe worth reorganizing. however, it + // will probably be better for actually using the error to extract what we + // need when resolving validation set errors in snapstate. it is also more + // representative of how the contraints are represented in the actual + // assertion. + + var missingComps, invalidComps, wrongRevComps strings.Builder + for snapName, vcerr := range e.ComponentErrors { + for _, compName := range sortedStringKeys(vcerr.MissingComponents) { + writeMissingError( + &missingComps, + naming.NewComponentRef(snapName, compName).String(), + vcerr.MissingComponents[compName], + ) + } + + for _, compName := range sortedStringKeys(vcerr.InvalidComponents) { + fmt.Fprintf( + &invalidComps, + "\n - %s (invalid for sets %s)", + naming.NewComponentRef(snapName, compName).String(), + strings.Join(vcerr.InvalidComponents[compName], ","), + ) + } + + for _, compName := range sortedStringKeys(vcerr.WrongRevisionComponents) { + writeWrongRevisionError( + &wrongRevComps, + naming.NewComponentRef(snapName, compName).String(), + vcerr.WrongRevisionComponents[compName], + ) } } - // TODO: add components to error message + if missingComps.Len() > 0 { + buf.WriteString("\n- missing required components:") + buf.WriteString(missingComps.String()) + } + if invalidComps.Len() > 0 { + buf.WriteString("\n- invalid components:") + buf.WriteString(invalidComps.String()) + } + if wrongRevComps.Len() > 0 { + buf.WriteString("\n- components at wrong revisions:") + buf.WriteString(wrongRevComps.String()) + } return buf.String() } +func sortedStringKeys[T any](m map[string]T) []string { + keys := make([]string, 0, len(m)) + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func writeWrongRevisionError(w io.Writer, name string, revisions map[snap.Revision][]string) { + revisionsSorted := make([]snap.Revision, 0, len(revisions)) + for rev := range revisions { + revisionsSorted = append(revisionsSorted, rev) + } + sort.Sort(byRevision(revisionsSorted)) + t := make([]string, 0, len(revisionsSorted)) + for _, rev := range revisionsSorted { + keys := revisions[rev] + t = append(t, fmt.Sprintf("at revision %s by sets %s", rev, strings.Join(keys, ","))) + } + fmt.Fprintf(w, "\n - %s (required %s)", name, strings.Join(t, ", ")) +} + +func writeMissingError(w io.Writer, name string, revisions map[snap.Revision][]string) { + revisionsSorted := make([]snap.Revision, 0, len(revisions)) + for rev := range revisions { + revisionsSorted = append(revisionsSorted, rev) + } + sort.Sort(byRevision(revisionsSorted)) + t := make([]string, 0, len(revisionsSorted)) + for _, rev := range revisionsSorted { + keys := revisions[rev] + if rev.Unset() { + t = append(t, fmt.Sprintf("at any revision by sets %s", strings.Join(keys, ","))) + } else { + t = append(t, fmt.Sprintf("at revision %s by sets %s", rev, strings.Join(keys, ","))) + } + } + fmt.Fprintf(w, "\n - %s (required %s)", name, strings.Join(t, ", ")) +} + // ValidationSets can hold a combination of validation-set assertions // and can check for conflicts or help applying them. type ValidationSets struct { diff --git a/asserts/snapasserts/validation_sets_test.go b/asserts/snapasserts/validation_sets_test.go index 781929ec435..aecede0e599 100644 --- a/asserts/snapasserts/validation_sets_test.go +++ b/asserts/snapasserts/validation_sets_test.go @@ -777,7 +777,6 @@ func (s *validationSetsSuite) TestCheckInstalledSnapsWithComponents(c *C) { assertions []string installed []*snapasserts.InstalledSnap verr *snapasserts.ValidationSetsValidationError - err error ignoreValidation map[string]bool } @@ -932,19 +931,64 @@ func (s *validationSetsSuite) TestCheckInstalledSnapsWithComponents(c *C) { c.Check(valSets.Conflict(), IsNil, Commentf(prefix)) err := valSets.CheckInstalledSnaps(tc.installed, tc.ignoreValidation) - switch { - case tc.err != nil: - c.Assert(err, testutil.ErrorIs, tc.err, Commentf("%s: unexpected error: %v", prefix, err)) - case tc.verr != nil: + if tc.verr == nil { + c.Assert(err, IsNil, Commentf("%s: unexpected error: %v", prefix, err)) + } else { verr, ok := err.(*snapasserts.ValidationSetsValidationError) c.Assert(ok, Equals, true, Commentf("%s: expected ValidationSetsValidationError, got: %v", prefix, err)) c.Check(verr, DeepEquals, tc.verr, Commentf(prefix)) - default: - c.Assert(err, IsNil, Commentf("%s: unexpected error: %v", prefix, err)) } } } +func (s *validationSetsSuite) TestValidationSetsValidationErrorStringWithComponents(c *C) { + verr := &snapasserts.ValidationSetsValidationError{ + Sets: map[string]*asserts.ValidationSet{ + "acme/one": nil, // can be nil for simplicity, not used in the test + "acme/two": nil, + "acme/three": nil, + }, + ComponentErrors: map[string]*snapasserts.ValidationSetsComponentValidationError{ + "snap-a": { + MissingComponents: map[string]map[snap.Revision][]string{ + "comp-3": { + snap.R(0): {"acme/one", "acme/two"}, + }, + "comp-4": { + snap.R(2): {"acme/one"}, + }, + }, + InvalidComponents: map[string][]string{ + "comp-5": {"acme/two"}, + }, + WrongRevisionComponents: map[string]map[snap.Revision][]string{ + "comp-6": { + snap.R(3): {"acme/three"}, + }, + }, + }, + }, + MissingSnaps: map[string]map[snap.Revision][]string{ + "snap-a": { + snap.R(0): {"acme/one"}, + }, + }, + } + + const expected = `validation sets assertions are not met: +- missing required snaps: + - snap-a (required at any revision by sets acme/one) +- missing required components: + - snap-a+comp-3 (required at any revision by sets acme/one,acme/two) + - snap-a+comp-4 (required at revision 2 by sets acme/one) +- invalid components: + - snap-a+comp-5 (invalid for sets acme/two) +- components at wrong revisions: + - snap-a+comp-6 (required at revision 3 by sets acme/three)` + + c.Check(verr.Error(), Equals, expected) +} + func (s *validationSetsSuite) TestCheckInstalledSnapsIgnoreValidation(c *C) { // require: snapB rev 3, snapC rev 2. // invalid: snapA