OCPBUGS-83667: inject restoreSnapshotURL into HostedControlPlane during restore

pkg/core/backup.go

@@ -164,6 +164,14 @@ func (p *BackupPlugin) Execute(item runtime.Unstructured, backup *velerov1.Backu
 		if err := p.waitForEtcdBackupCompletion(ctx); err != nil {
 			return nil, nil, err
 		}
+		if p.etcdSnapshotURL != "" {
+			metadata, err := meta.Accessor(item)
+			if err != nil {
+				return nil, nil, fmt.Errorf("error getting metadata accessor: %v", err)
+			}
+			common.AddAnnotation(metadata, common.EtcdSnapshotURLAnnotation, p.etcdSnapshotURL)
+			p.log.Infof("Added etcd snapshot URL annotation to HostedControlPlane %s", metadata.GetName())
+		}
 
 	case kind == common.HostedClusterKind:
 		metadata, err := meta.Accessor(item)

pkg/core/backup_test.go

@@ -134,6 +134,25 @@ func TestExecute(t *testing.T) {
 			},
 		},
 		// HostedControlPlane cases
+		{
+			name: "When Execute processes a HostedControlPlane with cached etcdSnapshotURL, It Should add etcd snapshot URL annotation",
+			setup: func(bp *BackupPlugin) {
+				bp.etcdSnapshotURL = "s3://bucket/backups/test/etcd-backup/snapshot.db"
+			},
+			item: func() *unstructured.Unstructured {
+				item := newUnstructuredItem("HostedControlPlane", "hypershift.openshift.io/v1beta1", "test-hcp", "clusters-test")
+				item.Object["spec"] = map[string]any{
+					"platform": map[string]any{"type": "AWS"},
+				}
+				return item
+			},
+			backup: newTestBackup,
+			assert: func(g *GomegaWithT, result runtime.Unstructured, _ *BackupPlugin) {
+				metadata := result.UnstructuredContent()["metadata"].(map[string]any)
+				annotations := metadata["annotations"].(map[string]any)
+				g.Expect(annotations[common.EtcdSnapshotURLAnnotation]).To(Equal("s3://bucket/backups/test/etcd-backup/snapshot.db"))
+			},
+		},
 		{
 			name: "When Execute processes a HostedControlPlane with volumeSnapshot method, It Should not create etcd backup",
 			item: func() *unstructured.Unstructured {

pkg/core/restore.go

@@ -165,6 +165,34 @@ func (p *RestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*v
 			return nil, fmt.Errorf("error checking platform configuration: %v", err)
 		}
 
+		metadata, err := meta.Accessor(input.Item)
+		if err != nil {
+			return nil, fmt.Errorf("error getting metadata accessor: %v", err)
+		}
+		annotations := metadata.GetAnnotations()
+		snapshotURL := annotations[common.EtcdSnapshotURLAnnotation]
+		if snapshotURL != "" {
+			if strings.HasPrefix(snapshotURL, "s3://") {
+				presigned, err := p.presignS3URL(ctx, backup, snapshotURL)
+				if err != nil {
+					return nil, fmt.Errorf("error generating pre-signed URL for etcd snapshot: %w", err)
+				}
+				p.log.Infof("Converted s3:// URL to pre-signed HTTPS URL for HostedControlPlane restore")
+				snapshotURL = presigned
+			}
+
+			if hcp.Spec.Etcd.Managed != nil {
+				hcp.Spec.Etcd.Managed.Storage.RestoreSnapshotURL = []string{snapshotURL}
+				p.log.Infof("Injected restoreSnapshotURL into HostedControlPlane %s", hcp.Name)
+
+				unstructuredHCP, err := runtime.DefaultUnstructuredConverter.ToUnstructured(hcp)
+				if err != nil {
+					return nil, fmt.Errorf("error converting HostedControlPlane to unstructured: %v", err)
+				}
+				input.Item.SetUnstructuredContent(unstructuredHCP)
+			}
+		}
+
 	case kind == "Pod":
 		p.log.Debugf("Pod found, skipping restore")
 		return velero.NewRestoreItemActionExecuteOutput(input.Item).WithoutRestore(), nil
@@ -187,7 +215,7 @@ func (p *RestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*v
 				if strings.HasPrefix(snapshotURL, "s3://") {
 					presigned, err := p.presignS3URL(ctx, backup, snapshotURL)
 					if err != nil {
-						return nil, fmt.Errorf("error generating pre-signed URL for etcd snapshot: %v", err)
+						return nil, fmt.Errorf("error generating pre-signed URL for etcd snapshot: %w", err)
 					}
 					p.log.Infof("Converted s3:// URL to pre-signed HTTPS URL for HostedCluster restore")
 					snapshotURL = presigned

pkg/core/restore_test.go

@@ -8,18 +8,32 @@ import (
 	"testing"
 
 	common "github.com/openshift/hypershift-oadp-plugin/pkg/common"
+	plugtypes "github.com/openshift/hypershift-oadp-plugin/pkg/core/types"
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 	"github.com/sirupsen/logrus"
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	veleroapiv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero"
 	corev1 "k8s.io/api/core/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+type mockRestoreValidator struct {
+	validatePlatformErr error
+}
+
+func (m *mockRestoreValidator) ValidatePluginConfig(_ map[string]string) (*plugtypes.RestoreOptions, error) {
+	return &plugtypes.RestoreOptions{}, nil
+}
+
+func (m *mockRestoreValidator) ValidatePlatformConfig(_ *hyperv1.HostedControlPlane, _ map[string]string) error {
+	return m.validatePlatformErr
+}
+
 func TestPresignS3URL(t *testing.T) {
 	scheme := runtime.NewScheme()
 	_ = hyperv1.AddToScheme(scheme)
@@ -240,6 +254,45 @@ func newHCUnstructured(name, namespace string, annotations map[string]string) *u
 	return hc
 }
 
+func newHCPUnstructured(t *testing.T, name, namespace string, annotations map[string]string) *unstructured.Unstructured {
+	t.Helper()
+	hcp := &hyperv1.HostedControlPlane{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "hypershift.openshift.io/v1beta1",
+			Kind:       "HostedControlPlane",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        name,
+			Namespace:   namespace,
+			Annotations: annotations,
+		},
+		Spec: hyperv1.HostedControlPlaneSpec{
+			ReleaseImage: "quay.io/openshift-release-dev/ocp-release:4.16.0-x86_64",
+			Platform:     hyperv1.PlatformSpec{Type: hyperv1.AWSPlatform},
+			PullSecret:   corev1.LocalObjectReference{Name: "pull-secret"},
+			IssuerURL:    "https://kubernetes.default.svc",
+			SSHKey:       corev1.LocalObjectReference{Name: "ssh-key"},
+			InfraID:      "test-infra",
+			Etcd: hyperv1.EtcdSpec{
+				ManagementType: hyperv1.Managed,
+				Managed: &hyperv1.ManagedEtcdSpec{
+					Storage: hyperv1.ManagedEtcdStorageSpec{
+						Type: hyperv1.PersistentVolumeEtcdStorage,
+						PersistentVolume: &hyperv1.PersistentVolumeEtcdStorageSpec{
+							Size: func() *resource.Quantity { q := resource.MustParse("8Gi"); return &q }(),
+						},
+					},
+				},
+			},
+		},
+	}
+	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(hcp)
+	if err != nil {
+		t.Fatalf("failed to convert HCP to unstructured: %v", err)
+	}
+	return &unstructured.Unstructured{Object: raw}
+}
+
 func TestRestoreExecuteSnapshotURL(t *testing.T) {
 	s := common.CustomScheme
 
@@ -360,9 +413,10 @@ func TestRestoreExecuteSnapshotURL(t *testing.T) {
 				Build()
 
 			plugin := &RestorePlugin{
-				log:    logrus.New(),
-				ctx:    context.Background(),
-				client: fakeClient,
+				log:       logrus.New(),
+				ctx:       context.Background(),
+				client:    fakeClient,
+				validator: &mockRestoreValidator{},
 			}
 
 			hc := newHCUnstructured("my-hc", "clusters", tt.annotations)
@@ -386,3 +440,162 @@ func TestRestoreExecuteSnapshotURL(t *testing.T) {
 		})
 	}
 }
+
+func TestRestoreExecuteHCPSnapshotURL(t *testing.T) {
+	s := common.CustomScheme
+
+	credentialData := []byte("[default]\naws_access_key_id = AKIAIOSFODNN7EXAMPLE\naws_secret_access_key = wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY\n")
+
+	hcpCRD := &apiextensionsv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "hostedcontrolplanes.hypershift.openshift.io"},
+	}
+
+	bsl := &velerov1api.BackupStorageLocation{
+		ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "openshift-adp"},
+		Spec: velerov1api.BackupStorageLocationSpec{
+			Config: map[string]string{"region": "us-east-1"},
+			Credential: &corev1.SecretKeySelector{
+				LocalObjectReference: corev1.LocalObjectReference{Name: "cloud-credentials"},
+				Key:                  "cloud",
+			},
+		},
+	}
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "cloud-credentials", Namespace: "openshift-adp"},
+		Data:       map[string][]byte{"cloud": credentialData},
+	}
+	backup := &velerov1api.Backup{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-backup", Namespace: "openshift-adp"},
+		Spec: velerov1api.BackupSpec{
+			StorageLocation:    "default",
+			IncludedNamespaces: []string{"clusters", "clusters-test"},
+		},
+	}
+	restore := &velerov1api.Restore{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-restore", Namespace: "openshift-adp"},
+		Spec:       velerov1api.RestoreSpec{BackupName: "test-backup"},
+	}
+
+	origSAPath := common.DefaultK8sSAFilePath
+	nsDir := t.TempDir()
+	if err := os.WriteFile(nsDir+"/namespace", []byte("openshift-adp"), 0644); err != nil {
+		t.Fatalf("failed to write namespace file: %v", err)
+	}
+	common.SetK8sSAFilePath(nsDir)
+	t.Cleanup(func() { common.SetK8sSAFilePath(origSAPath) })
+
+	extractRestoreSnapshotURL := func(output *veleroapiv1.RestoreItemActionExecuteOutput) ([]any, bool) {
+		spec := output.UpdatedItem.UnstructuredContent()["spec"].(map[string]any)
+		etcd := spec["etcd"].(map[string]any)
+		managed := etcd["managed"].(map[string]any)
+		storage := managed["storage"].(map[string]any)
+		urls, ok := storage["restoreSnapshotURL"].([]any)
+		return urls, ok
+	}
+
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		missingBSL  bool
+		wantErr     bool
+		assert      func(*testing.T, *veleroapiv1.RestoreItemActionExecuteOutput)
+	}{
+		{
+			name: "When HCP has s3 annotation and BSL is missing, it should return presign error",
+			annotations: map[string]string{
+				common.EtcdSnapshotURLAnnotation: "s3://my-bucket/path/to/snapshot.db",
+			},
+			missingBSL: true,
+			wantErr:    true,
+		},
+		{
+			name: "When HCP has etcd-snapshot-url annotation with s3 scheme, it should inject pre-signed restoreSnapshotURL",
+			annotations: map[string]string{
+				common.EtcdSnapshotURLAnnotation: "s3://my-bucket/path/to/snapshot.db",
+			},
+			assert: func(t *testing.T, output *veleroapiv1.RestoreItemActionExecuteOutput) {
+				urls, ok := extractRestoreSnapshotURL(output)
+				if !ok || len(urls) == 0 {
+					t.Fatal("expected restoreSnapshotURL to be set")
+				}
+				presignedURL, ok := urls[0].(string)
+				if !ok {
+					t.Fatal("expected restoreSnapshotURL[0] to be a string")
+				}
+				if !strings.HasPrefix(presignedURL, "https://") {
+					t.Errorf("expected pre-signed URL to start with https://, got %s", presignedURL)
+				}
+				if !strings.Contains(presignedURL, "X-Amz-Signature") {
+					t.Errorf("expected pre-signed URL to contain X-Amz-Signature, got %s", presignedURL)
+				}
+			},
+		},
+		{
+			name:        "When HCP has no etcd-snapshot-url annotation, it should not inject restoreSnapshotURL",
+			annotations: nil,
+			assert: func(t *testing.T, output *veleroapiv1.RestoreItemActionExecuteOutput) {
+				spec := output.UpdatedItem.UnstructuredContent()["spec"].(map[string]any)
+				etcd := spec["etcd"].(map[string]any)
+				managed := etcd["managed"].(map[string]any)
+				storage := managed["storage"].(map[string]any)
+				if _, exists := storage["restoreSnapshotURL"]; exists {
+					t.Error("expected restoreSnapshotURL to NOT be set when no annotation is present")
+				}
+			},
+		},
+		{
+			name: "When HCP has https annotation, it should inject it directly without presigning",
+			annotations: map[string]string{
+				common.EtcdSnapshotURLAnnotation: "https://my-bucket.s3.us-east-1.amazonaws.com/path/to/snapshot.db?X-Amz-Signature=abc123",
+			},
+			assert: func(t *testing.T, output *veleroapiv1.RestoreItemActionExecuteOutput) {
+				urls, ok := extractRestoreSnapshotURL(output)
+				if !ok || len(urls) == 0 {
+					t.Fatal("expected restoreSnapshotURL to be set for https URL")
+				}
+				expected := "https://my-bucket.s3.us-east-1.amazonaws.com/path/to/snapshot.db?X-Amz-Signature=abc123"
+				if urls[0].(string) != expected {
+					t.Errorf("expected URL to pass through unchanged, got %s", urls[0])
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			builder := fake.NewClientBuilder().
+				WithScheme(s).
+				WithObjects(hcpCRD, backup)
+			if !tt.missingBSL {
+				builder = builder.WithObjects(bsl, secret)
+			}
+			fakeClient := builder.Build()
+
+			plugin := &RestorePlugin{
+				log:       logrus.New(),
+				ctx:       context.Background(),
+				client:    fakeClient,
+				validator: &mockRestoreValidator{},
+			}
+
+			hcp := newHCPUnstructured(t, "my-hcp", "clusters-test", tt.annotations)
+
+			output, err := plugin.Execute(&veleroapiv1.RestoreItemActionExecuteInput{
+				Item:    hcp,
+				Restore: restore,
+			})
+			if tt.wantErr {
+				if err == nil {
+					t.Error("expected error but got nil")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if tt.assert != nil {
+				tt.assert(t, output)
+			}
+		})
+	}
+}