diff --git a/.golangci.yaml b/.golangci.yaml index 49e58ad..afbb0db 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -100,6 +100,7 @@ linters-settings: - github.com/go-logr/logr - k8s.io - sigs.k8s.io + - github.com/spf13/pflag dupl: threshold: 100 funlen: diff --git a/go.mod b/go.mod index 50359e1..6b0ca39 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/onsi/ginkgo/v2 v2.22.2 github.com/onsi/gomega v1.36.2 + github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.10.0 k8s.io/api v0.32.0 k8s.io/apiextensions-apiserver v0.32.0 @@ -69,7 +70,6 @@ require ( github.com/prometheus/procfs v0.15.1 // indirect github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/spf13/cobra v1.8.1 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/stretchr/objx v0.5.2 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xlab/treeprint v1.2.0 // indirect diff --git a/pkg/crdutil/crdutil.go b/pkg/crdutil/crdutil.go index 9fa4f9e..6f06eee 100644 --- a/pkg/crdutil/crdutil.go +++ b/pkg/crdutil/crdutil.go @@ -26,6 +26,7 @@ import ( "path/filepath" "strings" + "github.com/spf13/pflag" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" v1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1" @@ -37,38 +38,29 @@ import ( ctrl "sigs.k8s.io/controller-runtime" ) -type StringList []string - -func (s *StringList) String() string { - return strings.Join(*s, ", ") -} - -func (s *StringList) Set(value string) error { - *s = append(*s, value) - return nil -} - var ( - crdsDir StringList + crdsDir []string ) func initFlags() { - flag.Var(&crdsDir, "crds-dir", "Path to the directory containing the CRD manifests") - flag.Parse() + pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + pflag.StringSliceVarP(&crdsDir, "crds-dir", "f", crdsDir, "The files or directories that contain the CRDs to apply.") + pflag.Parse() if len(crdsDir) == 0 { - log.Fatalf("CRDs directory is required") + log.Fatalf("CRDs directory or single CRDs are required") } - for _, crdDir := range crdsDir { - if _, err := os.Stat(crdDir); os.IsNotExist(err) { - log.Fatalf("CRDs directory %s does not exist", crdsDir) + for _, path := range crdsDir { + if _, err := os.Stat(path); os.IsNotExist(err) { + log.Fatalf("path does not exist: %s", path) } } } // EnsureCRDsCmd reads each YAML file in the directory, splits it into documents, and applies each CRD to the cluster. // The parameter --crds-dir is required and should point to the directory containing the CRD manifests. +// TODO: add unit test for this command. func EnsureCRDsCmd() { ctx := context.Background() @@ -84,38 +76,71 @@ func EnsureCRDsCmd() { log.Fatalf("Failed to create API extensions client: %v", err) } - if err := walkCrdsDir(ctx, client.ApiextensionsV1().CustomResourceDefinitions()); err != nil { - log.Fatalf("Failed to apply CRDs: %v", err) + pathsToApply, err := collectYamlPaths(crdsDir) + if err != nil { + log.Fatalf("Failed to walk through CRDs: %v", err) + } + + for _, path := range pathsToApply { + log.Printf("Apply CRDs from file: %s", path) + if err := applyCRDs(ctx, client.ApiextensionsV1().CustomResourceDefinitions(), path); err != nil { + log.Fatalf("Failed to apply CRDs: %v", err) + } } } -// walkCrdsDir walks the CRDs directory and applies each YAML file. -func walkCrdsDir(ctx context.Context, crdClient v1.CustomResourceDefinitionInterface) error { - for _, crdDir := range crdsDir { +// collectYamlPaths processes a list of paths and returns all YAML files. +func collectYamlPaths(crdDirs []string) ([]string, error) { + paths := map[string]struct{}{} + for _, crdDir := range crdDirs { + // We need the parent directory to check if we are in the top-level directory. + // This is necessary for the recursive logic. + // We can skip the errors as it has been checked in initFlags. + parentDir, err := os.Stat(crdDir) + if err != nil { + return []string{}, fmt.Errorf("stat the path %s: %w", crdDir, err) + } // Walk the directory recursively and apply each YAML file. - err := filepath.Walk(crdDir, func(path string, info os.FileInfo, err error) error { + err = filepath.Walk(crdDir, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - if info.IsDir() || filepath.Ext(path) != ".yaml" { + // If this is a directory, skip it. + // filepath.Walk() is also called for directories, but we only want to apply CRDs from files. + if info.IsDir() { return nil } - - log.Printf("Apply CRDs from file: %s", path) - if err := applyCRDsFromFile(ctx, crdClient, path); err != nil { - return fmt.Errorf("apply CRD %s: %w", path, err) + if filepath.Ext(path) != ".yaml" && filepath.Ext(path) != ".yml" { + return nil } + // If we apply a dir we only want to apply the CRDs in the directory (i.e., not recursively). + // filepath.Dir() does not add a trailing slash, thus we need to trim it in the crdDir. + if parentDir.IsDir() && filepath.Dir(path) != strings.TrimRight(crdDir, "/") { + return nil + } + + paths[path] = struct{}{} return nil }) if err != nil { - return fmt.Errorf("walk the path %s: %w", crdsDir, err) + return []string{}, fmt.Errorf("walk the path %s: %w", crdDirs, err) } } - return nil + return mapToSlice(paths), nil } -// applyCRDsFromFile reads a YAML file, splits it into documents, and applies each CRD to the cluster. -func applyCRDsFromFile(ctx context.Context, crdClient v1.CustomResourceDefinitionInterface, filePath string) error { +// mapToSlice converts a map to a slice. +// The map is used to deduplicate the paths. +func mapToSlice(m map[string]struct{}) []string { + s := []string{} + for k := range m { + s = append(s, k) + } + return s +} + +// applyCRDs reads a YAML file, splits it into documents, and applies each CRD to the cluster. +func applyCRDs(ctx context.Context, crdClient v1.CustomResourceDefinitionInterface, filePath string) error { file, err := os.Open(filePath) if err != nil { return fmt.Errorf("open file %q: %w", filePath, err) @@ -168,16 +193,21 @@ func applyCRD( log.Printf("Create CRD %s", crd.Name) _, err = crdClient.Create(ctx, crd, metav1.CreateOptions{}) if err != nil { - return fmt.Errorf("create CRD %s: %w", crd.Name, err) - } - } else { - log.Printf("Update CRD %s", crd.Name) - // Set resource version to update an existing CRD. - crd.SetResourceVersion(curCRD.GetResourceVersion()) - _, err = crdClient.Update(ctx, crd, metav1.UpdateOptions{}) - if err != nil { - return fmt.Errorf("update CRD %s: %w", crd.Name, err) + return fmt.Errorf("create CRD: %w", err) } + return nil + } + if err != nil { + return fmt.Errorf("get CRD %s: %w", crd.Name, err) } + + log.Printf("Update CRD %s", crd.Name) + // Set resource version to update an existing CRD. + crd.SetResourceVersion(curCRD.GetResourceVersion()) + _, err = crdClient.Update(ctx, crd, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("update CRD %s: %w", crd.Name, err) + } + return nil } diff --git a/pkg/crdutil/crdutil_test.go b/pkg/crdutil/crdutil_test.go index 9691570..1212256 100644 --- a/pkg/crdutil/crdutil_test.go +++ b/pkg/crdutil/crdutil_test.go @@ -38,13 +38,48 @@ var _ = Describe("CRD Application", func() { Expect(testCRDClient.DeleteCollection(ctx, metav1.DeleteOptions{}, metav1.ListOptions{})).NotTo(HaveOccurred()) }) - Describe("applyCRDsFromFile", func() { + Describe("collectYamlPaths", func() { + It("should collect all YAML files in a directory", func() { + By("collecting YAML paths") + paths, err := collectYamlPaths([]string{"test-files"}) + Expect(err).NotTo(HaveOccurred()) + Expect(paths).To(ConsistOf( + "test-files/test-crds.yaml", + "test-files/updated-test-crds.yaml", + )) + }) + + It("should collect a single YAML file", func() { + By("collecting YAML paths") + paths, err := collectYamlPaths([]string{"test-files/test-crds.yaml"}) + Expect(err).NotTo(HaveOccurred()) + Expect(paths).To(ConsistOf("test-files/test-crds.yaml")) + }) + + It("should deduplicate YAML file", func() { + By("collecting YAML paths") + paths, err := collectYamlPaths([]string{"test-files/test-crds.yaml", "test-files"}) + Expect(err).NotTo(HaveOccurred()) + Expect(paths).To(ConsistOf( + "test-files/test-crds.yaml", + "test-files/updated-test-crds.yaml", + )) + }) + + It("should fail to collect non-existent YAML files", func() { + By("collecting YAML paths") + _, err := collectYamlPaths([]string{"test-files/non-existent.yaml"}) + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("applyCRDs", func() { It("should apply CRDs multiple times from a valid YAML file", func() { By("applying CRDs") - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) By("verifying CRDs are applied") crds, err := testCRDClient.List(ctx, metav1.ListOptions{}) @@ -54,7 +89,7 @@ var _ = Describe("CRD Application", func() { It("should update CRDs", func() { By("applying CRDs") - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/test-crds.yaml")).To(Succeed()) By("verifying CRDs do not have spec.foobar") for _, crdName := range []string{"bars.example.com", "foos.example.com"} { @@ -66,7 +101,7 @@ var _ = Describe("CRD Application", func() { } By("updating CRDs") - Expect(applyCRDsFromFile(ctx, testCRDClient, "test-files/updated-test-crds.yaml")).To(Succeed()) + Expect(applyCRDs(ctx, testCRDClient, "test-files/updated-test-crds.yaml")).To(Succeed()) By("verifying CRDs are updated") for _, crdName := range []string{"bars.example.com", "foos.example.com"} { diff --git a/pkg/crdutil/test-files/nested/do-not-apply.yaml b/pkg/crdutil/test-files/nested/do-not-apply.yaml new file mode 100644 index 0000000..154cb5c --- /dev/null +++ b/pkg/crdutil/test-files/nested/do-not-apply.yaml @@ -0,0 +1,22 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: foos.example.com +spec: + group: example.com + names: + kind: Foo + listKind: FooList + singular: foo + plural: foos + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object