fix: prevent overwriting schema in PreReconcile (#234)

* fix: skip schema write if unchanged in PreReconcile

* feat: added test for prereconcile

* fix: if the schema is missing then create the file instead of failing with an error

Author: kekcleader

listener/kcp/reconciler_factory.go

@@ -1,6 +1,7 @@
 package kcp
 
 import (
+	"bytes"
 	"context"
 	"errors"
 
@@ -35,6 +36,7 @@ var (
 	ErrCreatePathResolver    = errors.New("failed to create cluster path resolver")
 	ErrGetVWConfig           = errors.New("unable to get virtual workspace config, check if your kcp cluster is running")
 	ErrCreateHTTPClient      = errors.New("failed to create http client")
+	ErrReadJSON              = errors.New("failed to read JSON from filesystem")
 )
 
 type CustomReconciler interface {
@@ -107,12 +109,23 @@ func PreReconcile(
 	cr *apischema.CRDResolver,
 	io workspacefile.IOHandler,
 ) error {
-	JSON, err := cr.Resolve()
+	actualJSON, err := cr.Resolve()
 	if err != nil {
 		return errors.Join(ErrResolveSchema, err)
 	}
-	if err := io.Write(JSON, kubernetesClusterName); err != nil {
-		return errors.Join(ErrWriteJSON, err)
+
+	savedJSON, err := io.Read(kubernetesClusterName)
+	if err != nil {
+		if errors.Is(err, workspacefile.ErrNotFound) {
+			return io.Write(actualJSON, kubernetesClusterName)
+		}
+		return errors.Join(ErrReadJSON, err)
+	}
+
+	if !bytes.Equal(actualJSON, savedJSON) {
+		if err := io.Write(actualJSON, kubernetesClusterName); err != nil {
+			return errors.Join(ErrWriteJSON, err)
+		}
 	}
 
 	return nil

listener/kcp/reconciler_factory_test.go

@@ -106,3 +106,40 @@ func TestNewReconciler(t *testing.T) {
 		})
 	}
 }
+
+func TestPreReconcile(t *testing.T) {
+	tempDir := t.TempDir()
+
+	tests := map[string]struct {
+		cr  *apischema.CRDResolver
+		err error
+	}{
+		"error_on_empty_resolver": {
+			cr: func() *apischema.CRDResolver {
+				discovery := &mocks.MockDiscoveryInterface{}
+				discovery.On("ServerPreferredResources").Return(nil, errors.New("failed to get server resources"))
+
+				return &apischema.CRDResolver{
+					DiscoveryInterface: discovery,
+					RESTMapper:         &mocks.MockRESTMapper{},
+				}
+			}(),
+			err: errors.Join(ErrResolveSchema,
+				errors.New("failed to get server preferred resources"),
+				errors.New("failed to get server resources")),
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			ioHandler, err := workspacefile.NewIOHandler(tempDir)
+			assert.NoError(t, err)
+			err = PreReconcile(tc.cr, ioHandler)
+			if tc.err != nil {
+				assert.EqualError(t, err, tc.err.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}

listener/workspacefile/io_handler.go

@@ -11,6 +11,7 @@ var (
 	ErrReadJSONFile     = errors.New("failed to read JSON file")
 	ErrWriteJSONFile    = errors.New("failed to write JSON to file")
 	ErrDeleteJSONFile   = errors.New("failed to delete JSON file")
+	ErrNotFound         = errors.New("failed to find JSON file")
 )
 
 type IOHandler interface {
@@ -37,6 +38,9 @@ func (h *IOHandlerProvider) Read(clusterName string) ([]byte, error) {
 	fileName := path.Join(h.schemasDir, clusterName)
 	JSON, err := os.ReadFile(fileName)
 	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, ErrNotFound
+		}
 		return nil, errors.Join(ErrReadJSONFile, err)
 	}
 	return JSON, nil