From 8325b7f6c41b2d8f8a64ffb129d57a96770674e4 Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Fri, 21 Jan 2022 14:12:55 -0800 Subject: [PATCH] fix: Use the dynamic client in the InventoryClient --- pkg/apply/common_test.go | 160 ++++++++----------------- pkg/inventory/inventory-client.go | 135 ++++++++------------- pkg/inventory/inventory-client_test.go | 116 +++++++----------- pkg/object/infos.go | 6 +- pkg/object/unstructured.go | 12 ++ 5 files changed, 159 insertions(+), 270 deletions(-) diff --git a/pkg/apply/common_test.go b/pkg/apply/common_test.go index a8e78d3d..fee82d2f 100644 --- a/pkg/apply/common_test.go +++ b/pkg/apply/common_test.go @@ -26,7 +26,7 @@ import ( clienttesting "k8s.io/client-go/testing" "k8s.io/klog/v2" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" - "sigs.k8s.io/cli-utils/pkg/apply/info" + "k8s.io/kubectl/pkg/scheme" "sigs.k8s.io/cli-utils/pkg/apply/poller" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" @@ -42,8 +42,13 @@ type inventoryInfo struct { set object.ObjMetadataSet } -func (i inventoryInfo) toWrapped() inventory.InventoryInfo { - inv := &unstructured.Unstructured{ +func (i inventoryInfo) toUnstructured() *unstructured.Unstructured { + invMap := make(map[string]interface{}) + for _, objMeta := range i.set { + invMap[objMeta.String()] = "" + } + + return &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -54,9 +59,13 @@ func (i inventoryInfo) toWrapped() inventory.InventoryInfo { common.InventoryLabel: i.id, }, }, + "data": invMap, }, } - return inventory.WrapInventoryInfoObj(inv) +} + +func (i inventoryInfo) toWrapped() inventory.InventoryInfo { + return inventory.WrapInventoryInfoObj(i.toUnstructured()) } func newTestApplier( @@ -73,7 +82,7 @@ func newTestApplier( factory: tf, } - invClient := newTestInventory(t, tf, infoHelper) + invClient := newTestInventory(t, tf) applier, err := NewApplier(tf, invClient) require.NoError(t, err) @@ -95,11 +104,7 @@ func newTestDestroyer( tf := newTestFactory(t, invInfo, object.UnstructuredSet{}, clusterObjs) defer tf.Cleanup() - infoHelper := &fakeInfoHelper{ - factory: tf, - } - - invClient := newTestInventory(t, tf, infoHelper) + invClient := newTestInventory(t, tf) destroyer, err := NewDestroyer(tf, invClient) require.NoError(t, err) @@ -111,18 +116,11 @@ func newTestDestroyer( func newTestInventory( t *testing.T, tf *cmdtesting.TestFactory, - infoHelper info.InfoHelper, ) inventory.InventoryClient { // Use an InventoryClient with a fakeInfoHelper to allow generating Info // objects that use the FakeRESTClient as the UnstructuredClient. invClient, err := inventory.ClusterInventoryClientFactory{}.NewInventoryClient(tf) require.NoError(t, err) - - // TODO(mortent): This is not great, but at least this keeps the - // ugliness in the test code until we can find a way to wire it - // up so to avoid it. - invClient.(*inventory.ClusterInventoryClient).InfoHelper = infoHelper - return invClient } @@ -159,12 +157,6 @@ func newTestFactory( handlers := []handler{ &nsHandler{}, - &inventoryObjectHandler{ - inventoryName: invInfo.name, - inventoryNamespace: invInfo.namespace, - inventoryID: invInfo.id, - inventorySet: invInfo.set, - }, &genericHandler{ resources: objs, mapper: mapper, @@ -172,7 +164,7 @@ func newTestFactory( } tf.UnstructuredClient = newFakeRESTClient(t, handlers) - tf.FakeDynamicClient = fakeDynamicClient(t, mapper, objs...) + tf.FakeDynamicClient = fakeDynamicClient(t, mapper, invInfo, objs...) return tf } @@ -285,95 +277,39 @@ func (g *genericHandler) handle(t *testing.T, req *http.Request) (*http.Response return nil, false, nil } -// inventoryObjectHandler knows how to handle requests on the inventory objects. -// It knows how to handle creation, list and get requests for inventory objects. -type inventoryObjectHandler struct { - inventoryName string - inventoryNamespace string - inventoryID string - inventorySet object.ObjMetadataSet - inventoryObj *v1.ConfigMap +func newInventoryReactor(invInfo inventoryInfo) *inventoryReactor { + return &inventoryReactor{ + inventoryObj: invInfo.toUnstructured(), + } } -var ( - cmPathRegex = regexp.MustCompile(`^/namespaces/([^/]+)/configmaps$`) - invObjPathRegex = regexp.MustCompile(`^/namespaces/([^/]+)/configmaps/[a-zA-Z]+-[a-z0-9]+$`) -) - -func (i *inventoryObjectHandler) handle(t *testing.T, req *http.Request) (*http.Response, bool, error) { - klog.V(5).Infof("inventoryObjectHandler: handling %s request for %q", req.Method, req.URL) - if (req.Method == "POST" && cmPathRegex.Match([]byte(req.URL.Path))) || - (req.Method == "PUT" && invObjPathRegex.Match([]byte(req.URL.Path))) { - b, err := ioutil.ReadAll(req.Body) - if err != nil { - return nil, false, err - } - cm := v1.ConfigMap{} - err = runtime.DecodeInto(codec, b, &cm) - if err != nil { - return nil, false, err - } - if cm.Name == i.inventoryName && cm.Namespace == i.inventoryNamespace { - i.inventoryObj = &cm - inventorySet, err := object.FromStringMap(cm.Data) - if err != nil { - return nil, false, err - } - i.inventorySet = inventorySet - - bodyRC := ioutil.NopCloser(bytes.NewReader(b)) - var statusCode int - if req.Method == "POST" { - statusCode = http.StatusCreated - } else { - statusCode = http.StatusOK - } - return &http.Response{StatusCode: statusCode, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, true, nil - } - return nil, false, nil - } +type inventoryReactor struct { + inventoryObj *unstructured.Unstructured +} - if req.Method == http.MethodGet && cmPathRegex.Match([]byte(req.URL.Path)) { - cmList := v1.ConfigMapList{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "List", - }, - Items: []v1.ConfigMap{}, - } - if len(i.inventorySet) > 0 { - cmList.Items = append(cmList.Items, i.currentInvObj()) +func (ir *inventoryReactor) updateFakeDynamicClient(fdc *dynamicfake.FakeDynamicClient) { + fdc.PrependReactor("create", "configmaps", func(action clienttesting.Action) (bool, runtime.Object, error) { + obj := *action.(clienttesting.CreateAction).GetObject().(*unstructured.Unstructured) + ir.inventoryObj = &obj + return true, nil, nil + }) + fdc.PrependReactor("list", "configmaps", func(action clienttesting.Action) (bool, runtime.Object, error) { + uList := &unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{}, } - bodyRC := ioutil.NopCloser(bytes.NewReader(toJSONBytes(t, &cmList))) - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, true, nil - } - - if req.Method == http.MethodGet && invObjPathRegex.Match([]byte(req.URL.Path)) { - if len(i.inventorySet) == 0 { - return &http.Response{StatusCode: http.StatusNotFound, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.StringBody("")}, true, nil + if ir.inventoryObj != nil { + uList.Items = append(uList.Items, *ir.inventoryObj) } - invObj := i.currentInvObj() - bodyRC := ioutil.NopCloser(bytes.NewReader(toJSONBytes(t, &invObj))) - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, true, nil - } - return nil, false, nil -} - -func (i *inventoryObjectHandler) currentInvObj() v1.ConfigMap { - return v1.ConfigMap{ - TypeMeta: metav1.TypeMeta{ - APIVersion: v1.SchemeGroupVersion.String(), - Kind: "ConfigMap", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: i.inventoryName, - Namespace: i.inventoryNamespace, - Labels: map[string]string{ - common.InventoryLabel: i.inventoryID, - }, - }, - Data: i.inventorySet.ToStringMap(), - } + return true, uList, nil + }) + fdc.PrependReactor("get", "configmaps", func(action clienttesting.Action) (bool, runtime.Object, error) { + return true, ir.inventoryObj, nil + }) + fdc.PrependReactor("update", "configmaps", func(action clienttesting.Action) (bool, runtime.Object, error) { + obj := *action.(clienttesting.UpdateAction).GetObject().(*unstructured.Unstructured) + ir.inventoryObj = &obj + return true, nil, nil + }) } // nsHandler can handle requests for a namespace. It will behave as if @@ -486,8 +422,11 @@ func (f *fakeInfoHelper) getClient(gv schema.GroupVersion) (resource.RESTClient, } // fakeDynamicClient returns a fake dynamic client. -func fakeDynamicClient(t *testing.T, mapper meta.RESTMapper, objs ...resourceInfo) *dynamicfake.FakeDynamicClient { - fakeClient := dynamicfake.NewSimpleDynamicClient(runtime.NewScheme()) +func fakeDynamicClient(t *testing.T, mapper meta.RESTMapper, invInfo inventoryInfo, objs ...resourceInfo) *dynamicfake.FakeDynamicClient { + fakeClient := dynamicfake.NewSimpleDynamicClient(scheme.Scheme) + + invReactor := newInventoryReactor(invInfo) + invReactor.updateFakeDynamicClient(fakeClient) for i := range objs { obj := objs[i] @@ -507,6 +446,7 @@ func fakeDynamicClient(t *testing.T, mapper meta.RESTMapper, objs ...resourceInf return true, nil, nil }) } + return fakeClient } diff --git a/pkg/inventory/inventory-client.go b/pkg/inventory/inventory-client.go index 379671bf..e439e43a 100644 --- a/pkg/inventory/inventory-client.go +++ b/pkg/inventory/inventory-client.go @@ -4,17 +4,17 @@ package inventory import ( + "context" "fmt" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/cli-runtime/pkg/resource" + "k8s.io/client-go/dynamic" "k8s.io/klog/v2" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/util" - "k8s.io/kubectl/pkg/validation" - "sigs.k8s.io/cli-utils/pkg/apply/info" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" ) @@ -47,12 +47,10 @@ type InventoryClient interface { // ClusterInventoryClient is a concrete implementation of the // InventoryClient interface. type ClusterInventoryClient struct { - builderFunc func() *resource.Builder + dc dynamic.Interface mapper meta.RESTMapper - validator validation.Schema InventoryFactoryFunc InventoryFactoryFunc invToUnstructuredFunc InventoryToUnstructuredFunc - InfoHelper info.InfoHelper } var _ InventoryClient = &ClusterInventoryClient{} @@ -62,23 +60,19 @@ var _ InventoryClient = &ClusterInventoryClient{} func NewInventoryClient(factory cmdutil.Factory, invFunc InventoryFactoryFunc, invToUnstructuredFunc InventoryToUnstructuredFunc) (*ClusterInventoryClient, error) { - var err error - mapper, err := factory.ToRESTMapper() + dc, err := factory.DynamicClient() if err != nil { return nil, err } - validator, err := factory.Validator(false) + mapper, err := factory.ToRESTMapper() if err != nil { return nil, err } - builderFunc := factory.NewBuilder clusterInventoryClient := ClusterInventoryClient{ - builderFunc: builderFunc, + dc: dc, mapper: mapper, - validator: validator, InventoryFactoryFunc: invFunc, invToUnstructuredFunc: invToUnstructuredFunc, - InfoHelper: info.NewInfoHelper(mapper, factory), } return &clusterInventoryClient, nil } @@ -263,7 +257,7 @@ func (cic *ClusterInventoryClient) getClusterInventoryObjsByLabel(inv InventoryI return nil, fmt.Errorf("retrieving cluster inventory object with nil local inventory") } localObj := object.UnstructuredToObjMetadata(localInv) - mapping, err := cic.mapper.RESTMapping(localObj.GroupKind) + mapping, err := cic.getMapping(localInv) if err != nil { return nil, err } @@ -275,22 +269,18 @@ func (cic *ClusterInventoryClient) getClusterInventoryObjsByLabel(inv InventoryI } labelSelector := fmt.Sprintf("%s=%s", common.InventoryLabel, label) klog.V(4).Infof("inventory object fetch by label (group: %q, namespace: %q, selector: %q)", groupResource, namespace, labelSelector) - builder := cic.builderFunc() - retrievedInventoryInfos, err := builder. - Unstructured(). - // TODO: Check if this validator is necessary. - Schema(cic.validator). - ContinueOnError(). - NamespaceParam(namespace).DefaultNamespace(). - ResourceTypes(groupResource). - LabelSelectorParam(labelSelector). - Flatten(). - Do(). - Infos() + + uList, err := cic.dc.Resource(mapping.Resource).Namespace(namespace).List(context.TODO(), metav1.ListOptions{ + LabelSelector: labelSelector, + }) if err != nil { return nil, err } - return object.InfosToUnstructureds(retrievedInventoryInfos), nil + var invList []*unstructured.Unstructured + for i := range uList.Items { + invList = append(invList, &uList.Items[i]) + } + return invList, nil } func (cic *ClusterInventoryClient) getClusterInventoryObjsByName(inv InventoryInfo) (object.UnstructuredSet, error) { @@ -299,25 +289,20 @@ func (cic *ClusterInventoryClient) getClusterInventoryObjsByName(inv InventoryIn return nil, fmt.Errorf("retrieving cluster inventory object with nil local inventory") } - invInfo, err := cic.toInfo(localInv) + mapping, err := cic.getMapping(localInv) if err != nil { return nil, err } - helper := cic.helperFromInfo(invInfo) - klog.V(4).Infof("inventory object fetch by name (namespace: %q, name: %q)", inv.Namespace(), inv.Name()) - res, err := helper.Get(inv.Namespace(), inv.Name()) + clusterInv, err := cic.dc.Resource(mapping.Resource).Namespace(inv.Namespace()). + Get(context.TODO(), inv.Name(), metav1.GetOptions{}) if err != nil && !apierrors.IsNotFound(err) { return nil, err } if apierrors.IsNotFound(err) { return object.UnstructuredSet{}, nil } - clusterInv, ok := res.(*unstructured.Unstructured) - if !ok { - return nil, fmt.Errorf("retrieved cluster inventory object is not of type *Unstructured") - } return object.UnstructuredSet{clusterInv}, nil } @@ -348,19 +333,16 @@ func (cic *ClusterInventoryClient) applyInventoryObj(obj *unstructured.Unstructu if obj == nil { return fmt.Errorf("attempting apply a nil inventory object") } - invInfo, err := cic.toInfo(obj) - if err != nil { - return err - } - helper := cic.helperFromInfo(invInfo) - klog.V(4).Infof("replacing inventory object: %s/%s", invInfo.Namespace, invInfo.Name) - var overwrite = true - replacedObj, err := helper.Replace(invInfo.Namespace, invInfo.Name, overwrite, invInfo.Object) + + mapping, err := cic.getMapping(obj) if err != nil { return err } - var ignoreError = true - return invInfo.Refresh(replacedObj, ignoreError) + + klog.V(4).Infof("replacing inventory object: %s/%s", obj.GetNamespace(), obj.GetName()) + _, err = cic.dc.Resource(mapping.Resource).Namespace(obj.GetNamespace()). + Update(context.TODO(), obj, metav1.UpdateOptions{}) + return err } // createInventoryObj creates the passed inventory object on the APIServer. @@ -378,19 +360,16 @@ func (cic *ClusterInventoryClient) createInventoryObj(obj *unstructured.Unstruct if err != nil { return err } - invInfo, err := cic.toInfo(obj) - if err != nil { - return err - } - helper := cic.helperFromInfo(invInfo) - klog.V(4).Infof("creating inventory object: %s/%s", invInfo.Namespace, invInfo.Name) - var clearResourceVersion = false - createdObj, err := helper.Create(invInfo.Namespace, clearResourceVersion, invInfo.Object) + + mapping, err := cic.getMapping(obj) if err != nil { return err } - var ignoreError = true - return invInfo.Refresh(createdObj, ignoreError) + + klog.V(4).Infof("creating inventory object: %s/%s", obj.GetNamespace(), obj.GetName()) + _, err = cic.dc.Resource(mapping.Resource).Namespace(obj.GetNamespace()). + Create(context.TODO(), obj, metav1.CreateOptions{}) + return err } // deleteInventoryObjByName deletes the passed inventory object from the APIServer, or @@ -403,14 +382,15 @@ func (cic *ClusterInventoryClient) deleteInventoryObjByName(obj *unstructured.Un if obj == nil { return fmt.Errorf("attempting delete a nil inventory object") } - invInfo, err := cic.toInfo(obj) + + mapping, err := cic.getMapping(obj) if err != nil { return err } - helper := cic.helperFromInfo(invInfo) - klog.V(4).Infof("deleting inventory object: %s/%s", invInfo.Namespace, invInfo.Name) - _, err = helper.Delete(invInfo.Namespace, invInfo.Name) - return err + + klog.V(4).Infof("deleting inventory object: %s/%s", obj.GetNamespace(), obj.GetName()) + return cic.dc.Resource(mapping.Resource).Namespace(obj.GetNamespace()). + Delete(context.TODO(), obj.GetName(), metav1.DeleteOptions{}) } // ApplyInventoryNamespace creates the passed namespace if it does not already @@ -420,33 +400,24 @@ func (cic *ClusterInventoryClient) ApplyInventoryNamespace(obj *unstructured.Uns klog.V(4).Infof("dry-run apply inventory namespace (%s): not applied", obj.GetName()) return nil } - invInfo, err := cic.toInfo(obj) - if err != nil { - return err - } - helper := cic.helperFromInfo(invInfo) - klog.V(4).Infof("applying inventory namespace: %s", invInfo.Name) - if err := util.CreateApplyAnnotation(invInfo.Object, unstructured.UnstructuredJSONScheme); err != nil { + + invNamespace := obj.DeepCopy() + klog.V(4).Infof("applying inventory namespace: %s", obj.GetName()) + object.StripKyamlAnnotations(invNamespace) + if err := util.CreateApplyAnnotation(invNamespace, unstructured.UnstructuredJSONScheme); err != nil { return err } - var clearResourceVersion = false - createdObj, err := helper.Create(invInfo.Namespace, clearResourceVersion, invInfo.Object) + + mapping, err := cic.getMapping(obj) if err != nil { - if !apierrors.IsAlreadyExists(err) { - return err - } - return nil + return err } - var ignoreError = true - return invInfo.Refresh(createdObj, ignoreError) -} -func (cic *ClusterInventoryClient) toInfo(obj *unstructured.Unstructured) (*resource.Info, error) { - return cic.InfoHelper.BuildInfo(obj) + _, err = cic.dc.Resource(mapping.Resource).Create(context.TODO(), invNamespace, metav1.CreateOptions{}) + return err } -// helperFromInfo returns the resource.Helper to talk to the APIServer based -// on the information from the passed "info", or an error if one occurred. -func (cic *ClusterInventoryClient) helperFromInfo(info *resource.Info) *resource.Helper { - return resource.NewHelper(info.Client, info.Mapping) +// getMapping returns the RESTMapping for the provided resource. +func (cic *ClusterInventoryClient) getMapping(obj *unstructured.Unstructured) (*meta.RESTMapping, error) { + return cic.mapper.RESTMapping(obj.GroupVersionKind().GroupKind(), obj.GroupVersionKind().Version) } diff --git a/pkg/inventory/inventory-client_test.go b/pkg/inventory/inventory-client_test.go index 795033bf..cd711721 100644 --- a/pkg/inventory/inventory-client_test.go +++ b/pkg/inventory/inventory-client_test.go @@ -4,18 +4,14 @@ package inventory import ( - "bytes" - "io/ioutil" - "net/http" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" - "k8s.io/client-go/rest/fake" + clienttesting "k8s.io/client-go/testing" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/object" @@ -62,9 +58,7 @@ func TestGetClusterInventoryInfo(t *testing.T) { invClient, err := NewInventoryClient(tf, WrapInventoryObj, InvInfoToConfigMap) require.NoError(t, err) - fakeBuilder := FakeBuilder{} - fakeBuilder.SetInventoryObjs(tc.localObjs) - invClient.builderFunc = fakeBuilder.GetBuilder() + var inv *unstructured.Unstructured if tc.inv != nil { inv = storeObjsInInventory(tc.inv, tc.localObjs) @@ -157,22 +151,19 @@ func TestMerge(t *testing.T) { }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - for name, tc := range tests { for i := range common.Strategies { drs := common.Strategies[i] t.Run(name, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + + tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) // Create the local inventory object storing "tc.localObjs" invClient, err := NewInventoryClient(tf, WrapInventoryObj, InvInfoToConfigMap) require.NoError(t, err) - // Create a fake builder to return "tc.clusterObjs" from - // the cluster inventory object. - fakeBuilder := FakeBuilder{} - fakeBuilder.SetInventoryObjs(tc.clusterObjs) - invClient.builderFunc = fakeBuilder.GetBuilder() + // Call "Merge" to create the union of clusterObjs and localObjs. pruneObjs, err := invClient.Merge(tc.localInv, tc.localObjs, drs) if tc.isError { @@ -222,33 +213,18 @@ func TestCreateInventory(t *testing.T) { }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - - // The fake client must see a POST to the confimap URL. - tf.UnstructuredClient = &fake.RESTClient{ - NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - if req.Method == "POST" && cmPathRegex.Match([]byte(req.URL.Path)) { - b, err := ioutil.ReadAll(req.Body) - if err != nil { - return nil, err - } - cm := corev1.ConfigMap{} - err = runtime.DecodeInto(codec, b, &cm) - if err != nil { - return nil, err - } - bodyRC := ioutil.NopCloser(bytes.NewReader(b)) - return &http.Response{StatusCode: http.StatusCreated, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil - } - return nil, nil - }), - } - tf.ClientConfigVal = cmdtesting.DefaultClientConfig() - for name, tc := range tests { t.Run(name, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + + var storedInventory map[string]string + tf.FakeDynamicClient.PrependReactor("create", "configmaps", func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) { + obj := *action.(clienttesting.CreateAction).GetObject().(*unstructured.Unstructured) + storedInventory, _, _ = unstructured.NestedStringMap(obj.Object, "data") + return true, nil, nil + }) + invClient, err := NewInventoryClient(tf, WrapInventoryObj, InvInfoToConfigMap) require.NoError(t, err) @@ -262,6 +238,12 @@ func TestCreateInventory(t *testing.T) { } else { assert.NoError(t, err) } + + expectedInventory := tc.localObjs.ToStringMap() + // handle empty inventories special to avoid problems with empty vs nil maps + if len(expectedInventory) != 0 || len(storedInventory) != 0 { + assert.Equal(t, expectedInventory, storedInventory) + } }) } } @@ -378,19 +360,15 @@ func TestGetClusterObjs(t *testing.T) { }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - for name, tc := range tests { t.Run(name, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + tf.FakeDynamicClient.PrependReactor("list", "configmaps", toReactionFunc(tc.clusterObjs)) + invClient, err := NewInventoryClient(tf, WrapInventoryObj, InvInfoToConfigMap) require.NoError(t, err) - // Create fake builder returning "tc.clusterObjs" from cluster inventory. - fakeBuilder := FakeBuilder{} - fakeBuilder.SetInventoryObjs(tc.clusterObjs) - invClient.builderFunc = fakeBuilder.GetBuilder() - // Call "GetClusterObjs" and compare returned cluster inventory objs to expected. clusterObjs, err := invClient.GetClusterObjs(tc.localInv) if tc.isError { if err == nil { @@ -436,34 +414,13 @@ func TestDeleteInventoryObj(t *testing.T) { }, } - tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) - defer tf.Cleanup() - - tf.UnstructuredClient = &fake.RESTClient{ - NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - if req.Method == "DELETE" && cmPathRegex.Match([]byte(req.URL.Path)) { - b, err := ioutil.ReadAll(req.Body) - if err != nil { - return nil, err - } - cm := corev1.ConfigMap{} - err = runtime.DecodeInto(codec, b, &cm) - if err != nil { - return nil, err - } - bodyRC := ioutil.NopCloser(bytes.NewReader(b)) - return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: bodyRC}, nil - } - return nil, nil - }), - } - tf.ClientConfigVal = cmdtesting.DefaultClientConfig() - for name, tc := range tests { for i := range common.Strategies { drs := common.Strategies[i] t.Run(name, func(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace(testNamespace) + defer tf.Cleanup() + invClient, err := NewInventoryClient(tf, WrapInventoryObj, InvInfoToConfigMap) require.NoError(t, err) @@ -484,3 +441,16 @@ func ignoreErrInfoToObjMeta(info *resource.Info) object.ObjMetadata { objMeta, _ := object.InfoToObjMeta(info) return objMeta } + +func toReactionFunc(objs object.ObjMetadataSet) clienttesting.ReactionFunc { + return func(action clienttesting.Action) (bool, runtime.Object, error) { + u := copyInventoryInfo() + err := unstructured.SetNestedStringMap(u.Object, objs.ToStringMap(), "data") + if err != nil { + return true, nil, err + } + list := &unstructured.UnstructuredList{} + list.Items = []unstructured.Unstructured{*u} + return true, list, err + } +} diff --git a/pkg/object/infos.go b/pkg/object/infos.go index e927fdc2..93b8e6cf 100644 --- a/pkg/object/infos.go +++ b/pkg/object/infos.go @@ -56,12 +56,8 @@ func UnstructuredToInfo(obj *unstructured.Unstructured) (*resource.Info, error) path, ok := annos[kioutil.PathAnnotation] if ok { source = path - // kyaml adds both annotations for the time being, so we need to remove them both - // before apply. - delete(annos, kioutil.PathAnnotation) - delete(annos, kioutil.LegacyPathAnnotation) //nolint:staticcheck - obj.SetAnnotations(annos) } + StripKyamlAnnotations(obj) return &resource.Info{ Name: obj.GetName(), diff --git a/pkg/object/unstructured.go b/pkg/object/unstructured.go index 9ff4df6d..de652b44 100644 --- a/pkg/object/unstructured.go +++ b/pkg/object/unstructured.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) var ( @@ -189,3 +190,14 @@ func crdDefinesVersion(crd *unstructured.Unstructured, version string) (bool, er } return false, nil } + +// StripKyamlAnnotations removes any path and index annotations from the +// unstructured resource. +func StripKyamlAnnotations(u *unstructured.Unstructured) { + annos := u.GetAnnotations() + delete(annos, kioutil.PathAnnotation) + delete(annos, kioutil.LegacyPathAnnotation) //nolint:staticcheck + delete(annos, kioutil.IndexAnnotation) + delete(annos, kioutil.LegacyIndexAnnotation) //nolint:staticcheck + u.SetAnnotations(annos) +}