diff --git a/docs/cli-reference/ark_backup_create.md b/docs/cli-reference/ark_backup_create.md index 0b317b229..d9568c80c 100644 --- a/docs/cli-reference/ark_backup_create.md +++ b/docs/cli-reference/ark_backup_create.md @@ -14,18 +14,19 @@ ark backup create NAME [flags] ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the backup - --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io - -h, --help help for create - --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the backup - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - -l, --selector labelSelector only back up resources matching this label selector (default ) - --show-labels show labels in the last column - --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup - --ttl duration how long before the backup can be garbage collected (default 24h0m0s) + --exclude-namespaces stringArray namespaces to exclude from the backup + --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io + -h, --help help for create + --include-cluster-resources optionalBool[=true] include cluster-scoped resources in the backup + --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the backup + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + -l, --selector labelSelector only back up resources matching this label selector (default ) + --show-labels show labels in the last column + --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup + --ttl duration how long before the backup can be garbage collected (default 24h0m0s) ``` ### Options inherited from parent commands diff --git a/docs/cli-reference/ark_schedule_create.md b/docs/cli-reference/ark_schedule_create.md index a3ea62726..4548e71b0 100644 --- a/docs/cli-reference/ark_schedule_create.md +++ b/docs/cli-reference/ark_schedule_create.md @@ -14,19 +14,20 @@ ark schedule create NAME [flags] ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the backup - --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io - -h, --help help for create - --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the backup - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - --schedule string a cron expression specifying a recurring schedule for this backup to run - -l, --selector labelSelector only back up resources matching this label selector (default ) - --show-labels show labels in the last column - --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup - --ttl duration how long before the backup can be garbage collected (default 24h0m0s) + --exclude-namespaces stringArray namespaces to exclude from the backup + --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io + -h, --help help for create + --include-cluster-resources optionalBool[=true] include cluster-scoped resources in the backup + --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the backup + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + --schedule string a cron expression specifying a recurring schedule for this backup to run + -l, --selector labelSelector only back up resources matching this label selector (default ) + --show-labels show labels in the last column + --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup + --ttl duration how long before the backup can be garbage collected (default 24h0m0s) ``` ### Options inherited from parent commands diff --git a/pkg/apis/ark/v1/backup.go b/pkg/apis/ark/v1/backup.go index 55e544ae4..16f9075ef 100644 --- a/pkg/apis/ark/v1/backup.go +++ b/pkg/apis/ark/v1/backup.go @@ -49,6 +49,10 @@ type BackupSpec struct { // TTL is a time.Duration-parseable string describing how long // the Backup should be retained for. TTL metav1.Duration `json:"ttl"` + + // IncludeClusterResources specifies whether cluster-scoped resources + // should be included for consideration in the backup. + IncludeClusterResources *bool `json:"includeClusterResources"` } // BackupPhase is a string representation of the lifecycle phase diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index f70d41530..dff5e0cc8 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -289,6 +289,26 @@ func (kb *kubernetesBackupper) backupResource( gr := schema.GroupResource{Group: gv.Group, Resource: resource.Name} grString := gr.String() + switch { + case ctx.backup.Spec.IncludeClusterResources == nil: + // when IncludeClusterResources == nil (auto), only directly + // back up cluster-scoped resources if we're doing a full-cluster + // (all namespaces) backup. Note that in the case of a subset of + // namespaces being backed up, some related cluster-scoped resources + // may still be backed up if triggered by a custom action (e.g. PVC->PV). + if !resource.Namespaced && !ctx.namespaceIncludesExcludes.IncludeEverything() { + ctx.infof("Skipping resource %s because it's cluster-scoped and only specific namespaces are included in the backup", grString) + return nil + } + case *ctx.backup.Spec.IncludeClusterResources == false: + if !resource.Namespaced { + ctx.infof("Skipping resource %s because it's cluster-scoped", grString) + return nil + } + case *ctx.backup.Spec.IncludeClusterResources == true: + // include the resource, no action required + } + if !ctx.resourceIncludesExcludes.ShouldInclude(grString) { ctx.infof("Resource %s is excluded", grString) return nil @@ -411,11 +431,14 @@ func (ib *realItemBackupper) backupItem(ctx *backupContext, item map[string]inte namespace, err := collections.GetString(item, "metadata.namespace") // a non-nil error is assumed to be due to a cluster-scoped item - if err == nil { - if !ctx.namespaceIncludesExcludes.ShouldInclude(namespace) { - ctx.infof("Excluding item %s because namespace %s is excluded", name, namespace) - return nil - } + if err == nil && !ctx.namespaceIncludesExcludes.ShouldInclude(namespace) { + ctx.infof("Excluding item %s because namespace %s is excluded", name, namespace) + return nil + } + + if namespace == "" && ctx.backup.Spec.IncludeClusterResources != nil && *ctx.backup.Spec.IncludeClusterResources == false { + ctx.infof("Excluding item %s because resource %s is cluster-scoped and IncludeClusterResources is false", name, groupResource.String()) + return nil } if !ctx.resourceIncludesExcludes.ShouldInclude(groupResource.String()) { diff --git a/pkg/backup/backup_pv_action_test.go b/pkg/backup/backup_pv_action_test.go new file mode 100644 index 000000000..d3c6c8844 --- /dev/null +++ b/pkg/backup/backup_pv_action_test.go @@ -0,0 +1,95 @@ +/* +Copyright 2017 Heptio Inc. + +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" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + + testutil "github.com/heptio/ark/pkg/util/test" + testlogger "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/assert" +) + +func TestBackupPVAction(t *testing.T) { + tests := []struct { + name string + item map[string]interface{} + volumeName string + expectedErr bool + }{ + { + name: "execute PV backup in normal case", + item: map[string]interface{}{ + "metadata": map[string]interface{}{"name": "pvc-1"}, + "spec": map[string]interface{}{"volumeName": "pv-1"}, + }, + volumeName: "pv-1", + expectedErr: false, + }, + { + name: "error when PVC has no metadata.name", + item: map[string]interface{}{ + "metadata": map[string]interface{}{}, + "spec": map[string]interface{}{"volumeName": "pv-1"}, + }, + expectedErr: true, + }, + { + name: "error when PVC has no spec.volumeName", + item: map[string]interface{}{ + "metadata": map[string]interface{}{"name": "pvc-1"}, + "spec": map[string]interface{}{}, + }, + expectedErr: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var ( + discoveryHelper = testutil.NewFakeDiscoveryHelper(true, nil) + dynamicFactory = &testutil.FakeDynamicFactory{} + dynamicClient = &testutil.FakeDynamicClient{} + testLogger, _ = testlogger.NewNullLogger() + ctx = &backupContext{discoveryHelper: discoveryHelper, dynamicFactory: dynamicFactory, logger: testLogger} + backupper = &fakeItemBackupper{} + action = NewBackupPVAction() + pv = &unstructured.Unstructured{} + pvGVR = schema.GroupVersionResource{Resource: "persistentvolumes"} + ) + + dynamicFactory.On("ClientForGroupVersionResource", + pvGVR, + metav1.APIResource{Name: "persistentvolumes"}, + "", + ).Return(dynamicClient, nil) + + dynamicClient.On("Get", test.volumeName, metav1.GetOptions{}).Return(pv, nil) + + backupper.On("backupItem", ctx, pv.UnstructuredContent(), pvGVR.GroupResource()).Return(nil) + + // method under test + res := action.Execute(ctx, test.item, backupper) + + assert.Equal(t, test.expectedErr, res != nil) + }) + } +} diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index d8887e7eb..ed05e7102 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -46,6 +46,13 @@ import ( . "github.com/heptio/ark/pkg/util/test" ) +var ( + trueVal = true + falseVal = false + truePointer = &trueVal + falsePointer = &falseVal +) + type fakeAction struct { ids []string backups []*v1.Backup @@ -434,8 +441,8 @@ func TestBackupMethod(t *testing.T) { expectedFiles := sets.NewString( "namespaces/a/configmaps/configMap1.json", "namespaces/b/configmaps/configMap2.json", - "cluster/certificatesigningrequests.certificates.k8s.io/csr1.json", "namespaces/a/roles.rbac.authorization.k8s.io/role1.json", + // CSRs are not expected because they're unrelated cluster-scoped resources ) expectedData := map[string]string{ @@ -464,24 +471,6 @@ func TestBackupMethod(t *testing.T) { } } `, - "cluster/certificatesigningrequests.certificates.k8s.io/csr1.json": ` - { - "apiVersion": "certificates.k8s.io/v1beta1", - "kind": "CertificateSigningRequest", - "metadata": { - "name": "csr1" - }, - "spec": { - "request": "some request", - "username": "bob", - "uid": "12345", - "groups": [ - "group1", - "group2" - ] - } - } - `, "namespaces/a/roles.rbac.authorization.k8s.io/role1.json": ` { "apiVersion": "rbac.authorization.k8s.io/v1beta1", @@ -499,6 +488,7 @@ func TestBackupMethod(t *testing.T) { ] } `, + // CSRs are not expected because they're unrelated cluster-scoped resources } seenFiles := sets.NewString() @@ -548,10 +538,10 @@ func TestBackupMethod(t *testing.T) { } expectedCMActionIDs := []string{"a/configMap1", "b/configMap2"} - expectedCSRActionIDs := []string{"csr1"} assert.Equal(t, expectedCMActionIDs, cmAction.ids) - assert.Equal(t, expectedCSRActionIDs, csrAction.ids) + // CSRs are not expected because they're unrelated cluster-scoped resources + assert.Nil(t, csrAction.ids) } func TestBackupResource(t *testing.T) { @@ -573,6 +563,7 @@ func TestBackupResource(t *testing.T) { expectedDeploymentsBackedUp bool networkPoliciesBackedUp bool expectedNetworkPoliciesBackedUp bool + includeClusterResources *bool }{ { name: "should not include resource", @@ -744,6 +735,117 @@ func TestBackupResource(t *testing.T) { "certificatesigningrequests": {"1"}, }, }, + { + name: "should include cluster-scoped resource if backing up subset of namespaces and --include-cluster-resources=true", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("ns-1"), + includeClusterResources: truePointer, + lists: []string{ + `{ + "apiVersion": "foogroup/v1", + "kind": "BarList", + "items": [ + { + "metadata": { + "namespace": "", + "name": "1" + } + } + ] + }`, + }, + expectedListedNamespaces: []string{""}, + }, + { + name: "should not include cluster-scoped resource if backing up subset of namespaces and --include-cluster-resources=false", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("ns-1"), + includeClusterResources: falsePointer, + }, + { + name: "should not include cluster-scoped resource if backing up subset of namespaces and --include-cluster-resources=", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("ns-1"), + includeClusterResources: nil, + }, + { + name: "should include cluster-scoped resources if backing up all namespaces and --include-cluster-resources=true", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + includeClusterResources: truePointer, + lists: []string{ + `{ + "apiVersion": "foogroup/v1", + "kind": "BarList", + "items": [ + { + "metadata": { + "namespace": "", + "name": "1" + } + } + ] + }`, + }, + expectedListedNamespaces: []string{""}, + }, + { + name: "should not include cluster-scoped resource if backing up all namespaces and --include-cluster-resources=false", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + includeClusterResources: falsePointer, + }, + { + name: "should include cluster-scoped resource if backing up all namespaces and --include-cluster-resources=", + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceGroup: "foogroup", + resourceVersion: "v1", + resourceGV: "foogroup/v1", + resourceName: "bars", + resourceNamespaced: false, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + includeClusterResources: nil, + lists: []string{ + `{ + "apiVersion": "foogroup/v1", + "kind": "BarList", + "items": [ + { + "metadata": { + "namespace": "", + "name": "1" + } + } + ] + }`, + }, + expectedListedNamespaces: []string{""}, + }, } for _, test := range tests { @@ -760,7 +862,8 @@ func TestBackupResource(t *testing.T) { ctx := &backupContext{ backup: &v1.Backup{ Spec: v1.BackupSpec{ - LabelSelector: labelSelector, + LabelSelector: labelSelector, + IncludeClusterResources: test.includeClusterResources, }, }, resourceIncludesExcludes: test.resourceIncludesExcludes, @@ -869,6 +972,9 @@ func TestBackupItem(t *testing.T) { name string item string namespaceIncludesExcludes *collections.IncludesExcludes + resourceIncludesExcludes *collections.IncludesExcludes + includeClusterResources *bool + backedUpItems map[itemKey]struct{} expectError bool expectExcluded bool expectedTarHeaderName string @@ -956,6 +1062,30 @@ func TestBackupItem(t *testing.T) { customAction: true, expectedActionID: "myns/bar", }, + { + name: "cluster-scoped item not backed up when --include-cluster-resources=false", + item: `{"metadata":{"namespace":"","name":"bar"}}`, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + includeClusterResources: falsePointer, + expectError: false, + expectExcluded: true, + }, + { + name: "item not backed up when resource includes/excludes excludes it", + item: `{"metadata":{"namespace":"","name":"bar"}}`, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*").Excludes("resource.group"), + expectError: false, + expectExcluded: true, + }, + { + name: "item not backed up when it's already been backed up", + item: `{"metadata":{"namespace":"","name":"bar"}}`, + namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), + backedUpItems: map[itemKey]struct{}{itemKey{resource: "resource.group", namespace: "", name: "bar"}: struct{}{}}, + expectError: false, + expectExcluded: true, + }, } for _, test := range tests { @@ -980,7 +1110,7 @@ func TestBackupItem(t *testing.T) { var ( action *fakeAction - backup = &v1.Backup{} + backup = &v1.Backup{Spec: v1.BackupSpec{IncludeClusterResources: test.includeClusterResources}} groupResource = schema.ParseGroupResource("resource.group") log, _ = testlogger.NewNullLogger() ) @@ -994,6 +1124,14 @@ func TestBackupItem(t *testing.T) { resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), } + if test.resourceIncludesExcludes != nil { + ctx.resourceIncludesExcludes = test.resourceIncludesExcludes + } + + if test.backedUpItems != nil { + ctx.backedUpItems = test.backedUpItems + } + if test.customAction { action = &fakeAction{} ctx.actions = map[schema.GroupResource]Action{ diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index a5656b642..77de7d0d1 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -54,23 +54,25 @@ func NewCreateCommand(f client.Factory) *cobra.Command { } type CreateOptions struct { - Name string - TTL time.Duration - SnapshotVolumes flag.OptionalBool - IncludeNamespaces flag.StringArray - ExcludeNamespaces flag.StringArray - IncludeResources flag.StringArray - ExcludeResources flag.StringArray - Labels flag.Map - Selector flag.LabelSelector + Name string + TTL time.Duration + SnapshotVolumes flag.OptionalBool + IncludeNamespaces flag.StringArray + ExcludeNamespaces flag.StringArray + IncludeResources flag.StringArray + ExcludeResources flag.StringArray + Labels flag.Map + Selector flag.LabelSelector + IncludeClusterResources flag.OptionalBool } func NewCreateOptions() *CreateOptions { return &CreateOptions{ - TTL: 24 * time.Hour, - IncludeNamespaces: flag.NewStringArray("*"), - Labels: flag.NewMap(), - SnapshotVolumes: flag.NewOptionalBool(nil), + TTL: 24 * time.Hour, + IncludeNamespaces: flag.NewStringArray("*"), + Labels: flag.NewMap(), + SnapshotVolumes: flag.NewOptionalBool(nil), + IncludeClusterResources: flag.NewOptionalBool(nil), } } @@ -86,6 +88,9 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { // this allows the user to just specify "--snapshot-volumes" as shorthand for "--snapshot-volumes=true" // like a normal bool flag f.NoOptDefVal = "true" + + f = flags.VarPF(&o.IncludeClusterResources, "include-cluster-resources", "", "include cluster-scoped resources in the backup") + f.NoOptDefVal = "true" } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { @@ -125,6 +130,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { LabelSelector: o.Selector.LabelSelector, SnapshotVolumes: o.SnapshotVolumes.Value, TTL: metav1.Duration{Duration: o.TTL}, + IncludeClusterResources: o.IncludeClusterResources.Value, }, } diff --git a/pkg/util/collections/includes_excludes.go b/pkg/util/collections/includes_excludes.go index 62d165968..668a2193a 100644 --- a/pkg/util/collections/includes_excludes.go +++ b/pkg/util/collections/includes_excludes.go @@ -73,6 +73,12 @@ func (ie *IncludesExcludes) ShouldInclude(s string) bool { return ie.includes.Has("*") || ie.includes.Has(s) } +// IncludeEverything returns true if the Includes list is '*' +// and the Excludes list is empty, or false otherwise. +func (ie *IncludesExcludes) IncludeEverything() bool { + return ie.excludes.Len() == 0 && ie.includes.Len() == 1 && ie.includes.Has("*") +} + // ValidateIncludesExcludes checks provided lists of included and excluded // items to ensure they are a valid set of IncludesExcludes data. func ValidateIncludesExcludes(includesList, excludesList []string) []error { @@ -109,7 +115,7 @@ func ValidateIncludesExcludes(includesList, excludesList []string) []error { // GenerateIncludesExcludes constructs an IncludesExcludes struct by taking the provided // include/exclude slices, applying the specified mapping function to each item in them, // and adding the output of the function to the new struct. If the mapping function returns -// an error for an item, it is omitted from the result. +// an empty string for an item, it is omitted from the result. func GenerateIncludesExcludes(includes, excludes []string, mapFunc func(string) string) *IncludesExcludes { res := NewIncludesExcludes()