From 550efddd88f7612556d7f78948520467518a81e2 Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Wed, 3 Nov 2021 18:11:32 -0400 Subject: [PATCH] Verify group before treating resource as cohabitating (#4126) Signed-off-by: Scott Seago --- changelogs/unreleased/4126-sseago | 1 + pkg/backup/backup_test.go | 24 ++++++++++ pkg/backup/item_collector.go | 20 ++++---- pkg/builder/testcr_builder.go | 77 +++++++++++++++++++++++++++++++ pkg/test/api_server.go | 1 + pkg/test/resources.go | 12 +++++ 6 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/4126-sseago create mode 100644 pkg/builder/testcr_builder.go diff --git a/changelogs/unreleased/4126-sseago b/changelogs/unreleased/4126-sseago new file mode 100644 index 000000000..baefcbf48 --- /dev/null +++ b/changelogs/unreleased/4126-sseago @@ -0,0 +1 @@ +Verify group before treating resource as cohabitating diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 351c60429..0747409bb 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -970,6 +970,30 @@ func TestBackupResourceCohabitation(t *testing.T) { "resources/deployments.apps/v1-preferredversion/namespaces/zoo/raz.json", }, }, + { + name: "when deployments exist that are not in the cohabitating groups those are backed up along with apps/deployments", + backup: defaultBackup().Result(), + apiResources: []*test.APIResource{ + test.VeleroDeployments( + builder.ForTestCR("Deployment", "foo", "bar").Result(), + builder.ForTestCR("Deployment", "zoo", "raz").Result(), + ), + test.Deployments( + builder.ForDeployment("foo", "bar").Result(), + builder.ForDeployment("zoo", "raz").Result(), + ), + }, + want: []string{ + "resources/deployments.apps/namespaces/foo/bar.json", + "resources/deployments.apps/namespaces/zoo/raz.json", + "resources/deployments.apps/v1-preferredversion/namespaces/foo/bar.json", + "resources/deployments.apps/v1-preferredversion/namespaces/zoo/raz.json", + "resources/deployments.velero.io/namespaces/foo/bar.json", + "resources/deployments.velero.io/namespaces/zoo/raz.json", + "resources/deployments.velero.io/v1-preferredversion/namespaces/foo/bar.json", + "resources/deployments.velero.io/v1-preferredversion/namespaces/zoo/raz.json", + }, + }, } for _, tc := range tests { diff --git a/pkg/backup/item_collector.go b/pkg/backup/item_collector.go index 6029d8f97..bb740b8ae 100644 --- a/pkg/backup/item_collector.go +++ b/pkg/backup/item_collector.go @@ -209,16 +209,18 @@ func (r *itemCollector) getResourceItems(log logrus.FieldLogger, gv schema.Group } if cohabitator, found := r.cohabitatingResources[resource.Name]; found { - if cohabitator.seen { - log.WithFields( - logrus.Fields{ - "cohabitatingResource1": cohabitator.groupResource1.String(), - "cohabitatingResource2": cohabitator.groupResource2.String(), - }, - ).Infof("Skipping resource because it cohabitates and we've already processed it") - return nil, nil + if gv.Group == cohabitator.groupResource1.Group || gv.Group == cohabitator.groupResource2.Group { + if cohabitator.seen { + log.WithFields( + logrus.Fields{ + "cohabitatingResource1": cohabitator.groupResource1.String(), + "cohabitatingResource2": cohabitator.groupResource2.String(), + }, + ).Infof("Skipping resource because it cohabitates and we've already processed it") + return nil, nil + } + cohabitator.seen = true } - cohabitator.seen = true } namespacesToList := getNamespacesToList(r.backupRequest.NamespaceIncludesExcludes) diff --git a/pkg/builder/testcr_builder.go b/pkg/builder/testcr_builder.go new file mode 100644 index 000000000..85bc83b1c --- /dev/null +++ b/pkg/builder/testcr_builder.go @@ -0,0 +1,77 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package builder + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +// CustomResourceBuilder builds objects based on velero APIVersion CRDs. +type TestCRBuilder struct { + object *TestCR +} + +// ForTestCR is the constructor for a TestCRBuilder. +func ForTestCR(crdKind, ns, name string) *TestCRBuilder { + return &TestCRBuilder{ + object: &TestCR{ + TypeMeta: metav1.TypeMeta{ + APIVersion: velerov1api.SchemeGroupVersion.String(), + Kind: crdKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns, + Name: name, + }, + }, + } +} + +// Result returns the built TestCR. +func (b *TestCRBuilder) Result() *TestCR { + return b.object +} + +// ObjectMeta applies functional options to the TestCR's ObjectMeta. +func (b *TestCRBuilder) ObjectMeta(opts ...ObjectMetaOpt) *TestCRBuilder { + for _, opt := range opts { + opt(b.object) + } + + return b +} + +type TestCR struct { + metav1.TypeMeta `json:",inline"` + + // +optional + metav1.ObjectMeta `json:"metadata,omitempty"` + + // +optional + Spec TestCRSpec `json:"spec,omitempty"` + + // +optional + Status TestCRStatus `json:"status,omitempty"` +} + +type TestCRSpec struct { +} + +type TestCRStatus struct { +} diff --git a/pkg/test/api_server.go b/pkg/test/api_server.go index a18fbd2d0..06439bca1 100644 --- a/pkg/test/api_server.go +++ b/pkg/test/api_server.go @@ -57,6 +57,7 @@ func NewAPIServer(t *testing.T) *APIServer { {Group: "apiextensions.k8s.io", Version: "v1beta1", Resource: "customresourcedefinitions"}: "CRDList", {Group: "velero.io", Version: "v1", Resource: "volumesnapshotlocations"}: "VSLList", {Group: "extensions", Version: "v1", Resource: "deployments"}: "ExtDeploymentsList", + {Group: "velero.io", Version: "v1", Resource: "deployments"}: "VeleroDeploymentsList", }) discoveryClient = &DiscoveryClient{FakeDiscovery: kubeClient.Discovery().(*discoveryfake.FakeDiscovery)} ) diff --git a/pkg/test/resources.go b/pkg/test/resources.go index e83f171d4..69e8dbf90 100644 --- a/pkg/test/resources.go +++ b/pkg/test/resources.go @@ -108,6 +108,18 @@ func ExtensionsDeployments(items ...metav1.Object) *APIResource { } } +// test CRD +func VeleroDeployments(items ...metav1.Object) *APIResource { + return &APIResource{ + Group: "velero.io", + Version: "v1", + Name: "deployments", + ShortName: "deploy", + Namespaced: true, + Items: items, + } +} + func Namespaces(items ...metav1.Object) *APIResource { return &APIResource{ Group: "",