Account for possible missing schemas on v1 CRDs (#2264)

* Account for possible missing schemas on v1 CRDs

If a v1beta1 CRD without a Schema was submitted to a Kubernets v1.16
cluster, then Kubernetes will server it back as a v1 CRD without a
schema.

However, when Velero tries to restore this document, the request will be
rejected as a v1 CRD must have a schema.

This commit has some defensive coding on the restore side, as well as
potential fixes on the backup side for getting around this.

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Back up nonstructural CRDs as v1beta1

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Add tests for remapping plugin

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Add builders for v1 CRDs

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Address review feedback

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Remove extraneous log message

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

* Add changelog

Signed-off-by: Nolan Brubaker <brubakern@vmware.com>

changelogs/unreleased/2264-nrb

@@ -0,0 +1 @@
+Back up schema-less CustomResourceDefinitions as v1beta1, even if they are retrieved via the v1 endpoint.

pkg/backup/remap_crd_version_action.go

@@ -0,0 +1,114 @@
+/*
+Copyright 2020 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 backup
+
+import (
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/plugin/velero"
+)
+
+// RemapCRDVersionAction inspects CustomResourceDefinition and decides if it is a v1
+// CRD that needs to be backed up as v1beta1.
+type RemapCRDVersionAction struct {
+	logger logrus.FieldLogger
+}
+
+// NewRemapCRDVersionAction instantiates a new RemapCRDVersionAction plugin.
+func NewRemapCRDVersionAction(logger logrus.FieldLogger) *RemapCRDVersionAction {
+	return &RemapCRDVersionAction{logger: logger}
+}
+
+// AppliesTo selects the resources the plugin should run against. In this case, CustomResourceDefinitions.
+func (a *RemapCRDVersionAction) AppliesTo() (velero.ResourceSelector, error) {
+	return velero.ResourceSelector{
+		IncludedResources: []string{"customresourcedefinition.apiextensions.k8s.io"},
+	}, nil
+}
+
+// Execute executes logic necessary to check a CustomResourceDefinition and inspect it for characteristics that necessitate saving it as v1beta1 instead of v1.
+func (a *RemapCRDVersionAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) {
+	a.logger.Info("Executing RemapCRDVersionAction")
+
+	// This plugin is only relevant for CRDs retrieved from the v1 endpoint that were installed via the v1beta1
+	// endpoint, so we can exit immediately if the resource in question isn't v1.
+	apiVersion, ok, err := unstructured.NestedString(item.UnstructuredContent(), "apiVersion")
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "unable to read apiVersion from CRD")
+	}
+	if ok && apiVersion != "apiextensions.k8s.io/v1" {
+		a.logger.Info("Exiting RemapCRDVersionAction, CRD is not v1")
+		return item, nil, nil
+	}
+
+	// We've got a v1 CRD, so proceed.
+	var crd apiextv1.CustomResourceDefinition
+	if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.UnstructuredContent(), &crd); err != nil {
+		return nil, nil, errors.Wrap(err, "unable to convert unstructured item to a v1 CRD")
+	}
+
+	log := a.logger.WithField("plugin", "RemapCRDVersionAction").WithField("CRD", crd.Name)
+
+	// Looking for 1 version should be enough to tell if it's a v1beta1 CRD, as all v1beta1 CRD versions share the same schema.
+	// v1 CRDs can have different schemas per version
+	// The silently upgraded versions will often have a `versions` entry that looks like this:
+	//   versions:
+	//   - name: v1
+	//     served:  true
+	//     storage: true
+	// This is acceptable when re-submitted to a v1beta1 endpoint on restore.
+	if len(crd.Spec.Versions) > 0 {
+		if crd.Spec.Versions[0].Schema == nil || crd.Spec.Versions[0].Schema.OpenAPIV3Schema == nil {
+			log.Debug("CRD is a candidate for v1beta1 backup")
+
+			if err := setV1beta1Version(item); err != nil {
+				return nil, nil, err
+			}
+		}
+	}
+
+	// If the NonStructuralSchema condition was applied, be sure to back it up as v1beta1.
+	for _, c := range crd.Status.Conditions {
+		if c.Type == apiextv1.NonStructuralSchema {
+			log.Debug("CRD is a non-structural schema")
+
+			if err := setV1beta1Version(item); err != nil {
+				return nil, nil, err
+			}
+
+			break
+		}
+	}
+
+	return item, nil, nil
+}
+
+// setV1beta1Version updates the apiVersion field of an Unstructured CRD to be the v1beta1 string instead of v1.
+func setV1beta1Version(u runtime.Unstructured) error {
+	// Since we can't manipulate an Unstructured's Object map directly, get a copy to manipulate before setting it back
+	tempMap := u.UnstructuredContent()
+	if err := unstructured.SetNestedField(tempMap, "apiextensions.k8s.io/v1beta1", "apiVersion"); err != nil {
+		return errors.Wrap(err, "unable to set apiversion to v1beta1")
+	}
+	u.SetUnstructuredContent(tempMap)
+	return nil
+}

pkg/backup/remap_crd_version_action_test.go

@@ -0,0 +1,61 @@
+/*
+Copyright 2020 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 backup
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+
+	v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
+	"github.com/vmware-tanzu/velero/pkg/builder"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
+)
+
+func TestRemapCRDVersionAction(t *testing.T) {
+	backup := &v1.Backup{}
+	a := NewRemapCRDVersionAction(velerotest.NewLogger())
+
+	t.Run("Test a v1 CRD without any Schema information", func(t *testing.T) {
+		b := builder.ForV1CustomResourceDefinition("test.velero.io")
+		// Set a version that does not include and schema information.
+		b.Version(builder.ForV1CustomResourceDefinitionVersion("v1").Served(true).Storage(true).Result())
+		c := b.Result()
+		obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&c)
+		require.NoError(t, err)
+
+		item, _, err := a.Execute(&unstructured.Unstructured{Object: obj}, backup)
+		require.NoError(t, err)
+		assert.Equal(t, "apiextensions.k8s.io/v1beta1", item.UnstructuredContent()["apiVersion"])
+	})
+
+	t.Run("Test a v1 CRD with a NonStructuralSchema Condition", func(t *testing.T) {
+		b := builder.ForV1CustomResourceDefinition("test.velero.io")
+		b.Condition(builder.ForV1CustomResourceDefinitionCondition().Type(apiextv1.NonStructuralSchema).Result())
+		c := b.Result()
+		obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&c)
+		require.NoError(t, err)
+
+		item, _, err := a.Execute(&unstructured.Unstructured{Object: obj}, backup)
+		require.NoError(t, err)
+		assert.Equal(t, "apiextensions.k8s.io/v1beta1", item.UnstructuredContent()["apiVersion"])
+	})
+}

pkg/builder/v1_customresourcedefinition_builder.go

@@ -0,0 +1,132 @@
+/*
+Copyright 2020 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 (
+	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// V1CustomResourceDefinitionBuilder builds CustomResourceDefinition objects.
+type V1CustomResourceDefinitionBuilder struct {
+	object *apiextv1.CustomResourceDefinition
+}
+
+// ForV1CustomResourceDefinition is the constructor for a V1CustomResourceDefinitionBuilder.
+func ForV1CustomResourceDefinition(name string) *V1CustomResourceDefinitionBuilder {
+	return &V1CustomResourceDefinitionBuilder{
+		object: &apiextv1.CustomResourceDefinition{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: apiextv1.SchemeGroupVersion.String(),
+				Kind:       "CustomResourceDefinition",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name: name,
+			},
+		},
+	}
+}
+
+// Condition adds a CustomResourceDefinitionCondition objects to a V1CustomResourceDefinitionBuilder.
+func (b *V1CustomResourceDefinitionBuilder) Condition(cond apiextv1.CustomResourceDefinitionCondition) *V1CustomResourceDefinitionBuilder {
+	b.object.Status.Conditions = append(b.object.Status.Conditions, cond)
+	return b
+}
+
+// Version adds a CustomResourceDefinitionVersion object to a V1CustomResourceDefinitionBuilder.
+func (b *V1CustomResourceDefinitionBuilder) Version(ver apiextv1.CustomResourceDefinitionVersion) *V1CustomResourceDefinitionBuilder {
+	b.object.Spec.Versions = append(b.object.Spec.Versions, ver)
+	return b
+}
+
+// PreserveUnknownFields sets PreserveUnknownFields on a CustomResourceDefinition.
+func (b *V1CustomResourceDefinitionBuilder) PreserveUnknownFields(preserve bool) *V1CustomResourceDefinitionBuilder {
+	b.object.Spec.PreserveUnknownFields = preserve
+	return b
+}
+
+// Result returns the built CustomResourceDefinition.
+func (b *V1CustomResourceDefinitionBuilder) Result() *apiextv1.CustomResourceDefinition {
+	return b.object
+}
+
+// ObjectMeta applies functional options to the CustomResourceDefinition's ObjectMeta.
+func (b *V1CustomResourceDefinitionBuilder) ObjectMeta(opts ...ObjectMetaOpt) *V1CustomResourceDefinitionBuilder {
+	for _, opt := range opts {
+		opt(b.object)
+	}
+
+	return b
+}
+
+// V1CustomResourceDefinitionConditionBuilder builds CustomResourceDefinitionCondition objects.
+type V1CustomResourceDefinitionConditionBuilder struct {
+	object apiextv1.CustomResourceDefinitionCondition
+}
+
+// ForV1V1CustomResourceDefinitionConditionBuilder is the constructor for a V1CustomResourceDefinitionConditionBuilder.
+func ForV1CustomResourceDefinitionCondition() *V1CustomResourceDefinitionConditionBuilder {
+	return &V1CustomResourceDefinitionConditionBuilder{
+		object: apiextv1.CustomResourceDefinitionCondition{},
+	}
+}
+
+// Type sets the Condition's type.
+func (c *V1CustomResourceDefinitionConditionBuilder) Type(t apiextv1.CustomResourceDefinitionConditionType) *V1CustomResourceDefinitionConditionBuilder {
+	c.object.Type = t
+	return c
+}
+
+// Status sets the Condition's status.
+func (c *V1CustomResourceDefinitionConditionBuilder) Status(cs apiextv1.ConditionStatus) *V1CustomResourceDefinitionConditionBuilder {
+	c.object.Status = cs
+	return c
+}
+
+// Result returns the built CustomResourceDefinitionCondition.
+func (b *V1CustomResourceDefinitionConditionBuilder) Result() apiextv1.CustomResourceDefinitionCondition {
+	return b.object
+}
+
+// V1CustomResourceDefinitionVersionBuilder builds CustomResourceDefinitionVersion objects.
+type V1CustomResourceDefinitionVersionBuilder struct {
+	object apiextv1.CustomResourceDefinitionVersion
+}
+
+// ForV1CustomResourceDefinitionVersion is the constructor for a V1CustomResourceDefinitionVersionBuilder.
+func ForV1CustomResourceDefinitionVersion(name string) *V1CustomResourceDefinitionVersionBuilder {
+	return &V1CustomResourceDefinitionVersionBuilder{
+		object: apiextv1.CustomResourceDefinitionVersion{Name: name},
+	}
+}
+
+// Served sets the Served field on a CustomResourceDefinitionVersion.
+func (b *V1CustomResourceDefinitionVersionBuilder) Served(s bool) *V1CustomResourceDefinitionVersionBuilder {
+	b.object.Served = s
+	return b
+}
+
+// Storage sets the Storage field on a CustomResourceDefinitionVersion.
+func (b *V1CustomResourceDefinitionVersionBuilder) Storage(s bool) *V1CustomResourceDefinitionVersionBuilder {
+	b.object.Storage = s
+	return b
+}
+
+// Result returns the built CustomResourceDefinitionVersion.
+func (b *V1CustomResourceDefinitionVersionBuilder) Result() apiextv1.CustomResourceDefinitionVersion {
+	return b.object
+}

pkg/cmd/server/plugin/plugin.go

@@ -38,6 +38,7 @@ func NewCommand(f client.Factory) *cobra.Command {
 				RegisterBackupItemAction("velero.io/pv", newPVBackupItemAction).
 				RegisterBackupItemAction("velero.io/pod", newPodBackupItemAction).
 				RegisterBackupItemAction("velero.io/service-account", newServiceAccountBackupItemAction(f)).
+				RegisterBackupItemAction("velero.io/crd-remap-version", newRemapCRDVersionAction).
 				RegisterRestoreItemAction("velero.io/job", newJobRestoreItemAction).
 				RegisterRestoreItemAction("velero.io/pod", newPodRestoreItemAction).
 				RegisterRestoreItemAction("velero.io/restic", newResticRestoreItemAction(f)).
@@ -91,6 +92,10 @@ func newServiceAccountBackupItemAction(f client.Factory) veleroplugin.HandlerIni
 	}
 }
 
+func newRemapCRDVersionAction(logger logrus.FieldLogger) (interface{}, error) {
+	return backup.NewRemapCRDVersionAction(logger), nil
+}
+
 func newJobRestoreItemAction(logger logrus.FieldLogger) (interface{}, error) {
 	return restore.NewJobAction(logger), nil
 }

pkg/restore/crd_v1_preserve_unknown_fields_action.go

@@ -46,7 +46,12 @@ func (c *CRDV1PreserveUnknownFieldsAction) AppliesTo() (velero.ResourceSelector,
 func (c *CRDV1PreserveUnknownFieldsAction) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) {
 	c.logger.Info("Executing CRDV1PreserveUnknownFieldsAction")
 
-	log := c.logger.WithField("plugin", "CRDV1PreserveUnknownFieldsAction")
+	name, _, err := unstructured.NestedString(input.Item.UnstructuredContent(), "name")
+	if err != nil {
+		return nil, errors.Wrap(err, "could not get CRD name")
+	}
+
+	log := c.logger.WithField("plugin", "CRDV1PreserveUnknownFieldsAction").WithField("CRD", name)
 
 	version, _, err := unstructured.NestedString(input.Item.UnstructuredContent(), "apiVersion")
 	if err != nil {
@@ -74,7 +79,13 @@ func (c *CRDV1PreserveUnknownFieldsAction) Execute(input *velero.RestoreItemActi
 
 		// Make sure all versions are set to preserve unknown fields
 		for _, v := range crd.Spec.Versions {
-			// Use the address, since the XPreserveUnknownFields value is undefined or true (false is not allowed)
+			// If the schema fields are nil, there are no nested fields to set, so skip over it for this version.
+			if v.Schema == nil || v.Schema.OpenAPIV3Schema == nil {
+				continue
+			}
+
+			// Use the address, since the XPreserveUnknownFields value is nil or
+			// a pointer to true (false is not allowed)
 			preserve := true
 			v.Schema.OpenAPIV3Schema.XPreserveUnknownFields = &preserve
 			log.Debugf("Set x-preserve-unknown-fields in Open API for schema version %s", v.Name)