diff --git a/pkg/cmd/validate.go b/pkg/cmd/validate.go index 130d3f23..2d116f0b 100644 --- a/pkg/cmd/validate.go +++ b/pkg/cmd/validate.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io/fs" "net/http" "os" @@ -164,16 +165,27 @@ func errorToStatus(err error) metav1.Status { } func (c *commandFlags) Run(cmd *cobra.Command, args []string) error { + var schemaPatchesFs, localSchemasFs fs.FS + if c.schemaPatchesDir != "" { + schemaPatchesFs = os.DirFS(c.schemaPatchesDir) + } + if c.localSchemasDir != "" { + localSchemasFs = os.DirFS(c.localSchemasDir) + } + var localCRDsFileSystems []fs.FS + for _, current := range c.localCRDsDir { + localCRDsFileSystems = append(localCRDsFileSystems, os.DirFS(current)) + } // tool fetches openapi in the following priority order: factory, err := validator.New( openapiclient.NewOverlay( // apply user defined patches on top of the final schema - openapiclient.PatchLoaderFromDirectory(nil, c.schemaPatchesDir), + openapiclient.PatchLoaderFromDirectory(schemaPatchesFs), openapiclient.NewComposite( // consult local OpenAPI - openapiclient.NewLocalSchemaFiles(nil, c.localSchemasDir), + openapiclient.NewLocalSchemaFiles(localSchemasFs), // consult local CRDs - openapiclient.NewLocalCRDFiles(nil, c.localCRDsDir), + openapiclient.NewLocalCRDFiles(localCRDsFileSystems...), openapiclient.NewOverlay( // Hand-written hardcoded patches. openapiclient.HardcodedPatchLoader(c.version), diff --git a/pkg/openapiclient/local_crds.go b/pkg/openapiclient/local_crds.go index 340d286b..4c379bdc 100644 --- a/pkg/openapiclient/local_crds.go +++ b/pkg/openapiclient/local_crds.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io/fs" - "path" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apiserver" @@ -33,44 +32,43 @@ var metadataSchemas map[string]*spec.Schema = func() map[string]*spec.Schema { // client which provides openapi read from files on disk type localCRDsClient struct { - fs fs.FS - dirs []string + fileSystems []fs.FS } // Dir should have openapi files following directory layout: // myCRD.yaml (groupversions read from file) -func NewLocalCRDFiles(fs fs.FS, dirPaths []string) openapi.Client { +func NewLocalCRDFiles(fs ...fs.FS) openapi.Client { return &localCRDsClient{ - fs: fs, - dirs: dirPaths, + fileSystems: fs, } } func (k *localCRDsClient) Paths() (map[string]openapi.GroupVersion, error) { - if len(k.dirs) == 0 { + if len(k.fileSystems) == 0 { return nil, nil } var documents []utils.Document - for _, dir := range k.dirs { - files, err := fs.ReadDir(k.fs, dir) + for _, current := range k.fileSystems { + files, err := fs.ReadDir(current, ".") if err != nil { - return nil, fmt.Errorf("error listing %s: %w", dir, err) + if crossPlatformCheckDirExists(current, ".") { + return nil, fmt.Errorf("error listing: %w", err) + } } - for _, f := range files { - path := path.Join(dir, f.Name()) + path := f.Name() if f.IsDir() { continue } if utils.IsJson(f.Name()) { - fileBytes, err := fs.ReadFile(k.fs, path) + fileBytes, err := fs.ReadFile(current, path) if err != nil { return nil, fmt.Errorf("failed to read %s: %w", path, err) } documents = append(documents, fileBytes) } else if utils.IsYaml(f.Name()) { - fileBytes, err := fs.ReadFile(k.fs, path) + fileBytes, err := fs.ReadFile(current, path) if err != nil { return nil, fmt.Errorf("failed to read %s: %w", path, err) } diff --git a/pkg/openapiclient/local_crds_test.go b/pkg/openapiclient/local_crds_test.go index b9c44251..4d2ac290 100644 --- a/pkg/openapiclient/local_crds_test.go +++ b/pkg/openapiclient/local_crds_test.go @@ -14,43 +14,28 @@ import ( func TestNewLocalCRDFiles(t *testing.T) { tests := []struct { - name string - fs fs.FS - dirPaths []string - want openapi.Client + name string + fileSystems []fs.FS + want openapi.Client }{{ - name: "fs nil and dir empty", + name: "no fs", want: &localCRDsClient{}, }, { - name: "only dir", - dirPaths: []string{"test"}, + name: "one fs", + fileSystems: []fs.FS{os.DirFS("test")}, want: &localCRDsClient{ - dirs: []string{"test"}, + fileSystems: []fs.FS{os.DirFS("test")}, }, }, { - name: "multiple dirs", - dirPaths: []string{"test", "test2"}, + name: "multiple dirs", + fileSystems: []fs.FS{os.DirFS("test"), os.DirFS("test2")}, want: &localCRDsClient{ - dirs: []string{"test", "test2"}, - }, - }, { - name: "only fs", - fs: os.DirFS("."), - want: &localCRDsClient{ - fs: os.DirFS("."), - }, - }, { - name: "both fs and dir", - fs: os.DirFS("."), - dirPaths: []string{"test"}, - want: &localCRDsClient{ - fs: os.DirFS("."), - dirs: []string{"test"}, + fileSystems: []fs.FS{os.DirFS("test"), os.DirFS("test2")}, }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := NewLocalCRDFiles(tt.fs, tt.dirPaths) + got := NewLocalCRDFiles(tt.fileSystems...) require.Equal(t, tt.want, got, "NewLocalCRDFiles not equal") }) } @@ -58,16 +43,15 @@ func TestNewLocalCRDFiles(t *testing.T) { func Test_localCRDsClient_Paths(t *testing.T) { tests := []struct { - name string - fs fs.FS - dirs []string - want map[string]sets.Set[string] - wantErr bool + name string + fileSystems []fs.FS + want map[string]sets.Set[string] + wantErr bool }{{ - name: "fs nil and dir empty", + name: "no fs", }, { - name: "only dir", - dirs: []string{"../../testcases/crds"}, + name: "one fs", + fileSystems: []fs.FS{os.DirFS("../../testcases/crds")}, want: map[string]sets.Set[string]{ "apis/batch.x-k8s.io/v1alpha1": sets.New( "batch.x-k8s.io/v1alpha1.JobSet", @@ -87,8 +71,8 @@ func Test_localCRDsClient_Paths(t *testing.T) { ), }, }, { - name: "multiple dirs", - dirs: []string{"../../testcases/crds", "../../testcases/more-crds"}, + name: "two fs", + fileSystems: []fs.FS{os.DirFS("../../testcases/crds"), os.DirFS("../../testcases/more-crds")}, want: map[string]sets.Set[string]{ "apis/batch.x-k8s.io/v1alpha1": sets.New( "batch.x-k8s.io/v1alpha1.JobSet", @@ -111,43 +95,17 @@ func Test_localCRDsClient_Paths(t *testing.T) { ), }, }, { - name: "only fs", - fs: os.DirFS("../../testcases/crds"), - }, { - name: "both fs and dir", - fs: os.DirFS("../../testcases"), - dirs: []string{"crds"}, - want: map[string]sets.Set[string]{ - "apis/batch.x-k8s.io/v1alpha1": sets.New( - "batch.x-k8s.io/v1alpha1.JobSet", - ), - "apis/stable.example.com/v1": sets.New( - "stable.example.com/v1.CELBasic", - ), - "apis/acme.cert-manager.io/v1": sets.New( - "acme.cert-manager.io/v1.Challenge", - "acme.cert-manager.io/v1.Order", - ), - "apis/cert-manager.io/v1": sets.New( - "cert-manager.io/v1.Certificate", - "cert-manager.io/v1.CertificateRequest", - "cert-manager.io/v1.ClusterIssuer", - "cert-manager.io/v1.Issuer", - ), - }, - }, { - name: "invalid dir", - dirs: []string{"invalid"}, - wantErr: true, + name: "does not exist", + fileSystems: []fs.FS{os.DirFS("../../invalid")}, + want: map[string]sets.Set[string]{}, }, { - name: "invalid fs", - fs: os.DirFS("../../invalid"), - dirs: []string{"."}, - wantErr: true, + name: "not a directory", + fileSystems: []fs.FS{os.DirFS("../../testcases/schemas/error_not_a_dir")}, + wantErr: true, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := NewLocalCRDFiles(tt.fs, tt.dirs) + k := NewLocalCRDFiles(tt.fileSystems...) paths, err := k.Paths() if (err != nil) != tt.wantErr { t.Errorf("localCRDsClient.Paths() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/openapiclient/local_schemas.go b/pkg/openapiclient/local_schemas.go index 2652f646..d247ca3f 100644 --- a/pkg/openapiclient/local_schemas.go +++ b/pkg/openapiclient/local_schemas.go @@ -14,34 +14,40 @@ import ( // client which provides openapi read from files on disk type localSchemasClient struct { - fs fs.FS - dir string + fs fs.FS } // Dir should have openapi files following directory layout: // ///.json // /api/.json -func NewLocalSchemaFiles(fs fs.FS, dirPath string) openapi.Client { +func NewLocalSchemaFiles(fs fs.FS) openapi.Client { return &localSchemasClient{ - fs: fs, - dir: dirPath, + fs: fs, } } func (k *localSchemasClient) Paths() (map[string]openapi.GroupVersion, error) { - if len(k.dir) == 0 { + if k.fs == nil { return nil, nil } + // check if '.' can be listed + if _, err := fs.ReadDir(k.fs, "."); err != nil { + if crossPlatformCheckDirExists(k.fs, ".") { + return nil, fmt.Errorf("error listing %s: %w", ".", err) + } + } res := map[string]openapi.GroupVersion{} - apiGroups, err := fs.ReadDir(k.fs, path.Join(k.dir, "apis")) - if err != nil && !errors.Is(err, fs.ErrNotExist) { - return nil, fmt.Errorf("failed reading local files dir %s: %w", path.Join(k.dir, "apis"), err) + apiGroups, err := fs.ReadDir(k.fs, "apis") + if err != nil { + if crossPlatformCheckDirExists(k.fs, "apis") { + return nil, fmt.Errorf("error listing %s: %w", "apis", err) + } } for _, f := range apiGroups { - groupPath := path.Join(k.dir, "apis", f.Name()) + groupPath := path.Join("apis", f.Name()) versions, err := fs.ReadDir(k.fs, groupPath) if err != nil { - return nil, fmt.Errorf("failed reading local files dir %s: %w", groupPath, err) + return nil, fmt.Errorf("error listing %s: %w", groupPath, err) } for _, v := range versions { if !utils.IsJson(v.Name()) { @@ -52,9 +58,11 @@ func (k *localSchemasClient) Paths() (map[string]openapi.GroupVersion, error) { res[apisPath] = groupversion.NewForFile(k.fs, path.Join(groupPath, v.Name())) } } - coregroup, err := fs.ReadDir(k.fs, path.Join(k.dir, "api")) - if err != nil && !errors.Is(err, fs.ErrNotExist) { - return nil, fmt.Errorf("failed reading local files dir %s: %w", path.Join(k.dir, "api"), err) + coregroup, err := fs.ReadDir(k.fs, "api") + if err != nil { + if crossPlatformCheckDirExists(k.fs, "api") { + return nil, fmt.Errorf("error listing %s: %w", "api", err) + } } for _, v := range coregroup { if !utils.IsJson(v.Name()) { @@ -62,7 +70,15 @@ func (k *localSchemasClient) Paths() (map[string]openapi.GroupVersion, error) { } name := strings.TrimSuffix(v.Name(), path.Ext(v.Name())) apiPath := path.Join("api", name) - res[apiPath] = groupversion.NewForFile(k.fs, path.Join(k.dir, "api", v.Name())) + res[apiPath] = groupversion.NewForFile(k.fs, path.Join("api", v.Name())) } return res, nil } + +func crossPlatformCheckDirExists(f fs.FS, path string) bool { + _, err := fs.Stat(f, path) + if err != nil { + return !errors.Is(err, fs.ErrNotExist) + } + return true +} diff --git a/pkg/openapiclient/local_schemas_test.go b/pkg/openapiclient/local_schemas_test.go index b18c050a..bb59a176 100644 --- a/pkg/openapiclient/local_schemas_test.go +++ b/pkg/openapiclient/local_schemas_test.go @@ -3,48 +3,46 @@ package openapiclient import ( "io/fs" "os" - "reflect" "testing" + "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/openapi" ) func TestNewLocalSchemaFiles(t *testing.T) { tests := []struct { - name string - fs fs.FS - dirPath string - want openapi.Client + name string + fs fs.FS + want openapi.Client }{{ - name: "fs nil and dir empty", + name: "without fs", want: &localSchemasClient{}, }, { - name: "only dir", - dirPath: "test", - want: &localSchemasClient{ - dir: "test", - }, - }, { - name: "only fs", + name: "with fs", fs: os.DirFS("."), want: &localSchemasClient{ fs: os.DirFS("."), }, }, { - name: "both fs and dir", - fs: os.DirFS("."), - dirPath: "test", + name: "with sub fs", + fs: func() fs.FS { + sub, err := fs.Sub(os.DirFS("."), "test") + assert.NoError(t, err) + return sub + }(), want: &localSchemasClient{ - fs: os.DirFS("."), - dir: "test", + fs: func() fs.FS { + sub, err := fs.Sub(os.DirFS("."), "test") + assert.NoError(t, err) + return sub + }(), }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := NewLocalSchemaFiles(tt.fs, tt.dirPath); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewLocalSchemaFiles() = %v, want %v", got, tt.want) - } + got := NewLocalSchemaFiles(tt.fs) + assert.Equal(t, tt.want, got) }) } } @@ -53,52 +51,13 @@ func Test_localSchemasClient_Paths(t *testing.T) { tests := []struct { name string fs fs.FS - dir string want sets.Set[string] wantErr bool }{{ - name: "fs nil and dir empty", - }, { - name: "only dir", - dir: "./builtins/1.27", - want: sets.New( - "api/v1", - "apis/admissionregistration.k8s.io/v1", - "apis/admissionregistration.k8s.io/v1alpha1", - "apis/apiextensions.k8s.io/v1", - "apis/apiregistration.k8s.io/v1", - "apis/apps/v1", - "apis/authentication.k8s.io/v1", - "apis/authentication.k8s.io/v1alpha1", - "apis/authentication.k8s.io/v1beta1", - "apis/authorization.k8s.io/v1", - "apis/autoscaling/v1", - "apis/autoscaling/v2", - "apis/batch/v1", - "apis/certificates.k8s.io/v1", - "apis/certificates.k8s.io/v1alpha1", - "apis/coordination.k8s.io/v1", - "apis/discovery.k8s.io/v1", - "apis/events.k8s.io/v1", - "apis/flowcontrol.apiserver.k8s.io/v1beta2", - "apis/flowcontrol.apiserver.k8s.io/v1beta3", - "apis/internal.apiserver.k8s.io/v1alpha1", - "apis/networking.k8s.io/v1", - "apis/networking.k8s.io/v1alpha1", - "apis/node.k8s.io/v1", - "apis/policy/v1", - "apis/rbac.authorization.k8s.io/v1", - "apis/resource.k8s.io/v1alpha2", - "apis/scheduling.k8s.io/v1", - "apis/storage.k8s.io/v1", - ), + name: "without fs", }, { - name: "only fs", + name: "with fs", fs: os.DirFS("./builtins/1.27"), - }, { - name: "both fs and dir", - fs: os.DirFS("./builtins"), - dir: "1.27", want: sets.New( "api/v1", "apis/admissionregistration.k8s.io/v1", @@ -130,29 +89,24 @@ func Test_localSchemasClient_Paths(t *testing.T) { "apis/scheduling.k8s.io/v1", "apis/storage.k8s.io/v1", ), - }, { - name: "invalid dir", - dir: "invalid", - want: sets.New[string](), }, { name: "invalid fs", fs: os.DirFS("../../invalid"), - dir: ".", want: sets.New[string](), }, { name: "not a dir", - fs: os.DirFS("../../testcases/schemas"), - dir: "error_not_a_dir", + fs: os.DirFS("../../testcases/schemas/error_not_a_dir"), wantErr: true, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := NewLocalSchemaFiles(tt.fs, tt.dir) + k := NewLocalSchemaFiles(tt.fs) paths, err := k.Paths() - if (err != nil) != tt.wantErr { - t.Errorf("localSchemasClient.Paths() error = %v, wantErr %v", err, tt.wantErr) + if tt.wantErr { + assert.Error(t, err) return } + assert.NoError(t, err) var got sets.Set[string] if paths != nil { got = sets.New[string]() @@ -160,9 +114,7 @@ func Test_localSchemasClient_Paths(t *testing.T) { got.Insert(key) } } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("localSchemasClient.Paths() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, got) }) } } diff --git a/pkg/openapiclient/overlay.go b/pkg/openapiclient/overlay.go index 8a1c9175..9eee5154 100644 --- a/pkg/openapiclient/overlay.go +++ b/pkg/openapiclient/overlay.go @@ -13,19 +13,23 @@ import ( var patchesFS embed.FS func HardcodedPatchLoader(version string) groupversion.PatchLoaderFn { - return PatchLoaderFromDirectory(patchesFS, path.Join("patches", version)) + sub, err := fs.Sub(patchesFS, path.Join("patches", version)) + if err != nil { + return nil + } + return PatchLoaderFromDirectory(sub) } -func PatchLoaderFromDirectory(filesystem fs.FS, dir string) groupversion.PatchLoaderFn { - if len(dir) == 0 && filesystem == nil { +func PatchLoaderFromDirectory(filesystem fs.FS) groupversion.PatchLoaderFn { + if filesystem == nil { return nil } return func(s string) []byte { - if res, err := fs.ReadFile(filesystem, path.Join(dir, s+".json")); err == nil { + if res, err := fs.ReadFile(filesystem, path.Join(s+".json")); err == nil { return res - } else if res, err := fs.ReadFile(filesystem, path.Join(dir, s+".yaml")); err == nil { + } else if res, err := fs.ReadFile(filesystem, path.Join(s+".yaml")); err == nil { return res - } else if res, err := fs.ReadFile(filesystem, path.Join(dir, s+".yml")); err == nil { + } else if res, err := fs.ReadFile(filesystem, path.Join(s+".yml")); err == nil { return res } return nil diff --git a/pkg/validator/factory_test.go b/pkg/validator/factory_test.go index 14db264e..80187d91 100644 --- a/pkg/validator/factory_test.go +++ b/pkg/validator/factory_test.go @@ -42,7 +42,7 @@ func TestValidatorFactory_TestPatcher(t *testing.T) { { name: "cronjob_crd", file: "./testdata/cronjob.yaml", - client: openapiclient.NewLocalCRDFiles(nil, []string{"./testdata/crds/"}), + client: openapiclient.NewLocalCRDFiles(os.DirFS("./testdata/crds/")), want: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "batch.tutorial.kubebuilder.io/v1",