-
Notifications
You must be signed in to change notification settings - Fork 529
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
feat: api v2 init #2832
feat: api v2 init #2832
Changes from 17 commits
69aea84
e9a61fe
14fba3e
a408313
a77cf37
0e5e0d9
f26a308
be8764f
d5ad52c
e7a2d1a
b817e7d
d2d30e7
6b2f19b
53a5d11
1463151
0760e6a
323d4bb
58214d7
b575be9
2a936d7
b9b2c30
6490adb
7ca44ed
dc5f9de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -31,6 +31,23 @@ services: | |||||||||||||
ports: | ||||||||||||||
- 3900:3900 | ||||||||||||||
|
||||||||||||||
apiv2: | ||||||||||||||
deploy: | ||||||||||||||
replicas: 1 | ||||||||||||||
endpoint_mode: vip | ||||||||||||||
|
||||||||||||||
command: [ "api", "--config", "config.docker.json"] | ||||||||||||||
build: | ||||||||||||||
context: ../go | ||||||||||||||
dockerfile: ./Dockerfile | ||||||||||||||
depends_on: | ||||||||||||||
- mysql | ||||||||||||||
- clickhouse | ||||||||||||||
environment: | ||||||||||||||
PORT: 8080 | ||||||||||||||
DATABASE_PRIMARY_DSN: "mysql://unkey:password@tcp(mysql:3900)/unkey" | ||||||||||||||
CLICKHOUSE_URL: "clickhouse://default:password@clickhouse:9000" | ||||||||||||||
Comment on lines
+47
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use secrets management for sensitive information. Hard-coded credentials in environment variables pose a security risk. Consider using Docker secrets or environment files: environment:
PORT: 8080
- DATABASE_PRIMARY_DSN: "mysql://unkey:password@tcp(mysql:3900)/unkey"
- CLICKHOUSE_URL: "clickhouse://default:password@clickhouse:9000"
+ DATABASE_PRIMARY_DSN: "${DATABASE_PRIMARY_DSN}"
+ CLICKHOUSE_URL: "${CLICKHOUSE_URL}" 📝 Committable suggestion
Suggested change
|
||||||||||||||
|
||||||||||||||
agent: | ||||||||||||||
deploy: | ||||||||||||||
replicas: 3 | ||||||||||||||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,20 @@ | ||||||||||||||||||||||||||||||||||||||
FROM golang:1.23-alpine AS builder | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix invalid Go version in base image. The specified Go version 1.23 doesn't exist. The latest stable version is 1.21.5. -FROM golang:1.23-alpine AS builder
+FROM golang:1.21.5-alpine AS builder 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
WORKDIR /go/src/github.com/unkeyed/unkey/go | ||||||||||||||||||||||||||||||||||||||
COPY go.sum go.mod ./ | ||||||||||||||||||||||||||||||||||||||
RUN go mod download | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
COPY . . | ||||||||||||||||||||||||||||||||||||||
ARG VERSION | ||||||||||||||||||||||||||||||||||||||
RUN go build -o bin/unkey -ldflags "-X 'github.com/unkeyed/unkey/go/pkg/version.Version=${VERSION}'" ./cmd/main.go | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
FROM golang:1.23-alpine | ||||||||||||||||||||||||||||||||||||||
WORKDIR /usr/local/bin | ||||||||||||||||||||||||||||||||||||||
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/bin/unkey . | ||||||||||||||||||||||||||||||||||||||
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/config.local.json . | ||||||||||||||||||||||||||||||||||||||
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/config.docker.json . | ||||||||||||||||||||||||||||||||||||||
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/pkg/openapi/openapi.json ./pkg/openapi/openapi.json | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
ENTRYPOINT [ "/usr/local/bin/unkey"] | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add security hardening to the final image. The container runs as root by default, which is a security risk. Consider:
FROM golang:1.23-alpine
+RUN adduser -D -u 10001 appuser
WORKDIR /usr/local/bin
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/bin/unkey .
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/config.local.json .
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/config.docker.json .
COPY --from=builder /go/src/github.com/unkeyed/unkey/go/pkg/openapi/openapi.json ./pkg/openapi/openapi.json
+RUN chown -R appuser:appuser /usr/local/bin
+USER appuser
ENTRYPOINT [ "/usr/local/bin/unkey"] 📝 Committable suggestion
Suggested change
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
version: '3' | ||
|
||
tasks: | ||
install: | ||
cmd: | ||
go mod tidy | ||
fmt: | ||
cmds: | ||
- go fmt ./... | ||
- task: lint | ||
test: | ||
cmds: | ||
- go test -cover -json -failfast ./... | tparse -all -progress | ||
|
||
build: | ||
cmds: | ||
- go build -o unkey ./cmd/main.go | ||
|
||
|
||
lint: | ||
cmds: | ||
- golangci-lint run |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package api | ||
|
||
type nodeConfig struct { | ||
Platform string `json:"platform,omitempty" description:"The platform this agent is running on"` | ||
NodeId string `json:"nodeId,omitempty" description:"A unique node id"` | ||
Image string `json:"image,omitempty" description:"The image this agent is running"` | ||
|
||
Schema string `json:"$schema,omitempty" description:"Make jsonschema happy"` | ||
Region string `json:"region,omitempty" description:"The region this agent is running in"` | ||
Port string `json:"port,omitempty" default:"8080" description:"Port to listen on"` | ||
Heartbeat *struct { | ||
URL string `json:"url" minLength:"1" description:"URL to send heartbeat to"` | ||
Interval int `json:"interval" min:"1" description:"Interval in seconds to send heartbeat"` | ||
} `json:"heartbeat,omitempty" description:"Send heartbeat to a URL"` | ||
|
||
Clickhouse *struct { | ||
Url string `json:"url" minLength:"1"` | ||
} `json:"clickhouse,omitempty"` | ||
|
||
Database struct { | ||
// DSN of the primary database for reads and writes. | ||
Primary string `json:"primary"` | ||
|
||
// An optional read replica DSN. | ||
ReadonlyReplica string `json:"readonlyReplica,omitempty"` | ||
} `json:"database"` | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
package api | ||
|
||
import ( | ||
"fmt" | ||
"log/slog" | ||
"os" | ||
"os/signal" | ||
"runtime/debug" | ||
"syscall" | ||
|
||
"github.com/unkeyed/unkey/go/pkg/config" | ||
"github.com/unkeyed/unkey/go/pkg/logging" | ||
"github.com/unkeyed/unkey/go/pkg/uid" | ||
"github.com/unkeyed/unkey/go/pkg/version" | ||
"github.com/unkeyed/unkey/go/pkg/zen" | ||
"github.com/unkeyed/unkey/go/pkg/zen/validation" | ||
"github.com/urfave/cli/v2" | ||
) | ||
|
||
var Cmd = &cli.Command{ | ||
Name: "api", | ||
Flags: []cli.Flag{ | ||
&cli.StringFlag{ | ||
Name: "config", | ||
Aliases: []string{"c"}, | ||
Usage: "Load configuration file", | ||
Value: "unkey.json", | ||
DefaultText: "unkey.json", | ||
EnvVars: []string{"AGENT_CONFIG_FILE"}, | ||
}, | ||
}, | ||
Action: run, | ||
} | ||
|
||
func run(c *cli.Context) error { | ||
configFile := c.String("config") | ||
|
||
cfg := nodeConfig{} | ||
err := config.LoadFile(&cfg, configFile) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if cfg.NodeId == "" { | ||
cfg.NodeId = uid.Node() | ||
|
||
} | ||
|
||
if cfg.Region == "" { | ||
cfg.Region = "unknown" | ||
} | ||
logger := logging.New(logging.Config{Development: true}) | ||
logger = logger.With( | ||
|
||
slog.String("nodeId", cfg.NodeId), | ||
slog.String("platform", cfg.Platform), | ||
slog.String("region", cfg.Region), | ||
slog.String("version", version.Version), | ||
) | ||
|
||
// Catch any panics now after we have a logger but before we start the server | ||
defer func() { | ||
if r := recover(); r != nil { | ||
logger.Error(c.Context, "panic", | ||
slog.Any("panic", r), | ||
slog.String("stack", string(debug.Stack())), | ||
) | ||
} | ||
}() | ||
|
||
logger.Info(c.Context, "configration loaded", slog.String("file", configFile)) | ||
|
||
srv, err := zen.New(zen.Config{ | ||
NodeId: cfg.NodeId, | ||
Logger: logger, | ||
Clickhouse: nil, | ||
}) | ||
chronark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
validator, err := validation.New() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
srv.SetGlobalMiddleware( | ||
// metrics should always run first, so it can capture the latency of the entire request | ||
zen.WithMetrics(nil), // TODO: add eventbuffer | ||
zen.WithLogging(logger), | ||
zen.WithErrorHandling(), | ||
zen.WithValidation(validator), | ||
) | ||
|
||
go func() { | ||
listenErr := srv.Listen(c.Context, fmt.Sprintf(":%s", cfg.Port)) | ||
if listenErr != nil { | ||
panic(listenErr) | ||
} | ||
}() | ||
chronark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
cShutdown := make(chan os.Signal, 1) | ||
signal.Notify(cShutdown, os.Interrupt, syscall.SIGTERM) | ||
|
||
<-cShutdown | ||
logger.Info(c.Context, "shutting down") | ||
|
||
return nil | ||
chronark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// TODO: generating this every time is a bit stupid, we should make this its own command | ||
// | ||
// and then run it as part of the build process | ||
func init() { | ||
_, err := config.GenerateJsonSchema(nodeConfig{}, "schema.json") | ||
if err != nil { | ||
panic(err) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Address the "TODO" comment and move schema generation to a dedicated command or build step. Do you want a new command or separate script to handle this schema generation? I can help create that. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package main | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
|
||
"github.com/Southclaws/fault" | ||
"github.com/unkeyed/unkey/go/cmd/api" | ||
"github.com/urfave/cli/v2" | ||
) | ||
|
||
func main() { | ||
app := &cli.App{ | ||
Name: "unkey", | ||
Usage: "Run unkey ", | ||
|
||
Commands: []*cli.Command{ | ||
api.Cmd, | ||
}, | ||
} | ||
Comment on lines
+12
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Validate CLI arguments. While the CLI references |
||
|
||
err := app.Run(os.Args) | ||
if err != nil { | ||
chain := fault.Flatten(err) | ||
|
||
fmt.Println() | ||
fmt.Println() | ||
|
||
for _, e := range chain { | ||
fmt.Printf(" - ") | ||
if e.Location != "" { | ||
fmt.Printf("%s\n", e.Location) | ||
fmt.Printf(" > ") | ||
} | ||
fmt.Printf("%s\n", e.Message) | ||
} | ||
fmt.Println() | ||
os.Exit(1) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"$schema": "schema.json", | ||
"port": "8080", | ||
"nodeId": "node_1", | ||
"database": { | ||
"primary": "mysql://" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"$schema": "schema.json", | ||
"port": "8080", | ||
"nodeId": "node_1", | ||
"database": { | ||
"primary": "${DATABASE_PRIMARY_DSN}" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add health checks and resource limits.
The service should include health checks and resource constraints for better reliability and resource management.
📝 Committable suggestion