feat: various minor style changes (#93)

.github/workflows/rock-build.yaml

@@ -68,7 +68,7 @@ jobs:
             --data "{\"certificate\":\"${COMBINED_CERTIFICATE}\"}"
           
           docker exec notary /usr/bin/pebble notices
-          docker exec notary /usr/bin/pebble notices | grep notary\\.com/certificate/update
+          docker exec notary /usr/bin/pebble notices | grep canonical\\.com
           docker exec notary /usr/bin/pebble notice 3
         
       - uses: actions/upload-artifact@v4

cmd/notary/main.go

@@ -1,16 +1,19 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"log"
+	"net/http"
 	"os"
+	"os/signal"
 
 	"github.com/canonical/notary/internal/config"
 	"github.com/canonical/notary/internal/server"
 )
 
 func main() {
-	log.SetOutput(os.Stdout)
+	log.SetOutput(os.Stderr)
 	configFilePtr := flag.String("config", "", "The config file to be provided to the server")
 	flag.Parse()
 	if *configFilePtr == "" {
@@ -24,8 +27,23 @@ func main() {
 	if err != nil {
 		log.Fatalf("Couldn't create server: %s", err)
 	}
+
+	idleConnsClosed := make(chan struct{})
+	go func() {
+		sigint := make(chan os.Signal, 1)
+		signal.Notify(sigint, os.Interrupt)
+		<-sigint
+		log.Println("Interrupt signal received")
+		if err := srv.Shutdown(context.Background()); err != nil {
+			log.Printf("HTTP server Shutdown error: %v", err)
+		}
+		close(idleConnsClosed)
+	}()
+
 	log.Printf("Starting server at %s", srv.Addr)
-	if err := srv.ListenAndServeTLS("", ""); err != nil {
-		log.Fatalf("Server ran into error: %s", err)
+	if err := srv.ListenAndServeTLS("", ""); err != http.ErrServerClosed {
+		log.Fatalf("HTTP server ListenAndServe: %v", err)
 	}
+	log.Printf("Shutting down server")
+	<-idleConnsClosed
 }

internal/config/config.go

@@ -2,17 +2,19 @@ package config
 
 import (
 	"errors"
+	"fmt"
 	"os"
+	"os/exec"
 
 	"gopkg.in/yaml.v3"
 )
 
 type ConfigYAML struct {
-	KeyPath                    string `yaml:"key_path"`
-	CertPath                   string `yaml:"cert_path"`
-	DBPath                     string `yaml:"db_path"`
-	Port                       int    `yaml:"port"`
-	Pebblenotificationsenabled bool   `yaml:"pebble_notifications"`
+	KeyPath             string `yaml:"key_path"`
+	CertPath            string `yaml:"cert_path"`
+	DBPath              string `yaml:"db_path"`
+	Port                int    `yaml:"port"`
+	PebbleNotifications bool   `yaml:"pebble_notifications"`
 }
 
 type Config struct {
@@ -25,49 +27,54 @@ type Config struct {
 
 // Validate opens and processes the given yaml file, and catches errors in the process
 func Validate(filePath string) (Config, error) {
-	validationErr := errors.New("config file validation failed: ")
 	config := Config{}
 	configYaml, err := os.ReadFile(filePath)
 	if err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	c := ConfigYAML{}
 	if err := yaml.Unmarshal(configYaml, &c); err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	if c.CertPath == "" {
-		return config, errors.Join(validationErr, errors.New("`cert_path` is empty"))
+		return Config{}, errors.New("`cert_path` is empty")
 	}
 	cert, err := os.ReadFile(c.CertPath)
 	if err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	if c.KeyPath == "" {
-		return config, errors.Join(validationErr, errors.New("`key_path` is empty"))
+		return Config{}, errors.New("`key_path` is empty")
 	}
 	key, err := os.ReadFile(c.KeyPath)
 	if err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	if c.DBPath == "" {
-		return config, errors.Join(validationErr, errors.New("`db_path` is empty"))
+		return Config{}, errors.New("`db_path` is empty")
 	}
 	dbfile, err := os.OpenFile(c.DBPath, os.O_CREATE|os.O_RDONLY, 0o644)
 	if err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	err = dbfile.Close()
 	if err != nil {
-		return config, errors.Join(validationErr, err)
+		return Config{}, err
 	}
 	if c.Port == 0 {
-		return config, errors.Join(validationErr, errors.New("`port` is empty"))
+		return Config{}, errors.New("`port` is empty")
+	}
+	if c.PebbleNotifications {
+		_, err := exec.LookPath("pebble")
+		if err != nil {
+			return Config{}, fmt.Errorf("pebble binary not found: %w", err)
+		}
 	}
 
 	config.Cert = cert
 	config.Key = key
 	config.DBPath = c.DBPath
 	config.Port = c.Port
-	config.PebbleNotificationsEnabled = c.Pebblenotificationsenabled
+	config.PebbleNotificationsEnabled = c.PebbleNotifications
 	return config, nil
 }

internal/db/db.go

@@ -31,16 +31,16 @@ const (
 const queryCreateUsersTable = `CREATE TABLE IF NOT EXISTS %s (
     user_id INTEGER PRIMARY KEY AUTOINCREMENT,
     username TEXT NOT NULL UNIQUE,
-	password TEXT NOT NULL,
+	hashed_password TEXT NOT NULL,
 	permissions INTEGER
 )`
 
 const (
 	queryGetAllUsers       = "SELECT * FROM %s"
 	queryGetUser           = "SELECT * FROM %s WHERE user_id=?"
 	queryGetUserByUsername = "SELECT * FROM %s WHERE username=?"
-	queryCreateUser        = "INSERT INTO %s (username, password, permissions) VALUES (?, ?, ?)"
-	queryUpdateUser        = "UPDATE %s SET password=? WHERE user_id=?"
+	queryCreateUser        = "INSERT INTO %s (username, hashed_password, permissions) VALUES (?, ?, ?)"
+	queryUpdateUser        = "UPDATE %s SET hashed_password=? WHERE user_id=?"
 	queryDeleteUser        = "DELETE FROM %s WHERE user_id=?"
 	queryGetNumUsers       = "SELECT COUNT(*) FROM %s"
 )
@@ -136,15 +136,11 @@ func (db *Database) UpdateCSR(id string, cert string) (int64, error) {
 		}
 		cert = sanitizeCertificateBundle(cert)
 	}
-	result, err := db.conn.Exec(fmt.Sprintf(queryUpdateCSR, db.certificateTable), cert, csr.ID)
+	_, err = db.conn.Exec(fmt.Sprintf(queryUpdateCSR, db.certificateTable), cert, csr.ID)
 	if err != nil {
 		return 0, err
 	}
-	affectedRows, err := result.RowsAffected()
-	if err != nil {
-		return 0, err
-	}
-	return affectedRows, nil
+	return int64(csr.ID), nil
 }
 
 // DeleteCSR removes a CSR from the database alongside the certificate that may have been generated for it.

internal/metrics/metrics_test.go

@@ -10,7 +10,7 @@ import (
 	"math/big"
 	"net/http"
 	"net/http/httptest"
-	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -110,14 +110,8 @@ func initializeTestDB(t *testing.T, db *db.Database) {
 
 // TestMetrics tests some of the metrics that we currently collect.
 func TestMetrics(t *testing.T) {
-	f, err := os.CreateTemp("./", "*.db")
-	fmt.Print(f.Name())
-	if err != nil {
-		t.Fatal("couldn't create temp db file: " + err.Error())
-	}
-	defer f.Close()
-	defer os.Remove(f.Name())
-	db, err := db.NewDatabase(f.Name())
+	tempDir := t.TempDir()
+	db, err := db.NewDatabase(filepath.Join(tempDir, "db.sqlite3"))
 	if err != nil {
 		t.Fatal(err)
 	}

internal/server/handlers_certificate_requests.go

@@ -190,7 +190,7 @@ func CreateCertificate(env *HandlerConfig) http.HandlerFunc {
 		}
 		insertIdStr := strconv.FormatInt(insertId, 10)
 		if env.SendPebbleNotifications {
-			err := SendPebbleNotification("notary.com/certificate/update", insertIdStr)
+			err := SendPebbleNotification("canonical.com/notary/certificate/update", insertIdStr)
 			if err != nil {
 				log.Printf("pebble notify failed: %s. continuing silently.", err.Error())
 			}
@@ -222,7 +222,7 @@ func RejectCertificate(env *HandlerConfig) http.HandlerFunc {
 		}
 		insertIdStr := strconv.FormatInt(insertId, 10)
 		if env.SendPebbleNotifications {
-			err := SendPebbleNotification("notary.com/certificate/update", insertIdStr)
+			err := SendPebbleNotification("canonical.com/notary/certificate/update", insertIdStr)
 			if err != nil {
 				log.Printf("pebble notify failed: %s. continuing silently.", err.Error())
 			}
@@ -256,7 +256,7 @@ func DeleteCertificate(env *HandlerConfig) http.HandlerFunc {
 		}
 		insertIdStr := strconv.FormatInt(insertId, 10)
 		if env.SendPebbleNotifications {
-			err := SendPebbleNotification("notary.com/certificate/update", insertIdStr)
+			err := SendPebbleNotification("canonical.com/notary/certificate/update", insertIdStr)
 			if err != nil {
 				log.Printf("pebble notify failed: %s. continuing silently.", err.Error())
 			}

internal/server/handlers_login.go

@@ -12,6 +12,10 @@ import (
 	"golang.org/x/crypto/bcrypt"
 )
 
+func expireAfter(hours int) int64 {
+	return time.Now().Add(time.Hour * 1).Unix()
+}
+
 type jwtNotaryClaims struct {
 	ID          int    `json:"id"`
 	Username    string `json:"username"`
@@ -35,7 +39,7 @@ func generateJWT(id int, username string, jwtSecret []byte, permissions int) (st
 		Username:    username,
 		Permissions: permissions,
 		StandardClaims: jwt.StandardClaims{
-			ExpiresAt: time.Now().Add(time.Hour * 1).Unix(),
+			ExpiresAt: expireAfter(1),
 		},
 	})
 	tokenString, err := token.SignedString(jwtSecret)

internal/server/server.go

@@ -4,9 +4,7 @@ package server
 import (
 	"crypto/rand"
 	"crypto/tls"
-	"errors"
 	"fmt"
-	"log"
 	"net/http"
 	"os/exec"
 	"time"
@@ -23,7 +21,7 @@ type HandlerConfig struct {
 func SendPebbleNotification(key, request_id string) error {
 	cmd := exec.Command("pebble", "notify", key, fmt.Sprintf("request_id=%s", request_id))
 	if err := cmd.Run(); err != nil {
-		return errors.Join(errors.New("couldn't execute a pebble notify: "), err)
+		return fmt.Errorf("couldn't execute a pebble notify: %w", err)
 	}
 	return nil
 }
@@ -44,7 +42,7 @@ func New(port int, cert []byte, key []byte, dbPath string, pebbleNotificationsEn
 	}
 	db, err := db.NewDatabase(dbPath)
 	if err != nil {
-		log.Fatalf("Couldn't connect to database: %s", err)
+		return nil, err
 	}
 
 	jwtSecret, err := generateJWTSecret()