diff --git a/.github/.golangci.yml b/.github/.golangci.yml new file mode 100644 index 000000000..3f49df427 --- /dev/null +++ b/.github/.golangci.yml @@ -0,0 +1,14 @@ +linters: + disable-all: true + enable: + - gci +linters-settings: + gci: + sections: + - standard + - default + - Prefix(github.com/canonical/pebble) +issues: + # these values ensure that all issues will be surfaced + max-issues-per-linter: 0 + max-same-issues: 0 \ No newline at end of file diff --git a/.github/.trivyignore b/.github/.trivyignore new file mode 100644 index 000000000..0d0839f3a --- /dev/null +++ b/.github/.trivyignore @@ -0,0 +1,6 @@ +# ignore known CVEs that are not backported before Go 1.17 + +CVE-2022-41721 +CVE-2022-41717 +CVE-2022-41723 +CVE-2022-32149 diff --git a/.github/trivy.yaml b/.github/trivy.yaml new file mode 100644 index 000000000..7337ce08c --- /dev/null +++ b/.github/trivy.yaml @@ -0,0 +1,4 @@ +timeout: 20m +scan: + offline-scan: true +ignore-file: .github/.trivyignore diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 000000000..6122d1eaf --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,30 @@ +name: Lint +on: [push, pull_request] + +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-go@v4 + with: + go-mod-file: 'go.mod' + cache: false + + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + id: lint + with: + version: latest + args: '-c .github/.golangci.yml --out-format=colored-line-number' + skip-cache: true + + - name: Print error message + if: always() && steps.lint.outcome == 'failure' + run: | + echo ' + Linting failed. On your local machine, please run + golangci-lint run -c .github/.golangci.yml --fix + and check in the changes.' + exit 1 \ No newline at end of file diff --git a/.github/workflows/scanning.yml b/.github/workflows/scanning.yml new file mode 100644 index 000000000..83a362333 --- /dev/null +++ b/.github/workflows/scanning.yml @@ -0,0 +1,22 @@ +name: Vulnerability scanning + +on: + push: + branches: [master] + pull_request: + branches: [master] + +jobs: + scan: + name: Scan for known vulnerabilities + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v2 + + - name: Run Github Trivy FS Action + uses: aquasecurity/trivy-action@master + with: + scan-type: 'fs' + scan-ref: '.' + trivy-config: .github/trivy.yaml diff --git a/.github/workflows/snap.yml b/.github/workflows/snap.yml index dc3cc9fed..78a6a5bdb 100644 --- a/.github/workflows/snap.yml +++ b/.github/workflows/snap.yml @@ -1,6 +1,13 @@ name: Pebble snap -on: [pull_request] +on: + pull_request: + branches: [master] + release: + types: [published] + +env: + SNAP_NAME: pebble jobs: build: @@ -27,6 +34,8 @@ jobs: test: runs-on: ubuntu-latest needs: [build] + outputs: + pebble-version: ${{ steps.install-pebble.outputs.version }} steps: - uses: actions/download-artifact@v3 @@ -34,12 +43,58 @@ jobs: name: ${{ needs.build.outputs.pebble-snap }} - name: Install the Pebble snap + id: install-pebble run: | + set -ex # Install the Pebble snap from the artifact built in the previous job sudo snap install --dangerous --classic ${{ needs.build.outputs.pebble-snap }} # Make sure Pebble is installed - pebble version + echo "version=$(pebble version --client)" >> "$GITHUB_OUTPUT" - name: Run smoke test - run: pebble enter exec echo Hello | grep Hello + run: pebble enter --create-dirs exec echo Hello | grep Hello + + promote: + if: ${{ github.event_name == 'release' }} + runs-on: ubuntu-latest + needs: [test] + strategy: + fail-fast: false + matrix: + arch: [amd64, arm64, ppc64el, armhf, s390x] + env: + TRACK: latest + DEFAULT_RISK: edge + TO_RISK: candidate + steps: + - name: Install Snapcraft + run: sudo snap install snapcraft --classic + + - name: Wait for ${{ needs.test.outputs.pebble-version }} to be released + env: + SNAPCRAFT_STORE_CREDENTIALS: ${{ secrets.SNAPCRAFT_STORE_CREDENTIALS }} + run: | + while ! `snapcraft status ${{ env.SNAP_NAME }} --track ${{ env.TRACK }} --arch ${{ matrix.arch }} \ + | grep "${{ env.DEFAULT_RISK }}" \ + | awk -F' ' '{print $2}' \ + | grep -Fxq "${{ needs.test.outputs.pebble-version }}"`; do + echo "[${{ matrix.arch }}] Waiting for ${{ needs.test.outputs.pebble-version }} \ + to be released to ${{ env.TRACK }}/${{ env.DEFAULT_RISK }}..." + sleep 10 + done + + # It would be easier to use `snapcraft promote`, but there's an error when trying + # to avoid the prompt with the "--yes" option: + # > 'latest/edge' is not a valid set value for --from-channel when using --yes. + - name: Promote ${{ needs.test.outputs.pebble-version }} (${{ matrix.arch }}) to ${{ env.TO_RISK }} + env: + SNAPCRAFT_STORE_CREDENTIALS: ${{ secrets.SNAPCRAFT_STORE_CREDENTIALS }} + run: | + revision="$(snapcraft status ${{ env.SNAP_NAME }} \ + --track ${{ env.TRACK }} --arch ${{ matrix.arch }} \ + | grep "${{ env.DEFAULT_RISK }}" | awk -F' ' '{print $3}')" + + snapcraft release ${{ env.SNAP_NAME }} \ + $revision \ + ${{ env.TRACK }}/${{ env.TO_RISK }} diff --git a/HACKING.md b/HACKING.md index 9ec335851..69f4cf72b 100644 --- a/HACKING.md +++ b/HACKING.md @@ -83,6 +83,39 @@ $ curl --unix-socket ~/pebble/.pebble.socket 'http://localhost/v1/services?names ``` +## Code style + +Pebble imports should be arranged in three groups: +- standard library imports +- third-party / non-Pebble imports +- Pebble imports (i.e. those prefixed with `github.com/canonical/pebble`) + +Imports should be sorted alphabetically within each group. + +We use the [`gopkg.in/check.v1`](https://pkg.go.dev/gopkg.in/check.v1) package for testing. Inside a test file, import this as follows: +```go +. "gopkg.in/check.v1" +``` +so that identifiers from that package will be added to the local namespace. + + +Here is an example of correctly arranged imports: + +```go +import ( + "fmt" + "net" + "os" + + "github.com/gorilla/mux" + . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/systemd" + "github.com/canonical/pebble/internals/testutil" +) +``` + + ## Running the tests Pebble has a suite of Go unit tests, which you can run using the regular `go test` command. To test all packages in the Pebble repository: diff --git a/README.md b/README.md index a0fe9e3ef..ab9a0cc27 100644 --- a/README.md +++ b/README.md @@ -403,6 +403,60 @@ $ pebble run --verbose ... ``` + + ## Container usage Pebble works well as a local service manager, but if running Pebble in a separate container, you can use the exec and file management APIs to coordinate with the remote system over the shared unix socket. @@ -523,6 +577,10 @@ services: # group and group-id are specified, the group's GID must match group-id. group-id: + # (Optional) Working directory to run command in. By default, the + # command is run in the service manager's current directory. + working-dir: + # (Optional) Defines what happens when the service exits with a zero # exit code. Possible values are: "restart" (default) which restarts # the service after the backoff delay, "shutdown" which shuts down and @@ -630,6 +688,13 @@ checks: # directly, not interpreted by a shell. command: + # (Optional) Run the command in the context of this service. + # Specifically, inherit its environment variables, user/group + # settings, and working directory. The check's context (the + # settings below) will override the service's; the check's + # environment map will be merged on top of the service's. + service-context: + # (Optional) A list of key/value pairs defining environment # variables that should be set when running the command. environment: @@ -653,7 +718,8 @@ checks: # match group-id. group-id: - # (Optional) Working directory to run command in. + # (Optional) Working directory to run command in. By default, the + # command is run in the service manager's current directory. working-dir: ``` diff --git a/client/exec.go b/client/exec.go index 4b74242dd..88d405bd4 100644 --- a/client/exec.go +++ b/client/exec.go @@ -30,22 +30,28 @@ type ExecOptions struct { // Required: command and arguments (first element is the executable). Command []string + // Optional: run the command in the context of this service. Specifically, + // inherit its environment variables, user/group settings, and working + // and working directory. The other options in this struct will override + // the service context; Environment will be merged on top of the service's. + ServiceContext string + // Optional environment variables. Environment map[string]string // Optional working directory (default is $HOME or "/" if $HOME not set). WorkingDir string - // Optional timeout for the command execution, after which the process - // will be terminated. If zero, no timeout applies. - Timeout time.Duration - // Optional user ID and group ID for the process to run as. UserID *int User string GroupID *int Group string + // Optional timeout for the command execution, after which the process + // will be terminated. If zero, no timeout applies. + Timeout time.Duration + // True to ask the server to set up a pseudo-terminal (PTY) for stdout // (this also allows window resizing). The default is no PTY, and just // to use pipes for stdout/stderr. @@ -74,19 +80,20 @@ type ExecOptions struct { } type execPayload struct { - Command []string `json:"command"` - Environment map[string]string `json:"environment,omitempty"` - WorkingDir string `json:"working-dir,omitempty"` - Timeout string `json:"timeout,omitempty"` - UserID *int `json:"user-id,omitempty"` - User string `json:"user,omitempty"` - GroupID *int `json:"group-id,omitempty"` - Group string `json:"group,omitempty"` - Terminal bool `json:"terminal,omitempty"` - Interactive bool `json:"interactive,omitempty"` - SplitStderr bool `json:"split-stderr,omitempty"` - Width int `json:"width,omitempty"` - Height int `json:"height,omitempty"` + Command []string `json:"command"` + ServiceContext string `json:"service-context,omitempty"` + Environment map[string]string `json:"environment,omitempty"` + WorkingDir string `json:"working-dir,omitempty"` + Timeout string `json:"timeout,omitempty"` + UserID *int `json:"user-id,omitempty"` + User string `json:"user,omitempty"` + GroupID *int `json:"group-id,omitempty"` + Group string `json:"group,omitempty"` + Terminal bool `json:"terminal,omitempty"` + Interactive bool `json:"interactive,omitempty"` + SplitStderr bool `json:"split-stderr,omitempty"` + Width int `json:"width,omitempty"` + Height int `json:"height,omitempty"` } type execResult struct { @@ -122,19 +129,20 @@ func (client *Client) Exec(opts *ExecOptions) (*ExecProcess, error) { timeoutStr = opts.Timeout.String() } payload := execPayload{ - Command: opts.Command, - Environment: opts.Environment, - WorkingDir: opts.WorkingDir, - Timeout: timeoutStr, - UserID: opts.UserID, - User: opts.User, - GroupID: opts.GroupID, - Group: opts.Group, - Terminal: opts.Terminal, - Interactive: opts.Interactive, - SplitStderr: opts.Stderr != nil, - Width: opts.Width, - Height: opts.Height, + Command: opts.Command, + ServiceContext: opts.ServiceContext, + Environment: opts.Environment, + WorkingDir: opts.WorkingDir, + Timeout: timeoutStr, + UserID: opts.UserID, + User: opts.User, + GroupID: opts.GroupID, + Group: opts.Group, + Terminal: opts.Terminal, + Interactive: opts.Interactive, + SplitStderr: opts.Stderr != nil, + Width: opts.Width, + Height: opts.Height, } var body bytes.Buffer err := json.NewEncoder(&body).Encode(&payload) diff --git a/internals/cli/cli.go b/internals/cli/cli.go index 18a378510..cfdd4cb36 100644 --- a/internals/cli/cli.go +++ b/internals/cli/cli.go @@ -26,7 +26,6 @@ import ( "unicode/utf8" "github.com/canonical/go-flags" - "golang.org/x/crypto/ssh/terminal" "github.com/canonical/pebble/client" diff --git a/internals/cli/cli_test.go b/internals/cli/cli_test.go index 45943c925..16cf4f68a 100644 --- a/internals/cli/cli_test.go +++ b/internals/cli/cli_test.go @@ -11,7 +11,6 @@ import ( "testing" "golang.org/x/crypto/ssh/terminal" - . "gopkg.in/check.v1" "github.com/canonical/pebble/cmd" diff --git a/internals/cli/cmd_exec.go b/internals/cli/cmd_exec.go index 0298f2b29..1dae53005 100644 --- a/internals/cli/cmd_exec.go +++ b/internals/cli/cmd_exec.go @@ -39,6 +39,7 @@ type cmdExec struct { GroupID *int `long:"gid"` Group string `long:"group"` Timeout time.Duration `long:"timeout"` + Context string `long:"context"` Terminal bool `short:"t"` NoTerminal bool `short:"T"` Interactive bool `short:"i"` @@ -56,6 +57,7 @@ var execDescs = map[string]string{ "gid": "Group ID to run command as", "group": "Group name to run command as (group's GID must match gid if both present)", "timeout": "Timeout after which to terminate command", + "context": "Inherit the context of the named service (overridden by -w, --env, --uid/user, --gid/group)", "t": "Allocate remote pseudo-terminal and connect stdout to it (default if stdout is a TTY)", "T": "Disable remote pseudo-terminal allocation", "i": "Interactive mode: connect stdin to the pseudo-terminal (default if stdin and stdout are TTYs)", @@ -143,21 +145,22 @@ func (cmd *cmdExec) Execute(args []string) error { } opts := &client.ExecOptions{ - Command: command, - Environment: env, - WorkingDir: cmd.WorkingDir, - Timeout: cmd.Timeout, - UserID: cmd.UserID, - User: cmd.User, - GroupID: cmd.GroupID, - Group: cmd.Group, - Terminal: terminal, - Interactive: interactive, - Width: width, - Height: height, - Stdin: Stdin, - Stdout: Stdout, - Stderr: Stderr, + Command: command, + ServiceContext: cmd.Context, + Environment: env, + WorkingDir: cmd.WorkingDir, + Timeout: cmd.Timeout, + UserID: cmd.UserID, + User: cmd.User, + GroupID: cmd.GroupID, + Group: cmd.Group, + Terminal: terminal, + Interactive: interactive, + Width: width, + Height: height, + Stdin: Stdin, + Stdout: Stdout, + Stderr: Stderr, } // If stdout and stderr both refer to the same file or device (e.g., diff --git a/internals/cli/format_test.go b/internals/cli/format_test.go index 3b92d37a2..b6c6c5ea3 100644 --- a/internals/cli/format_test.go +++ b/internals/cli/format_test.go @@ -18,9 +18,6 @@ import ( "os" "runtime" - // "fmt" - // "net/http" - "gopkg.in/check.v1" "github.com/canonical/pebble/internals/cli" diff --git a/internals/daemon/api_changes_test.go b/internals/daemon/api_changes_test.go index 0f5f6228d..c06015de2 100644 --- a/internals/daemon/api_changes_test.go +++ b/internals/daemon/api_changes_test.go @@ -23,9 +23,9 @@ import ( "net/http/httptest" "time" - "github.com/canonical/pebble/internals/overlord/state" - "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/state" ) func setupChanges(st *state.State) []string { diff --git a/internals/daemon/api_exec.go b/internals/daemon/api_exec.go index 41c9d189a..eab7bd43d 100644 --- a/internals/daemon/api_exec.go +++ b/internals/daemon/api_exec.go @@ -24,22 +24,24 @@ import ( "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/overlord/cmdstate" "github.com/canonical/pebble/internals/overlord/state" + "github.com/canonical/pebble/internals/plan" ) type execPayload struct { - Command []string `json:"command"` - Environment map[string]string `json:"environment"` - WorkingDir string `json:"working-dir"` - Timeout string `json:"timeout"` - UserID *int `json:"user-id"` - User string `json:"user"` - GroupID *int `json:"group-id"` - Group string `json:"group"` - Terminal bool `json:"terminal"` - Interactive bool `json:"interactive"` - SplitStderr bool `json:"split-stderr"` - Width int `json:"width"` - Height int `json:"height"` + Command []string `json:"command"` + ServiceContext string `json:"service-context"` + Environment map[string]string `json:"environment"` + WorkingDir string `json:"working-dir"` + Timeout string `json:"timeout"` + UserID *int `json:"user-id"` + User string `json:"user"` + GroupID *int `json:"group-id"` + Group string `json:"group"` + Terminal bool `json:"terminal"` + Interactive bool `json:"interactive"` + SplitStderr bool `json:"split-stderr"` + Width int `json:"width"` + Height int `json:"height"` } func v1PostExec(c *Command, req *http.Request, _ *userState) Response { @@ -63,12 +65,42 @@ func v1PostExec(c *Command, req *http.Request, _ *userState) Response { // Check up-front that the executable exists. _, err := exec.LookPath(payload.Command[0]) + if err != nil { + return statusBadRequest("cannot find executable %q", payload.Command[0]) + } + + // Also check that the working directory exists, to avoid a confusing + // error message later that implies the command doesn't exist: + // + // fork/exec /usr/local/bin/realcommand: no such file or directory + // + // Note that this check still doesn't check that the permissions are + // correct to use it as a working directory, but this is a good start. + if payload.WorkingDir != "" { + if !osutil.IsDir(payload.WorkingDir) { + return statusBadRequest("cannot find working directory %q", payload.WorkingDir) + } + } + + p, err := c.d.overlord.ServiceManager().Plan() + if err != nil { + return statusBadRequest("%v", err) + } + overrides := plan.ContextOptions{ + Environment: payload.Environment, + UserID: payload.UserID, + User: payload.User, + GroupID: payload.GroupID, + Group: payload.Group, + WorkingDir: payload.WorkingDir, + } + merged, err := plan.MergeServiceContext(p, payload.ServiceContext, overrides) if err != nil { return statusBadRequest("%v", err) } // Convert User/UserID and Group/GroupID combinations into raw uid/gid. - uid, gid, err := osutil.NormalizeUidGid(payload.UserID, payload.GroupID, payload.User, payload.Group) + uid, gid, err := osutil.NormalizeUidGid(merged.UserID, merged.GroupID, merged.User, merged.Group) if err != nil { return statusBadRequest("%v", err) } @@ -79,8 +111,8 @@ func v1PostExec(c *Command, req *http.Request, _ *userState) Response { args := &cmdstate.ExecArgs{ Command: payload.Command, - Environment: payload.Environment, - WorkingDir: payload.WorkingDir, + Environment: merged.Environment, + WorkingDir: merged.WorkingDir, Timeout: timeout, UserID: uid, GroupID: gid, diff --git a/internals/daemon/api_exec_test.go b/internals/daemon/api_exec_test.go index bba0ea025..db38ed728 100644 --- a/internals/daemon/api_exec_test.go +++ b/internals/daemon/api_exec_test.go @@ -31,6 +31,7 @@ import ( "github.com/canonical/pebble/client" "github.com/canonical/pebble/internals/logger" + "github.com/canonical/pebble/internals/plan" ) var _ = Suite(&execSuite{}) @@ -164,6 +165,54 @@ func (s *execSuite) TestTimeout(c *C) { c.Check(stderr, Equals, "") } +func (s *execSuite) TestContextNoOverrides(c *C) { + dir := c.MkDir() + err := s.daemon.overlord.ServiceManager().AppendLayer(&plan.Layer{ + Label: "layer1", + Services: map[string]*plan.Service{"svc1": { + Name: "svc1", + Override: "replace", + Command: "dummy", + Environment: map[string]string{"FOO": "foo", "BAR": "bar"}, + WorkingDir: dir, + }}, + }) + c.Assert(err, IsNil) + + stdout, stderr, err := s.exec(c, "", &client.ExecOptions{ + Command: []string{"/bin/sh", "-c", "echo FOO=$FOO BAR=$BAR; pwd"}, + ServiceContext: "svc1", + }) + c.Assert(err, IsNil) + c.Check(stdout, Equals, "FOO=foo BAR=bar\n"+dir+"\n") + c.Check(stderr, Equals, "") +} + +func (s *execSuite) TestContextOverrides(c *C) { + err := s.daemon.overlord.ServiceManager().AppendLayer(&plan.Layer{ + Label: "layer1", + Services: map[string]*plan.Service{"svc1": { + Name: "svc1", + Override: "replace", + Command: "dummy", + Environment: map[string]string{"FOO": "foo", "BAR": "bar"}, + WorkingDir: c.MkDir(), + }}, + }) + c.Assert(err, IsNil) + + overrideDir := c.MkDir() + stdout, stderr, err := s.exec(c, "", &client.ExecOptions{ + Command: []string{"/bin/sh", "-c", "echo FOO=$FOO BAR=$BAR; pwd"}, + ServiceContext: "svc1", + Environment: map[string]string{"FOO": "oof"}, + WorkingDir: overrideDir, + }) + c.Assert(err, IsNil) + c.Check(stdout, Equals, "FOO=oof BAR=bar\n"+overrideDir+"\n") + c.Check(stderr, Equals, "") +} + func (s *execSuite) TestCurrentUserGroup(c *C) { current, err := user.Current() c.Assert(err, IsNil) @@ -336,7 +385,7 @@ func (s *execSuite) TestCommandNotFound(c *C) { c.Check(httpResp.StatusCode, Equals, http.StatusBadRequest) c.Check(execResp.StatusCode, Equals, http.StatusBadRequest) c.Check(execResp.Type, Equals, "error") - c.Check(execResp.Result["message"], Matches, ".*executable file not found.*") + c.Check(execResp.Result["message"], Matches, "cannot find executable .*") } func (s *execSuite) TestUserGroupError(c *C) { diff --git a/internals/daemon/api_files_test.go b/internals/daemon/api_files_test.go index ab41554da..3abbb7dfc 100644 --- a/internals/daemon/api_files_test.go +++ b/internals/daemon/api_files_test.go @@ -32,9 +32,10 @@ import ( "syscall" "time" + . "gopkg.in/check.v1" + "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/osutil/sys" - . "gopkg.in/check.v1" ) var _ = Suite(&filesSuite{}) diff --git a/internals/daemon/api_services_test.go b/internals/daemon/api_services_test.go index e05017122..09ba46fcf 100644 --- a/internals/daemon/api_services_test.go +++ b/internals/daemon/api_services_test.go @@ -25,9 +25,9 @@ import ( "strings" "time" - "github.com/canonical/pebble/internals/overlord/state" - . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/state" ) var servicesLayer = ` diff --git a/internals/daemon/api_warnings_test.go b/internals/daemon/api_warnings_test.go index e8318f60b..6d97a60f3 100644 --- a/internals/daemon/api_warnings_test.go +++ b/internals/daemon/api_warnings_test.go @@ -21,9 +21,9 @@ import ( "net/url" "time" - "github.com/canonical/pebble/internals/overlord/state" - "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/state" ) func (s *apiSuite) testWarnings(c *check.C, all bool, body io.Reader) (calls string, result interface{}) { diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go index b7405ee06..e85fdd02c 100644 --- a/internals/daemon/daemon.go +++ b/internals/daemon/daemon.go @@ -31,9 +31,8 @@ import ( "syscall" "time" - "gopkg.in/tomb.v2" - "github.com/gorilla/mux" + "gopkg.in/tomb.v2" "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/osutil" diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go index bff34730c..996e5d42f 100644 --- a/internals/daemon/daemon_test.go +++ b/internals/daemon/daemon_test.go @@ -30,10 +30,6 @@ import ( "time" "github.com/gorilla/mux" - - "gopkg.in/check.v1" - - // XXX Delete import above and make this file like the other ones. . "gopkg.in/check.v1" "github.com/canonical/pebble/internals/osutil" @@ -47,7 +43,7 @@ import ( ) // Hook up check.v1 into the "go test" runner -func Test(t *testing.T) { check.TestingT(t) } +func Test(t *testing.T) { TestingT(t) } type daemonSuite struct { pebbleDir string @@ -60,7 +56,7 @@ type daemonSuite struct { restoreBackends func() } -var _ = check.Suite(&daemonSuite{}) +var _ = Suite(&daemonSuite{}) func (s *daemonSuite) SetUpTest(c *check.C) { err := reaper.Start() @@ -73,7 +69,7 @@ func (s *daemonSuite) SetUpTest(c *check.C) { } } -func (s *daemonSuite) TearDownTest(c *check.C) { +func (s *daemonSuite) TearDownTest(c *C) { systemdSdNotify = systemd.SdNotify s.notified = nil s.authorized = false @@ -82,13 +78,13 @@ func (s *daemonSuite) TearDownTest(c *check.C) { c.Assert(err, check.IsNil) } -func (s *daemonSuite) newDaemon(c *check.C) *Daemon { +func (s *daemonSuite) newDaemon(c *C) *Daemon { d, err := New(&Options{ Dir: s.pebbleDir, SocketPath: s.socketPath, HTTPAddress: s.httpAddress, }) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) d.addRoutes() return d } @@ -120,13 +116,13 @@ func (s *daemonSuite) TestExplicitPaths(c *C) { c.Assert(info.Mode(), Equals, os.ModeSocket|0666) } -func (s *daemonSuite) TestCommandMethodDispatch(c *check.C) { +func (s *daemonSuite) TestCommandMethodDispatch(c *C) { fakeUserAgent := "some-agent-talking-to-pebble/1.0" cmd := &Command{d: s.newDaemon(c)} handler := &fakeHandler{cmd: cmd} rf := func(innerCmd *Command, req *http.Request, user *userState) Response { - c.Assert(cmd, check.Equals, innerCmd) + c.Assert(cmd, Equals, innerCmd) return handler } cmd.GET = rf @@ -137,30 +133,30 @@ func (s *daemonSuite) TestCommandMethodDispatch(c *check.C) { for _, method := range []string{"GET", "POST", "PUT", "DELETE"} { req, err := http.NewRequest(method, "", nil) req.Header.Add("User-Agent", fakeUserAgent) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) rec := httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 401, check.Commentf(method)) + c.Check(rec.Code, Equals, 401, Commentf(method)) rec = httptest.NewRecorder() req.RemoteAddr = "pid=100;uid=0;socket=;" cmd.ServeHTTP(rec, req) - c.Check(handler.lastMethod, check.Equals, method) - c.Check(rec.Code, check.Equals, 200) + c.Check(handler.lastMethod, Equals, method) + c.Check(rec.Code, Equals, 200) } req, err := http.NewRequest("POTATO", "", nil) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) req.RemoteAddr = "pid=100;uid=0;socket=;" rec := httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 405) + c.Check(rec.Code, Equals, 405) } -func (s *daemonSuite) TestCommandRestartingState(c *check.C) { +func (s *daemonSuite) TestCommandRestartingState(c *C) { d := s.newDaemon(c) cmd := &Command{d: d} @@ -168,18 +164,18 @@ func (s *daemonSuite) TestCommandRestartingState(c *check.C) { return SyncResponse(nil) } req, err := http.NewRequest("GET", "", nil) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) req.RemoteAddr = "pid=100;uid=0;socket=;" rec := httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) var rst struct { Maintenance *errorResult `json:"maintenance"` } err = json.Unmarshal(rec.Body.Bytes(), &rst) - c.Assert(err, check.IsNil) - c.Check(rst.Maintenance, check.IsNil) + c.Assert(err, IsNil) + c.Check(rst.Maintenance, IsNil) state := d.overlord.State() @@ -188,10 +184,10 @@ func (s *daemonSuite) TestCommandRestartingState(c *check.C) { state.Unlock() rec = httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) err = json.Unmarshal(rec.Body.Bytes(), &rst) - c.Assert(err, check.IsNil) - c.Check(rst.Maintenance, check.DeepEquals, &errorResult{ + c.Assert(err, IsNil) + c.Check(rst.Maintenance, DeepEquals, &errorResult{ Kind: errorKindSystemRestart, Message: "system is restarting", }) @@ -201,16 +197,16 @@ func (s *daemonSuite) TestCommandRestartingState(c *check.C) { state.Unlock() rec = httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) err = json.Unmarshal(rec.Body.Bytes(), &rst) - c.Assert(err, check.IsNil) - c.Check(rst.Maintenance, check.DeepEquals, &errorResult{ + c.Assert(err, IsNil) + c.Check(rst.Maintenance, DeepEquals, &errorResult{ Kind: errorKindDaemonRestart, Message: "daemon is restarting", }) } -func (s *daemonSuite) TestFillsWarnings(c *check.C) { +func (s *daemonSuite) TestFillsWarnings(c *C) { d := s.newDaemon(c) cmd := &Command{d: d} @@ -218,20 +214,20 @@ func (s *daemonSuite) TestFillsWarnings(c *check.C) { return SyncResponse(nil) } req, err := http.NewRequest("GET", "", nil) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) req.RemoteAddr = "pid=100;uid=0;socket=;" rec := httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) var rst struct { WarningTimestamp *time.Time `json:"warning-timestamp,omitempty"` WarningCount int `json:"warning-count,omitempty"` } err = json.Unmarshal(rec.Body.Bytes(), &rst) - c.Assert(err, check.IsNil) - c.Check(rst.WarningCount, check.Equals, 0) - c.Check(rst.WarningTimestamp, check.IsNil) + c.Assert(err, IsNil) + c.Check(rst.WarningCount, Equals, 0) + c.Check(rst.WarningTimestamp, IsNil) st := d.overlord.State() st.Lock() @@ -240,14 +236,14 @@ func (s *daemonSuite) TestFillsWarnings(c *check.C) { rec = httptest.NewRecorder() cmd.ServeHTTP(rec, req) - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) err = json.Unmarshal(rec.Body.Bytes(), &rst) - c.Assert(err, check.IsNil) - c.Check(rst.WarningCount, check.Equals, 1) - c.Check(rst.WarningTimestamp, check.NotNil) + c.Assert(err, IsNil) + c.Check(rst.WarningCount, Equals, 1) + c.Check(rst.WarningTimestamp, NotNil) } -func (s *daemonSuite) TestGuestAccess(c *check.C) { +func (s *daemonSuite) TestGuestAccess(c *C) { d := s.newDaemon(c) get := &http.Request{Method: "GET"} @@ -256,31 +252,31 @@ func (s *daemonSuite) TestGuestAccess(c *check.C) { del := &http.Request{Method: "DELETE"} cmd := &Command{d: d} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(del, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized) cmd = &Command{d: d, AdminOnly: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(del, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized) cmd = &Command{d: d, UserOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(del, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized) cmd = &Command{d: d, GuestOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(del, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(pst, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(del, nil), Equals, accessUnauthorized) } -func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithUser(c *check.C) { +func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithUser(c *C) { d := s.newDaemon(c) remoteAddr := "pid=100;uid=1000;socket=" + d.untrustedSocketPath + ";" @@ -290,13 +286,13 @@ func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithUser(c *check.C) { del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr} cmd := &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(del, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) + c.Check(cmd.canAccess(pst, nil), Equals, accessOK) + c.Check(cmd.canAccess(del, nil), Equals, accessOK) } -func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithRoot(c *check.C) { +func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithRoot(c *C) { d := s.newDaemon(c) remoteAddr := "pid=100;uid=0;socket=" + d.untrustedSocketPath + ";" @@ -306,43 +302,43 @@ func (s *daemonSuite) TestUntrustedAccessUntrustedOKWithRoot(c *check.C) { del := &http.Request{Method: "DELETE", RemoteAddr: remoteAddr} cmd := &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(pst, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(del, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) + c.Check(cmd.canAccess(pst, nil), Equals, accessOK) + c.Check(cmd.canAccess(del, nil), Equals, accessOK) } -func (s *daemonSuite) TestUserAccess(c *check.C) { +func (s *daemonSuite) TestUserAccess(c *C) { d := s.newDaemon(c) get := &http.Request{Method: "GET", RemoteAddr: "pid=100;uid=42;socket=;"} put := &http.Request{Method: "PUT", RemoteAddr: "pid=100;uid=42;socket=;"} cmd := &Command{d: d} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) cmd = &Command{d: d, AdminOnly: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) cmd = &Command{d: d, UserOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) cmd = &Command{d: d, GuestOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) // Since this request has a RemoteAddr, it must be coming from the pebble server // socket instead of the pebble one. In that case, UntrustedOK should have no // bearing on the default behavior, which is to deny access. cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, nil), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, nil), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, nil), Equals, accessUnauthorized) } -func (s *daemonSuite) TestLoggedInUserAccess(c *check.C) { +func (s *daemonSuite) TestLoggedInUserAccess(c *C) { d := s.newDaemon(c) user := &userState{} @@ -350,27 +346,27 @@ func (s *daemonSuite) TestLoggedInUserAccess(c *check.C) { put := &http.Request{Method: "PUT", RemoteAddr: "pid=100;uid=42;socket=;"} cmd := &Command{d: d} - c.Check(cmd.canAccess(get, user), check.Equals, accessOK) - c.Check(cmd.canAccess(put, user), check.Equals, accessOK) + c.Check(cmd.canAccess(get, user), Equals, accessOK) + c.Check(cmd.canAccess(put, user), Equals, accessOK) cmd = &Command{d: d, AdminOnly: true} - c.Check(cmd.canAccess(get, user), check.Equals, accessUnauthorized) - c.Check(cmd.canAccess(put, user), check.Equals, accessUnauthorized) + c.Check(cmd.canAccess(get, user), Equals, accessUnauthorized) + c.Check(cmd.canAccess(put, user), Equals, accessUnauthorized) cmd = &Command{d: d, UserOK: true} - c.Check(cmd.canAccess(get, user), check.Equals, accessOK) - c.Check(cmd.canAccess(put, user), check.Equals, accessOK) + c.Check(cmd.canAccess(get, user), Equals, accessOK) + c.Check(cmd.canAccess(put, user), Equals, accessOK) cmd = &Command{d: d, GuestOK: true} - c.Check(cmd.canAccess(get, user), check.Equals, accessOK) - c.Check(cmd.canAccess(put, user), check.Equals, accessOK) + c.Check(cmd.canAccess(get, user), Equals, accessOK) + c.Check(cmd.canAccess(put, user), Equals, accessOK) cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, user), check.Equals, accessOK) - c.Check(cmd.canAccess(put, user), check.Equals, accessOK) + c.Check(cmd.canAccess(get, user), Equals, accessOK) + c.Check(cmd.canAccess(put, user), Equals, accessOK) } -func (s *daemonSuite) TestSuperAccess(c *check.C) { +func (s *daemonSuite) TestSuperAccess(c *C) { d := s.newDaemon(c) for _, uid := range []int{0, os.Getuid()} { @@ -379,28 +375,28 @@ func (s *daemonSuite) TestSuperAccess(c *check.C) { put := &http.Request{Method: "PUT", RemoteAddr: remoteAddr} cmd := &Command{d: d} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) cmd = &Command{d: d, AdminOnly: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) cmd = &Command{d: d, UserOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) cmd = &Command{d: d, GuestOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) cmd = &Command{d: d, UntrustedOK: true} - c.Check(cmd.canAccess(get, nil), check.Equals, accessOK) - c.Check(cmd.canAccess(put, nil), check.Equals, accessOK) + c.Check(cmd.canAccess(get, nil), Equals, accessOK) + c.Check(cmd.canAccess(put, nil), Equals, accessOK) } } -func (s *daemonSuite) TestAddRoutes(c *check.C) { +func (s *daemonSuite) TestAddRoutes(c *C) { d := s.newDaemon(c) expected := make([]string, len(api)) @@ -416,9 +412,9 @@ func (s *daemonSuite) TestAddRoutes(c *check.C) { c.Assert(d.router.Walk(func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error { got = append(got, route.GetName()) return nil - }), check.IsNil) + }), IsNil) - c.Check(got, check.DeepEquals, expected) // this'll stop being true if routes are added that aren't commands (e.g. for the favicon) + c.Check(got, DeepEquals, expected) // this'll stop being true if routes are added that aren't commands (e.g. for the favicon) } type witnessAcceptListener struct { @@ -450,13 +446,13 @@ func (l *witnessAcceptListener) Close() error { return l.closeErr } -func (s *daemonSuite) TestStartStop(c *check.C) { +func (s *daemonSuite) TestStartStop(c *C) { d := s.newDaemon(c) l1, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) l2, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l1, accept: generalAccept} @@ -490,14 +486,14 @@ func (s *daemonSuite) TestStartStop(c *check.C) { <-untrustedDone err = d.Stop(nil) - c.Check(err, check.IsNil) + c.Check(err, IsNil) } -func (s *daemonSuite) TestRestartWiring(c *check.C) { +func (s *daemonSuite) TestRestartWiring(c *C) { d := s.newDaemon(c) l, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l, accept: generalAccept} @@ -543,7 +539,7 @@ func (s *daemonSuite) TestRestartWiring(c *check.C) { } } -func (s *daemonSuite) TestGracefulStop(c *check.C) { +func (s *daemonSuite) TestGracefulStop(c *C) { d := s.newDaemon(c) responding := make(chan struct{}) @@ -560,10 +556,10 @@ func (s *daemonSuite) TestGracefulStop(c *check.C) { }) generalL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) untrustedL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) generalAccept := make(chan struct{}) generalClosed := make(chan struct{}) @@ -601,12 +597,12 @@ func (s *daemonSuite) TestGracefulStop(c *check.C) { go func() { res, err := http.Get(fmt.Sprintf("http://%s/endp", generalL.Addr())) - c.Assert(err, check.IsNil) - c.Check(res.StatusCode, check.Equals, 200) + c.Assert(err, IsNil) + c.Check(res.StatusCode, Equals, 200) body, err := ioutil.ReadAll(res.Body) res.Body.Close() - c.Assert(err, check.IsNil) - c.Check(string(body), check.Equals, "OKOK") + c.Assert(err, IsNil) + c.Check(string(body), Equals, "OKOK") close(alright) }() go func() { @@ -618,7 +614,7 @@ func (s *daemonSuite) TestGracefulStop(c *check.C) { <-responding err = d.Stop(nil) doRespond <- false - c.Check(err, check.IsNil) + c.Check(err, IsNil) select { case <-alright: @@ -627,11 +623,11 @@ func (s *daemonSuite) TestGracefulStop(c *check.C) { } } -func (s *daemonSuite) TestRestartSystemWiring(c *check.C) { +func (s *daemonSuite) TestRestartSystemWiring(c *C) { d := s.newDaemon(c) l, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) generalAccept := make(chan struct{}) d.generalListener = &witnessAcceptListener{Listener: l, accept: generalAccept} @@ -703,33 +699,33 @@ func (s *daemonSuite) TestRestartSystemWiring(c *check.C) { rs := d.restartSystem d.mu.Unlock() - c.Check(rs, check.Equals, true) + c.Check(rs, Equals, true) - c.Check(delays, check.HasLen, 1) - c.Check(delays[0], check.DeepEquals, rebootWaitTimeout) + c.Check(delays, HasLen, 1) + c.Check(delays[0], DeepEquals, rebootWaitTimeout) now := time.Now() err = d.Stop(nil) - c.Check(err, check.ErrorMatches, "expected reboot did not happen") + c.Check(err, ErrorMatches, "expected reboot did not happen") - c.Check(delays, check.HasLen, 2) - c.Check(delays[1], check.DeepEquals, 1*time.Minute) + c.Check(delays, HasLen, 2) + c.Check(delays[1], DeepEquals, 1*time.Minute) // we are not stopping, we wait for the reboot instead - c.Check(s.notified, check.DeepEquals, []string{"READY=1"}) + c.Check(s.notified, DeepEquals, []string{"READY=1"}) st.Lock() defer st.Unlock() var rebootAt time.Time err = st.Get("daemon-system-restart-at", &rebootAt) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) approxAt := now.Add(time.Minute) - c.Check(rebootAt.After(approxAt) || rebootAt.Equal(approxAt), check.Equals, true) + c.Check(rebootAt.After(approxAt) || rebootAt.Equal(approxAt), Equals, true) } -func (s *daemonSuite) TestRebootHelper(c *check.C) { +func (s *daemonSuite) TestRebootHelper(c *C) { cmd := testutil.FakeCommand(c, "shutdown", "", true) defer cmd.Restore() @@ -746,8 +742,8 @@ func (s *daemonSuite) TestRebootHelper(c *check.C) { for _, t := range tests { err := reboot(t.delay) - c.Assert(err, check.IsNil) - c.Check(cmd.Calls(), check.DeepEquals, [][]string{ + c.Assert(err, IsNil) + c.Check(cmd.Calls(), DeepEquals, [][]string{ {"shutdown", "-r", t.delayArg, "reboot scheduled to update the system"}, }) @@ -755,12 +751,12 @@ func (s *daemonSuite) TestRebootHelper(c *check.C) { } } -func makeDaemonListeners(c *check.C, d *Daemon) { +func makeDaemonListeners(c *C, d *Daemon) { generalL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) untrustedL, err := net.Listen("tcp", "127.0.0.1:0") - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) generalAccept := make(chan struct{}) generalClosed := make(chan struct{}) @@ -773,7 +769,7 @@ func makeDaemonListeners(c *check.C, d *Daemon) { // This test tests that when a restart of the system is called // a sigterm (from e.g. systemd) is handled when it arrives before // stop is fully done. -func (s *daemonSuite) TestRestartShutdownWithSigtermInBetween(c *check.C) { +func (s *daemonSuite) TestRestartShutdownWithSigtermInBetween(c *C) { oldRebootNoticeWait := rebootNoticeWait defer func() { rebootNoticeWait = oldRebootNoticeWait @@ -797,12 +793,12 @@ func (s *daemonSuite) TestRestartShutdownWithSigtermInBetween(c *check.C) { ch <- syscall.SIGTERM // stop will check if we got a sigterm in between (which we did) err := d.Stop(ch) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) } // This test tests that when there is a shutdown we close the sigterm // handler so that systemd can kill pebble. -func (s *daemonSuite) TestRestartShutdown(c *check.C) { +func (s *daemonSuite) TestRestartShutdown(c *C) { oldRebootNoticeWait := rebootNoticeWait oldRebootWaitTimeout := rebootWaitTimeout defer func() { @@ -831,16 +827,16 @@ func (s *daemonSuite) TestRestartShutdown(c *check.C) { // ensure that the sigCh got closed as part of the stop _, chOpen := <-sigCh - c.Assert(chOpen, check.Equals, false) + c.Assert(chOpen, Equals, false) } -func (s *daemonSuite) TestRestartExpectedRebootIsMissing(c *check.C) { +func (s *daemonSuite) TestRestartExpectedRebootIsMissing(c *C) { curBootID, err := osutil.BootID() - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) fakeState := []byte(fmt.Sprintf(`{"data":{"patch-level":%d,"patch-sublevel":%d,"some":"data","system-restart-from-boot-id":%q,"daemon-system-restart-at":"%s"},"changes":null,"tasks":null,"last-change-id":0,"last-task-id":0,"last-lane-id":0}`, patch.Level, patch.Sublevel, curBootID, time.Now().UTC().Format(time.RFC3339))) err = ioutil.WriteFile(s.statePath, fakeState, 0600) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) oldRebootNoticeWait := rebootNoticeWait oldRebootRetryWaitTimeout := rebootRetryWaitTimeout @@ -855,19 +851,19 @@ func (s *daemonSuite) TestRestartExpectedRebootIsMissing(c *check.C) { defer cmd.Restore() d := s.newDaemon(c) - c.Check(d.overlord, check.IsNil) - c.Check(d.rebootIsMissing, check.Equals, true) + c.Check(d.overlord, IsNil) + c.Check(d.rebootIsMissing, Equals, true) var n int d.state.Lock() err = d.state.Get("daemon-system-restart-tentative", &n) d.state.Unlock() - c.Check(err, check.IsNil) - c.Check(n, check.Equals, 1) + c.Check(err, IsNil) + c.Check(n, Equals, 1) d.Start() - c.Check(s.notified, check.DeepEquals, []string{"READY=1"}) + c.Check(s.notified, DeepEquals, []string{"READY=1"}) select { case <-d.Dying(): @@ -880,57 +876,57 @@ func (s *daemonSuite) TestRestartExpectedRebootIsMissing(c *check.C) { d.Stop(sigCh) // an immediate shutdown was scheduled again - c.Check(cmd.Calls(), check.DeepEquals, [][]string{ + c.Check(cmd.Calls(), DeepEquals, [][]string{ {"shutdown", "-r", "+0", "reboot scheduled to update the system"}, }) } -func (s *daemonSuite) TestRestartExpectedRebootOK(c *check.C) { +func (s *daemonSuite) TestRestartExpectedRebootOK(c *C) { fakeState := []byte(fmt.Sprintf(`{"data":{"patch-level":%d,"patch-sublevel":%d,"some":"data","system-restart-from-boot-id":%q,"daemon-system-restart-at":"%s"},"changes":null,"tasks":null,"last-change-id":0,"last-task-id":0,"last-lane-id":0}`, patch.Level, patch.Sublevel, "boot-id-0", time.Now().UTC().Format(time.RFC3339))) err := ioutil.WriteFile(s.statePath, fakeState, 0600) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) cmd := testutil.FakeCommand(c, "shutdown", "", true) defer cmd.Restore() d := s.newDaemon(c) - c.Assert(d.overlord, check.NotNil) + c.Assert(d.overlord, NotNil) st := d.overlord.State() st.Lock() defer st.Unlock() var v interface{} // these were cleared - c.Check(st.Get("daemon-system-restart-at", &v), check.Equals, state.ErrNoState) - c.Check(st.Get("system-restart-from-boot-id", &v), check.Equals, state.ErrNoState) + c.Check(st.Get("daemon-system-restart-at", &v), Equals, state.ErrNoState) + c.Check(st.Get("system-restart-from-boot-id", &v), Equals, state.ErrNoState) } -func (s *daemonSuite) TestRestartExpectedRebootGiveUp(c *check.C) { +func (s *daemonSuite) TestRestartExpectedRebootGiveUp(c *C) { // we give up trying to restart the system after 3 retry tentatives curBootID, err := osutil.BootID() - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) fakeState := []byte(fmt.Sprintf(`{"data":{"patch-level":%d,"patch-sublevel":%d,"some":"data","system-restart-from-boot-id":%q,"daemon-system-restart-at":"%s","daemon-system-restart-tentative":3},"changes":null,"tasks":null,"last-change-id":0,"last-task-id":0,"last-lane-id":0}`, patch.Level, patch.Sublevel, curBootID, time.Now().UTC().Format(time.RFC3339))) err = ioutil.WriteFile(s.statePath, fakeState, 0600) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) cmd := testutil.FakeCommand(c, "shutdown", "", true) defer cmd.Restore() d := s.newDaemon(c) - c.Assert(d.overlord, check.NotNil) + c.Assert(d.overlord, NotNil) st := d.overlord.State() st.Lock() defer st.Unlock() var v interface{} // these were cleared - c.Check(st.Get("daemon-system-restart-at", &v), check.Equals, state.ErrNoState) - c.Check(st.Get("system-restart-from-boot-id", &v), check.Equals, state.ErrNoState) - c.Check(st.Get("daemon-system-restart-tentative", &v), check.Equals, state.ErrNoState) + c.Check(st.Get("daemon-system-restart-at", &v), Equals, state.ErrNoState) + c.Check(st.Get("system-restart-from-boot-id", &v), Equals, state.ErrNoState) + c.Check(st.Get("daemon-system-restart-tentative", &v), Equals, state.ErrNoState) } -func (s *daemonSuite) TestRestartIntoSocketModeNoNewChanges(c *check.C) { +func (s *daemonSuite) TestRestartIntoSocketModeNoNewChanges(c *C) { notifySocket := filepath.Join(c.MkDir(), "notify.socket") os.Setenv("NOTIFY_SOCKET", notifySocket) defer os.Setenv("NOTIFY_SOCKET", "") @@ -949,10 +945,10 @@ func (s *daemonSuite) TestRestartIntoSocketModeNoNewChanges(c *check.C) { time.Sleep(5 * time.Millisecond) } - c.Assert(d.standbyOpinions.CanStandby(), check.Equals, false) + c.Assert(d.standbyOpinions.CanStandby(), Equals, false) f, _ := os.Create(notifySocket) f.Close() - c.Assert(d.standbyOpinions.CanStandby(), check.Equals, true) + c.Assert(d.standbyOpinions.CanStandby(), Equals, true) select { case <-d.Dying(): @@ -961,11 +957,11 @@ func (s *daemonSuite) TestRestartIntoSocketModeNoNewChanges(c *check.C) { c.Errorf("daemon did not stop after 15s") } err := d.Stop(nil) - c.Check(err, check.Equals, ErrRestartSocket) - c.Check(d.restartSocket, check.Equals, true) + c.Check(err, Equals, ErrRestartSocket) + c.Check(d.restartSocket, Equals, true) } -func (s *daemonSuite) TestRestartIntoSocketModePendingChanges(c *check.C) { +func (s *daemonSuite) TestRestartIntoSocketModePendingChanges(c *C) { os.Setenv("NOTIFY_SOCKET", c.MkDir()) defer os.Setenv("NOTIFY_SOCKET", "") @@ -998,38 +994,38 @@ func (s *daemonSuite) TestRestartIntoSocketModePendingChanges(c *check.C) { chgStatus := chg.Status() st.Unlock() // ensure our change is valid and ready - c.Check(chgStatus, check.Equals, state.DoStatus) + c.Check(chgStatus, Equals, state.DoStatus) case <-time.After(5 * time.Second): c.Errorf("daemon did not stop after 5s") } // when the daemon got a pending change it just restarts err := d.Stop(nil) - c.Check(err, check.IsNil) - c.Check(d.restartSocket, check.Equals, false) + c.Check(err, IsNil) + c.Check(d.restartSocket, Equals, false) } -func (s *daemonSuite) TestConnTrackerCanShutdown(c *check.C) { +func (s *daemonSuite) TestConnTrackerCanShutdown(c *C) { ct := &connTracker{conns: make(map[net.Conn]struct{})} - c.Check(ct.CanStandby(), check.Equals, true) + c.Check(ct.CanStandby(), Equals, true) con := &net.IPConn{} ct.trackConn(con, http.StateActive) - c.Check(ct.CanStandby(), check.Equals, false) + c.Check(ct.CanStandby(), Equals, false) ct.trackConn(con, http.StateIdle) - c.Check(ct.CanStandby(), check.Equals, true) + c.Check(ct.CanStandby(), Equals, true) } -func doTestReq(c *check.C, cmd *Command, mth string) *httptest.ResponseRecorder { +func doTestReq(c *C, cmd *Command, mth string) *httptest.ResponseRecorder { req, err := http.NewRequest(mth, "", nil) - c.Assert(err, check.IsNil) + c.Assert(err, IsNil) req.RemoteAddr = "pid=100;uid=0;socket=;" rec := httptest.NewRecorder() cmd.ServeHTTP(rec, req) return rec } -func (s *daemonSuite) TestDegradedModeReply(c *check.C) { +func (s *daemonSuite) TestDegradedModeReply(c *C) { d := s.newDaemon(c) cmd := &Command{d: d} cmd.GET = func(*Command, *http.Request, *userState) Response { @@ -1044,22 +1040,22 @@ func (s *daemonSuite) TestDegradedModeReply(c *check.C) { // GET is ok even in degraded mode rec := doTestReq(c, cmd, "GET") - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) // POST is not allowed rec = doTestReq(c, cmd, "POST") - c.Check(rec.Code, check.Equals, 500) + c.Check(rec.Code, Equals, 500) // verify we get the error var v struct{ Result errorResult } - c.Assert(json.NewDecoder(rec.Body).Decode(&v), check.IsNil) - c.Check(v.Result.Message, check.Equals, "foo error") + c.Assert(json.NewDecoder(rec.Body).Decode(&v), IsNil) + c.Check(v.Result.Message, Equals, "foo error") // clean degraded mode d.SetDegradedMode(nil) rec = doTestReq(c, cmd, "POST") - c.Check(rec.Code, check.Equals, 200) + c.Check(rec.Code, Equals, 200) } -func (s *daemonSuite) TestHTTPAPI(c *check.C) { +func (s *daemonSuite) TestHTTPAPI(c *C) { s.httpAddress = ":0" // Go will choose port (use listener.Addr() to find it) d := s.newDaemon(c) d.Init() diff --git a/internals/osutil/bootid_test.go b/internals/osutil/bootid_test.go index 48f5b61bd..5d8f5fb44 100644 --- a/internals/osutil/bootid_test.go +++ b/internals/osutil/bootid_test.go @@ -20,9 +20,9 @@ package osutil_test import ( - "github.com/canonical/pebble/internals/osutil" - . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/osutil" ) type bootIdSuite struct{} diff --git a/internals/osutil/env_test.go b/internals/osutil/env_test.go index a5e05f4bf..5ad3a410d 100644 --- a/internals/osutil/env_test.go +++ b/internals/osutil/env_test.go @@ -15,8 +15,9 @@ package osutil_test import ( - "github.com/canonical/pebble/internals/osutil" . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/osutil" ) type envSuite struct{} diff --git a/internals/overlord/checkstate/manager.go b/internals/overlord/checkstate/manager.go index abbde8e7c..3708559c7 100644 --- a/internals/overlord/checkstate/manager.go +++ b/internals/overlord/checkstate/manager.go @@ -27,6 +27,7 @@ import ( // CheckManager starts and manages the health checks. type CheckManager struct { mutex sync.Mutex + wg sync.WaitGroup checks map[string]*checkData failureHandlers []FailureFunc } @@ -58,6 +59,12 @@ func (m *CheckManager) PlanChanged(p *plan.Plan) { for _, check := range m.checks { check.cancel() } + // Wait for all context cancellations to propagate and allow + // each goroutine to cleanly exit. + m.wg.Wait() + + // Set the size of the next wait group + m.wg.Add(len(p.Checks)) // Then configure and start new checks. checks := make(map[string]*checkData, len(p.Checks)) @@ -65,13 +72,16 @@ func (m *CheckManager) PlanChanged(p *plan.Plan) { ctx, cancel := context.WithCancel(context.Background()) check := &checkData{ config: config, - checker: newChecker(config), + checker: newChecker(config, p), ctx: ctx, cancel: cancel, action: m.callFailureHandlers, } checks[name] = check - go check.loop() + go func() { + defer m.wg.Done() + check.loop() + }() } m.checks = checks } @@ -83,7 +93,7 @@ func (m *CheckManager) callFailureHandlers(name string) { } // newChecker creates a new checker of the configured type. -func newChecker(config *plan.Check) checker { +func newChecker(config *plan.Check, p *plan.Plan) checker { switch { case config.HTTP != nil: return &httpChecker{ @@ -100,15 +110,28 @@ func newChecker(config *plan.Check) checker { } case config.Exec != nil: + overrides := plan.ContextOptions{ + Environment: config.Exec.Environment, + UserID: config.Exec.UserID, + User: config.Exec.User, + GroupID: config.Exec.GroupID, + Group: config.Exec.Group, + WorkingDir: config.Exec.WorkingDir, + } + merged, err := plan.MergeServiceContext(p, config.Exec.ServiceContext, overrides) + if err != nil { + // Context service name has already been checked when plan was loaded. + panic("internal error: " + err.Error()) + } return &execChecker{ name: config.Name, command: config.Exec.Command, - environment: config.Exec.Environment, - userID: config.Exec.UserID, - user: config.Exec.User, - groupID: config.Exec.GroupID, - group: config.Exec.Group, - workingDir: config.Exec.WorkingDir, + environment: merged.Environment, + userID: merged.UserID, + user: merged.User, + groupID: merged.GroupID, + group: merged.Group, + workingDir: merged.WorkingDir, } default: diff --git a/internals/overlord/checkstate/manager_test.go b/internals/overlord/checkstate/manager_test.go index 62ee24abd..8e5cf5895 100644 --- a/internals/overlord/checkstate/manager_test.go +++ b/internals/overlord/checkstate/manager_test.go @@ -45,12 +45,14 @@ func (s *ManagerSuite) SetUpSuite(c *C) { setLoggerOnce.Do(func() { logger.SetLogger(logger.New(os.Stderr, "[test] ")) }) +} +func (s *ManagerSuite) SetUpTest(c *C) { err := reaper.Start() c.Assert(err, IsNil) } -func (s *ManagerSuite) TearDownSuite(c *C) { +func (s *ManagerSuite) TearDownTest(c *C) { err := reaper.Stop() c.Assert(err, IsNil) } @@ -137,7 +139,6 @@ func (s *ManagerSuite) TestTimeout(c *C) { c.Assert(check.Failures, Equals, 1) c.Assert(check.Threshold, Equals, 1) c.Assert(check.LastError, Equals, "exec check timed out") - c.Assert(check.ErrorDetails, Equals, "FOO") } func (s *ManagerSuite) TestCheckCanceled(c *C) { @@ -161,17 +162,15 @@ func (s *ManagerSuite) TestCheckCanceled(c *C) { }, }) - // Wait for command to start (output file grows in size) - prevSize := 0 + // Wait for command to start (output file is not zero in size) for i := 0; ; i++ { if i >= 100 { c.Fatalf("failed waiting for command to start") } b, _ := ioutil.ReadFile(tempFile) - if len(b) != prevSize { + if len(b) > 0 { break } - prevSize = len(b) time.Sleep(time.Millisecond) } @@ -185,7 +184,6 @@ func (s *ManagerSuite) TestCheckCanceled(c *C) { stopChecks(c, mgr) // Ensure command was terminated (output file didn't grow in size) - time.Sleep(50 * time.Millisecond) b1, err := ioutil.ReadFile(tempFile) c.Assert(err, IsNil) time.Sleep(20 * time.Millisecond) @@ -269,8 +267,20 @@ func (s *ManagerSuite) TestFailures(c *C) { c.Assert(failureName, Equals, "") } +// waitCheck is a time based approach to wait for a checker run to complete. +// The timeout value does not impact the general time it takes for tests to +// complete, but determines a worst case waiting period before giving up. +// The timeout value must take into account single core or very busy machines +// so it makes sense to pick a conservative number here as failing a test +// due to a busy test resource is more extensive than waiting a few more +// seconds. func waitCheck(c *C, mgr *CheckManager, name string, f func(check *CheckInfo) bool) *CheckInfo { - for i := 0; i < 100; i++ { + // Worst case waiting time for checker run(s) to complete. This + // period should be much longer (10x is good) than the longest + // check timeout value. + timeout := time.Second * 10 + + for start := time.Now(); time.Since(start) < timeout; { checks, err := mgr.Checks() c.Assert(err, IsNil) for _, check := range checks { @@ -280,6 +290,7 @@ func waitCheck(c *C, mgr *CheckManager, name string, f func(check *CheckInfo) bo } time.Sleep(time.Millisecond) } + c.Fatalf("timed out waiting for check %q", name) return nil } @@ -291,7 +302,7 @@ func (s *CheckersSuite) TestNewChecker(c *C) { URL: "https://example.com/foo", Headers: map[string]string{"k": "v"}, }, - }) + }, nil) http, ok := chk.(*httpChecker) c.Assert(ok, Equals, true) c.Check(http.name, Equals, "http") @@ -304,7 +315,7 @@ func (s *CheckersSuite) TestNewChecker(c *C) { Port: 80, Host: "localhost", }, - }) + }, nil) tcp, ok := chk.(*tcpChecker) c.Assert(ok, Equals, true) c.Check(tcp.name, Equals, "tcp") @@ -323,7 +334,7 @@ func (s *CheckersSuite) TestNewChecker(c *C) { Group: "group", WorkingDir: "/working/dir", }, - }) + }, nil) exec, ok := chk.(*execChecker) c.Assert(ok, Equals, true) c.Assert(exec.name, Equals, "exec") @@ -334,3 +345,70 @@ func (s *CheckersSuite) TestNewChecker(c *C) { c.Assert(exec.groupID, Equals, &groupID) c.Assert(exec.workingDir, Equals, "/working/dir") } + +func (s *CheckersSuite) TestExecContextNoOverride(c *C) { + svcUserID, svcGroupID := 10, 20 + chk := newChecker(&plan.Check{ + Name: "exec", + Exec: &plan.ExecCheck{ + Command: "sleep 1", + ServiceContext: "svc1", + }, + }, &plan.Plan{Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Environment: map[string]string{"k": "x", "a": "1"}, + UserID: &svcUserID, + User: "svcuser", + GroupID: &svcGroupID, + Group: "svcgroup", + WorkingDir: "/working/svc", + }, + }}) + exec, ok := chk.(*execChecker) + c.Assert(ok, Equals, true) + c.Check(exec.name, Equals, "exec") + c.Check(exec.command, Equals, "sleep 1") + c.Check(exec.environment, DeepEquals, map[string]string{"k": "x", "a": "1"}) + c.Check(exec.userID, DeepEquals, &svcUserID) + c.Check(exec.user, Equals, "svcuser") + c.Check(exec.groupID, DeepEquals, &svcGroupID) + c.Check(exec.workingDir, Equals, "/working/svc") +} + +func (s *CheckersSuite) TestExecContextOverride(c *C) { + userID, groupID := 100, 200 + svcUserID, svcGroupID := 10, 20 + chk := newChecker(&plan.Check{ + Name: "exec", + Exec: &plan.ExecCheck{ + Command: "sleep 1", + ServiceContext: "svc1", + Environment: map[string]string{"k": "v"}, + UserID: &userID, + User: "user", + GroupID: &groupID, + Group: "group", + WorkingDir: "/working/dir", + }, + }, &plan.Plan{Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Environment: map[string]string{"k": "x", "a": "1"}, + UserID: &svcUserID, + User: "svcuser", + GroupID: &svcGroupID, + Group: "svcgroup", + WorkingDir: "/working/svc", + }, + }}) + exec, ok := chk.(*execChecker) + c.Assert(ok, Equals, true) + c.Check(exec.name, Equals, "exec") + c.Check(exec.command, Equals, "sleep 1") + c.Check(exec.environment, DeepEquals, map[string]string{"k": "v", "a": "1"}) + c.Check(exec.userID, DeepEquals, &userID) + c.Check(exec.user, Equals, "user") + c.Check(exec.groupID, DeepEquals, &groupID) + c.Check(exec.workingDir, Equals, "/working/dir") +} diff --git a/internals/overlord/patch/patch_test.go b/internals/overlord/patch/patch_test.go index ea858ce5a..afe9fd8db 100644 --- a/internals/overlord/patch/patch_test.go +++ b/internals/overlord/patch/patch_test.go @@ -24,10 +24,10 @@ import ( "sort" "testing" + . "gopkg.in/check.v1" + "github.com/canonical/pebble/internals/overlord/patch" "github.com/canonical/pebble/internals/overlord/state" - - . "gopkg.in/check.v1" ) func Test(t *testing.T) { TestingT(t) } diff --git a/internals/overlord/servstate/handlers.go b/internals/overlord/servstate/handlers.go index 1beb7d502..32348a253 100644 --- a/internals/overlord/servstate/handlers.go +++ b/internals/overlord/servstate/handlers.go @@ -347,6 +347,8 @@ func (s *serviceData) startInternal() error { environment[k] = v } + s.cmd.Dir = s.config.WorkingDir + // Start as another user if specified in plan. uid, gid, err := osutil.NormalizeUidGid(s.config.UserID, s.config.GroupID, s.config.User, s.config.Group) if err != nil { diff --git a/internals/overlord/servstate/manager_test.go b/internals/overlord/servstate/manager_test.go index 58a4d920f..629cf0298 100644 --- a/internals/overlord/servstate/manager_test.go +++ b/internals/overlord/servstate/manager_test.go @@ -1763,3 +1763,62 @@ type fakeLogManager struct{} func (f fakeLogManager) ServiceStarted(serviceName string, logs *servicelog.RingBuffer) { // no-op } + +func (s *S) TestNoWorkingDir(c *C) { + // Service command should run in current directory (package directory) + // if "working-dir" config option not set. + dir := c.MkDir() + err := os.Mkdir(filepath.Join(dir, "layers"), 0755) + c.Assert(err, IsNil) + manager, err := servstate.NewManager(s.st, s.runner, dir, nil, nil, fakeLogManager{}) + c.Assert(err, IsNil) + defer manager.Stop() + + outputPath := filepath.Join(dir, "output") + layer := parseLayer(c, 0, "layer", fmt.Sprintf(` +services: + nowrkdir: + override: replace + command: /bin/sh -c "pwd >%s; sleep %g" +`, outputPath, shortOkayDelay.Seconds()+0.01)) + err = manager.AppendLayer(layer) + c.Assert(err, IsNil) + + chg := s.startServices(c, []string{"nowrkdir"}, 1) + s.st.Lock() + c.Assert(chg.Err(), IsNil) + s.st.Unlock() + + output, err := ioutil.ReadFile(outputPath) + c.Assert(err, IsNil) + c.Check(string(output), Matches, ".*/overlord/servstate\n") +} + +func (s *S) TestWorkingDir(c *C) { + dir := c.MkDir() + err := os.Mkdir(filepath.Join(dir, "layers"), 0755) + c.Assert(err, IsNil) + manager, err := servstate.NewManager(s.st, s.runner, dir, nil, nil, fakeLogManager{}) + c.Assert(err, IsNil) + defer manager.Stop() + + outputPath := filepath.Join(dir, "output") + layer := parseLayer(c, 0, "layer", fmt.Sprintf(` +services: + wrkdir: + override: replace + command: /bin/sh -c "pwd >%s; sleep %g" + working-dir: %s +`, outputPath, shortOkayDelay.Seconds()+0.01, dir)) + err = manager.AppendLayer(layer) + c.Assert(err, IsNil) + + chg := s.startServices(c, []string{"wrkdir"}, 1) + s.st.Lock() + c.Assert(chg.Err(), IsNil) + s.st.Unlock() + + output, err := ioutil.ReadFile(outputPath) + c.Assert(err, IsNil) + c.Check(string(output), Equals, dir+"\n") +} diff --git a/internals/overlord/servstate/request.go b/internals/overlord/servstate/request.go index 0d525a875..49c23f4f3 100644 --- a/internals/overlord/servstate/request.go +++ b/internals/overlord/servstate/request.go @@ -1,9 +1,9 @@ package servstate import ( - "github.com/canonical/pebble/internals/overlord/state" - "fmt" + + "github.com/canonical/pebble/internals/overlord/state" ) // ServiceRequest holds the details required to perform service tasks. diff --git a/internals/overlord/state/change_test.go b/internals/overlord/state/change_test.go index 52b77bfcd..339c94344 100644 --- a/internals/overlord/state/change_test.go +++ b/internals/overlord/state/change_test.go @@ -26,9 +26,9 @@ import ( "strings" "time" - "github.com/canonical/pebble/internals/overlord/state" - . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/overlord/state" ) type changeSuite struct{} diff --git a/internals/plan/plan.go b/internals/plan/plan.go index d142e184a..eccf42a90 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -29,6 +29,7 @@ import ( "github.com/canonical/x-go/strutil/shlex" "gopkg.in/yaml.v3" + "github.com/canonical/pebble/internals/logger" "github.com/canonical/pebble/internals/osutil" ) @@ -79,6 +80,7 @@ type Service struct { User string `yaml:"user,omitempty"` GroupID *int `yaml:"group-id,omitempty"` Group string `yaml:"group,omitempty"` + WorkingDir string `yaml:"working-dir,omitempty"` // Auto-restart and backoff functionality OnSuccess ServiceAction `yaml:"on-success,omitempty"` @@ -88,9 +90,6 @@ type Service struct { BackoffFactor OptionalFloat `yaml:"backoff-factor,omitempty"` BackoffLimit OptionalDuration `yaml:"backoff-limit,omitempty"` KillDelay OptionalDuration `yaml:"kill-delay,omitempty"` - - // Log forwarding - LogTargets []string `yaml:"log-targets,omitempty"` } // Copy returns a deep copy of the service. @@ -106,12 +105,10 @@ func (s *Service) Copy() *Service { } } if s.UserID != nil { - userID := *s.UserID - copied.UserID = &userID + copied.UserID = copyIntPtr(s.UserID) } if s.GroupID != nil { - groupID := *s.GroupID - copied.GroupID = &groupID + copied.GroupID = copyIntPtr(s.GroupID) } if s.OnCheckFailure != nil { copied.OnCheckFailure = make(map[string]ServiceAction) @@ -119,7 +116,6 @@ func (s *Service) Copy() *Service { copied.OnCheckFailure[k] = v } } - copied.LogTargets = append([]string(nil), s.LogTargets...) return &copied } @@ -141,19 +137,20 @@ func (s *Service) Merge(other *Service) { s.KillDelay = other.KillDelay } if other.UserID != nil { - userID := *other.UserID - s.UserID = &userID + s.UserID = copyIntPtr(other.UserID) } if other.User != "" { s.User = other.User } if other.GroupID != nil { - groupID := *other.GroupID - s.GroupID = &groupID + s.GroupID = copyIntPtr(other.GroupID) } if other.Group != "" { s.Group = other.Group } + if other.WorkingDir != "" { + s.WorkingDir = other.WorkingDir + } s.After = append(s.After, other.After...) s.Before = append(s.Before, other.Before...) s.Requires = append(s.Requires, other.Requires...) @@ -184,23 +181,6 @@ func (s *Service) Merge(other *Service) { if other.BackoffLimit.IsSet { s.BackoffLimit = other.BackoffLimit } - s.LogTargets = appendUnique(s.LogTargets, other.LogTargets...) -} - -// appendUnique appends into a the elements from b which are not yet present -// and returns the modified slice. -// TODO: move this function into canonical/x-go/strutil -func appendUnique(a []string, b ...string) []string { -Outer: - for _, bn := range b { - for _, an := range a { - if an == bn { - continue Outer - } - } - a = append(a, bn) - } - return a } // Equal returns true when the two services are equal in value. @@ -271,23 +251,22 @@ func CommandString(base, extra []string) string { } // LogsTo returns true if the logs from s should be forwarded to target t. -// This happens if: -// - t.Selection is "opt-out" or empty, and s.LogTargets is empty; or -// - t.Selection is not "disabled", and s.LogTargets contains t. func (s *Service) LogsTo(t *LogTarget) bool { - if t.Selection == DisabledSelection { - return false - } - if len(s.LogTargets) == 0 { - if t.Selection == UnsetSelection || t.Selection == OptOutSelection { + // Iterate backwards through t.Services until we find something matching + // s.Name. + for i := len(t.Services) - 1; i >= 0; i-- { + switch t.Services[i] { + case s.Name: return true - } - } - for _, targetName := range s.LogTargets { - if targetName == t.Name { + case ("-" + s.Name): + return false + case "all": return true + case "-all": + return false } } + // Nothing matching the service name, so it was not specified. return false } @@ -448,13 +427,14 @@ func (c *TCPCheck) Merge(other *TCPCheck) { // ExecCheck holds the configuration for an exec health check. type ExecCheck struct { - Command string `yaml:"command,omitempty"` - Environment map[string]string `yaml:"environment,omitempty"` - UserID *int `yaml:"user-id,omitempty"` - User string `yaml:"user,omitempty"` - GroupID *int `yaml:"group-id,omitempty"` - Group string `yaml:"group,omitempty"` - WorkingDir string `yaml:"working-dir,omitempty"` + Command string `yaml:"command,omitempty"` + ServiceContext string `yaml:"service-context,omitempty"` + Environment map[string]string `yaml:"environment,omitempty"` + UserID *int `yaml:"user-id,omitempty"` + User string `yaml:"user,omitempty"` + GroupID *int `yaml:"group-id,omitempty"` + Group string `yaml:"group,omitempty"` + WorkingDir string `yaml:"working-dir,omitempty"` } // Copy returns a deep copy of the exec check configuration. @@ -467,12 +447,10 @@ func (c *ExecCheck) Copy() *ExecCheck { } } if c.UserID != nil { - userID := *c.UserID - copied.UserID = &userID + copied.UserID = copyIntPtr(c.UserID) } if c.GroupID != nil { - groupID := *c.GroupID - copied.GroupID = &groupID + copied.GroupID = copyIntPtr(c.GroupID) } return &copied } @@ -482,6 +460,9 @@ func (c *ExecCheck) Merge(other *ExecCheck) { if other.Command != "" { c.Command = other.Command } + if other.ServiceContext != "" { + c.ServiceContext = other.ServiceContext + } for k, v := range other.Environment { if c.Environment == nil { c.Environment = make(map[string]string) @@ -489,15 +470,13 @@ func (c *ExecCheck) Merge(other *ExecCheck) { c.Environment[k] = v } if other.UserID != nil { - userID := *other.UserID - c.UserID = &userID + c.UserID = copyIntPtr(other.UserID) } if other.User != "" { c.User = other.User } if other.GroupID != nil { - groupID := *other.GroupID - c.GroupID = &groupID + c.GroupID = copyIntPtr(other.GroupID) } if other.Group != "" { c.Group = other.Group @@ -509,11 +488,11 @@ func (c *ExecCheck) Merge(other *ExecCheck) { // LogTarget specifies a remote server to forward logs to. type LogTarget struct { - Name string `yaml:"-"` - Type LogTargetType `yaml:"type"` - Location string `yaml:"location"` - Selection Selection `yaml:"selection,omitempty"` - Override Override `yaml:"override,omitempty"` + Name string `yaml:"-"` + Type LogTargetType `yaml:"type"` + Location string `yaml:"location"` + Services []string `yaml:"services"` + Override Override `yaml:"override,omitempty"` } // LogTargetType defines the protocol to use to forward logs. @@ -525,19 +504,10 @@ const ( UnsetLogTarget LogTargetType = "" ) -// Selection describes which services' logs will be forwarded to this target. -type Selection string - -const ( - OptOutSelection Selection = "opt-out" - OptInSelection Selection = "opt-in" - DisabledSelection Selection = "disabled" - UnsetSelection Selection = "" -) - // Copy returns a deep copy of the log target configuration. func (t *LogTarget) Copy() *LogTarget { copied := *t + copied.Services = append([]string(nil), t.Services...) return &copied } @@ -549,9 +519,7 @@ func (t *LogTarget) Merge(other *LogTarget) { if other.Location != "" { t.Location = other.Location } - if other.Selection != "" { - t.Selection = other.Selection - } + t.Services = append(t.Services, other.Services...) } // FormatError is the error returned when a layer has a format error, such as @@ -761,6 +729,13 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Message: fmt.Sprintf("plan check %q command invalid: %v", name, err), } } + _, contextExists := combined.Services[check.Exec.ServiceContext] + if check.Exec.ServiceContext != "" && !contextExists { + return nil, &FormatError{ + Message: fmt.Sprintf("plan check %q service context specifies non-existent service %q", + name, check.Exec.ServiceContext), + } + } _, _, err = osutil.NormalizeUidGid(check.Exec.UserID, check.Exec.GroupID, check.Exec.User, check.Exec.Group) if err != nil { return nil, &FormatError{ @@ -792,35 +767,28 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { } } - switch target.Selection { - case OptOutSelection, OptInSelection, DisabledSelection, UnsetSelection: - // valid, continue - default: + // Validate service names specified in log target + for _, serviceName := range target.Services { + serviceName = strings.TrimPrefix(serviceName, "-") + if serviceName == "all" { + continue + } + if _, ok := combined.Services[serviceName]; ok { + continue + } return nil, &FormatError{ - Message: fmt.Sprintf(`log target %q has invalid selection %q, must be %q, %q or %q`, - name, target.Selection, OptOutSelection, OptInSelection, DisabledSelection), + Message: fmt.Sprintf(`log target %q specifies unknown service %q`, + target.Name, serviceName), } } - if target.Location == "" && target.Selection != DisabledSelection { + if target.Location == "" { return nil, &FormatError{ Message: fmt.Sprintf(`plan must define "location" for log target %q`, name), } } } - // Validate service log targets - for serviceName, service := range combined.Services { - for _, targetName := range service.LogTargets { - _, ok := combined.LogTargets[targetName] - if !ok { - return nil, &FormatError{ - Message: fmt.Sprintf(`unknown log target %q for service %q`, targetName, serviceName), - } - } - } - } - // Ensure combined layers don't have cycles. err := combined.checkCycles() if err != nil { @@ -959,6 +927,15 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { Message: fmt.Sprintf("cannot use reserved service name %q", name), } } + // Deprecated service names + if name == "all" || name == "default" || name == "none" { + logger.Noticef("Using keyword %q as a service name is deprecated", name) + } + if strings.HasPrefix(name, "-") { + return nil, &FormatError{ + Message: fmt.Sprintf(`cannot use service name %q: starting with "-" not allowed`, name), + } + } if service == nil { return nil, &FormatError{ Message: fmt.Sprintf("service object cannot be null for service %q", name), @@ -1101,3 +1078,79 @@ func ReadDir(dir string) (*Plan, error) { } return plan, err } + +// MergeServiceContext merges the overrides on top of the service context +// specified by serviceName, returning a new ContextOptions value. If +// serviceName is "" (context not specified), return overrides directly. +func MergeServiceContext(p *Plan, serviceName string, overrides ContextOptions) (ContextOptions, error) { + if serviceName == "" { + return overrides, nil + } + var service *Service + for _, s := range p.Services { + if s.Name == serviceName { + service = s + break + } + } + if service == nil { + return ContextOptions{}, fmt.Errorf("context service %q not found", serviceName) + } + + // Start with the config values from the context service. + merged := ContextOptions{ + Environment: make(map[string]string), + } + for k, v := range service.Environment { + merged.Environment[k] = v + } + if service.UserID != nil { + merged.UserID = copyIntPtr(service.UserID) + } + merged.User = service.User + if service.GroupID != nil { + merged.GroupID = copyIntPtr(service.GroupID) + } + merged.Group = service.Group + merged.WorkingDir = service.WorkingDir + + // Merge in fields from the overrides, if set. + for k, v := range overrides.Environment { + merged.Environment[k] = v + } + if overrides.UserID != nil { + merged.UserID = copyIntPtr(overrides.UserID) + } + if overrides.User != "" { + merged.User = overrides.User + } + if overrides.GroupID != nil { + merged.GroupID = copyIntPtr(overrides.GroupID) + } + if overrides.Group != "" { + merged.Group = overrides.Group + } + if overrides.WorkingDir != "" { + merged.WorkingDir = overrides.WorkingDir + } + + return merged, nil +} + +// ContextOptions holds service context config fields. +type ContextOptions struct { + Environment map[string]string + UserID *int + User string + GroupID *int + Group string + WorkingDir string +} + +func copyIntPtr(p *int) *int { + if p == nil { + return nil + } + copied := *p + return &copied +} diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index bc76513a5..c9f999d0d 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -107,12 +107,14 @@ var planTests = []planTest{{ backoff-delay: 1s backoff-factor: 1.5 backoff-limit: 10s + working-dir: /workdir/srv1 srv2: override: replace startup: enabled command: cmd before: - srv3 + working-dir: /workdir/srv2 srv3: override: replace command: cmd @@ -131,6 +133,7 @@ var planTests = []planTest{{ - srv4 before: - srv5 + working-dir: /workdir/srv1/override srv2: override: replace startup: disabled @@ -173,16 +176,18 @@ var planTests = []planTest{{ "var0": "val0", "var2": "val2", }, + WorkingDir: "/workdir/srv1", BackoffDelay: plan.OptionalDuration{Value: time.Second, IsSet: true}, BackoffFactor: plan.OptionalFloat{Value: 1.5, IsSet: true}, BackoffLimit: plan.OptionalDuration{Value: 10 * time.Second, IsSet: true}, }, "srv2": { - Name: "srv2", - Override: "replace", - Command: "cmd", - Startup: plan.StartupEnabled, - Before: []string{"srv3"}, + Name: "srv2", + Override: "replace", + Command: "cmd", + WorkingDir: "/workdir/srv2", + Startup: plan.StartupEnabled, + Before: []string{"srv3"}, }, "srv3": { Name: "srv3", @@ -213,6 +218,7 @@ var planTests = []planTest{{ Environment: map[string]string{ "var3": "val3", }, + WorkingDir: "/workdir/srv1/override", }, "srv2": { Name: "srv2", @@ -269,6 +275,7 @@ var planTests = []planTest{{ "var2": "val2", "var3": "val3", }, + WorkingDir: "/workdir/srv1/override", BackoffDelay: plan.OptionalDuration{Value: time.Second, IsSet: true}, BackoffFactor: plan.OptionalFloat{Value: 1.5, IsSet: true}, BackoffLimit: plan.OptionalDuration{Value: 10 * time.Second, IsSet: true}, @@ -816,6 +823,17 @@ var planTests = []planTest{{ exec: command: foo ' `}, +}, { + summary: `Invalid exec check service context`, + error: `plan check "chk1" service context specifies non-existent service "nosvc"`, + input: []string{` + checks: + chk1: + override: replace + exec: + command: foo + service-context: nosvc + `}, }, { summary: "Simple layer with log targets", input: []string{` @@ -824,24 +842,21 @@ var planTests = []planTest{{ command: foo override: merge startup: enabled - log-targets: - - tgt1 svc2: command: bar override: merge startup: enabled - log-targets: - - tgt1 - - tgt2 log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [all] override: merge tgt2: type: syslog location: udp://0.0.0.0:514 + services: [svc2] override: merge `}, result: &plan.Layer{ @@ -854,7 +869,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1"}, }, "svc2": { Name: "svc2", @@ -864,7 +878,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1", "tgt2"}, }, }, Checks: map[string]*plan.Check{}, @@ -873,12 +886,14 @@ var planTests = []planTest{{ Name: "tgt1", Type: plan.LokiTarget, Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, "tgt2": { Name: "tgt2", Type: plan.SyslogTarget, Location: "udp://0.0.0.0:514", + Services: []string{"svc2"}, Override: plan.MergeOverride, }, }, @@ -891,50 +906,50 @@ var planTests = []planTest{{ command: foo override: merge startup: enabled - log-targets: - - tgt1 svc2: command: bar override: merge startup: enabled - log-targets: - - tgt1 - - tgt2 log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [all] override: merge tgt2: type: syslog location: udp://0.0.0.0:514 + services: [svc2] + override: merge + tgt3: + type: loki + location: http://10.1.77.206:3100/loki/api/v1/push + services: [all] override: merge `, ` services: svc1: command: foo override: merge - log-targets: - - tgt3 svc2: command: bar override: replace startup: enabled - log-targets: - - tgt3 log-targets: tgt1: + services: [-all, svc1] override: merge - selection: opt-in tgt2: type: syslog + location: udp://1.2.3.4:514 + services: [] override: replace - selection: disabled tgt3: - type: loki - location: http://10.1.77.206:3100/loki/api/v1/push + type: syslog + location: udp://0.0.0.0:514 + services: [-svc1] override: merge `}, layers: []*plan.Layer{{ @@ -942,18 +957,16 @@ var planTests = []planTest{{ Order: 0, Services: map[string]*plan.Service{ "svc1": { - Name: "svc1", - Command: "foo", - Override: plan.MergeOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt1"}, + Name: "svc1", + Command: "foo", + Override: plan.MergeOverride, + Startup: plan.StartupEnabled, }, "svc2": { - Name: "svc2", - Command: "bar", - Override: plan.MergeOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt1", "tgt2"}, + Name: "svc2", + Command: "bar", + Override: plan.MergeOverride, + Startup: plan.StartupEnabled, }, }, Checks: map[string]*plan.Check{}, @@ -962,12 +975,21 @@ var planTests = []planTest{{ Name: "tgt1", Type: plan.LokiTarget, Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, "tgt2": { Name: "tgt2", Type: plan.SyslogTarget, Location: "udp://0.0.0.0:514", + Services: []string{"svc2"}, + Override: plan.MergeOverride, + }, + "tgt3": { + Name: "tgt3", + Type: plan.LokiTarget, + Location: "http://10.1.77.206:3100/loki/api/v1/push", + Services: []string{"all"}, Override: plan.MergeOverride, }, }, @@ -976,36 +998,36 @@ var planTests = []planTest{{ Order: 1, Services: map[string]*plan.Service{ "svc1": { - Name: "svc1", - Command: "foo", - Override: plan.MergeOverride, - LogTargets: []string{"tgt3"}, + Name: "svc1", + Command: "foo", + Override: plan.MergeOverride, }, "svc2": { - Name: "svc2", - Command: "bar", - Override: plan.ReplaceOverride, - Startup: plan.StartupEnabled, - LogTargets: []string{"tgt3"}, + Name: "svc2", + Command: "bar", + Override: plan.ReplaceOverride, + Startup: plan.StartupEnabled, }, }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{ "tgt1": { - Name: "tgt1", - Override: plan.MergeOverride, - Selection: plan.OptInSelection, + Name: "tgt1", + Services: []string{"-all", "svc1"}, + Override: plan.MergeOverride, }, "tgt2": { - Name: "tgt2", - Type: plan.SyslogTarget, - Override: plan.ReplaceOverride, - Selection: plan.DisabledSelection, + Name: "tgt2", + Type: plan.SyslogTarget, + Location: "udp://1.2.3.4:514", + Services: []string{}, + Override: plan.ReplaceOverride, }, "tgt3": { Name: "tgt3", - Type: plan.LokiTarget, - Location: "http://10.1.77.206:3100/loki/api/v1/push", + Type: plan.SyslogTarget, + Location: "udp://0.0.0.0:514", + Services: []string{"-svc1"}, Override: plan.MergeOverride, }, }, @@ -1020,7 +1042,6 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt1", "tgt3"}, }, "svc2": { Name: "svc2", @@ -1030,28 +1051,28 @@ var planTests = []planTest{{ BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - LogTargets: []string{"tgt3"}, }, }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{ "tgt1": { - Name: "tgt1", - Type: plan.LokiTarget, - Location: "http://10.1.77.196:3100/loki/api/v1/push", - Override: plan.MergeOverride, - Selection: plan.OptInSelection, + Name: "tgt1", + Type: plan.LokiTarget, + Location: "http://10.1.77.196:3100/loki/api/v1/push", + Services: []string{"all", "-all", "svc1"}, + Override: plan.MergeOverride, }, "tgt2": { - Name: "tgt2", - Type: plan.SyslogTarget, - Override: plan.ReplaceOverride, - Selection: plan.DisabledSelection, + Name: "tgt2", + Type: plan.SyslogTarget, + Location: "udp://1.2.3.4:514", + Override: plan.ReplaceOverride, }, "tgt3": { Name: "tgt3", - Type: plan.LokiTarget, - Location: "http://10.1.77.206:3100/loki/api/v1/push", + Type: plan.SyslogTarget, + Location: "udp://0.0.0.0:514", + Services: []string{"all", "-svc1"}, Override: plan.MergeOverride, }, }, @@ -1072,6 +1093,7 @@ var planTests = []planTest{{ log-targets: tgt1: type: loki + services: [all] override: merge `}}, { summary: "Unsupported log target type", @@ -1084,31 +1106,24 @@ var planTests = []planTest{{ override: merge `}, }, { - summary: "Invalid selection for log target", - error: `log target "tgt1" has invalid selection "foobar", must be "opt-out", "opt-in" or "disabled"`, + summary: "Log target specifies invalid service", + error: `log target "tgt1" specifies unknown service "nonexistent"`, input: []string{` log-targets: tgt1: type: loki location: http://10.1.77.196:3100/loki/api/v1/push + services: [nonexistent] override: merge - selection: foobar `}, }, { - summary: "Service specifies unknown log target", - error: `unknown log target "tgt2" for service "svc1"`, + summary: `Service name can't start with "-"`, + error: `cannot use service name "-svc1": starting with "-" not allowed`, input: []string{` services: - svc1: + -svc1: command: foo override: merge - log-targets: - - tgt2 - log-targets: - tgt1: - type: loki - location: http://10.1.77.196:3100/loki/api/v1/push - override: merge `}, }} @@ -1455,41 +1470,164 @@ func (s *S) TestParseCommand(c *C) { } } -func (s *S) TestSelectTargets(c *C) { - logTargets := []*plan.LogTarget{ - {Name: "unset", Selection: plan.UnsetSelection}, - {Name: "optout", Selection: plan.OptOutSelection}, - {Name: "optin", Selection: plan.OptInSelection}, - {Name: "disabled", Selection: plan.DisabledSelection}, - } - services := []*plan.Service{ - {Name: "svc1", LogTargets: nil}, - {Name: "svc2", LogTargets: []string{}}, - {Name: "svc3", LogTargets: []string{"unset"}}, - {Name: "svc4", LogTargets: []string{"optout"}}, - {Name: "svc5", LogTargets: []string{"optin"}}, - {Name: "svc6", LogTargets: []string{"disabled"}}, - {Name: "svc7", LogTargets: []string{"unset", "optin", "disabled"}}, +func (s *S) TestLogsTo(c *C) { + tests := []struct { + services []string + logsTo map[string]bool + }{{ + services: nil, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + }, + }, { + services: []string{}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + }, + }, { + services: []string{"all"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + }, + }, { + services: []string{"svc1"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + }, + }, { + services: []string{"svc1", "svc2"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + "svc3": false, + }, + }, { + services: []string{"all", "-svc2"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + "svc3": true, + }, + }, { + services: []string{"svc1", "svc2", "-svc1", "all"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": true, + "svc3": true, + }, + }, { + services: []string{"svc1", "svc2", "-all"}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + "svc3": false, + }, + }, { + services: []string{"all", "-all"}, + logsTo: map[string]bool{ + "svc1": false, + "svc2": false, + "svc3": false, + }, + }, { + services: []string{"svc1", "svc2", "-all", "svc3", "svc1", "-svc3"}, + logsTo: map[string]bool{ + "svc1": true, + "svc2": false, + "svc3": false, + }, + }} + + for _, test := range tests { + target := &plan.LogTarget{ + Services: test.services, + } + + for serviceName, shouldLogTo := range test.logsTo { + service := &plan.Service{ + Name: serviceName, + } + c.Check(service.LogsTo(target), Equals, shouldLogTo, + Commentf("matching service %q against 'services: %v'", serviceName, test.services)) + } } +} - // Use pointers to bools so the test will fail if we forget to set a value - t, f := true, false - expected := map[string]map[string]*bool{ - "svc1": {"unset": &t, "optout": &t, "optin": &f, "disabled": &f}, - "svc2": {"unset": &t, "optout": &t, "optin": &f, "disabled": &f}, - "svc3": {"unset": &t, "optout": &f, "optin": &f, "disabled": &f}, - "svc4": {"unset": &f, "optout": &t, "optin": &f, "disabled": &f}, - "svc5": {"unset": &f, "optout": &f, "optin": &t, "disabled": &f}, - "svc6": {"unset": &f, "optout": &f, "optin": &f, "disabled": &f}, - "svc7": {"unset": &t, "optout": &f, "optin": &t, "disabled": &f}, +func (s *S) TestMergeServiceContextNoContext(c *C) { + userID, groupID := 10, 20 + overrides := plan.ContextOptions{ + Environment: map[string]string{"x": "y"}, + UserID: &userID, + User: "usr", + GroupID: &groupID, + Group: "grp", + WorkingDir: "/working/dir", } + merged, err := plan.MergeServiceContext(nil, "", overrides) + c.Assert(err, IsNil) + c.Check(merged, DeepEquals, overrides) +} - for _, service := range services { - for _, target := range logTargets { - exp := expected[service.Name][target.Name] - c.Assert(exp, NotNil, Commentf("no expected value defined for %s.LogsTo(%s)", service.Name, target.Name)) - c.Check(service.LogsTo(target), Equals, *exp, - Commentf("unexpected value for %s.LogsTo(%s)", service.Name, target.Name)) - } +func (s *S) TestMergeServiceContextBadService(c *C) { + _, err := plan.MergeServiceContext(&plan.Plan{}, "nosvc", plan.ContextOptions{}) + c.Assert(err, ErrorMatches, `context service "nosvc" not found`) +} + +func (s *S) TestMergeServiceContextNoOverrides(c *C) { + userID, groupID := 11, 22 + p := &plan.Plan{Services: map[string]*plan.Service{"svc1": { + Name: "svc1", + Environment: map[string]string{"x": "y"}, + UserID: &userID, + User: "svcuser", + GroupID: &groupID, + Group: "svcgroup", + WorkingDir: "/working/svc", + }}} + merged, err := plan.MergeServiceContext(p, "svc1", plan.ContextOptions{}) + c.Assert(err, IsNil) + c.Check(merged, DeepEquals, plan.ContextOptions{ + Environment: map[string]string{"x": "y"}, + UserID: &userID, + User: "svcuser", + GroupID: &groupID, + Group: "svcgroup", + WorkingDir: "/working/svc", + }) +} + +func (s *S) TestMergeServiceContextOverrides(c *C) { + svcUserID, svcGroupID := 10, 20 + p := &plan.Plan{Services: map[string]*plan.Service{"svc1": { + Name: "svc1", + Environment: map[string]string{"x": "y", "w": "z"}, + UserID: &svcUserID, + User: "svcuser", + GroupID: &svcGroupID, + Group: "svcgroup", + WorkingDir: "/working/svc", + }}} + userID, groupID := 11, 22 + overrides := plan.ContextOptions{ + Environment: map[string]string{"x": "a"}, + UserID: &userID, + User: "usr", + GroupID: &groupID, + Group: "grp", + WorkingDir: "/working/dir", } + merged, err := plan.MergeServiceContext(p, "svc1", overrides) + c.Assert(err, IsNil) + c.Check(merged, DeepEquals, plan.ContextOptions{ + Environment: map[string]string{"x": "a", "w": "z"}, + UserID: &userID, + User: "usr", + GroupID: &groupID, + Group: "grp", + WorkingDir: "/working/dir", + }) } diff --git a/internals/servicelog/ringbuffer_test.go b/internals/servicelog/ringbuffer_test.go index 08b27eb54..4af940c75 100644 --- a/internals/servicelog/ringbuffer_test.go +++ b/internals/servicelog/ringbuffer_test.go @@ -20,8 +20,9 @@ import ( "io" "testing" - "github.com/canonical/pebble/internals/servicelog" . "gopkg.in/check.v1" + + "github.com/canonical/pebble/internals/servicelog" ) type ringBufferSuite struct{} diff --git a/internals/systemd/systemd_test.go b/internals/systemd/systemd_test.go index f32c363d7..45a211922 100644 --- a/internals/systemd/systemd_test.go +++ b/internals/systemd/systemd_test.go @@ -34,9 +34,8 @@ import ( "github.com/canonical/pebble/internals/osutil" "github.com/canonical/pebble/internals/osutil/squashfs" - "github.com/canonical/pebble/internals/testutil" - "github.com/canonical/pebble/internals/systemd" + "github.com/canonical/pebble/internals/testutil" ) type testreporter struct { diff --git a/internals/testutil/exec.go b/internals/testutil/exec.go index 113f7cebc..86245b0e0 100644 --- a/internals/testutil/exec.go +++ b/internals/testutil/exec.go @@ -92,7 +92,7 @@ type FakeCmd struct { // faking commands that need "\n" in their args (like zenity) // we use the following convention: // - generate \0 to separate args -// - generate \0\0 to separate commands +// - generate \0\f\n\r magic sequence to separate commands var scriptTpl = `#!/bin/bash printf "%%s" "$(basename "$0")" >> %[1]q printf '\0' >> %[1]q @@ -102,7 +102,7 @@ for arg in "$@"; do printf '\0' >> %[1]q done -printf '\0' >> %[1]q +printf '\f\n\r' >> %[1]q %s ` @@ -176,12 +176,11 @@ func (cmd *FakeCmd) Calls() [][]string { if err != nil { panic(err) } - logContent := strings.TrimSuffix(string(raw), "\000") + logContent := strings.TrimSuffix(string(raw), "\000\f\n\r") allCalls := [][]string{} - calls := strings.Split(logContent, "\000\000") + calls := strings.Split(logContent, "\000\f\n\r") for _, call := range calls { - call = strings.TrimSuffix(call, "\000") allCalls = append(allCalls, strings.Split(call, "\000")) } return allCalls diff --git a/internals/testutil/exec_test.go b/internals/testutil/exec_test.go index 2bdffaf85..705bda1be 100644 --- a/internals/testutil/exec_test.go +++ b/internals/testutil/exec_test.go @@ -46,9 +46,15 @@ func (s *fakeCommandSuite) TestFakeCommand(c *check.C) { c.Assert(err, check.IsNil) err = exec.Command("cmd", "second-run", "--arg1", "arg2", "a %s").Run() c.Assert(err, check.IsNil) + err = exec.Command("cmd", "third-run", "--arg1", "arg2", "").Run() + c.Assert(err, check.IsNil) + err = exec.Command("cmd", "forth-run", "--arg1", "arg2", "", "a %s").Run() + c.Assert(err, check.IsNil) c.Assert(fake.Calls(), check.DeepEquals, [][]string{ {"cmd", "first-run", "--arg1", "arg2", "a space"}, {"cmd", "second-run", "--arg1", "arg2", "a %s"}, + {"cmd", "third-run", "--arg1", "arg2", ""}, + {"cmd", "forth-run", "--arg1", "arg2", "", "a %s"}, }) }