openapi: Make file client more easy to re-use

A few notes about the change:
1. I need to initialize the fileclient once, in an init function, so I
don't have access to `testing.T` yet.
2. I want to be able to configure the openapi files that I use
3. We already have a "cache" client that wraps another client, we don't
need to re-implement caching here, one can just do:
`cache.NewClient(openapitest.NewFileClient("some/path"))` to do a cached
client. Or initialize it in an init/global var.

Since there is still some value to use the embedded file, make an
alternative constructor while using fs.FS interface to be able to
manipulate both virtual and disk-based filesystems.

Kubernetes-commit: 29503fd8d45bc2c9438e92936bf4111162529b40

openapi/openapitest/fileclient.go

@@ -19,33 +19,35 @@ package openapitest
 import (
 	"embed"
 	"errors"
-	"path/filepath"
+	"io/fs"
+	"os"
 	"strings"
-	"sync"
-	"testing"
 
 	"k8s.io/client-go/openapi"
 )
 
 //go:embed testdata/*_openapi.json
-var f embed.FS
+var embedded embed.FS
 
 // NewFileClient returns a test client implementing the openapi.Client
-// interface, which serves a subset of hard-coded GroupVersion
+// interface, which serves Open API V3 specifications files from the
-// Open API V3 specifications files. The subset of specifications is
+// given path, as prepared in `api/openapi-spec/v3`.
-// located in the "testdata" subdirectory.
+func NewFileClient(path string) openapi.Client {
-func NewFileClient(t *testing.T) openapi.Client {
+	return &fileClient{f: os.DirFS(path)}
-	if t == nil {
+}
-		panic("non-nil testing.T required; this package is only for use in tests")
+
+// NewEmbeddedFileClient returns a test client that uses the embedded
+// `testdata` openapi files.
+func NewEmbeddedFileClient() openapi.Client {
+	f, err := fs.Sub(embedded, "testdata")
+	if err != nil {
+		panic(err)
 	}
-	return &fileClient{t: t}
+	return &fileClient{f: f}
 }
 
 type fileClient struct {
-	t     *testing.T
+	f fs.FS
-	init  sync.Once
-	paths map[string]openapi.GroupVersion
-	err   error
 }
 
 // fileClient implements the openapi.Client interface.
@@ -60,29 +62,23 @@ var _ openapi.Client = &fileClient{}
 //
 // The file contents are read only once. All files must parse correctly
 // into an api path, or an error is returned.
-func (t *fileClient) Paths() (map[string]openapi.GroupVersion, error) {
+func (f *fileClient) Paths() (map[string]openapi.GroupVersion, error) {
-	t.init.Do(func() {
+	paths := map[string]openapi.GroupVersion{}
-		t.paths = map[string]openapi.GroupVersion{}
+	entries, err := fs.ReadDir(f.f, ".")
-		entries, err := f.ReadDir("testdata")
 	if err != nil {
-			t.err = err
+		return nil, err
-			t.t.Error(err)
 	}
 	for _, e := range entries {
 		// this reverses the transformation done in hack/update-openapi-spec.sh
 		path := strings.ReplaceAll(strings.TrimSuffix(e.Name(), "_openapi.json"), "__", "/")
-			t.paths[path] = &fileGroupVersion{t: t.t, filename: filepath.Join("testdata", e.Name())}
+		paths[path] = &fileGroupVersion{f: f.f, filename: e.Name()}
 	}
-	})
+	return paths, nil
-	return t.paths, t.err
 }
 
 type fileGroupVersion struct {
-	t        *testing.T
+	f        fs.FS
-	init     sync.Once
 	filename string
-	data     []byte
-	err      error
 }
 
 // fileGroupVersion implements the openapi.GroupVersion interface.
@@ -91,17 +87,10 @@ var _ openapi.GroupVersion = &fileGroupVersion{}
 // Schema returns the OpenAPI V3 specification for the GroupVersion as
 // unstructured bytes, or an error if the contentType is not
 // "application/json" or there is an error reading the spec file. The
-// file is read only once. The embedded file is located in the "testdata"
+// file is read only once.
-// subdirectory.
+func (f *fileGroupVersion) Schema(contentType string) ([]byte, error) {
-func (t *fileGroupVersion) Schema(contentType string) ([]byte, error) {
 	if contentType != "application/json" {
 		return nil, errors.New("openapitest only supports 'application/json' contentType")
 	}
-	t.init.Do(func() {
+	return fs.ReadFile(f.f, f.filename)
-		t.data, t.err = f.ReadFile(t.filename)
-		if t.err != nil {
-			t.t.Error(t.err)
-		}
-	})
-	return t.data, t.err
 }

openapi/openapitest/fileclient_test.go

@@ -14,16 +14,61 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package openapitest
+package openapitest_test
 
 import (
+	"testing"
+
+	"k8s.io/client-go/openapi/openapitest"
 	"k8s.io/kube-openapi/pkg/spec3"
 	kjson "sigs.k8s.io/json"
-	"testing"
 )
 
-func TestOpenAPITest(t *testing.T) {
+func TestOpenAPIEmbeddedTest(t *testing.T) {
-	client := NewFileClient(t)
+	client := openapitest.NewEmbeddedFileClient()
+
+	// make sure we get paths
+	paths, err := client.Paths()
+	if err != nil {
+		t.Fatalf("error fetching paths: %v", err)
+	}
+	if len(paths) == 0 {
+		t.Error("empty paths")
+	}
+
+	// spot check specific paths
+	expectedPaths := []string{
+		"api/v1",
+		"apis/apps/v1",
+		"apis/batch/v1",
+		"apis/networking.k8s.io/v1alpha1",
+		"apis/discovery.k8s.io/v1",
+	}
+	for _, p := range expectedPaths {
+		if _, ok := paths[p]; !ok {
+			t.Fatalf("expected %s", p)
+		}
+	}
+
+	// make sure all paths can load
+	for path, gv := range paths {
+		data, err := gv.Schema("application/json")
+		if err != nil {
+			t.Fatalf("error reading schema for %v: %v", path, err)
+		}
+		o := &spec3.OpenAPI{}
+		stricterrs, err := kjson.UnmarshalStrict(data, o)
+		if err != nil {
+			t.Fatalf("error unmarshaling schema for %v: %v", path, err)
+		}
+		if len(stricterrs) > 0 {
+			t.Fatalf("strict errors unmarshaling schema for %v: %v", path, stricterrs)
+		}
+	}
+}
+
+func TestOpenAPITest(t *testing.T) {
+	client := openapitest.NewFileClient("testdata")
 
 	// make sure we get paths
 	paths, err := client.Paths()

openapi3/root_test.go

@@ -150,7 +150,7 @@ func TestOpenAPIV3Root_GVSpec(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			client := openapitest.NewFileClient(t)
+			client := openapitest.NewEmbeddedFileClient()
 			root := NewRoot(client)
 			gvSpec, err := root.GVSpec(test.gv)
 			if test.err != nil {
@@ -209,8 +209,7 @@ func TestOpenAPIV3Root_GVSpecAsMap(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			client := openapitest.NewFileClient(t)
+			root := NewRoot(openapitest.NewEmbeddedFileClient())
-			root := NewRoot(client)
 			gvSpecAsMap, err := root.GVSpecAsMap(test.gv)
 			if test.err != nil {
 				assert.True(t, reflect.DeepEqual(test.err, err))