Skip to content

Commit

Permalink
addressed all TODOs in the source code
Browse files Browse the repository at this point in the history
Signed-off-by: Volkan Özçelik <[email protected]>
  • Loading branch information
v0lkan committed Nov 16, 2024
1 parent 7fc650b commit acbee73
Show file tree
Hide file tree
Showing 14 changed files with 167 additions and 62 deletions.
10 changes: 2 additions & 8 deletions app/spike/internal/cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/spiffe/go-spiffe/v2/workloadapi"

"github.com/spiffe/spike/app/spike/internal/net"
"github.com/spiffe/spike/app/spike/internal/state"
)

// NewDeleteCommand creates and returns a new cobra.Command for deleting secrets.
Expand Down Expand Up @@ -59,13 +58,8 @@ Examples:
spike delete secret/apocalyptica -v 0,1,2 # Deletes current version plus versions 1 and 2`,
Args: cobra.ExactArgs(1),
Run: func(cmd *cobra.Command, args []string) {
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return
}
adminToken := adminToken()
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return
}

Expand Down Expand Up @@ -94,7 +88,7 @@ Examples:
}
}

err = net.DeleteSecret(source, path, versionList)
err := net.DeleteSecret(source, path, versionList)
if err != nil {
fmt.Printf("Error: %v\n", err)
return
Expand Down
8 changes: 1 addition & 7 deletions app/spike/internal/cmd/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/spiffe/go-spiffe/v2/workloadapi"

"github.com/spiffe/spike/app/spike/internal/net"
"github.com/spiffe/spike/app/spike/internal/state"
)

// NewGetCommand creates and returns a new cobra.Command for retrieving secrets.
Expand Down Expand Up @@ -46,13 +45,8 @@ func NewGetCommand(source *workloadapi.X509Source) *cobra.Command {
Short: "Get secrets from the specified path",
Args: cobra.ExactArgs(1),
Run: func(cmd *cobra.Command, args []string) {
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return
}
adminToken := adminToken()
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return
}

Expand Down
2 changes: 1 addition & 1 deletion app/spike/internal/cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
// 2. If not initialized:
// - Generate a new admin token
// - Save the token using the provided X.509 source
// - Store the token in ./.spike-admin-token file
// - Store the token in SpikeAdminTokenFile()
//
// Error cases:
// - Already initialized: Notifies user and exits
Expand Down
21 changes: 14 additions & 7 deletions app/spike/internal/cmd/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,19 @@ import (
"github.com/spiffe/spike/app/spike/internal/state"
)

func adminToken() string {
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return ""
}
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return ""
}
return adminToken
}

// NewListCommand creates and returns a new cobra.Command for listing all secret
// paths. It configures a command that retrieves and displays all available
// secret paths from the system.
Expand Down Expand Up @@ -40,14 +53,8 @@ func NewListCommand(source *workloadapi.X509Source) *cobra.Command {
Use: "list",
Short: "List all secret paths",
Run: func(cmd *cobra.Command, args []string) {
// TODO: this check is repeated in every command, consider refactoring
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return
}
adminToken := adminToken()
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return
}

Expand Down
28 changes: 25 additions & 3 deletions app/spike/internal/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,33 @@ import (
"github.com/spf13/cobra"
"github.com/spiffe/go-spiffe/v2/workloadapi"
"github.com/spiffe/spike/app/spike/internal/net"
"github.com/spiffe/spike/internal/config"
"golang.org/x/term"
"os"
"syscall"
)

// NewLoginCommand creates and returns a Cobra command that handles user
// authentication with SPIKE Nexus. The command prompts for an admin password
// and stores the resulting authentication token.
//
// The command performs the following steps:
// 1. Prompts the user for an admin password via secure password input
// 2. Attempts to authenticate with SPIKE Nexus using the provided X509Source
// 3. On successful authentication, saves the JWT token to a local file
//
// Parameters:
// - source: An X509Source used for authenticating the connection to SPIKE Nexus
//
// The command supports the following flags:
// - --password: Optional flag to provide password
// (not recommended for security reasons)
//
// The token is saved to the path specified by config.SpikePilotAdminTokenFile()
// with 0600 permissions.
//
// If any step fails (password input, authentication, or token storage), the command
// prints an error message and returns without saving the token.
func NewLoginCommand(source *workloadapi.X509Source) *cobra.Command {
cmd := &cobra.Command{
Use: "login",
Expand All @@ -36,9 +58,9 @@ func NewLoginCommand(source *workloadapi.X509Source) *cobra.Command {
return
}

// TODO: where the token is stored should be configurable.
// Save token to file:
err = os.WriteFile(".spike-admin-token", []byte(token), 0600)
err = os.WriteFile(
config.SpikePilotAdminTokenFile(), []byte(token), 0600,
)
if err != nil {
fmt.Println("Failed to save token to file:")
fmt.Println(err.Error())
Expand Down
10 changes: 2 additions & 8 deletions app/spike/internal/cmd/put.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/spiffe/go-spiffe/v2/workloadapi"

"github.com/spiffe/spike/app/spike/internal/net"
"github.com/spiffe/spike/app/spike/internal/state"
)

// NewPutCommand creates and returns a new cobra.Command for storing secrets.
Expand Down Expand Up @@ -52,13 +51,8 @@ func NewPutCommand(source *workloadapi.X509Source) *cobra.Command {
Short: "Put secrets at the specified path",
Args: cobra.MinimumNArgs(2),
Run: func(cmd *cobra.Command, args []string) {
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return
}
adminToken := adminToken()
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return
}

Expand All @@ -79,7 +73,7 @@ func NewPutCommand(source *workloadapi.X509Source) *cobra.Command {
return
}

err = net.PutSecret(source, path, values)
err := net.PutSecret(source, path, values)
if err != nil {
fmt.Printf("Error: %v\n", err)
return
Expand Down
11 changes: 2 additions & 9 deletions app/spike/internal/cmd/undelete.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package cmd
import (
"fmt"
"github.com/spiffe/spike/app/spike/internal/net"
"github.com/spiffe/spike/app/spike/internal/state"
"strconv"
"strings"

Expand Down Expand Up @@ -61,19 +60,13 @@ Examples:
spike undelete secret/ella -v 0,1,2 # Undeletes current version plus versions 1 and 2`,
Args: cobra.ExactArgs(1),
Run: func(cmd *cobra.Command, args []string) {
adminToken, err := state.AdminToken()
if err != nil {
fmt.Println("Please login first with `spike login`.")
return
}
adminToken := adminToken()
if adminToken == "" {
fmt.Println("Please login first with `spike login`.")
return
}

path := args[0]
versions, _ := cmd.Flags().GetString("versions")

if versions == "" {
versions = "0"
}
Expand All @@ -94,7 +87,7 @@ Examples:
}
}

err = net.UndeleteSecret(source, path, versionList)
err := net.UndeleteSecret(source, path, versionList)
if err != nil {
fmt.Printf("Error: %v\n", err)
return
Expand Down
14 changes: 4 additions & 10 deletions app/spike/internal/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ package state

import (
"errors"
"github.com/spiffe/spike/internal/config"
"os"
"sync"
)

var tokenMutex sync.RWMutex

// AdminToken retrieves the admin token from the ".spike-admin-token" file.
// AdminToken retrieves the admin token from the SpikeAdminTokenFile().
// The function is thread-safe through a read mutex lock.
//
// Returns:
Expand All @@ -25,19 +26,12 @@ var tokenMutex sync.RWMutex
// file is inaccessible or doesn't exist.
//
// The token file is expected to be in the current working directory with
// the name ".spike-admin-token".
// the name SpikeAdminTokenFile().
func AdminToken() (string, error) {
tokenMutex.RLock()
defer tokenMutex.RUnlock()

// TODO: make this configurable.
// TODO: Explicitly set strict file permissions (0600) when writing the token
// TODO: file should be on local fs; no NEF or shared storage.
// TODO: maybe follow the establish convention of `~/.kube/config`
// or `~/.aws/credentials` and store the token in `~/.spike/config`
// or `~/.spike/credentials`.
// Try to read from file:
tokenBytes, err := os.ReadFile(".spike-admin-token")
tokenBytes, err := os.ReadFile(config.SpikePilotAdminTokenFile())
if err != nil {
return "", errors.Join(
errors.New("failed to read token from file"),
Expand Down
3 changes: 2 additions & 1 deletion docs/adrs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@
* [ADR-0010: Session Token Storage Strategy for SPIKE Nexus](adrs/adr-0010.md)
* [ADR-0011: PostgreSQL as SPIKE's Backing Store](adrs/adr-0011.md)
* [ADR-0012: HTTP Methods for SPIKE API](adrs/adr-0012.md)
* [ADR-0013: S3-Compatible Storage as SPIKE's Backing Store](adrs/adr-0013.md)
* [ADR-0013: S3-Compatible Storage as SPIKE's Backing Store](adrs/adr-0013.md)
* [ADR-0014: Maintaining SQLite as SPIKE's Primary Storage Backend](adrs/adr-0014.md)
1 change: 1 addition & 0 deletions docs/adrs/adr-0012.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Status: accepted
- Date: 2024-11-04
- Tags: API, TLS, Semantics, Network, Operations

## Context

SPIKE is a secrets management system that provides an HTTP API for CRUD
Expand Down
87 changes: 87 additions & 0 deletions docs/adrs/adr-0014.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# ADR-0014: Maintaining SQLite as SPIKE's Primary Storage Backend

- Status: accepted
- Date: 2024-11-15
- Tags: Persistence, Storage, SQLite, File-System

## Context

SPIKE currently uses SQLite as its backing store for secret management. There
is a proposal to implement a file-system-based backing store as an alternative
storage solution. SPIKE uses SPIFFE as its identity control plane for
authentication and authorization.

The key considerations are:
- SQLite is already implemented and proven to work well for SPIKE's needs
- SQLite itself is fundamentally a file-system-based database
- A new file-system backing store would need to implement similar functionality
- Both solutions ultimately interact with the filesystem at their core

## Decision

We will maintain SQLite as the primary storage backend and not implement a
separate file-system-based storage solution.

## Rationale

### Technical Advantages of SQLite

1. **File-System Foundation**: SQLite already operates directly on the file
system, using efficient file I/O operations. As per SQLite's documentation,
it competes directly with `fs.open()` for performance.

2. **ACID Compliance**: SQLite provides built-in:
- Atomicity for operations
- Consistency in data storage
- Isolation for concurrent operations
- Durability of stored data

3. **Proven Security**: SQLite has undergone extensive security auditing and
has a well-understood security model.

### Implementation Considerations

1. **Redundant Development**: Creating a separate file-system store would:
- Duplicate existing functionality
- Require implementing concurrent access controls
- Need additional security auditing
- Require new testing infrastructure

2. **Maintenance Overhead**: Supporting two storage backends would:
- Increase maintenance complexity
- Require maintaining two sets of documentation
- Complicate troubleshooting
- Potentially create inconsistencies in behavior

### Performance

- SQLite is highly optimized for the types of operations SPIKE performs
- The overhead of SQLite compared to direct file system operations is
negligible for SPIKE's use case
- SQLite's page cache provides performance benefits that would need to be
re-implemented in a file-system solution

## Consequences

### Positive

1. Reduced development effort and maintenance burden
2. Continued use of a well-tested, secure storage solution
3. Simplified codebase and deployment
4. Consistent behavior across deployments

### Negative

1. Less flexibility in storage formats
2. Continued dependency on SQLite
3. Potential perception of over-engineering for simple storage needs

### Mitigations

1. Document SQLite's role as a file-system interface
2. Maintain clear interface boundaries for potential future storage implementations
3. Monitor performance and storage requirements to validate continued suitability

## Related Documents

- [SQLite Documentation](https://www.sqlite.org/docs.html)
21 changes: 20 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,24 @@ const NexusIssuer = "spike-nexus"
const NexusAdminSubject = "spike-admin"
const NexusAdminTokenId = "spike-admin-jwt"

func SpikePilotAdminTokenPath() string {
// SpikePilotAdminTokenFile returns the file path where the SPIKE Pilot admin
// JWT should be stored. The function creates the necessary directory structure
// if it doesn't exist.
//
// The function attempts to create a .spike directory in the user's home
// directory. If the home directory cannot be determined, it falls back to
// using /tmp.
//
// The directory is created with 0600 permissions for security.
//
// The token file path and name are hardcoded for security reasons and cannot be
// configured by the user.
//
// Returns the absolute path to the admin JWT token file.
//
// The function will panic if it fails to create the required directory
// structure.
func SpikePilotAdminTokenFile() string {
// Get user's home directory
homeDir, err := os.UserHomeDir()
if err != nil {
Expand All @@ -33,6 +50,8 @@ func SpikePilotAdminTokenPath() string {
panic(err)
}

// The file path and file name are NOT configurable for security reasons.

return filepath.Join(spikeDir, ".spike-admin.jwt")
}

Expand Down
Loading

0 comments on commit acbee73

Please sign in to comment.