Adding tests for improved schema cache

pkg/debounce/refresher_test.go

@@ -0,0 +1,47 @@
+package debounce
+
+import (
+	"fmt"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+)
+
+type refreshable struct {
+	wasRefreshed atomic.Bool
+	retErr       error
+}
+
+func (r *refreshable) Refresh() error {
+	r.wasRefreshed.Store(true)
+	return r.retErr
+}
+
+func TestRefreshAfter(t *testing.T) {
+	ref := refreshable{}
+	debounce := DebounceableRefresher{
+		Refreshable: &ref,
+	}
+	debounce.RefreshAfter(time.Millisecond * 2)
+	debounce.RefreshAfter(time.Microsecond * 2)
+	time.Sleep(time.Millisecond * 1)
+	// test that the second refresh call overrode the first - Micro < Milli so this should have ran
+	require.True(t, ref.wasRefreshed.Load())
+	ref.wasRefreshed.Store(false)
+	time.Sleep(time.Millisecond * 2)
+	// test that the call was debounced - though we called this twice only one refresh should be called
+	require.False(t, ref.wasRefreshed.Load())
+
+	ref = refreshable{
+		retErr: fmt.Errorf("Some error"),
+	}
+	debounce = DebounceableRefresher{
+		Refreshable: &ref,
+	}
+	debounce.RefreshAfter(time.Microsecond * 2)
+	// test the error case
+	time.Sleep(time.Millisecond * 1)
+	require.True(t, ref.wasRefreshed.Load())
+}

pkg/schema/definitions/handler_test.go

@@ -3,7 +3,6 @@ package definitions
 import (
 	"fmt"
 	"testing"
-	"time"
 
 	openapi_v2 "github.com/google/gnostic-models/openapiv2"
 	"github.com/rancher/apiserver/pkg/apierror"
@@ -16,166 +15,50 @@ import (
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/openapi"
 	restclient "k8s.io/client-go/rest"
+	"k8s.io/kube-openapi/pkg/util/proto"
 )
 
-var globalRoleObject = types.APIObject{
+func TestRefresh(t *testing.T) {
-	ID:   "management.cattle.io.globalrole",
+	defaultDocument, err := openapi_v2.ParseDocument([]byte(openapi_raw))
-	Type: "schemaDefinition",
+	require.NoError(t, err)
-	Object: schemaDefinition{
+	defaultModels, err := proto.NewOpenAPIData(defaultDocument)
-		DefinitionType: "io.cattle.management.v2.GlobalRole",
+	require.NoError(t, err)
-		Definitions: map[string]definition{
+	defaultSchemaToModel := map[string]string{
-			"io.cattle.management.v2.GlobalRole": {
+		"management.cattle.io.globalrole": "io.cattle.management.v2.GlobalRole",
-				ResourceFields: map[string]definitionField{
-					"apiVersion": {
-						Type:        "string",
-						Description: "The APIVersion of this resource",
-					},
-					"kind": {
-						Type:        "string",
-						Description: "The kind",
-					},
-					"metadata": {
-						Type:        "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
-						Description: "The metadata",
-					},
-					"spec": {
-						Type: "io.cattle.management.v2.GlobalRole.spec", Description: "The spec for the project",
-					},
-				},
-				Type:        "io.cattle.management.v2.GlobalRole",
-				Description: "A Global Role V2 provides Global Permissions in Rancher",
-			},
-			"io.cattle.management.v2.GlobalRole.spec": {
-				ResourceFields: map[string]definitionField{
-					"clusterName": {
-						Type:        "string",
-						Description: "The name of the cluster",
-						Required:    true,
-					},
-					"displayName": {
-						Type:        "string",
-						Description: "The UI readable name",
-						Required:    true,
-					},
-					"newField": {
-						Type:        "string",
-						Description: "A new field not present in v1",
-					},
-					"notRequired": {
-						Type:        "boolean",
-						Description: "Some field that isn't required",
-					},
-				},
-				Type:        "io.cattle.management.v2.GlobalRole.spec",
-				Description: "The spec for the project",
-			},
-			"io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": {
-				ResourceFields: map[string]definitionField{
-					"annotations": {
-						Type:        "map",
-						SubType:     "string",
-						Description: "annotations of the resource",
-					},
-					"name": {
-						Type:        "string",
-						SubType:     "",
-						Description: "name of the resource",
-					},
-				},
-				Type:        "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
-				Description: "Object Metadata",
-			},
-		},
-	},
-}
-
-func TestByID(t *testing.T) {
-	schemas := types.EmptyAPISchemas()
-	addBaseSchema := func(names ...string) {
-		for _, name := range names {
-			schemas.MustAddSchema(types.APISchema{
-				Schema: &wschemas.Schema{
-					ID:                name,
-					CollectionMethods: []string{"get"},
-					ResourceMethods:   []string{"get"},
-				},
-			})
-		}
 	}
-
-	intPtr := func(input int) *int {
-		return &input
-	}
-
-	addBaseSchema("management.cattle.io.globalrole", "management.cattle.io.missingfrommodel")
-
 	tests := []struct {
 		name                     string
-		schemaName               string
-		needsRefresh             bool
 		openapiError             error
 		serverGroupsResourcesErr error
 		useBadOpenApiDoc         bool
 		unparseableGV            bool
-		wantObject               *types.APIObject
+		wantModels               *proto.Models
+		wantSchemaToModel        map[string]string
 		wantError                bool
-		wantErrorCode            *int
 	}{
 		{
-			name:         "global role definition",
+			name:              "success",
-			schemaName:   "management.cattle.io.globalrole",
+			wantModels:        &defaultModels,
-			needsRefresh: true,
+			wantSchemaToModel: defaultSchemaToModel,
-			wantObject:   &globalRoleObject,
 		},
 		{
-			name:          "missing definition",
+			name:         "error - openapi doc unavailable",
-			schemaName:    "management.cattle.io.cluster",
+			openapiError: fmt.Errorf("server unavailable"),
-			needsRefresh:  true,
+			wantError:    true,
-			wantError:     true,
-			wantErrorCode: intPtr(404),
 		},
 		{
-			name:          "not refreshed",
+			name:             "error - unable to parse openapi doc",
-			schemaName:    "management.cattle.io.globalrole",
-			needsRefresh:  false,
-			wantError:     true,
-			wantErrorCode: intPtr(503),
-		},
-		{
-			name:          "missing from model",
-			schemaName:    "management.cattle.io.missingfrommodel",
-			needsRefresh:  true,
-			wantError:     true,
-			wantErrorCode: intPtr(503),
-		},
-		{
-			name:          "refresh error - openapi doc unavailable",
-			schemaName:    "management.cattle.io.globalrole",
-			needsRefresh:  true,
-			openapiError:  fmt.Errorf("server unavailable"),
-			wantError:     true,
-			wantErrorCode: intPtr(500),
-		},
-		{
-			name:             "refresh error - unable to parse openapi doc",
-			schemaName:       "management.cattle.io.globalrole",
-			needsRefresh:     true,
 			useBadOpenApiDoc: true,
 			wantError:        true,
-			wantErrorCode:    intPtr(500),
 		},
 		{
-			name:                     "refresh error - unable to retrieve groups and resources",
+			name:                     "error - unable to retrieve groups and resources",
-			schemaName:               "management.cattle.io.globalrole",
-			needsRefresh:             true,
 			serverGroupsResourcesErr: fmt.Errorf("server not available"),
+			wantModels:               &defaultModels,
 			wantError:                true,
-			wantErrorCode:            intPtr(500),
 		},
 		{
-			name:         "refresh error - unable to retrieve all groups and resources",
+			name: "error - unable to retrieve all groups and resources",
-			schemaName:   "management.cattle.io.globalrole",
-			needsRefresh: true,
 			serverGroupsResourcesErr: &discovery.ErrGroupDiscoveryFailed{
 				Groups: map[schema.GroupVersion]error{
 					{
@@ -184,19 +67,18 @@ func TestByID(t *testing.T) {
 					}: fmt.Errorf("some group error"),
 				},
 			},
-			wantError:     true,
+			wantModels:        &defaultModels,
-			wantErrorCode: intPtr(500),
+			wantSchemaToModel: defaultSchemaToModel,
+			wantError:         true,
 		},
 		{
-			name:          "refresh error - unparesable gv",
+			name:              "error - unparesable gv",
-			schemaName:    "management.cattle.io.globalrole",
+			unparseableGV:     true,
-			needsRefresh:  true,
+			wantModels:        &defaultModels,
-			unparseableGV: true,
+			wantSchemaToModel: defaultSchemaToModel,
-			wantError:     true,
+			wantError:         true,
-			wantErrorCode: intPtr(500),
 		},
 	}
-
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
@@ -216,12 +98,175 @@ func TestByID(t *testing.T) {
 				})
 			}
 			require.Nil(t, err)
-			handler := schemaDefinitionHandler{
+			handler := SchemaDefinitionHandler{
 				client: client,
 			}
-			if !test.needsRefresh {
+			err = handler.Refresh()
-				handler.lastRefresh = time.Now()
+			if test.wantError {
-				handler.refreshStale = time.Minute * 1
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			require.Equal(t, test.wantModels, handler.models)
+			require.Equal(t, test.wantSchemaToModel, handler.schemaToModel)
+		})
+
+	}
+}
+
+func Test_byID(t *testing.T) {
+	defaultDocument, err := openapi_v2.ParseDocument([]byte(openapi_raw))
+	require.NoError(t, err)
+	defaultModels, err := proto.NewOpenAPIData(defaultDocument)
+	require.NoError(t, err)
+	defaultSchemaToModel := map[string]string{
+		"management.cattle.io.globalrole": "io.cattle.management.v2.GlobalRole",
+	}
+	schemas := types.EmptyAPISchemas()
+	addBaseSchema := func(names ...string) {
+		for _, name := range names {
+			schemas.MustAddSchema(types.APISchema{
+				Schema: &wschemas.Schema{
+					ID:                name,
+					CollectionMethods: []string{"get"},
+					ResourceMethods:   []string{"get"},
+				},
+			})
+		}
+	}
+
+	intPtr := func(input int) *int {
+		return &input
+	}
+
+	addBaseSchema("management.cattle.io.globalrole", "management.cattle.io.missingfrommodel", "management.cattle.io.notakind")
+
+	tests := []struct {
+		name          string
+		schemaName    string
+		models        *proto.Models
+		schemaToModel map[string]string
+		wantObject    *types.APIObject
+		wantError     bool
+		wantErrorCode *int
+	}{
+		{
+			name:          "global role definition",
+			schemaName:    "management.cattle.io.globalrole",
+			models:        &defaultModels,
+			schemaToModel: defaultSchemaToModel,
+			wantObject: &types.APIObject{
+				ID:   "management.cattle.io.globalrole",
+				Type: "schemaDefinition",
+				Object: schemaDefinition{
+					DefinitionType: "io.cattle.management.v2.GlobalRole",
+					Definitions: map[string]definition{
+						"io.cattle.management.v2.GlobalRole": {
+							ResourceFields: map[string]definitionField{
+								"apiVersion": {
+									Type:        "string",
+									Description: "The APIVersion of this resource",
+								},
+								"kind": {
+									Type:        "string",
+									Description: "The kind",
+								},
+								"metadata": {
+									Type:        "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
+									Description: "The metadata",
+								},
+								"spec": {
+									Type: "io.cattle.management.v2.GlobalRole.spec", Description: "The spec for the project",
+								},
+							},
+							Type:        "io.cattle.management.v2.GlobalRole",
+							Description: "A Global Role V2 provides Global Permissions in Rancher",
+						},
+						"io.cattle.management.v2.GlobalRole.spec": {
+							ResourceFields: map[string]definitionField{
+								"clusterName": {
+									Type:        "string",
+									Description: "The name of the cluster",
+									Required:    true,
+								},
+								"displayName": {
+									Type:        "string",
+									Description: "The UI readable name",
+									Required:    true,
+								},
+								"newField": {
+									Type:        "string",
+									Description: "A new field not present in v1",
+								},
+								"notRequired": {
+									Type:        "boolean",
+									Description: "Some field that isn't required",
+								},
+							},
+							Type:        "io.cattle.management.v2.GlobalRole.spec",
+							Description: "The spec for the project",
+						},
+						"io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta": {
+							ResourceFields: map[string]definitionField{
+								"annotations": {
+									Type:        "map",
+									SubType:     "string",
+									Description: "annotations of the resource",
+								},
+								"name": {
+									Type:        "string",
+									SubType:     "",
+									Description: "name of the resource",
+								},
+							},
+							Type:        "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta",
+							Description: "Object Metadata",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:          "missing definition",
+			schemaName:    "management.cattle.io.cluster",
+			models:        &defaultModels,
+			schemaToModel: defaultSchemaToModel,
+			wantError:     true,
+			wantErrorCode: intPtr(404),
+		},
+		{
+			name:          "not refreshed",
+			schemaName:    "management.cattle.io.globalrole",
+			wantError:     true,
+			wantErrorCode: intPtr(503),
+		},
+		{
+			name:          "has schema, missing from model",
+			schemaName:    "management.cattle.io.missingfrommodel",
+			models:        &defaultModels,
+			schemaToModel: defaultSchemaToModel,
+			wantError:     true,
+			wantErrorCode: intPtr(503),
+		},
+		{
+			name:       "has schema, model is not a kind",
+			schemaName: "management.cattle.io.notakind",
+			models:     &defaultModels,
+			schemaToModel: map[string]string{
+				"management.cattle.io.notakind": "io.management.cattle.NotAKind",
+			},
+			wantError:     true,
+			wantErrorCode: intPtr(500),
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			handler := SchemaDefinitionHandler{
+				models:        test.models,
+				schemaToModel: test.schemaToModel,
 			}
 			request := types.APIRequest{
 				Schemas: schemas,

pkg/schema/definitions/openapi_test.go

@@ -82,7 +82,7 @@ definitions:
     - group: "management.cattle.io"
       version: "v2"
       kind: "GlobalRole"
-  io.management.cattle.NotAKind:
+  io.cattle.management.NotAKind:
     type: "string"
     description: "Some string which isn't a kind"
   io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta:

pkg/schema/definitions/refresh_test.go

@@ -0,0 +1,84 @@
+package definitions
+
+import (
+	"context"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/rancher/steve/pkg/debounce"
+	"github.com/stretchr/testify/require"
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	apiregv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
+)
+
+type refreshable struct {
+	wasRefreshed atomic.Bool
+}
+
+func (r *refreshable) Refresh() error {
+	r.wasRefreshed.Store(true)
+	return nil
+}
+
+func Test_onChangeCRD(t *testing.T) {
+	internalRefresh := refreshable{}
+	refresher := debounce.DebounceableRefresher{
+		Refreshable: &internalRefresh,
+	}
+	refreshHandler := refreshHandler{
+		debounceRef:      &refresher,
+		debounceDuration: time.Microsecond * 5,
+	}
+	input := apiextv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-crd",
+		},
+	}
+	output, err := refreshHandler.onChangeCRD("test-crd", &input)
+	require.Nil(t, err)
+	require.Equal(t, input, *output)
+	// waiting to allow the debouncer to refresh the refreshable
+	time.Sleep(time.Millisecond * 2)
+	require.True(t, internalRefresh.wasRefreshed.Load())
+}
+
+func Test_onChangeAPIService(t *testing.T) {
+	internalRefresh := refreshable{}
+	refresher := debounce.DebounceableRefresher{
+		Refreshable: &internalRefresh,
+	}
+	refreshHandler := refreshHandler{
+		debounceRef:      &refresher,
+		debounceDuration: time.Microsecond * 5,
+	}
+	input := apiregv1.APIService{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-apiservice",
+		},
+	}
+	output, err := refreshHandler.onChangeAPIService("test-apiservice", &input)
+	require.Nil(t, err)
+	require.Equal(t, input, *output)
+	// waiting to allow the debouncer to refresh the refreshable
+	time.Sleep(time.Millisecond * 2)
+	require.True(t, internalRefresh.wasRefreshed.Load())
+
+}
+
+func Test_startBackgroundRefresh(t *testing.T) {
+	internalRefresh := refreshable{}
+	refresher := debounce.DebounceableRefresher{
+		Refreshable: &internalRefresh,
+	}
+	refreshHandler := refreshHandler{
+		debounceRef:      &refresher,
+		debounceDuration: time.Microsecond * 5,
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	refreshHandler.startBackgroundRefresh(ctx, time.Microsecond*10)
+	time.Sleep(time.Millisecond * 2)
+	require.True(t, internalRefresh.wasRefreshed.Load())
+	cancel()
+}

pkg/schema/definitions/schema_test.go

@@ -0,0 +1,76 @@
+package definitions
+
+import (
+	"context"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/golang/mock/gomock"
+	"github.com/rancher/apiserver/pkg/types"
+	"github.com/rancher/wrangler/v2/pkg/generic/fake"
+	"github.com/stretchr/testify/require"
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiregv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
+)
+
+func TestRegister(t *testing.T) {
+	schemas := types.EmptyAPISchemas()
+	client := fakeDiscovery{}
+	ctrl := gomock.NewController(t)
+	crdController := fake.NewMockNonNamespacedControllerInterface[*apiextv1.CustomResourceDefinition, *apiextv1.CustomResourceDefinitionList](ctrl)
+	apisvcController := fake.NewMockNonNamespacedControllerInterface[*apiregv1.APIService, *apiregv1.APIServiceList](ctrl)
+	ctx, cancel := context.WithCancel(context.Background())
+	crdController.EXPECT().OnChange(ctx, handlerKey, gomock.Any())
+	apisvcController.EXPECT().OnChange(ctx, handlerKey, gomock.Any())
+	Register(ctx, schemas, &client, crdController, apisvcController)
+	registeredSchema := schemas.LookupSchema("schemaDefinition")
+	require.NotNil(t, registeredSchema)
+	require.Len(t, registeredSchema.ResourceMethods, 1)
+	require.Equal(t, registeredSchema.ResourceMethods[0], "GET")
+	require.NotNil(t, registeredSchema.ByIDHandler)
+	// Register will spawn a background thread, so we want to stop that to not impact other tests
+	cancel()
+}
+
+func Test_getDurationEnvVarOrDefault(t *testing.T) {
+	os.Setenv("VALID", "1")
+	os.Setenv("INVALID", "NOTANUMBER")
+	tests := []struct {
+		name         string
+		envVar       string
+		defaultValue int
+		unit         time.Duration
+		wantDuration time.Duration
+	}{
+		{
+			name:         "not found, use default",
+			envVar:       "NOT_FOUND",
+			defaultValue: 12,
+			unit:         time.Second,
+			wantDuration: time.Second * 12,
+		},
+		{
+			name:         "found but not an int",
+			envVar:       "INVALID",
+			defaultValue: 24,
+			unit:         time.Minute,
+			wantDuration: time.Minute * 24,
+		},
+		{
+			name:         "found and valid int",
+			envVar:       "VALID",
+			defaultValue: 30,
+			unit:         time.Hour,
+			wantDuration: time.Hour * 1,
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			got := getDurationEnvVarOrDefault(test.envVar, test.defaultValue, test.unit)
+			require.Equal(t, test.wantDuration, got)
+		})
+	}
+}