Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion listener/reconciler/clusteraccess/subroutines.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import (
"github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler"
)

// generateSchemaSubroutine processes ClusterAccess resources and generates schemas
const (
finalizerName = "gateway.openmfp.org/clusteraccess-finalizer"
lastSchemaPathAnnotation = "gateway.openmfp.org/last-schema-path"
)

// generateSchemaSubroutine processes ClusterAccess resources and generates schemas
type generateSchemaSubroutine struct {
reconciler *ClusterAccessReconciler
Expand Down Expand Up @@ -72,12 +78,41 @@ func (s *generateSchemaSubroutine) Process(ctx context.Context, instance runtime
return ctrl.Result{}, commonserrors.NewOperatorError(err, false, false)
}

// If path changed, delete the old schema file referenced in the annotation
prevPath := ""
if ann := clusterAccess.GetAnnotations(); ann != nil {
prevPath = ann[lastSchemaPathAnnotation]
}
if prevPath != "" && prevPath != clusterName {
if err := s.reconciler.ioHandler.Delete(prevPath); err != nil {
// Log and continue; do not fail reconciliation on cleanup issues
s.reconciler.log.Warn().Err(err).Str("previousPath", prevPath).Str("clusterAccess", clusterAccessName).Msg("failed to delete previous schema file")
}
}

// Write schema to file using cluster name from path or resource name
if err := s.reconciler.ioHandler.Write(schemaWithMetadata, clusterName); err != nil {
s.reconciler.log.Error().Err(err).Str("clusterAccess", clusterAccessName).Msg("failed to write schema")
return ctrl.Result{}, commonserrors.NewOperatorError(err, false, false)
}

// Ensure annotation reflects the current path for future cleanups
needUpdate := prevPath != clusterName
if needUpdate {
obj := clusterAccess.DeepCopy()
if obj.Annotations == nil {
obj.Annotations = map[string]string{}
}
obj.Annotations[lastSchemaPathAnnotation] = clusterName
if err := s.reconciler.opts.Client.Update(ctx, obj); err != nil {
// Log but do not fail reconciliation; file has been written already
s.reconciler.log.Warn().Err(err).Str("clusterAccess", clusterAccessName).Msg("failed to update last schema path annotation")
} else {
// Reflect update locally too to avoid future confusion in this reconcile loop
clusterAccess.Annotations = obj.Annotations
}
}

s.reconciler.log.Info().Str("clusterAccess", clusterAccessName).Msg("successfully processed ClusterAccess resource")
return ctrl.Result{}, nil
}
Expand All @@ -97,6 +132,36 @@ func (s *generateSchemaSubroutine) restMapperFromConfig(cfg *rest.Config) (meta.
}

func (s *generateSchemaSubroutine) Finalize(ctx context.Context, instance runtimeobject.RuntimeObject) (ctrl.Result, commonserrors.OperatorError) {
clusterAccess, ok := instance.(*gatewayv1alpha1.ClusterAccess)
if !ok {
s.reconciler.log.Error().Msg("instance is not a ClusterAccess resource in Finalize")
return ctrl.Result{}, commonserrors.NewOperatorError(errors.New("invalid resource type"), false, false)
}

// Determine current and previously used paths
currentPath := clusterAccess.Spec.Path
if currentPath == "" {
currentPath = clusterAccess.GetName()
}
prevPath := ""
if ann := clusterAccess.GetAnnotations(); ann != nil {
prevPath = ann[lastSchemaPathAnnotation]
}

// Try deleting current path file
if currentPath != "" {
if err := s.reconciler.ioHandler.Delete(currentPath); err != nil {
// Log and continue; do not block finalization just because file was missing or deletion failed
s.reconciler.log.Warn().Err(err).Str("path", currentPath).Str("clusterAccess", clusterAccess.GetName()).Msg("failed to delete schema file during finalization")
}
}
// If previous differs, try deleting it as well
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case, where old file remains? If this code works, we will receive failed to delete previous schema file during finalization every time.
I wonder if deletion of previous filename makes sense at all.

if prevPath != "" && prevPath != currentPath {
if err := s.reconciler.ioHandler.Delete(prevPath); err != nil {
s.reconciler.log.Warn().Err(err).Str("path", prevPath).Str("clusterAccess", clusterAccess.GetName()).Msg("failed to delete previous schema file during finalization")
}
}

return ctrl.Result{}, nil
}

Expand All @@ -105,5 +170,5 @@ func (s *generateSchemaSubroutine) GetName() string {
}

func (s *generateSchemaSubroutine) Finalizers() []string {
return nil
return []string{finalizerName}
}
122 changes: 122 additions & 0 deletions listener/reconciler/clusteraccess/subroutines_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
package clusteraccess

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/platform-mesh/golang-commons/logger"
gatewayv1alpha1 "github.com/platform-mesh/kubernetes-graphql-gateway/common/apis/v1alpha1"
workspacefile_mocks "github.com/platform-mesh/kubernetes-graphql-gateway/listener/pkg/workspacefile/mocks"
"github.com/platform-mesh/kubernetes-graphql-gateway/listener/reconciler"
)

func TestGenerateSchemaSubroutine_Process_InvalidResourceType(t *testing.T) {
mockIO := workspacefile_mocks.NewMockIOHandler(t)
log, _ := logger.New(logger.DefaultConfig())

r := &ClusterAccessReconciler{
ioHandler: mockIO,
log: log,
}
s := &generateSchemaSubroutine{reconciler: r}

_, opErr := s.Process(context.Background(), &metav1.PartialObjectMetadata{})

assert.NotNil(t, opErr)
}

func TestGenerateSchemaSubroutine_Process_MissingHostReturnsError(t *testing.T) {
scheme := runtime.NewScheme()
_ = gatewayv1alpha1.AddToScheme(scheme)

ca := &gatewayv1alpha1.ClusterAccess{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Annotations: map[string]string{},
},
Spec: gatewayv1alpha1.ClusterAccessSpec{
// Host is intentionally empty to trigger validation error
},
}

fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(ca).Build()

mockIO := workspacefile_mocks.NewMockIOHandler(t)
mockIO.EXPECT().Write(mock.Anything, mock.Anything).Maybe().Return(nil)
mockIO.EXPECT().Delete(mock.Anything).Maybe().Return(nil)

log, _ := logger.New(logger.DefaultConfig())

r := &ClusterAccessReconciler{
ioHandler: mockIO,
log: log,
opts: reconciler.ReconcilerOpts{
Client: fakeClient,
Config: &rest.Config{Host: "https://unit-test.invalid"},
ManagerOpts: ctrl.Options{Scheme: scheme},
Scheme: scheme,
},
}
s := &generateSchemaSubroutine{reconciler: r}

res, opErr := s.Process(context.Background(), ca)

assert.NotNil(t, opErr)
assert.Equal(t, ctrl.Result{}, res)
}

func TestGenerateSchemaSubroutine_Finalize_DeletesCurrentAndPreviousPaths(t *testing.T) {
mockIO := workspacefile_mocks.NewMockIOHandler(t)
log, _ := logger.New(logger.DefaultConfig())

// Expect deletion of both current and previous paths
mockIO.EXPECT().Delete("current-path").Return(nil).Once()
mockIO.EXPECT().Delete("previous-path").Return(nil).Once()

r := &ClusterAccessReconciler{
ioHandler: mockIO,
log: log,
}
s := &generateSchemaSubroutine{reconciler: r}

ca := &gatewayv1alpha1.ClusterAccess{
ObjectMeta: metav1.ObjectMeta{
Name: "my-resource",
Annotations: map[string]string{
lastSchemaPathAnnotation: "previous-path",
},
},
Spec: gatewayv1alpha1.ClusterAccessSpec{
Path: "current-path",
},
}

res, opErr := s.Finalize(context.Background(), ca)

assert.Nil(t, opErr)
assert.Equal(t, ctrl.Result{}, res)
}

func TestGenerateSchemaSubroutine_restMapperFromConfig_SucceedsWithMinimalConfig(t *testing.T) {
mockIO := workspacefile_mocks.NewMockIOHandler(t)
log, _ := logger.New(logger.DefaultConfig())

r := &ClusterAccessReconciler{
ioHandler: mockIO,
log: log,
}
s := &generateSchemaSubroutine{reconciler: r}

rm, err := s.restMapperFromConfig(&rest.Config{})

assert.NotNil(t, rm)
assert.NoError(t, err)
}
Loading