refactor: createPatch function for better handling of crds

deploy/apply.go

@@ -32,7 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/mergepatch"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/client-go/dynamic"
-	"k8s.io/kubectl/pkg/scheme"
+	"k8s.io/client-go/kubernetes/scheme"
 )
 
 const CustomResourceDefinitionName = "CustomResourceDefinition"
@@ -70,12 +70,6 @@ func defaultApplyResource(clients *K8sClients, res Resource, deployConfig Deploy
 		return err
 	}
 
-	// if res.Object.GetKind() == "Secret" || res.Object.GetKind() == "ConfigMap" || res.Object.GetKind() == CustomResourceDefinitionName {
-	// 	fmt.Printf("Replacing %s: %s\n", res.Object.GetKind(), res.Object.GetName())
-	// 	res.Object.SetResourceVersion(onClusterObj.GetResourceVersion())
-	// 	return ReplaceResource(gvr, clients, res)
-	// }
-
 	return PatchResource(gvr, clients, res, onClusterObj)
 }
 
@@ -101,7 +95,10 @@ func CreateResource(gvr schema.GroupVersionResource, clients *K8sClients, res Re
 
 	// creates kubectl.kubernetes.io/last-applied-configuration annotation
 	// inside the resource except for Secrets, ConfigMaps, and CRDs
-	if res.Object.GetKind() != "Secret" && res.Object.GetKind() != "ConfigMap" && res.Object.GetKind() != CustomResourceDefinitionName {
+	switch res.Object.GetKind() {
+	case "Secret", "ConfigMap", CustomResourceDefinitionName:
+		// Do nothing
+	default:
 		orignAnn := res.Object.GetAnnotations()
 		if orignAnn == nil {
 			orignAnn = make(map[string]string)
@@ -118,32 +115,14 @@ func CreateResource(gvr schema.GroupVersionResource, clients *K8sClients, res Re
 		return err
 	}
 
-	var err error
-
+	var resourceInterface dynamic.ResourceInterface
 	if res.Namespaced {
-		_, err = clients.dynamic.Resource(gvr).
-			Namespace(res.Object.GetNamespace()).
-			Create(context.Background(),
-				&res.Object,
-				metav1.CreateOptions{})
+		resourceInterface = clients.dynamic.Resource(gvr).Namespace(res.Object.GetNamespace())
 	} else {
-		_, err = clients.dynamic.Resource(gvr).
-			Create(context.Background(),
-				&res.Object,
-				metav1.CreateOptions{})
+		resourceInterface = clients.dynamic.Resource(gvr)
 	}
 
-	return err
-}
-
-// ReplaceResource handles resource replacement on the cluster
-// e.g. for Secrets, ConfigMaps, and CRDs
-func ReplaceResource(gvr schema.GroupVersionResource, clients *K8sClients, res Resource) error {
-	_, err := clients.dynamic.Resource(gvr).
-		Namespace(res.Object.GetNamespace()).
-		Update(context.Background(),
-			&res.Object,
-			metav1.UpdateOptions{})
+	_, err := resourceInterface.Create(context.Background(), &res.Object, metav1.CreateOptions{})
 	return err
 }
 
@@ -156,13 +135,14 @@ func PatchResource(gvr schema.GroupVersionResource, clients *K8sClients, res Res
 		return errors.Wrap(err, "failed to create patch")
 	}
 
-	// TODO: handle non-namespaced resources
-	if _, err = clients.dynamic.Resource(gvr).
-		Namespace(res.Object.GetNamespace()).
-		Patch(context.Background(),
-			res.Object.GetName(), patchType, patch, metav1.PatchOptions{}); err != nil {
-		return errors.Wrap(err, "failed to patch")
+	var resourceInterface dynamic.ResourceInterface
+	if res.Namespaced {
+		resourceInterface = clients.dynamic.Resource(gvr).Namespace(res.Object.GetNamespace())
+	} else {
+		resourceInterface = clients.dynamic.Resource(gvr)
 	}
+
+	_, err = resourceInterface.Patch(context.Background(), res.Object.GetName(), patchType, patch, metav1.PatchOptions{})
 	return err
 }
 
@@ -194,58 +174,36 @@ func annotateWithLastApplied(res Resource) (unstructured.Unstructured, error) {
 // object annotation, the actual resource state deployed inside the cluster and the desired state after
 // the update.
 func createPatch(currentObj unstructured.Unstructured, target Resource) ([]byte, types.PatchType, error) {
+	// Get the resource in the cluster
+	currentJSON, err := currentObj.MarshalJSON()
+	if err != nil {
+		return nil, "", errors.Wrap(err, "serializing live configuration")
+	}
 
-	patchType := types.StrategicMergePatchType
-
-	if target.Object.GetKind() == "Secret" && target.Object.GetKind() == "ConfigMap" && target.Object.GetKind() == CustomResourceDefinitionName {
-		// Get the resource scheme
-		versionedObject, err := scheme.Scheme.New(*target.GroupVersionKind)
-		if err != nil {
-			return nil, types.StrategicMergePatchType, err
-		}
-		patchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObject)
+	// Get last applied config from annotation if exists
+	lastAppliedConfigJSON := ""
+	var targetJSON []byte
+	if annotations := currentObj.GetAnnotations(); annotations != nil {
+		lastAppliedConfigJSON = annotations[corev1.LastAppliedConfigAnnotation]
+		annotatedTarget, err := annotateWithLastApplied(target)
 		if err != nil {
-			return nil, types.StrategicMergePatchType, errors.Wrap(err, "unable to create patch metadata from object")
+			return nil, "", err
 		}
-		currentJSON, err := currentObj.MarshalJSON()
+		targetJSON, err = annotatedTarget.MarshalJSON()
 		if err != nil {
-			return nil, types.StrategicMergePatchType, err
+			return nil, "", err
 		}
-		targetJSON, err := target.Object.MarshalJSON()
+	} else {
+		targetJSON, err = target.Object.MarshalJSON()
 		if err != nil {
-			return nil, types.StrategicMergePatchType, err
-		}
-
-		preconditions := []mergepatch.PreconditionFunc{}
-		if runtime.IsNotRegisteredError(err) {
-			patchType = types.MergePatchType
-			preconditions = []mergepatch.PreconditionFunc{mergepatch.RequireKeyUnchanged("apiVersion"),
-				mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")}
+			return nil, "", err
 		}
-
-		patch, err := strategicpatch.CreateTwoWayMergePatch(currentJSON, targetJSON, patchMeta, preconditions...)
-		return patch, patchType, err
-	}
-	// Get last applied config from current object annotation
-	lastAppliedConfigJSON := currentObj.GetAnnotations()[corev1.LastAppliedConfigAnnotation]
-	// Get the desired configuration
-	annotatedTarget, err := annotateWithLastApplied(target)
-	if err != nil {
-		return nil, types.StrategicMergePatchType, err
-	}
-	targetJSON, err := annotatedTarget.MarshalJSON()
-	if err != nil {
-		return nil, types.StrategicMergePatchType, err
-	}
-
-	// Get the resource in the cluster
-	currentJSON, err := currentObj.MarshalJSON()
-	if err != nil {
-		return nil, types.StrategicMergePatchType, errors.Wrap(err, "serializing live configuration")
 	}
 
-	// Get the resource scheme
 	versionedObject, err := scheme.Scheme.New(*target.GroupVersionKind)
+	if err != nil && !runtime.IsNotRegisteredError(err) {
+		return nil, "", err
+	}
 
 	// use a three way json merge if the resource is a CRD
 	if runtime.IsNotRegisteredError(err) {
@@ -255,8 +213,6 @@ func createPatch(currentObj unstructured.Unstructured, target Resource) ([]byte,
 			mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name")}
 		patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch([]byte(lastAppliedConfigJSON), targetJSON, currentJSON, preconditions...)
 		return patch, patchType, err
-	} else if err != nil {
-		return nil, types.StrategicMergePatchType, err
 	}
 
 	patchMeta, err := strategicpatch.NewPatchMetaFromStruct(versionedObject)

deploy/apply_test.go

@@ -185,7 +185,7 @@ func TestCreatePatch(t *testing.T) {
 			patch, patchType, err := createPatch(*tC.current, *tC.target)
 
 			require.Equal(t, tC.expected, string(patch))
-			require.Equal(t, patchType, types.StrategicMergePatchType)
+			require.Equal(t, types.StrategicMergePatchType, patchType)
 			require.Nil(t, err)
 		})
 	}

deploy/deploy.go

@@ -17,18 +17,14 @@ package jpl
 import (
 	"context"
 	"fmt"
-	"sync"
 
-	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/discovery"
 	discoveryFake "k8s.io/client-go/discovery/fake"
 	"k8s.io/client-go/dynamic"
 	dynamicFake "k8s.io/client-go/dynamic/fake"
-	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
 )
 
@@ -92,21 +88,9 @@ func InitRealK8sClients(opts *Options) *K8sClients {
 	return createRealK8sClients(restConfig)
 }
 
-var addToScheme sync.Once
-
 // createRealK8sClients returns an initialized K8sClients struct,
 // given a REST config
 func createRealK8sClients(cfg *rest.Config) *K8sClients {
-	// Add CRDs to the scheme. They are missing by default.
-	addToScheme.Do(func() {
-		if err := apiextv1.AddToScheme(scheme.Scheme); err != nil {
-			// This should never happen.
-			panic(err)
-		}
-		if err := apiextv1beta1.AddToScheme(scheme.Scheme); err != nil {
-			panic(err)
-		}
-	})
 	clients := &K8sClients{
 		dynamic:   dynamic.NewForConfigOrDie(cfg),
 		discovery: discovery.NewDiscoveryClientForConfigOrDie(cfg),

deploy/integration_test.go

@@ -140,7 +140,7 @@ var _ = Describe("deploy on mock kubernetes", func() {
 			Expect(err).NotTo(HaveOccurred())
 		})
 		It("replaces non-namespaced resources", func() {
-			err := execDeploy(clients, "test4", []string{"testdata/integration/apply-resources/non-namespaced.yaml"}, deployConfig)
+			err := execDeploy(clients, "test4", []string{"testdata/integration/apply-resources/non-namespaced-2.yaml"}, deployConfig)
 			Expect(err).NotTo(HaveOccurred())
 			_, err = clients.dynamic.Resource(gvrCRDs).
 				List(context.Background(), metav1.ListOptions{})

deploy/testdata/integration/apply-resources/non-namespaced-2.yaml

@@ -0,0 +1,38 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: secret-reader
+rules:
+- apiGroups: [""]
+  resources: ["secrets", "configmaps"]
+  verbs: ["get", "watch", "list"]
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: projects.example.jpl.com
+spec:
+  group: example.jpl.com
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          required: [spec]
+          type: object
+          properties:
+            spec:
+              required: [replicas]
+              type: object
+              properties:
+                replicas:
+                  type: integer
+                  minimum: 2
+  scope: Namespaced
+  names:
+    plural: projects
+    singular: project
+    kind: Project
+    shortNames:
+    - pj

go.mod

@@ -5,47 +5,38 @@ go 1.18
 require (
 	github.com/Masterminds/semver v1.5.0
 	github.com/onsi/ginkgo v1.16.5
-	github.com/onsi/gomega v1.20.1
+	github.com/onsi/gomega v1.22.1
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/afero v1.9.2
 	github.com/stretchr/testify v1.8.0
-	k8s.io/api v0.25.2
-	k8s.io/apiextensions-apiserver v0.25.0
-	k8s.io/apimachinery v0.25.2
-	k8s.io/client-go v0.25.2
-	k8s.io/kubectl v0.25.0
+	k8s.io/api v0.25.3
+	k8s.io/apimachinery v0.25.3
+	k8s.io/cli-runtime v0.25.3
+	k8s.io/client-go v0.25.3
 	sigs.k8s.io/controller-runtime v0.13.0
 	sigs.k8s.io/yaml v1.3.0
 )
 
-require (
-	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
-	github.com/fsnotify/fsnotify v1.5.4 // indirect
-	github.com/go-logr/zapr v1.2.3 // indirect
-	github.com/google/go-cmp v0.5.8 // indirect
-	github.com/nxadm/tail v1.4.8 // indirect
-	go.uber.org/atomic v1.7.0 // indirect
-	go.uber.org/multierr v1.6.0 // indirect
-	go.uber.org/zap v1.21.0 // indirect
-	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
-)
-
 require (
 	github.com/PuerkitoBio/purell v1.1.1 // indirect
 	github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.8.0 // indirect
 	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
+	github.com/evanphx/json-patch/v5 v5.6.0 // indirect
+	github.com/fsnotify/fsnotify v1.5.4 // indirect
 	github.com/go-errors/errors v1.0.1 // indirect
 	github.com/go-logr/logr v1.2.3 // indirect
+	github.com/go-logr/zapr v1.2.3 // indirect
 	github.com/go-openapi/jsonpointer v0.19.5 // indirect
 	github.com/go-openapi/jsonreference v0.19.5 // indirect
 	github.com/go-openapi/swag v0.19.14 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/btree v1.0.1 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
-	github.com/google/gofuzz v1.2.0 // indirect
+	github.com/google/go-cmp v0.5.8 // indirect
+	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/google/uuid v1.1.2 // indirect
 	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
@@ -59,24 +50,29 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/spf13/cobra v1.4.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/xlab/treeprint v1.1.0 // indirect
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
+	go.uber.org/atomic v1.7.0 // indirect
+	go.uber.org/multierr v1.6.0 // indirect
+	go.uber.org/zap v1.21.0 // indirect
 	golang.org/x/net v0.0.0-20220722155237-a158d28d115b // indirect
-	golang.org/x/oauth2 v0.0.0-20220622183110-fd043fe589d2 // indirect
+	golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 // indirect
 	golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/time v0.0.0-20220609170525-579cf78fd858 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
+	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/cli-runtime v0.25.1
+	k8s.io/apiextensions-apiserver v0.25.3 // indirect
 	k8s.io/klog/v2 v2.70.1 // indirect
 	k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect
 	k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed // indirect