many: remove snapasserts.ValidationSets.CheckPresence* methods

asserts/snapasserts/validation_sets.go

@@ -962,28 +962,6 @@ func (v *ValidationSets) constraintsForSnap(snapRef naming.SnapRef) *snapConstra
 	return nil
 }
 
-// CheckPresenceRequired returns the list of all validation sets that declare
-// presence of the given snap as required and the required revision (or
-// snap.R(0) if no specific revision is required). PresenceConstraintError is
-// returned if presence of the snap is "invalid".
-// The method assumes that validation sets are not in conflict.
-func (v *ValidationSets) CheckPresenceRequired(snapRef naming.SnapRef) ([]ValidationSetKey, snap.Revision, error) {
-	p, err := v.Presence(snapRef)
-	if err != nil {
-		return nil, snap.Revision{}, err
-	}
-
-	if p.Presence == asserts.PresenceInvalid {
-		return nil, snap.Revision{}, &PresenceConstraintError{snapRef.SnapName(), p.Presence}
-	}
-
-	if p.Presence != asserts.PresenceRequired {
-		return nil, unspecifiedRevision, nil
-	}
-
-	return p.Sets, p.Revision, nil
-}
-
 // CanBePresent returns true if a snap can be present in a situation in which
 // these validation sets are being applied.
 func (v *ValidationSets) CanBePresent(snapRef naming.SnapRef) bool {
@@ -1012,27 +990,6 @@ func (v *ValidationSets) SnapConstrained(snapRef naming.SnapRef) bool {
 	return v.constraintsForSnap(snapRef) != nil
 }
 
-// CheckPresenceInvalid returns the list of all validation sets that declare
-// presence of the given snap as invalid. PresenceConstraintError is returned if
-// presence of the snap is "optional" or "required".
-// The method assumes that validation sets are not in conflict.
-func (v *ValidationSets) CheckPresenceInvalid(snapRef naming.SnapRef) ([]ValidationSetKey, error) {
-	if !v.SnapConstrained(snapRef) {
-		return nil, nil
-	}
-
-	p, err := v.Presence(snapRef)
-	if err != nil {
-		return nil, err
-	}
-
-	if p.Presence != asserts.PresenceInvalid {
-		return nil, &PresenceConstraintError{snapRef.SnapName(), p.Presence}
-	}
-
-	return p.Sets, nil
-}
-
 type presence struct {
 	Presence asserts.Presence
 	Revision snap.Revision

asserts/snapasserts/validation_sets_test.go

@@ -1198,9 +1198,9 @@ func (s *validationSetsSuite) TestCheckPresenceRequired(c *C) {
 	valsets := snapasserts.NewValidationSets()
 
 	// no validation sets
-	vsKeys, _, err := valsets.CheckPresenceRequired(naming.Snap("my-snap"))
+	presence, err := valsets.Presence(naming.Snap("my-snap"))
 	c.Assert(err, IsNil)
-	c.Check(vsKeys, HasLen, 0)
+	c.Check(presence.Constrained(), Equals, false)
 
 	c.Assert(valsets.Add(valset1), IsNil)
 	c.Assert(valsets.Add(valset2), IsNil)
@@ -1209,39 +1209,31 @@ func (s *validationSetsSuite) TestCheckPresenceRequired(c *C) {
 	// validity
 	c.Assert(valsets.Conflict(), IsNil)
 
-	vsKeys, rev, err := valsets.CheckPresenceRequired(naming.Snap("my-snap"))
+	presence, err = valsets.Presence(naming.Snap("my-snap"))
 	c.Assert(err, IsNil)
-	c.Check(rev, DeepEquals, snap.Revision{N: 7})
-	c.Check(vsKeys, DeepEquals, []snapasserts.ValidationSetKey{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2", "16/account-id/my-snap-ctl3/1"})
+	c.Check(presence.Revision, DeepEquals, snap.Revision{N: 7})
+	c.Check(presence.Presence, DeepEquals, asserts.PresenceRequired)
+	c.Check(presence.Sets, DeepEquals, snapasserts.ValidationSetKeySlice{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2", "16/account-id/my-snap-ctl3/1"})
 
-	vsKeys, rev, err = valsets.CheckPresenceRequired(naming.NewSnapRef("my-snap", "mysnapididididididididididididid"))
+	presence, err = valsets.Presence(naming.NewSnapRef("my-snap", "mysnapididididididididididididid"))
 	c.Assert(err, IsNil)
-	c.Check(rev, DeepEquals, snap.Revision{N: 7})
-	c.Check(vsKeys, DeepEquals, []snapasserts.ValidationSetKey{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2", "16/account-id/my-snap-ctl3/1"})
-
-	// other-snap is not required
-	vsKeys, rev, err = valsets.CheckPresenceRequired(naming.Snap("other-snap"))
-	c.Assert(err, ErrorMatches, `unexpected presence "invalid" for snap "other-snap"`)
-	pr, ok := err.(*snapasserts.PresenceConstraintError)
-	c.Assert(ok, Equals, true)
-	c.Check(pr.SnapName, Equals, "other-snap")
-	c.Check(pr.Presence, Equals, asserts.PresenceInvalid)
-	c.Check(rev, DeepEquals, snap.Revision{N: 0})
-	c.Check(vsKeys, HasLen, 0)
+	c.Check(presence.Revision, DeepEquals, snap.Revision{N: 7})
+	c.Check(presence.Presence, DeepEquals, asserts.PresenceRequired)
+	c.Check(presence.Sets, DeepEquals, snapasserts.ValidationSetKeySlice{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2", "16/account-id/my-snap-ctl3/1"})
 
 	// unknown snap is not required
-	vsKeys, rev, err = valsets.CheckPresenceRequired(naming.NewSnapRef("unknown-snap", "00000000idididididididididididid"))
+	presence, err = valsets.Presence(naming.NewSnapRef("unknown-snap", "00000000idididididididididididid"))
 	c.Assert(err, IsNil)
-	c.Check(rev, DeepEquals, snap.Revision{N: 0})
-	c.Check(vsKeys, HasLen, 0)
+	c.Check(presence.Constrained(), Equals, false)
 
 	// just one set, required but no revision specified
 	valsets = snapasserts.NewValidationSets()
 	c.Assert(valsets.Add(valset3), IsNil)
-	vsKeys, rev, err = valsets.CheckPresenceRequired(naming.Snap("my-snap"))
+	presence, err = valsets.Presence(naming.Snap("my-snap"))
 	c.Assert(err, IsNil)
-	c.Check(rev, DeepEquals, snap.Revision{N: 0})
-	c.Check(vsKeys, DeepEquals, []snapasserts.ValidationSetKey{"16/account-id/my-snap-ctl3/1"})
+	c.Check(presence.Revision, DeepEquals, snap.Revision{N: 0})
+	c.Check(presence.Presence, DeepEquals, asserts.PresenceRequired)
+	c.Check(presence.Sets, DeepEquals, snapasserts.ValidationSetKeySlice{"16/account-id/my-snap-ctl3/1"})
 }
 
 func (s *validationSetsSuite) TestIsPresenceInvalid(c *C) {
@@ -1285,9 +1277,9 @@ func (s *validationSetsSuite) TestIsPresenceInvalid(c *C) {
 	valsets := snapasserts.NewValidationSets()
 
 	// no validation sets
-	vsKeys, err := valsets.CheckPresenceInvalid(naming.Snap("my-snap"))
+	presence, err := valsets.Presence(naming.Snap("my-snap"))
 	c.Assert(err, IsNil)
-	c.Check(vsKeys, HasLen, 0)
+	c.Check(presence.Constrained(), Equals, false)
 
 	c.Assert(valsets.Add(valset1), IsNil)
 	c.Assert(valsets.Add(valset2), IsNil)
@@ -1296,31 +1288,18 @@ func (s *validationSetsSuite) TestIsPresenceInvalid(c *C) {
 	c.Assert(valsets.Conflict(), IsNil)
 
 	// invalid in two sets
-	vsKeys, err = valsets.CheckPresenceInvalid(naming.Snap("my-snap"))
+	presence, err = valsets.Presence(naming.Snap("my-snap"))
 	c.Assert(err, IsNil)
-	c.Check(vsKeys, DeepEquals, []snapasserts.ValidationSetKey{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2"})
+	c.Check(presence.Sets, DeepEquals, snapasserts.ValidationSetKeySlice{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2"})
 
-	vsKeys, err = valsets.CheckPresenceInvalid(naming.NewSnapRef("my-snap", "mysnapididididididididididididid"))
+	presence, err = valsets.Presence(naming.NewSnapRef("my-snap", "mysnapididididididididididididid"))
 	c.Assert(err, IsNil)
-	c.Check(vsKeys, DeepEquals, []snapasserts.ValidationSetKey{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2"})
-
-	// other-snap isn't invalid
-	vsKeys, err = valsets.CheckPresenceInvalid(naming.Snap("other-snap"))
-	c.Assert(err, ErrorMatches, `unexpected presence "optional" for snap "other-snap"`)
-	pr, ok := err.(*snapasserts.PresenceConstraintError)
-	c.Assert(ok, Equals, true)
-	c.Check(pr.SnapName, Equals, "other-snap")
-	c.Check(pr.Presence, Equals, asserts.PresenceOptional)
-	c.Check(vsKeys, HasLen, 0)
-
-	vsKeys, err = valsets.CheckPresenceInvalid(naming.NewSnapRef("other-snap", "123456ididididididididididididid"))
-	c.Assert(err, ErrorMatches, `unexpected presence "optional" for snap "other-snap"`)
-	c.Check(vsKeys, HasLen, 0)
-
-	// unknown snap isn't invalid
-	vsKeys, err = valsets.CheckPresenceInvalid(naming.NewSnapRef("unknown-snap", "00000000idididididididididididid"))
+	c.Check(presence.Sets, DeepEquals, snapasserts.ValidationSetKeySlice{"16/account-id/my-snap-ctl/1", "16/account-id/my-snap-ctl2/2"})
+
+	// unknown snap isn't constrained
+	presence, err = valsets.Presence(naming.NewSnapRef("unknown-snap", "00000000idididididididididididid"))
 	c.Assert(err, IsNil)
-	c.Check(vsKeys, HasLen, 0)
+	c.Check(presence.Constrained(), Equals, false)
 }
 
 func (s *validationSetsSuite) TestParseValidationSet(c *C) {

image/image_linux.go

@@ -559,16 +559,18 @@ func (s *imageSeeder) validationSetKeysAndRevisionForSnap(snapName string) ([]sn
 		return nil, snap.Revision{}, err
 	}
 
-	// TODO: It's pointed out that here and some of the others uses of this
-	// may miss logic for optional snaps which have required revisions. This
-	// is not covered by the below check, and we may or may not have multiple places
-	// with a similar issue.
-	snapVsKeys, snapRev, err := allVss.CheckPresenceRequired(naming.Snap(snapName))
+	pres, err := allVss.Presence(naming.Snap(snapName))
 	if err != nil {
 		return nil, snap.Revision{}, err
 	}
-	if len(snapVsKeys) > 0 {
-		return snapVsKeys, snapRev, nil
+
+	// TODO: figure out if this is needed
+	if pres.Presence == asserts.PresenceInvalid {
+		return nil, snap.Revision{}, fmt.Errorf("snap %q is invalid in validation sets: %v", snapName, pres.Sets.CommaSeparated())
+	}
+
+	if pres.Constrained() {
+		return pres.Sets, pres.Revision, nil
 	}
 	return nil, s.w.Manifest().AllowedSnapRevision(snapName), nil
 }

overlord/devicestate/devicestate.go

@@ -1745,14 +1745,14 @@ func CreateRecoverySystem(st *state.State, label string, opts CreateRecoverySyst
 		}
 
 		if sn.Presence != "required" {
-			sets, _, err := valsets.CheckPresenceRequired(sn)
+			pres, err := valsets.Presence(sn)
 			if err != nil {
 				return nil, err
 			}
 
 			// snap isn't already installed, and it isn't required by model or
 			// any validation sets, so we should skip it
-			if len(sets) == 0 {
+			if pres.Presence != asserts.PresenceRequired {
 				continue
 			}
 		}

overlord/snapstate/snapstate.go

@@ -3464,30 +3464,24 @@ func canRemove(st *state.State, si *snap.Info, snapst *SnapState, removeAll bool
 	if enforcedSets == nil {
 		return nil
 	}
-	requiredValsets, requiredRevision, err := enforcedSets.CheckPresenceRequired(si)
+	pres, err := enforcedSets.Presence(si)
 	if err != nil {
-		if _, ok := err.(*snapasserts.PresenceConstraintError); !ok {
-			return err
-		}
-		// else - presence is invalid, nothing to do (not really possible since
-		// it shouldn't be allowed to get installed in the first place).
-		return nil
+		return err
 	}
-	if len(requiredValsets) == 0 {
-		// not required by any validation set (or is optional)
+	if pres.Presence != asserts.PresenceRequired {
 		return nil
 	}
 	// removeAll is set if we're removing the snap completely
 	if removeAll {
-		if requiredRevision.Unset() {
-			return fmt.Errorf("snap %q is required by validation sets: %s", si.InstanceName(), snapasserts.ValidationSetKeySlice(requiredValsets).CommaSeparated())
+		if pres.Revision.Unset() {
+			return fmt.Errorf("snap %q is required by validation sets: %s", si.InstanceName(), pres.Sets.CommaSeparated())
 		}
-		return fmt.Errorf("snap %q at revision %s is required by validation sets: %s", si.InstanceName(), requiredRevision, snapasserts.ValidationSetKeySlice(requiredValsets).CommaSeparated())
+		return fmt.Errorf("snap %q at revision %s is required by validation sets: %s", si.InstanceName(), pres.Revision, pres.Sets.CommaSeparated())
 	}
 
 	// rev is set at this point (otherwise we would hit removeAll case)
-	if requiredRevision.N == rev.N {
-		return fmt.Errorf("snap %q at revision %s is required by validation sets: %s", si.InstanceName(), rev, snapasserts.ValidationSetKeySlice(requiredValsets).CommaSeparated())
+	if pres.Revision.N == rev.N {
+		return fmt.Errorf("snap %q at revision %s is required by validation sets: %s", si.InstanceName(), rev, pres.Sets.CommaSeparated())
 	} // else - it's ok to remove a revision different than the required
 	return nil
 }

overlord/snapstate/target.go

@@ -25,6 +25,7 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/snapcore/snapd/asserts"
 	"github.com/snapcore/snapd/asserts/snapasserts"
 	"github.com/snapcore/snapd/client"
 	"github.com/snapcore/snapd/logger"
@@ -413,14 +414,12 @@ func completeStoreAction(action *store.SnapAction, revOpts RevisionOptions, igno
 
 		// if the caller didn't provide any validation sets, make sure that
 		// the snap is allowed by all of the enforced validation sets
-		invalidSets, err := vsets.CheckPresenceInvalid(naming.Snap(action.InstanceName))
+		pres, err := vsets.Presence(naming.Snap(action.InstanceName))
 		if err != nil {
-			if _, ok := err.(*snapasserts.PresenceConstraintError); !ok {
-				return err
-			} // else presence is optional or required, carry on
+			return err
 		}
 
-		if len(invalidSets) > 0 {
+		if pres.Presence == asserts.PresenceInvalid {
 			verb := "install"
 			if action.Action == "refresh" {
 				verb = "update"
@@ -430,29 +429,28 @@ func completeStoreAction(action *store.SnapAction, revOpts RevisionOptions, igno
 				"cannot %s snap %q due to enforcing rules of validation set %s",
 				verb,
 				action.InstanceName,
-				snapasserts.ValidationSetKeySlice(invalidSets).CommaSeparated(),
+				pres.Sets.CommaSeparated(),
 			)
 		}
 
-		requiredSets, requiredRev, err := vsets.CheckPresenceRequired(naming.Snap(action.InstanceName))
-		if err != nil {
-			return err
-		}
-
 		// make sure that the caller-requested revision matches the revision
 		// required by the enforced validation sets
-		if !requiredRev.Unset() && !revOpts.Revision.Unset() && requiredRev != revOpts.Revision {
-			return invalidRevisionError(action, requiredSets, revOpts.Revision, requiredRev)
+		if !pres.Revision.Unset() && !revOpts.Revision.Unset() && pres.Revision != revOpts.Revision {
+			return invalidRevisionError(action, pres.Sets, revOpts.Revision, pres.Revision)
 		}
 
 		// TODO:COMPS: handle validation sets and components here
 
-		action.ValidationSets = requiredSets
+		// we only need to send these if this snap is actually constrained by
+		// the validation sets in some way
+		if pres.Constrained() {
+			action.ValidationSets = pres.Sets
+		}
 
-		if !requiredRev.Unset() {
+		if !pres.Revision.Unset() {
 			// make sure that we use the revision required by the enforced
 			// validation sets
-			action.Revision = requiredRev
+			action.Revision = pres.Revision
 
 			// we ignore the cohort if a validation set requires that the
 			// snap is pinned to a specific revision