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

asserts: introduce SequenceMemberAfter in the asserts backstores #8906

Merged
merged 5 commits into from
Jul 6, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions asserts/asserts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (as *assertsSuite) TestTypeNames(c *C) {
"test-only-no-authority",
"test-only-no-authority-pk",
"test-only-rev",
"test-only-seq",
"validation",
"validation-set",
})
Expand All @@ -84,6 +85,7 @@ func (as *assertsSuite) TestMaxSupportedFormats(c *C) {
"snap-declaration": snapDeclMaxFormat,
"system-user": systemUserMaxFormat,
"test-only": 1,
"test-only-seq": 2,
})

// all
Expand Down
14 changes: 13 additions & 1 deletion asserts/database.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2015-2016 Canonical Ltd
* Copyright (C) 2015-2020 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
Expand Down Expand Up @@ -65,6 +65,14 @@ type Backstore interface {
// Search returns assertions matching the given headers.
// It invokes foundCb for each found assertion.
Search(assertType *AssertionType, headers map[string]string, foundCb func(Assertion), maxFormat int) error
// SequenceMemberAfter returns for a sequence-forming assertType the
// first assertion in the sequence under the given sequenceKey
// with sequence number larger than after. If after==-1 it
// returns the assertion with largest sequence number. If none
// exists it returns a NotFoundError, usually with omitted
// Headers. If assertType is not sequence-forming it can
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the part about omitted headers from this description alone. Does it mean that it can return both an error and a SequenceMember that are together non-nil?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, Headers here refer to the Headers field of NotFoundError itself, the with can refer only to that in the previous part of the sentence, so I'm not entirely sure how to make it clearer. We have that exact sentence in the docs of preexisting methods already.

// panic.
SequenceMemberAfter(assertType *AssertionType, sequenceKey []string, after, maxFormat int) (SequenceMember, error)
}

type nullBackstore struct{}
Expand All @@ -81,6 +89,10 @@ func (nbs nullBackstore) Search(t *AssertionType, h map[string]string, f func(As
return nil
}

func (nbs nullBackstore) SequenceMemberAfter(t *AssertionType, kp []string, after, maxFormat int) (SequenceMember, error) {
return nil, &NotFoundError{Type: t}
}

// A KeypairManager is a manager and backstore for private/public key pairs.
type KeypairManager interface {
// Put stores the given private/public key pair,
Expand Down
29 changes: 29 additions & 0 deletions asserts/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,33 @@ func assembleTestOnlyRev(assert assertionBase) (Assertion, error) {

var TestOnlyRevType = &AssertionType{"test-only-rev", []string{"h"}, assembleTestOnlyRev, 0}

// TestOnlySeq is a test-only assertion that is sequence-forming.
type TestOnlySeq struct {
assertionBase
seq int
}

func (seq *TestOnlyRev) N() string {
return seq.HeaderString("n")
}

func (seq *TestOnlySeq) Sequence() int {
return seq.seq
}

func assembleTestOnlySeq(assert assertionBase) (Assertion, error) {
seq, err := checkSequence(assert.headers, "sequence")
if err != nil {
return nil, err
}
return &TestOnlySeq{
assertionBase: assert,
seq: seq,
}, nil
}

var TestOnlySeqType = &AssertionType{"test-only-seq", []string{"n", "sequence"}, assembleTestOnlySeq, sequenceForming}

type TestOnlyNoAuthority struct {
assertionBase
}
Expand Down Expand Up @@ -208,6 +235,8 @@ func init() {
}
typeRegistry[TestOnlyDeclType.Name] = TestOnlyDeclType
typeRegistry[TestOnlyRevType.Name] = TestOnlyRevType
typeRegistry[TestOnlySeqType.Name] = TestOnlySeqType
maxSupportedFormat[TestOnlySeqType.Name] = 2
}

// AccountKeyIsKeyValidAt exposes isKeyValidAt on AccountKey for tests
Expand Down
88 changes: 82 additions & 6 deletions asserts/findwildcard.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ package asserts

import (
"fmt"
"io"
"os"
"path/filepath"
"sort"
"strconv"
"strings"
)

Expand All @@ -31,14 +34,21 @@ findWildcard invokes foundCb once for each parent directory of regular files mat

<top>/<descendantWithWildcard[0]>/<descendantWithWildcard[1]>...

where each descendantWithWildcard component can contain the * wildcard;
where each descendantWithWildcard component can contain the * wildcard.

One of the descendantWithWildcard components except the last
can be "#>" or "#<", in which case that level is assumed to have names
that can be parsed as positive integers, which will be enumerated in
ascending (#>) or descending order respectively (#<); if seqnum != -1
then only the values >seqnum or respectively <seqnum will be
considered.

foundCb is invoked with the paths of the found regular files relative to top (that means top/ is excluded).

Unlike filepath.Glob any I/O operation error stops the walking and bottoms out, so does a foundCb invocation that returns an error.
*/
func findWildcard(top string, descendantWithWildcard []string, foundCb func(relpath []string) error) error {
return findWildcardDescend(top, top, descendantWithWildcard, foundCb)
func findWildcard(top string, descendantWithWildcard []string, seqnum int, foundCb func(relpath []string) error) error {
return findWildcardDescend(top, top, descendantWithWildcard, seqnum, foundCb)
}

func findWildcardBottom(top, current string, pat string, names []string, foundCb func(relpath []string) error) error {
Expand Down Expand Up @@ -74,13 +84,21 @@ func findWildcardBottom(top, current string, pat string, names []string, foundCb
return foundCb(hits)
}

func findWildcardDescend(top, current string, descendantWithWildcard []string, foundCb func(relpath []string) error) error {
func findWildcardDescend(top, current string, descendantWithWildcard []string, seqnum int, foundCb func(relpath []string) error) error {
k := descendantWithWildcard[0]
if k == "#>" || k == "#<" {
if len(descendantWithWildcard) == 1 {
return fmt.Errorf("findWildcard: sequence wildcard (#>|<#) cannot be the last component")
}
return findWildcardSequence(top, current, k, descendantWithWildcard[1:], seqnum, foundCb)
}
if len(descendantWithWildcard) > 1 && strings.IndexByte(k, '*') == -1 {
return findWildcardDescend(top, filepath.Join(current, k), descendantWithWildcard[1:], foundCb)
return findWildcardDescend(top, filepath.Join(current, k), descendantWithWildcard[1:], seqnum, foundCb)
}

d, err := os.Open(current)
// ignore missing directory, higher level will produce
// NotFoundError as needed
if os.IsNotExist(err) {
return nil
}
Expand All @@ -101,11 +119,69 @@ func findWildcardDescend(top, current string, descendantWithWildcard []string, f
return fmt.Errorf("findWildcard: invoked with malformed wildcard: %v", err)
}
if ok {
err = findWildcardDescend(top, filepath.Join(current, name), descendantWithWildcard[1:], foundCb)
err = findWildcardDescend(top, filepath.Join(current, name), descendantWithWildcard[1:], seqnum, foundCb)
if err != nil {
return err
}
}
}
return nil
}

func findWildcardSequence(top, current, seqWildcard string, descendantWithWildcard []string, seqnum int, foundCb func(relpath []string) error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function looks good.

filter := func(i int) bool { return true }
if seqnum != -1 {
if seqWildcard == "#>" {
filter = func(i int) bool { return i > seqnum }
} else { // "#<", guaranteed by the caller
filter = func(i int) bool { return i < seqnum }
}
}

d, err := os.Open(current)
// ignore missing directory, higher level will produce
// NotFoundError as needed
if os.IsNotExist(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment explaining what is the meaning of not-exists here and why it is not propagated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

current can come straight from the query (if it doesn't have wildcards for its level), in which case we just ignore the missing directory and the callback isn't called and the end result will be a not-found error at the higher level. This kind of code exist already in the other findWildcard* functions in the file

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment

return nil
}
if err != nil {
return err
}
defer d.Close()
var seq []int
for {
names, err := d.Readdirnames(100)
zyga marked this conversation as resolved.
Show resolved Hide resolved
stolowski marked this conversation as resolved.
Show resolved Hide resolved
if err == io.EOF {
break
}
if err != nil {
return err
}
for _, n := range names {
sqn, err := strconv.Atoi(n)
if err != nil || sqn < 0 {
return fmt.Errorf("cannot parse %q name as a sequence number", filepath.Join(current, n))
}
if filter(sqn) {
seq = append(seq, sqn)
}
}
}
sort.Ints(seq)

var start, direction int
if seqWildcard == "#>" {
start = 0
direction = 1
} else {
start = len(seq) - 1
direction = -1
}
for i := start; i >= 0 && i < len(seq); i += direction {
err = findWildcardDescend(top, filepath.Join(current, strconv.Itoa(seq[i])), descendantWithWildcard, -1, foundCb)
Copy link
Contributor

Choose a reason for hiding this comment

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

Although the tree structure is controller, this will trigger interesting behavior if sequence directory name is eg. "01", we'll parse it as 1 and then Itoa() converts it back to "1"

Copy link
Collaborator Author

@pedronis pedronis Jul 3, 2020

Choose a reason for hiding this comment

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

we'll just miss things that arre not well-formed but you are right that I should probably review the uses of Atoi in the package (it's an orthogonal problem)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it sounds like a follow up topic if anything.

if err != nil {
return err
}
}
return nil
}
Loading