chore: add windows tests (#120)

* chore: add windows tests Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> * fix Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com> --------- Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
kubernetes-sigs · Apr 26, 2024 · 57b5e37 · 57b5e37
1 parent 4ffbdb6
commit 57b5e37
Show file tree

Hide file tree

Showing 15 changed files with 159 additions and 246 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -11,7 +11,11 @@ concurrency:
 
 jobs:
   build:
-    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
     steps:
       - name: Checkout
         uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.0
@@ -21,17 +25,17 @@ jobs:
         run: go mod tidy && git diff --exit-code
       - name: Build
         run: go build ./cmd/... ./pkg/...
-      - name: Check code format
-        run: |
-          diff=$(gofmt -s -d cmd pkg *.go)
-          if [[ -n "${diff}" ]]; then echo "${diff}"; exit 1; fi
       - name: Test
         run: go test ./...
   golangci-lint:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
         uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3.5.0
+      - name: Check code format
+        run: |
+          diff=$(gofmt -s -d cmd pkg *.go)
+          if [[ -n "${diff}" ]]; then echo "${diff}"; exit 1; fi
       - name: golangci-lint
         uses: golangci/golangci-lint-action@08e2f20817b15149a52b5b3ebe7de50aff2ba8c5 # v3.4.0
   required:

diff --git 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/github_builtins_test.go b/pkg/openapiclient/github_builtins_test.go
@@ -1,17 +1,15 @@
 package openapiclient_test
 
 import (
-	"fmt"
 	"testing"
 
 	"sigs.k8s.io/kubectl-validate/pkg/openapiclient"
 )
 
 func TestGitHubBuiltins(t *testing.T) {
 	c := openapiclient.NewGitHubBuiltins("1.27")
-	m, err := c.Paths()
+	_, err := c.Paths()
 	if err != nil {
 		t.Fatal(err)
 	}
-	fmt.Println(m)
 }
diff --git a/pkg/openapiclient/groupversion/file.go b/pkg/openapiclient/groupversion/file.go
@@ -7,7 +7,6 @@ import (
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/openapi"
-	"sigs.k8s.io/kubectl-validate/pkg/utils"
 )
 
 type fileGroupVersion struct {
@@ -19,7 +18,7 @@ func (gv *fileGroupVersion) Schema(contentType string) ([]byte, error) {
 	if strings.ToLower(contentType) != runtime.ContentTypeJSON {
 		return nil, fmt.Errorf("only application/json content type is supported")
 	}
-	return utils.ReadFile(gv.fs, gv.filepath)
+	return fs.ReadFile(gv.fs, gv.filepath)
 }
 
 func NewForFile(fs fs.FS, filepath string) openapi.GroupVersion {

diff --git a/pkg/openapiclient/hardcoded_builtins.go b/pkg/openapiclient/hardcoded_builtins.go
@@ -3,7 +3,7 @@ package openapiclient
 import (
 	"embed"
 	"fmt"
-	"path/filepath"
+	"path"
 	"strings"
 
 	"k8s.io/client-go/openapi"
@@ -38,29 +38,29 @@ func (k hardcodedResolver) Paths() (map[string]openapi.GroupVersion, error) {
 		if v.Name() == k.version {
 			res := map[string]openapi.GroupVersion{}
 
-			apiDir := filepath.Join("builtins", v.Name(), "api")
+			apiDir := path.Join("builtins", v.Name(), "api")
 			apiListing, _ := hardcodedBuiltins.ReadDir(apiDir)
 			for _, v := range apiListing {
 				// chop extension
-				ext := filepath.Ext(v.Name())
+				ext := path.Ext(v.Name())
 				version := strings.TrimSuffix(v.Name(), ext)
-				res[fmt.Sprintf("api/%s", version)] = groupversion.NewForFile(&hardcodedBuiltins, filepath.Join(apiDir, v.Name()))
+				res[fmt.Sprintf("api/%s", version)] = groupversion.NewForFile(&hardcodedBuiltins, path.Join(apiDir, v.Name()))
 			}
 
-			apisDir := filepath.Join("builtins", v.Name(), "apis")
+			apisDir := path.Join("builtins", v.Name(), "apis")
 			apisListing, _ := hardcodedBuiltins.ReadDir(apisDir)
 			for _, g := range apisListing {
-				gDir := filepath.Join(apisDir, g.Name())
+				gDir := path.Join(apisDir, g.Name())
 				vs, err := hardcodedBuiltins.ReadDir(gDir)
 				if err != nil {
 					return nil, err
 				}
 
 				for _, v := range vs {
 					// chop extension
-					ext := filepath.Ext(v.Name())
+					ext := path.Ext(v.Name())
 					version := strings.TrimSuffix(v.Name(), ext)
-					res[fmt.Sprintf("apis/%s/%s", g.Name(), version)] = groupversion.NewForFile(&hardcodedBuiltins, filepath.Join(gDir, v.Name()))
+					res[fmt.Sprintf("apis/%s/%s", g.Name(), version)] = groupversion.NewForFile(&hardcodedBuiltins, path.Join(gDir, v.Name()))
 				}
 			}
 

diff --git a/pkg/openapiclient/local_crds.go b/pkg/openapiclient/local_crds.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/fs"
-	"path/filepath"
 
 	"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 := utils.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 := filepath.Join(dir, f.Name())
+			path := f.Name()
 			if f.IsDir() {
 				continue
 			}
 			if utils.IsJson(f.Name()) {
-				fileBytes, err := utils.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 := utils.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
@@ -14,60 +14,44 @@ 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")
 		})
 	}
 }
 
 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)