Add `TestVolumeEncryption` by hamistao · Pull Request #751 · rancher/tests

hamistao · 2026-07-07T19:50:05Z

This PR introduces the test TestVolumeEncryption to our LonghornTestSuite. Among other small unrelated improvements to the codebase.

Said test required introducing new helper funcitions to handle longhorn resources such as voluemes, backups and backuptargets. These functions can also be moved to shepherd if we think it would make more sense for them to live there. Comments on the operations performed during the test are greatly appreciated as this is a very complex test and there are many possible approaches to satisfying the steps outlined in the pit schema for this test or possibly even improving the current schema.

So main topics that I am looking forward to getting feedback are:
1- Are the new helper functions fine here or should they be moved to shepherd?
2- Are the steps performed able to assure the features explored work as expected?
3- Since the test is quite big and only the second half of it requires interacting with S3, should we break this test into 2 or more tests?

This work performed as part of rancher/qa-tasks#2403, the other test will be available for review in a separate PR.

floatingman

Just a few comments, but I think the one in secrets.go might cause a panic if not handled.

floatingman · 2026-07-13T20:35:08Z

+			Namespace: backup.Namespace,
+		},
+		Spec: longhorn.VolumeSpec{
+			Size:       backup.Spec.BackupBlockSize,


Should this be the size stored in backup.Status.VolumeSize instead? Seems like this will create a volume that is too small.

You are probably correct, I will chase the best way to handle this.

floatingman · 2026-07-13T20:55:14Z

+	}
+
+	var volume longhorn.Volume
+	err = volumeController.Client().Get(context.Background(), namespace, volumeName, &volume, metav1.GetOptions{})


I noticed that some of the methods use namespace and then some use charts.LonghornNamespace . Which is the correct one to use?

The methods that take in a namespace parameter receive charts.LonghornNamespace so I hadn't noticed the inconsistency. This is now fixed and now CreateS3LonghornBackupTarget always use charts.LonghornNamespace since the target namespace matters very little, CreateLonghornVolumeBackup takes in the volume's namespace and creates the backup in the same namespace and RestoreLonghornVolumeFromBackup creates the restored volume in the same namespace as the backup for simplicity. In the end we always expect the value to be charts.LonghornNamespace since it is the approriate place for longhorn resources to live.

Copilot

Pull request overview

This pull request adds a new Longhorn end-to-end test (TestVolumeEncryption) to validate encrypted volume behavior across on-node replica data and S3 backups, while also refactoring several secret/storage helpers and updating module dependencies to support the new functionality.

Changes:

Add TestVolumeEncryption to LonghornTestSuite, including S3 backup/restore validation.
Introduce/adjust helper actions for Longhorn resources, S3 access, storage/PVC workflows, and secret templates (switching many call sites to map[string]string / StringData).
Update go.mod / go.sum dependencies (root, actions/, and interoperability/) to newer versions required by added Longhorn/AWS functionality.

Reviewed changes

Copilot reviewed 38 out of 41 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
validation/upgrade/workload.go	Switch secret helper import and update secret template to use `StringData`-style input.
validation/snapshot/rke2/snapshot_s3_restore_test.go	Use new `actions/storage/s3` helpers for bucket lifecycle.
validation/snapshot/k3s/snapshot_s3_restore_test.go	Use new `actions/storage/s3` helpers for bucket lifecycle.
validation/rbac/secrets/rbac_registry_secrets_test.go	Update secret data setup to `map[string]string` for secret creation.
validation/rbac/secrets/rbac_opaque_secrets_test.go	Update secret data setup to `map[string]string` and assertions for updated secret data.
validation/rbac/globalrolesv2/namespacedrules_test.go	Update secret data setup to `map[string]string`.
validation/rbac/clusterandprojectroles/cluster_role_test.go	Update secret data setup to `map[string]string`.
validation/rbac/certificates/certificate_rbac_test.go	Update TLS secret data setup to `map[string]string`.
validation/rbac/aggregatedclusterroles/aggregated_cluster_roles_prtb_test.go	Update secret data setup to `map[string]string`.
validation/rbac/aggregatedclusterroles/aggregated_cluster_roles_crtb_test.go	Update secret data setup to `map[string]string`.
validation/rbac/aggregatedclusterroles/aggregated_cluster_roles_cleanup_test.go	Update secret data setup to `map[string]string`.
validation/projects/project_scoped_secrets_test.go	Update project-scoped secret helper signature and secret data to `map[string]string`.
validation/longhorn/longhorn_test.go	Add `TestVolumeEncryption` and supporting imports/constants; adjust Longhorn config loading usage.
validation/longhorn/chartinstall/installation_test.go	Improve workload pod selection and mounted-volume checks; reuse chart constants.
validation/charts/webhook.go	Remove unused constant.
validation/charts/monitoring_test.go	Switch secret helper import path to `actions/kubeapi/secrets`.
validation/charts/backup_restore/backup_restore.go	Switch secret helper import path and use `StringData`-style secret template input.
validation/certificates/cert_test.go	Update TLS secret creation data to `map[string]string`.
interoperability/longhorn/longhorn.go	Change Longhorn test config loader to return a value instead of pointer.
interoperability/go.mod	Bump dependency versions for interoperability module.
interoperability/go.sum	Update dependency checksums for interoperability module.
go.mod	Bump dependency versions; add replace for `beszel`; update k8s/prometheus/x/* versions.
actions/storage/verify.go	Update node filesystem checks to poll via backoff; update mounted-volume checks to accept kubeconfig.
actions/storage/s3/s3.go	Rename package to `s3` and add helpers to locate/read backup objects.
actions/storage/pvc.go	Refactor PVC creation to separate `CreatePVC` and add workload labeling for pod lookup.
actions/secrets/secrets.go	Remove old secrets template helper (moved to `actions/kubeapi/secrets`).
actions/provisioning/verify.go	Remove unused constants and simplify SSH command string formatting.
actions/provisioning/creates.go	Switch secret helper import path and use `map[string]string` for secret template data.
actions/kubeapi/storageclasses/storageclasses.go	Expand `NewStorageClass` constructor to set provisioner/parameters/expansion.
actions/kubeapi/secrets/template.go	Move secret stevetype constant here and switch template to `StringData`.
actions/kubeapi/secrets/secrets.go	Update `SecretCopyWithNewData` to accept `map[string]string`.
actions/kubeapi/secrets/create.go	Update secret creation helpers to accept `map[string]string`.
actions/kubeapi/longhorn/longhorn.go	Add Longhorn helper actions for volumes, backup targets, backups, restore, and deletion.
actions/go.mod	Bump module dependencies for actions module (AWS SDK v2, Longhorn manager, etc.).
actions/go.sum	Update dependency checksums for actions module.
actions/cloudprovider/verify.go	Remove pod-status check and unused constants/import.
actions/cloudprovider/cloudprovider.go	Switch Harvester credential secret data to `map[string]string`.
actions/charts/ranchermonitoring.go	Adjust watch handling; update failure error formatting.
actions/charts/longhorn.go	Add exported Longhorn storage class constants alongside namespace/chart name.
.github/scripts/check-gomod.sh	Adjust messaging/quoting and fix an indentation/`fi` formatting issue.

Comments suppressed due to low confidence (1)

actions/storage/s3/s3.go:112

ReadS3Object ignores the error from GetObject and doesn't close the response body. This can cause panics on error and leak HTTP connections/file descriptors during test runs.

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Also moves the pre-existing functions to `actions/storage/s3` Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

CI tries to run these commands when they are between ` https://github.com/rancher/tests/actions/runs/28968388225/job/85957411738?pr=751 Signed-off-by: hamistao <pedro.ribeiro@suse.com>

lscalabrini01 · 2026-07-14T19:52:03Z

 		}
 		if state == string(catalogv1.StatusFailed) {
-			return false, fmt.Errorf("monitoring chart installation failed: %s", app.Status.Summary.Error)
+			return false, fmt.Errorf("monitoring chart installation failed: %t", app.Status.Summary.Error)


Is the app.Status.Summary.Error a boolean? if not I would recommend use %s

lscalabrini01 · 2026-07-14T19:56:16Z

+	l.client.Session.RegisterCleanupFunc(func() error {
+		return longhornActions.DeleteLonghornVolume(l.client, l.cluster.ID, charts.LonghornNamespace, volumeName)
+	})


I would recommend moving the cleanup function to the actions package instead of keeping it in the test. That way, it can be reused by any other test that needs to create a volume

lscalabrini01 · 2026-07-14T19:58:42Z

+}
+
+// CreateLonghornVolumeBackup creates a snapshot for a Longhorn Volume and then creates a backup from this snapshot.
+// This waits until the backuṕ is succesfully created and returns the URL for that backup in S3.


Suggestion: from backuṕ to backup

lscalabrini01 · 2026-07-14T20:36:31Z

 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"


Duplicate import

hamistao requested review from a team as code owners July 7, 2026 19:50

hamistao force-pushed the longhorn_encryption_test branch 3 times, most recently from 00c4fa3 to 3ced3f8 Compare July 8, 2026 20:25

floatingman reviewed Jul 13, 2026

View reviewed changes

floatingman requested review from khushboo-rancher, lscalabrini01 and slickwarren July 13, 2026 21:03

floatingman added the team/pit-crew slack notifier for pit crew label Jul 13, 2026

hamistao requested a review from Copilot July 14, 2026 01:54

Copilot started reviewing on behalf of hamistao July 14, 2026 01:54 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

hamistao added 16 commits July 13, 2026 23:03

refactor ranchermonitoring.go

64883a3

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

minor improvements

e149e49

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Minor improvements to installation tests

2633154

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Refactor Longhorn constants

c92f3f9

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Refactor CheckMountedVolume

5ec601f

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Wait for deployment on createNginxDeploymentWithPVC

aa479fe

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Use WatchAndWaitPodContainerRunning instead of kubectl

5d4195f

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Allow passing parameters in NewStorageClass

ad53877

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Remove NewSecretTemplate duplicate and use StringData instead of Data

09011b9

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Add S3 helper functions

0d3a47f

Also moves the pre-existing functions to `actions/storage/s3` Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Create CreatePVC and move GetTargetVolume

d07ae76

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

add longhorn-manager dependency

3dbaf0c

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Create Longhorn helper functions

447b899

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Use exponential backoff on CheckNodeFilesystem

0a41416

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Add TestVolumeEncryption

dbe435d

Signed-off-by: hamistao <pedro.ribeiro@suse.com>

Fix check-gomod.sh

12f6b33

CI tries to run these commands when they are between ` https://github.com/rancher/tests/actions/runs/28968388225/job/85957411738?pr=751 Signed-off-by: hamistao <pedro.ribeiro@suse.com>

hamistao force-pushed the longhorn_encryption_test branch from 3ced3f8 to 12f6b33 Compare July 14, 2026 02:05

lscalabrini01 requested changes Jul 14, 2026

View reviewed changes

hamistao self-assigned this Jul 16, 2026

		metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
		v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Uh oh!

Conversation

hamistao commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

floatingman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hamistao commented Jul 7, 2026 •

edited

Loading