Repository: kinvolk/lokomotive
Pull request #1149 (2020-10-30 06:37:47)

velero: Add provider field and remove default configuration in restic plugin
Brings back the addition of `provider` field in the component configuration.
This is done so to be able to provide default configurations for the plugins.

Also it removes the default BackupStorageLocation config in restic
plugin.

Name is optional and its default value is `default` in Helm chart.
Provider is required and making it a default of `aws` doesn't make sense
as the user has to provide  value to it anyway.

Region is also specifc to the provider used. If not `aws` then there is
no need to input that value.

Release notes: 

Add `provider` field in the component configuration.
example:

```
component `velero` {
  .
  .
  provider = "openebs"
 
  openebs {
    .
    .
  }
}
```

Signed-off-by: Imran Pochi <imran@kinvolk.io>

knrt10: Can you please address check linter and build errors?
ipochi: > Can you please address linter and build errors?

yep, I am looking into the bug introduced by this fix.

Patch:
diff --git a/docs/configuration-reference/components/velero.md b/docs/configuration-reference/components/velero.md
index ecc7927a..61943bb8 100644
--- a/docs/configuration-reference/components/velero.md
+++ b/docs/configuration-reference/components/velero.md
@@ -37,6 +37,7 @@ Velero component configuration example:
 # velero.lokocfg
 component "velero" {
 
+  # provider = "azure/openebs/restic"
   # azure {
   #   # Required arguments.
   #   subscription_id = "9e5ac23c-6df8-44c4-9790-6f6decf96268"
@@ -110,6 +111,7 @@ Table of all the arguments accepted by the component.
 | Argument                                             | Description                                                                                                                 | Default                                           | Type   | Required |
 |------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------|--------|----------|
 | `namespace`                                          | Namespace to install Velero.                                                                                                | "velero"                                          | string | false    |
+| `provider`                                           | Provider sets which provider block to use for the configuration. Supported values are: `azure`, `openebs` and `restic`.     | -                                                 | string | true     |
 | `metrics`                                            | Configure Prometheus to scrape Velero metrics. Needs the [Prometheus Operator component](prometheus-operator.md) installed. | -                                                 | object | false    |
 | `metrics.enabled`                                    | Adds Prometheus annotations to Velero deployment if enabled.                                                                | false                                             | bool   | false    |
 | `metrics.service_monitor`                            | Adds ServiceMonitor resource for Prometheus. Requires `metrics.enabled` as true.                                            | false                                             | bool   | false    |
@@ -145,10 +147,10 @@ Table of all the arguments accepted by the component.
 | `openebs.volume_snapshot_location.s3_url`            | S3 API URL.                                                                                                                 | -                                                 | string | false    |
 | `restic`                                             | Configure Restic provider for Velero.                                                                                       | -                                                 | object | false    |
 | `restic.credentials`                                 | Content of cloud provider credentials.                                                                                      | -                                                 | string | true     |
-| `restic.backup_storage_location.provider`            | Cloud provider name for storing backups.                                                                                    | -                                                 | string | false    |
+| `restic.backup_storage_location.provider`            | Cloud provider name for storing backups.                                                                                    | -                                                 | string | true     |
 | `restic.backup_storage_location.bucket`              | Cloud storage bucket name for storing backups.                                                                              | -                                                 | string | true     |
-| `restic.backup_storage_location.name`                | Name for backup location object on the cluster.                                                                             | -                                                 | string | false    |
-| `restic.backup_storage_location.region`              | Cloud provider region for storing snapshots.                                                                                | `eu-west-1`                                       | string | false    |
+| `restic.backup_storage_location.name`                | Name for backup location object on the cluster.                                                                             | "default"                                         | string | false    |
+| `restic.backup_storage_location.region`              | Cloud provider region for storing snapshots. Required if `restic.backup_storage_location.provider = aws`.                   | -                                                 | string | false    |
 
 ## Applying
 
diff --git a/pkg/components/velero/component.go b/pkg/components/velero/component.go
index c7afe9a0..7cee0849 100644
--- a/pkg/components/velero/component.go
+++ b/pkg/components/velero/component.go
@@ -43,7 +43,8 @@ type component struct {
 	Namespace string `hcl:"namespace,optional"`
 	// Metrics specific configuration
 	Metrics *Metrics `hcl:"metrics,block"`
-
+	// Provider specifies which Velero plugin is to be configured.
+	Provider string `hcl:"provider"`
 	// Azure specific parameters
 	Azure *azure.Configuration `hcl:"azure,block"`
 	// OpenEBS specific parameters.
@@ -72,6 +73,9 @@ func newComponent() *component {
 			Enabled:        false,
 			ServiceMonitor: false,
 		},
+		Azure:   &azure.Configuration{},
+		OpenEBS: &openebs.Configuration{},
+		Restic:  restic.NewConfiguration(),
 	}
 }
 
@@ -187,29 +191,16 @@ func (c *component) getSupportedProviders() []string {
 // If no providers are configured or there is more than one provider configured, error
 // is returned.
 func (c *component) getProvider() (provider, error) {
-	providers := []provider{}
-
-	if c.Azure != nil {
-		providers = append(providers, c.Azure)
-	}
-
-	if c.OpenEBS != nil {
-		providers = append(providers, c.OpenEBS)
-	}
-
-	if c.Restic != nil {
-		providers = append(providers, c.Restic)
+	switch c.Provider {
+	case "azure":
+		return c.Azure, nil
+	case "openebs":
+		return c.OpenEBS, nil
+	case "restic":
+		return c.Restic, nil
+	default:
+		return nil, fmt.Errorf("unknown provider: %s", c.Provider)
 	}
-
-	if len(providers) > 1 {
-		return nil, fmt.Errorf("more than one provider configured")
-	}
-
-	if len(providers) == 0 {
-		return nil, fmt.Errorf("no providers configured")
-	}
-
-	return providers[0], nil
 }
 
 func (c *component) Metadata() components.Metadata {
diff --git a/pkg/components/velero/openebs/openebs.go b/pkg/components/velero/openebs/openebs.go
index 2a059214..7982e584 100644
--- a/pkg/components/velero/openebs/openebs.go
+++ b/pkg/components/velero/openebs/openebs.go
@@ -53,7 +53,7 @@ func (b *BackupStorageLocation) validate(defaultProvider string) hcl.Diagnostics
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.backup_storage_location' block must be specified",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -61,7 +61,7 @@ func (b *BackupStorageLocation) validate(defaultProvider string) hcl.Diagnostics
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.backup_storage_location.bucket' cannot be empty",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -69,7 +69,7 @@ func (b *BackupStorageLocation) validate(defaultProvider string) hcl.Diagnostics
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.backup_storage_location.region' cannot be empty",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -77,7 +77,7 @@ func (b *BackupStorageLocation) validate(defaultProvider string) hcl.Diagnostics
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "either 'openebs.provider' or 'openebs.backup_storage_location.provider' must be set",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -115,7 +115,7 @@ func (v *VolumeSnapshotLocation) validate(defaultProvider string) hcl.Diagnostic
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.volume_snapshot_location' block must be specified",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -123,7 +123,7 @@ func (v *VolumeSnapshotLocation) validate(defaultProvider string) hcl.Diagnostic
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.volume_snapshot_location.bucket' cannot be empty",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -131,7 +131,7 @@ func (v *VolumeSnapshotLocation) validate(defaultProvider string) hcl.Diagnostic
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'openebs.volume_snapshot_location.region' cannot be empty",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
@@ -139,7 +139,7 @@ func (v *VolumeSnapshotLocation) validate(defaultProvider string) hcl.Diagnostic
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "either 'openebs.provider' or 'openebs.volume_snapshot_location.provider' must be set",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 
diff --git a/pkg/components/velero/restic/restic.go b/pkg/components/velero/restic/restic.go
index 2f50fd0e..dba71540 100644
--- a/pkg/components/velero/restic/restic.go
+++ b/pkg/components/velero/restic/restic.go
@@ -44,11 +44,7 @@ type BackupStorageLocation struct {
 // NewConfiguration returns the default restic configuration.
 func NewConfiguration() *Configuration {
 	return &Configuration{
-		BackupStorageLocation: &BackupStorageLocation{
-			Provider: "aws",
-			Name:     "default",
-			Region:   "eu-west-1",
-		},
+		BackupStorageLocation: &BackupStorageLocation{},
 	}
 }
 
@@ -83,7 +79,7 @@ func (c *Configuration) Validate() hcl.Diagnostics {
 		diagnostics = append(diagnostics, &hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "'restic.backup_storage_location' block must be specified",
-			Detail:   "Make sure to the set the field to valid non-empty value",
+			Detail:   "Make sure to set the field to valid non-empty value",
 		})
 	}
 


Test patch:
diff --git a/pkg/components/velero/component_test.go b/pkg/components/velero/component_test.go
index fc922b67..4fb11122 100644
--- a/pkg/components/velero/component_test.go
+++ b/pkg/components/velero/component_test.go
@@ -37,6 +37,7 @@ func TestEmptyConfig(t *testing.T) {
 func TestRenderManifestAzure(t *testing.T) {
 	configHCL := `
 component "velero" {
+  provider = "azure"
   azure {
     subscription_id  = "foo"
     tenant_id        = "foo"
@@ -78,6 +79,7 @@ component "velero" {
 func TestRenderManifestOpenEBS(t *testing.T) {
 	configHCL := `
 component "velero" {
+  provider = "openebs"
   openebs {
     credentials = "foo"
     provider    = "aws"
@@ -122,6 +124,7 @@ component "velero" {
 func TestRenderManifestConflictingProviders(t *testing.T) {
 	configHCL := `
 component "velero" {
+  provider = "azure"
   azure {}
   openebs {}
 }
@@ -159,6 +162,7 @@ component "velero" {}
 func TestRenderManifestRestic(t *testing.T) {
 	configHCL := `
 component "velero" {
+  provider = "restic"
   restic {
     credentials = "foo"