diff --git a/.golangci.yaml b/.golangci.yaml index 11485e5a739..d18bc2afb67 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -33,7 +33,6 @@ linters: - prealloc - revive - staticcheck - - thelper - unconvert - unused - usestdlibvars diff --git a/pkg/reconciler/cache/replication/replication_reconcile.go b/pkg/reconciler/cache/replication/replication_reconcile.go index d301fe9b86a..daad9f0eb93 100644 --- a/pkg/reconciler/cache/replication/replication_reconcile.go +++ b/pkg/reconciler/cache/replication/replication_reconcile.go @@ -19,7 +19,6 @@ package replication import ( "context" "fmt" - "reflect" "strings" kcpcache "github.com/kcp-dev/apimachinery/v2/pkg/cache" @@ -55,7 +54,10 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { }, func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localAPIExportLister.Cluster(cluster).Get(name) - return obj, true, err + if err != nil { + return nil, false, err + } + return obj, true, nil }) case apisv1alpha1.SchemeGroupVersion.WithResource("apiresourceschemas").String(): return c.reconcileObject(ctx, @@ -67,7 +69,10 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { }, func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localAPIResourceSchemaLister.Cluster(cluster).Get(name) - return obj, true, err + if err != nil { + return nil, false, err + } + return obj, true, nil }) case corev1alpha1.SchemeGroupVersion.WithResource("shards").String(): return c.reconcileObject(ctx, @@ -79,7 +84,10 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { }, func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localShardLister.Cluster(cluster).Get(name) - return obj, true, err + if err != nil { + return nil, false, err + } + return obj, true, nil }) case tenancyv1alpha1.SchemeGroupVersion.WithResource("workspacetypes").String(): return c.reconcileObject(ctx, @@ -91,7 +99,10 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { }, func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localWorkspaceTypeLister.Cluster(cluster).Get(name) - return obj, true, err + if err != nil { + return nil, false, err + } + return obj, true, nil }) case rbacv1.SchemeGroupVersion.WithResource("clusterroles").String(): return c.reconcileObject(ctx, @@ -104,9 +115,9 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localClusterRoleLister.Cluster(cluster).Get(name) if err != nil { - return nil, true, err + return nil, false, err } - return obj, obj.Annotations[core.ReplicateAnnotationKey] != "", err + return obj, obj.Annotations[core.ReplicateAnnotationKey] != "", nil }) case rbacv1.SchemeGroupVersion.WithResource("clusterrolebindings").String(): return c.reconcileObject(ctx, @@ -119,9 +130,9 @@ func (c *controller) reconcile(ctx context.Context, gvrKey string) error { func(cluster logicalcluster.Name, _, name string) (interface{}, bool, error) { obj, err := c.localClusterRoleBindingLister.Cluster(cluster).Get(name) if err != nil { - return nil, true, err + return nil, false, err } - return obj, obj.Annotations[core.ReplicateAnnotationKey] != "", err + return obj, obj.Annotations[core.ReplicateAnnotationKey] != "", nil }) default: return fmt.Errorf("unsupported resource %v", keyParts[0]) @@ -149,9 +160,6 @@ func (c *controller) reconcileObject(ctx context.Context, if err != nil && !errors.IsNotFound(err) { return err } - if !replicate { - return nil - } if errors.IsNotFound(err) { // issue a live GET to make sure the localObject was removed _, err = c.dynamicKcpLocalClient.Cluster(cluster.Path()).Resource(gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{}) @@ -161,11 +169,13 @@ func (c *controller) reconcileObject(ctx context.Context, if !errors.IsNotFound(err) { return err } + } else if !replicate { + localObject = nil // like if it wasn't there } var unstructuredCacheObject *unstructured.Unstructured var unstructuredLocalObject *unstructured.Unstructured - if isNotNil(cacheObject) { + if cacheObject != nil { unstructuredCacheObject, err = toUnstructured(cacheObject) if err != nil { return err @@ -173,7 +183,7 @@ func (c *controller) reconcileObject(ctx context.Context, unstructuredCacheObject.SetKind(gvk.Kind) unstructuredCacheObject.SetAPIVersion(gvr.GroupVersion().String()) } - if isNotNil(localObject) { + if localObject != nil { unstructuredLocalObject, err = toUnstructured(localObject) if err != nil { return err @@ -181,7 +191,7 @@ func (c *controller) reconcileObject(ctx context.Context, unstructuredLocalObject.SetKind(gvk.Kind) unstructuredLocalObject.SetAPIVersion(gvr.GroupVersion().String()) } - if cluster.Empty() && isNotNil(localObject) { + if cluster.Empty() && localObject != nil { metadata, err := meta.Accessor(localObject) if err != nil { return err @@ -205,7 +215,3 @@ func retrieveCacheObject(gvr *schema.GroupVersionResource, cacheIndex cache.Inde } return cacheObjects[0], nil } - -func isNotNil(obj interface{}) bool { - return obj != nil && (reflect.ValueOf(obj).Kind() == reflect.Ptr && !reflect.ValueOf(obj).IsNil()) -} diff --git a/pkg/reconciler/cache/replication/replication_reconcile_test.go b/pkg/reconciler/cache/replication/replication_reconcile_test.go index 1319df989ba..4a4107427c0 100644 --- a/pkg/reconciler/cache/replication/replication_reconcile_test.go +++ b/pkg/reconciler/cache/replication/replication_reconcile_test.go @@ -59,8 +59,6 @@ func TestReconcileAPIExports(t *testing.T) { initialLocalAPIExports: []runtime.Object{newAPIExport("foo")}, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - if len(localClientActions) != 0 { t.Fatalf("unexpected REST calls were made to the localDynamicClient: %#v", localClientActions) } @@ -105,8 +103,6 @@ func TestReconcileAPIExports(t *testing.T) { initCacheFakeClientWithInitialAPIExports: true, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - if len(localClientActions) != 0 { t.Fatalf("unexpected REST calls were made to the localDynamicClient: %#v", localClientActions) } @@ -135,8 +131,6 @@ func TestReconcileAPIExports(t *testing.T) { initCacheFakeClientWithInitialAPIExports: true, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - wasGlobalAPIExportDeletionValidated := false wasGlobalAPIExportRetrievalValidated := false for _, action := range localClientActions { @@ -186,8 +180,6 @@ func TestReconcileAPIExports(t *testing.T) { initCacheFakeClientWithInitialAPIExports: true, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - if len(localClientActions) != 0 { t.Fatalf("unexpected REST calls were made to the localDynamicClient: %#v", localClientActions) } @@ -231,8 +223,6 @@ func TestReconcileAPIExports(t *testing.T) { initCacheFakeClientWithInitialAPIExports: true, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - if len(localClientActions) != 0 { t.Fatalf("unexpected REST calls were made to the localDynamicClient: %#v", localClientActions) } @@ -277,8 +267,6 @@ func TestReconcileAPIExports(t *testing.T) { initCacheFakeClientWithInitialAPIExports: true, reconcileKey: fmt.Sprintf("%s::root|foo", apisv1alpha1.SchemeGroupVersion.WithResource("apiexports")), validateFunc: func(t *testing.T, cacheClientActions []kcptesting.Action, localClientActions []kcptesting.Action) { - t.Helper() - if len(localClientActions) != 0 { t.Fatalf("unexpected REST calls were made to the localDynamicClient: %#v", localClientActions) } diff --git a/pkg/reconciler/cache/replication/replication_reconcile_unstructured.go b/pkg/reconciler/cache/replication/replication_reconcile_unstructured.go index fed5aad7805..c852abf6c44 100644 --- a/pkg/reconciler/cache/replication/replication_reconcile_unstructured.go +++ b/pkg/reconciler/cache/replication/replication_reconcile_unstructured.go @@ -18,6 +18,7 @@ package replication import ( "context" + "encoding/json" "fmt" "reflect" @@ -25,7 +26,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" genericrequest "k8s.io/apiserver/pkg/endpoints/request" ) @@ -210,10 +210,12 @@ func ensureRemaining(cacheObject *unstructured.Unstructured, localObject *unstru func toUnstructured(obj interface{}) (*unstructured.Unstructured, error) { unstructured := &unstructured.Unstructured{Object: map[string]interface{}{}} - raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + bs, err := json.Marshal(obj) if err != nil { return nil, err } - unstructured.Object = raw + if err := json.Unmarshal(bs, &unstructured.Object); err != nil { + return nil, err + } return unstructured, nil }