-
Notifications
You must be signed in to change notification settings - Fork 9
Add entity type schemas #97
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
base: master
Are you sure you want to change the base?
Changes from all commits
44ece7c
988756a
c766eb9
5d54114
9f7f160
473803e
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 | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,7 @@ import ( | |||||||||||||||||||||||
| "github.com/square/etre/entity" | ||||||||||||||||||||||||
| "github.com/square/etre/metrics" | ||||||||||||||||||||||||
| "github.com/square/etre/query" | ||||||||||||||||||||||||
| "github.com/square/etre/schema" | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| func init() { | ||||||||||||||||||||||||
|
|
@@ -65,6 +66,7 @@ type API struct { | |||||||||||||||||||||||
| queryProfSampleRate int | ||||||||||||||||||||||||
| queryProfReportThreshold time.Duration | ||||||||||||||||||||||||
| srv *http.Server | ||||||||||||||||||||||||
| schemas schema.Config | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // NewAPI godoc | ||||||||||||||||||||||||
|
|
@@ -94,6 +96,7 @@ func NewAPI(appCtx app.Context) *API { | |||||||||||||||||||||||
| queryLatencySLA: queryLatencySLA, | ||||||||||||||||||||||||
| queryProfSampleRate: int(appCtx.Config.Metrics.QueryProfileSampleRate * 100), | ||||||||||||||||||||||||
| queryProfReportThreshold: queryProfReportThreshold, | ||||||||||||||||||||||||
| schemas: appCtx.Config.Schemas, | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| mux := http.NewServeMux() | ||||||||||||||||||||||||
|
|
@@ -120,6 +123,17 @@ func NewAPI(appCtx app.Context) *API { | |||||||||||||||||||||||
| mux.Handle("GET "+etre.API_ROOT+"/entity/{type}/{id}/labels", api.requestWrapper(api.id(http.HandlerFunc(api.getLabelsHandler)))) | ||||||||||||||||||||||||
| mux.Handle("DELETE "+etre.API_ROOT+"/entity/{type}/{id}/labels/{label}", api.requestWrapper(api.id(http.HandlerFunc(api.deleteLabelHandler)))) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
| // Schemas | ||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
| mux.HandleFunc("GET "+etre.API_ROOT+"/schemas/{type}", api.getSchemasHandler) | ||||||||||||||||||||||||
| mux.HandleFunc("GET "+etre.API_ROOT+"/schemas", api.getSchemasHandler) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
| // Entity Types | ||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
| mux.HandleFunc("GET "+etre.API_ROOT+"/entity-types", api.getEntityTypesHandler) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
| // Metrics and status | ||||||||||||||||||||||||
| // ///////////////////////////////////////////////////////////////////// | ||||||||||||||||||||||||
|
|
@@ -1173,6 +1187,52 @@ func (api *API) changesHandler(w http.ResponseWriter, r *http.Request) { | |||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // getSchemasHandler godoc | ||||||||||||||||||||||||
| // @Summary Get entity schemas | ||||||||||||||||||||||||
| // @Description Return the schema for one entity of the given :type | ||||||||||||||||||||||||
| // @ID getEntityHandler | ||||||||||||||||||||||||
| // @Produce json | ||||||||||||||||||||||||
| // @Param type path string true "Entity type" | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| // @Param type path string true "Entity type" | |
| // @Param type path string false "Entity type" |
Copilot
AI
Nov 11, 2025
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.
Potential nil pointer dereference: when an entity type is valid but not present in api.schemas.Entities, accessing api.schemas.Entities[entityType] at line 1215 will return a zero-value EntitySchema with a nil Schema field. While this may be intentional to return an empty schema for known types without configured schemas, it could be clearer to explicitly handle this case or document the behavior.
| cfg.Entities = map[string]schema.EntitySchema{ | |
| entityType: api.schemas.Entities[entityType], | |
| // If the entity type is valid but not present in api.schemas.Entities, | |
| // explicitly return an empty schema for that type. | |
| entitySchema, ok := api.schemas.Entities[entityType] | |
| if !ok { | |
| // Entity type is valid but has no configured schema; return empty schema. | |
| entitySchema = schema.EntitySchema{} | |
| } | |
| cfg.Entities = map[string]schema.EntitySchema{ | |
| entityType: entitySchema, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| package api_test | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "net/url" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/square/etre" | ||
| "github.com/square/etre/api" | ||
| "github.com/square/etre/app" | ||
| "github.com/square/etre/auth" | ||
| "github.com/square/etre/config" | ||
| "github.com/square/etre/entity" | ||
| "github.com/square/etre/metrics" | ||
| srv "github.com/square/etre/server" | ||
| "github.com/square/etre/test" | ||
| "github.com/square/etre/test/mock" | ||
| ) | ||
|
|
||
| func TestGetEntityTypes(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| entityTypes []string | ||
| expectTypes []string | ||
| }{ | ||
| { | ||
| name: "Single entity type", | ||
| entityTypes: []string{"nodes"}, | ||
| expectTypes: []string{"nodes"}, | ||
| }, | ||
| { | ||
| name: "Multiple entity types", | ||
| entityTypes: []string{"nodes", "racks", "hosts"}, | ||
| expectTypes: []string{"nodes", "racks", "hosts"}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Set up the server with custom entity types | ||
| config := defaultConfig | ||
| server := setupWithValidator(t, config, mock.EntityStore{}, entity.NewValidator(tt.entityTypes)) | ||
| defer server.ts.Close() | ||
|
|
||
| // Set up the request URL | ||
| etreurl := server.url + etre.API_ROOT + "/entity-types" | ||
|
|
||
| // Make the HTTP call | ||
| var gotTypes []string | ||
| statusCode, err := test.MakeHTTPRequest("GET", etreurl, nil, &gotTypes) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, statusCode, "response status = %d, expected %d, url %s", statusCode, http.StatusOK, etreurl) | ||
|
|
||
| // Make sure we got the expected entity types | ||
| assert.Equal(t, tt.expectTypes, gotTypes) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func setupWithValidator(t *testing.T, cfg config.Config, store mock.EntityStore, validator entity.Validator) *server { | ||
| etre.DebugEnabled = true | ||
|
|
||
| server := &server{ | ||
| store: store, | ||
| cfg: cfg, | ||
| auth: &mock.AuthRecorder{}, | ||
| cdcStore: &mock.CDCStore{}, | ||
| streamerFactory: &mock.StreamerFactory{}, | ||
| metricsrec: mock.NewMetricsRecorder(), | ||
| sysmetrics: mock.NewMetricsRecorder(), | ||
| } | ||
|
|
||
| acls, err := srv.MapConfigACLRoles(cfg.Security.ACL) | ||
| require.NoError(t, err, "invalid Config.ACL: %s", err) | ||
|
|
||
| ms := metrics.NewMemoryStore() | ||
| mf := metrics.GroupFactory{Store: ms} | ||
| sm := metrics.NewSystemMetrics() | ||
|
|
||
| appCtx := app.Context{ | ||
| Config: server.cfg, | ||
| EntityStore: server.store, | ||
| EntityValidator: validator, | ||
| Auth: auth.NewManager(acls, server.auth), | ||
| MetricsStore: ms, | ||
| MetricsFactory: mock.NewMetricsFactory(mf, server.metricsrec), | ||
| StreamerFactory: server.streamerFactory, | ||
| SystemMetrics: mock.NewSystemMetrics(sm, server.sysmetrics), | ||
| } | ||
| server.api = api.NewAPI(appCtx) | ||
| server.ts = httptest.NewServer(server.api) | ||
|
|
||
| u, err := url.Parse(server.ts.URL) | ||
| require.NoError(t, err) | ||
|
|
||
| server.url = fmt.Sprintf("http://%s", u.Host) | ||
|
|
||
| return server | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| package api_test | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/square/etre" | ||
| "github.com/square/etre/schema" | ||
| "github.com/square/etre/test" | ||
| "github.com/square/etre/test/mock" | ||
| ) | ||
|
|
||
| func TestGetSchemas(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| entityType string | ||
| config schema.Config | ||
| expect schema.Config | ||
| }{ | ||
| { | ||
| name: "No schemas configured", | ||
| entityType: "nodes", | ||
| expect: schema.Config{Entities: map[string]schema.EntitySchema{"nodes": {}}}, // the server will return an empty schema for known types | ||
| }, | ||
| { | ||
| name: "No schemas configured - No type param", | ||
| }, | ||
| { | ||
| name: "Schema configured - type param present", | ||
| entityType: "nodes", | ||
| config: schema.Config{ | ||
| Entities: map[string]schema.EntitySchema{ | ||
| "nodes": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "hostname", Type: "string", Required: true}, | ||
| {Name: "status", Type: "string", Required: false}, | ||
| }, | ||
| AdditionalProperties: true, | ||
| ValidationLevel: "strict", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| expect: schema.Config{ | ||
| Entities: map[string]schema.EntitySchema{ | ||
| "nodes": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "hostname", Type: "string", Required: true}, | ||
| {Name: "status", Type: "string", Required: false}, | ||
| }, | ||
| AdditionalProperties: true, | ||
| ValidationLevel: "strict", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| name: "Schema configured - no type param", | ||
| config: schema.Config{ | ||
| Entities: map[string]schema.EntitySchema{ | ||
| "nodes": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "hostname", Type: "string", Required: true}, | ||
| }, | ||
| AdditionalProperties: false, | ||
| }, | ||
| }, | ||
| "racks": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "rack_id", Type: "string", Required: true}, | ||
| {Name: "datacenter", Type: "string", Required: true}, | ||
| }, | ||
| ValidationLevel: "moderate", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| expect: schema.Config{ | ||
| Entities: map[string]schema.EntitySchema{ | ||
| "nodes": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "hostname", Type: "string", Required: true}, | ||
| }, | ||
| AdditionalProperties: false, | ||
| }, | ||
| }, | ||
| "racks": { | ||
| Schema: &schema.Schema{ | ||
| Fields: []schema.Field{ | ||
| {Name: "rack_id", Type: "string", Required: true}, | ||
| {Name: "datacenter", Type: "string", Required: true}, | ||
| }, | ||
| ValidationLevel: "moderate", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| // Set up the server | ||
| config := defaultConfig | ||
| config.Schemas = tt.config | ||
| server := setup(t, config, mock.EntityStore{}) | ||
| defer server.ts.Close() | ||
|
|
||
| // Set up the request URL | ||
| etreurl := server.url + etre.API_ROOT + "/schemas" | ||
| if tt.entityType != "" { | ||
| etreurl += "/" + tt.entityType | ||
| } | ||
|
|
||
| // Make the HTTP call | ||
| var gotSchemas schema.Config | ||
| statusCode, err := test.MakeHTTPRequest("GET", etreurl, nil, &gotSchemas) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, http.StatusOK, statusCode, "response status = %d, expected %d, url %s", statusCode, http.StatusOK, etreurl) | ||
|
|
||
| // Make sure we got the expected schemas | ||
| assert.Equal(t, tt.expect, gotSchemas) | ||
| }) | ||
| } | ||
| } |
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.
The godoc comment has an incorrect ID. The
@IDshould begetSchemasHandlerto match the function name, notgetEntityHandler.