Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MySQL storage and tests #5

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ jobs:

- name: Build
run: go build -v ./...

- name: Start test docker containers
if: matrix.platform == 'ubuntu-latest'
run: |
docker-compose -f docker-compose-test.yml up &

- name: Set
if: matrix.platform == 'ubuntu-latest'
run: echo "NANODEP_MYSQL_STORAGE_TEST=1" >> $GITHUB_ENV

- name: Test
run: go test -v -race ./...
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,4 @@ release: \
test:
go test -v -cover -race ./...

.PHONY: my docker $(DEPTOKENS) $(DEPSERVER) $(DEPSYNCER) clean release test
.PHONY: my docker $(DEPTOKENS) $(DEPSERVER) $(DEPSYNCER) clean release test
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Package client implements HTTP privitives for talking with and authenticating with the Apple DEP APIs.
// Package client implements HTTP primitives for talking with and authenticating with the Apple DEP APIs.
package client

import (
Expand Down
2 changes: 1 addition & 1 deletion client/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func GetName(ctx context.Context) string {
}

type AuthTokensRetriever interface {
RetrieveAuthTokens(context.Context, string) (*OAuth1Tokens, error)
RetrieveAuthTokens(ctx context.Context, name string) (*OAuth1Tokens, error)
}

type SessionStore interface {
Expand Down
42 changes: 0 additions & 42 deletions cmd/cli.go

This file was deleted.

6 changes: 3 additions & 3 deletions cmd/depserver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (
"time"

"github.com/micromdm/nanodep/client"
"github.com/micromdm/nanodep/cmd"
dephttp "github.com/micromdm/nanodep/http"
"github.com/micromdm/nanodep/http/api"
"github.com/micromdm/nanodep/log/stdlogfmt"
"github.com/micromdm/nanodep/parse"
"github.com/micromdm/nanodep/proxy"
)

Expand All @@ -37,7 +37,7 @@ func main() {
flListen = flag.String("listen", ":9001", "HTTP listen address")
flAPIKey = flag.String("api", "", "API key for API endpoints")
flVersion = flag.Bool("version", false, "print version")
flStorage = flag.String("storage", "", "storage backend")
flStorage = flag.String("storage", "file", "storage backend")
flDSN = flag.String("storage-dsn", "", "storage data source name")
)
flag.Parse()
Expand All @@ -55,7 +55,7 @@ func main() {

logger := stdlogfmt.New(stdlog.Default(), *flDebug)

storage, err := cmd.ParseStorage(*flStorage, *flDSN)
storage, err := parse.Storage(*flStorage, *flDSN)
if err != nil {
logger.Info("msg", "creating storage backend", "err", err)
os.Exit(1)
Expand Down
6 changes: 3 additions & 3 deletions cmd/depsyncer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
"syscall"
"time"

"github.com/micromdm/nanodep/cmd"
"github.com/micromdm/nanodep/godep"
"github.com/micromdm/nanodep/log/stdlogfmt"
"github.com/micromdm/nanodep/parse"
depsync "github.com/micromdm/nanodep/sync"
)

Expand All @@ -31,7 +31,7 @@ func main() {
flDebug = flag.Bool("debug", false, "log debug messages")
flADebug = flag.Bool("debug-assigner", false, "additional debug logging of the device assigner")
flStorage = flag.String("storage", "", "storage backend")
flDSN = flag.String("storage-dsn", "", "storage data source name")
flDSN = flag.String("storage-dsn", "file", "storage data source name")
lucasmrod marked this conversation as resolved.
Show resolved Hide resolved
flWebhook = flag.String("webhook-url", "", "URL to send requests to")
)
flag.Usage = func() {
Expand All @@ -53,7 +53,7 @@ func main() {

logger := stdlogfmt.New(stdlog.Default(), *flDebug)

storage, err := cmd.ParseStorage(*flStorage, *flDSN)
storage, err := parse.Storage(*flStorage, *flDSN)
if err != nil {
logger.Info("msg", "creating storage backend", "err", err)
os.Exit(1)
Expand Down
23 changes: 23 additions & 0 deletions docker-compose-test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
version: "2"
services:
mysql:
image: ${NANODEP_MYSQL_IMAGE:-mysql:5.7}
platform: ${NANODEP_MYSQL_PLATFORM:-linux/x86_64}
command:
[
"mysqld",
"--datadir=/tmp/mysqldata",
"--log-bin=bin.log",
"--server-id=master-01"
]
environment:
MYSQL_ROOT_PASSWORD: toor
MYSQL_DATABASE: nanodep
MYSQL_USER: nanodep
MYSQL_PASSWORD: insecure
tmpfs:
- /var/lib/mysql:rw,noexec,nosuid
- /tmpfs
ports:
- "4242:3306"
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ go 1.17
require go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352

require github.com/gomodule/oauth1 v0.2.0

require github.com/go-sql-driver/mysql v1.6.0 // indirect
lucasmrod marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfCHuOE=
github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
github.com/gomodule/oauth1 v0.2.0 h1:/nNHAD99yipOEspQFbAnNmwGTZ1UNXiD/+JLxwx79fo=
github.com/gomodule/oauth1 v0.2.0/go.mod h1:4r/a8/3RkhMBxJQWL5qzbOEcaQmNPIkNoI7P8sXeI08=
go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 h1:CCriYyAfq1Br1aIYettdHZTy8mBTIPo7We18TuO/bak=
Expand Down
2 changes: 1 addition & 1 deletion http/api/assigner.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func RetrieveAssignerProfileHandler(store sync.AssignerProfileRetriever, logger
}

type AssignerProfileStorer interface {
StoreAssignerProfile(context.Context, string, string) error
StoreAssignerProfile(ctx context.Context, name string, profileUUID string) error
}

// StoreAssignerProfileHandler saves the assigner profile UUID for the
Expand Down
2 changes: 1 addition & 1 deletion http/api/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func RetrieveConfigHandler(store client.ConfigRetriever, logger log.Logger) http
}

type ConfigStorer interface {
StoreConfig(context.Context, string, *client.Config) error
StoreConfig(ctx context.Context, name string, config *client.Config) error
}

// StoreConfigHandler stores the DEP server config for the DEP
Expand Down
16 changes: 4 additions & 12 deletions http/api/tokenpki.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,18 @@ import (
)

type TokenPKIRetriever interface {
RetrieveTokenPKI(context.Context, string) ([]byte, []byte, error)
RetrieveTokenPKI(ctx context.Context, name string) (pemCert []byte, pemKey []byte, err error)
}

type TokenPKIStorer interface {
StoreTokenPKI(context.Context, string, []byte, []byte) error
StoreTokenPKI(ctx context.Context, name string, pemCert []byte, pemKey []byte) error
}

const (
defaultCN = "depserver"
defaultDays = 1
)

// PEMRSAPrivateKey returns key as a PEM block.
func PEMRSAPrivateKey(key *rsa.PrivateKey) []byte {
block := &pem.Block{
Type: "RSA PRIVATE KEY",
Bytes: x509.MarshalPKCS1PrivateKey(key),
}
return pem.EncodeToMemory(block)
}

// GetCertTokenPKIHandler generates a new private key and certificate for
// the token PKI exchange with the ABM/ASM/BE portal. Every call to this
// handler generates a new keypair and stores it. The PEM-encoded certificate
Expand All @@ -55,14 +46,15 @@ func GetCertTokenPKIHandler(store TokenPKIStorer, logger log.Logger) http.Handle
return
}
logger = logger.With("name", r.URL.Path)
// TODO(lucas): defaultDays is 1.
lucasmrod marked this conversation as resolved.
Show resolved Hide resolved
key, cert, err := tokenpki.SelfSignedRSAKeypair(defaultCN, defaultDays)
if err != nil {
logger.Info("msg", "generating token keypair", "err", err)
jsonError(w, err)
return
}
pemCert := tokenpki.PEMCertificate(cert.Raw)
err = store.StoreTokenPKI(r.Context(), r.URL.Path, pemCert, PEMRSAPrivateKey(key))
err = store.StoreTokenPKI(r.Context(), r.URL.Path, pemCert, tokenpki.PEMRSAPrivateKey(key))
if err != nil {
logger.Info("msg", "storing token keypair", "err", err)
jsonError(w, err)
Expand Down
2 changes: 1 addition & 1 deletion http/api/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type AuthTokensStorer interface {
StoreAuthTokens(context.Context, string, *client.OAuth1Tokens) error
StoreAuthTokens(ctx context.Context, name string, tokens *client.OAuth1Tokens) error
}

// RetrieveAuthTokensHandler returns the DEP server OAuth1 tokens for the DEP
Expand Down
27 changes: 27 additions & 0 deletions parse/storage.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package parse
Copy link
Member

Choose a reason for hiding this comment

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

I like the idea of moving this out of the cmd/ tree. But can we rename this package to just cli to match micromdm/nanomdm@ff8b7af?


import (
"fmt"

"github.com/micromdm/nanodep/storage"
"github.com/micromdm/nanodep/storage/file"
"github.com/micromdm/nanodep/storage/mysql"
)

// Storage parses a storage name and dsn to determine which and return a storage backend.
func Storage(storageName, dsn string) (storage.AllStorage, error) {
var store storage.AllStorage
var err error
switch storageName {
case "file":
if dsn == "" {
dsn = "db"
}
store, err = file.New(dsn)
case "mysql":
lucasmrod marked this conversation as resolved.
Show resolved Hide resolved
store, err = mysql.New(mysql.WithDSN(dsn))
default:
return nil, fmt.Errorf("unknown storage: %q", storageName)
}
return store, err
}
19 changes: 14 additions & 5 deletions storage/file/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/micromdm/nanodep/client"
"github.com/micromdm/nanodep/storage"
)

const defaultFileMode = 0644
Expand All @@ -20,6 +21,8 @@ type FileStorage struct {
path string
}

var _ storage.AllStorage = (*FileStorage)(nil)
Copy link
Member

Choose a reason for hiding this comment

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

Curious what's the reason for this?

Copy link
Contributor Author

@lucasmrod lucasmrod Jul 21, 2022

Choose a reason for hiding this comment

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

Convention that serves both as a "compile time check" and as a form of documentation for code readers/reviewers that FileStorage implements storage.AllStorage interface.

Copy link
Member

Choose a reason for hiding this comment

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

I think this creates an unnecessary coupling. How do you feel about removing it? As Go is statically typed an implementation must conform to it's use. AllStorage isn't particularly useful outside of main.go so I'd not like to couple these.

Let me know what you think. :)


// New creates a new FileStorage backend.
func New(path string) (*FileStorage, error) {
err := os.Mkdir(path, 0755)
Expand Down Expand Up @@ -62,10 +65,14 @@ func (s *FileStorage) tokenpkiFilename(name, kind string) string {
// RetrieveAuthTokens reads the JSON DEP OAuth tokens from disk for name DEP name.
func (s *FileStorage) RetrieveAuthTokens(_ context.Context, name string) (*client.OAuth1Tokens, error) {
tokens := new(client.OAuth1Tokens)
return tokens, decodeJSONfile(s.tokensFilename(name), tokens)
err := decodeJSONfile(s.tokensFilename(name), tokens)
if err != nil {
return nil, err
Copy link
Member

Choose a reason for hiding this comment

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

Any problem with returning a non-nil empty token — most callers are going to check for errors anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A convention I've seen many projects use is to try (when possible) to always return zero-values when error is non-nil. In this case, the zero-value of a pointer is nil.

Copy link
Member

Choose a reason for hiding this comment

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

So I'm fine with this specific code change, although its a bit verbose. But in general I don't think its necessary to add a bunch of boilerplate to assure a zero-values for an error. Discussed here: https://macadmins.slack.com/archives/C07ME1284/p1663867171949419

}
return tokens, nil
}

// RetrieveAuthTokens saves the DEP OAuth tokens to disk as JSON for name DEP name.
// StoreAuthTokens saves the DEP OAuth tokens to disk as JSON for name DEP name.
func (s *FileStorage) StoreAuthTokens(_ context.Context, name string, tokens *client.OAuth1Tokens) error {
f, err := os.Create(s.tokensFilename(name))
if err != nil {
Expand All @@ -85,8 +92,8 @@ func decodeJSONfile(filename string, v interface{}) error {
}

// RetrieveConfig reads the JSON DEP config from disk for name DEP name.
// We return an empty config if the config does not exist on disk.
// This is to support a fallback default config.
//
// Returns an empty config if the config does not exist (to support a fallback default config).
func (s *FileStorage) RetrieveConfig(_ context.Context, name string) (*client.Config, error) {
config := new(client.Config)
err := decodeJSONfile(s.configFilename(name), config)
Expand All @@ -109,6 +116,8 @@ func (s *FileStorage) StoreConfig(_ context.Context, name string, config *client

// RetrieveAssignerProfile reads the assigner profile UUID and its configured
// timestamp from disk for name DEP name.
//
// Returns an empty profile if it does not exist.
func (s *FileStorage) RetrieveAssignerProfile(_ context.Context, name string) (string, time.Time, error) {
profileBytes, err := os.ReadFile(s.profileFilename(name))
if err != nil && errors.Is(err, os.ErrNotExist) {
Expand All @@ -119,7 +128,7 @@ func (s *FileStorage) RetrieveAssignerProfile(_ context.Context, name string) (s
if err == nil {
var stat fs.FileInfo
stat, err = os.Stat(s.profileFilename(name))
if err != nil {
if err == nil {
lucasmrod marked this conversation as resolved.
Show resolved Hide resolved
modTime = stat.ModTime()
}
}
Expand Down
18 changes: 18 additions & 0 deletions storage/file/file_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package file

import (
"testing"

"github.com/micromdm/nanodep/storage"
"github.com/micromdm/nanodep/storage/storagetest"
)

func TestFileStorage(t *testing.T) {
storagetest.Run(t, func(t *testing.T) storage.AllStorage {
s, err := New(t.TempDir())
if err != nil {
t.Fatal(err)
}
return s
})
}
Loading