Skip to content

Commit 51d8c88

Browse files
anurag4DSBanurag4DSB
committed
S3CSI-5: Add unit tests
1 parent 66a3ba7 commit 51d8c88

File tree

6 files changed

+323
-3
lines changed

6 files changed

+323
-3
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ require (
9191
github.com/google/uuid v1.6.0
9292
github.com/moby/sys/mountinfo v0.7.1 // indirect
9393
github.com/nxadm/tail v1.4.8 // indirect
94-
golang.org/x/net v0.36.0
94+
golang.org/x/net v0.36.0 // indirect
9595
golang.org/x/sys v0.30.0
9696
golang.org/x/text v0.22.0 // indirect
9797
google.golang.org/protobuf v1.34.2

‎pkg/driver/node/credentialprovider/provider_secret_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package credentialprovider_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"google.golang.org/grpc/codes"
8+
"google.golang.org/grpc/status"
9+
10+
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/credentialprovider"
11+
"github.com/scality/mountpoint-s3-csi-driver/pkg/util/testutil/assert"
12+
)
13+
14+
func TestProvideFromSecret(t *testing.T) {
15+
t.Skip("Internal method testing needs to be refactored to expose a testable function")
16+
}
17+
18+
func TestProvideWithSecretAuthentication(t *testing.T) {
19+
validSecret := map[string]string{
20+
"key_id": "AKIA123456789ABC",
21+
"access_key": "SECRET123456789ABCDEFGHIJKLMNOPQRSTUV",
22+
}
23+
24+
invalidSecret := map[string]string{
25+
"key_id": "invalid-format",
26+
}
27+
28+
tests := []struct {
29+
name string
30+
secretData map[string]string
31+
expectError bool
32+
}{
33+
{
34+
name: "valid credentials",
35+
secretData: validSecret,
36+
expectError: false,
37+
},
38+
{
39+
name: "invalid credentials",
40+
secretData: invalidSecret,
41+
expectError: true,
42+
},
43+
{
44+
name: "missing credentials",
45+
secretData: map[string]string{},
46+
expectError: true,
47+
},
48+
}
49+
50+
provider := credentialprovider.New(nil, nil)
51+
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
provideCtx := credentialprovider.ProvideContext{
55+
AuthenticationSource: credentialprovider.AuthenticationSourceSecret,
56+
VolumeID: "test-volume-id",
57+
SecretData: tt.secretData,
58+
}
59+
60+
env, authSource, err := provider.Provide(context.Background(), provideCtx)
61+
62+
if tt.expectError {
63+
if err == nil {
64+
t.Errorf("Expected error but got nil")
65+
}
66+
st, ok := status.FromError(err)
67+
if !ok {
68+
t.Errorf("Expected status error but got %v", err)
69+
}
70+
if st.Code() != codes.InvalidArgument {
71+
t.Errorf("Expected InvalidArgument code but got %v", st.Code())
72+
}
73+
} else {
74+
assert.NoError(t, err)
75+
assert.Equals(t, credentialprovider.AuthenticationSourceSecret, authSource)
76+
if env == nil {
77+
t.Errorf("Expected environment to be not nil")
78+
}
79+
}
80+
})
81+
}
82+
}

‎pkg/driver/node/credentialprovider/provider_test.go

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,3 +874,114 @@ func serviceAccount(name, namespace string, annotations map[string]string) *v1.S
874874
Annotations: annotations,
875875
}}
876876
}
877+
878+
// TestProvideWithSecretAuthSource tests authentication with Secret credentials
879+
func TestProvideWithSecretAuthSource(t *testing.T) {
880+
tests := []struct {
881+
name string
882+
secretData map[string]string
883+
expectError bool
884+
expectedAuth credentialprovider.AuthenticationSource
885+
}{
886+
{
887+
name: "valid secret credentials",
888+
secretData: map[string]string{
889+
"key_id": "AKIA123456789ABC",
890+
"access_key": "SECRET123456789ABCDEFGHIJKLMNOPQRSTUV",
891+
},
892+
expectError: false,
893+
expectedAuth: credentialprovider.AuthenticationSourceSecret,
894+
},
895+
{
896+
name: "invalid secret credentials",
897+
secretData: map[string]string{
898+
"key_id": "invalid",
899+
},
900+
expectError: true,
901+
},
902+
{
903+
name: "secret with unexpected keys",
904+
secretData: map[string]string{
905+
"key_id": "AKIA123456789ABC",
906+
"access_key": "SECRET123456789ABCDEFGHIJKLMNOPQRSTUV",
907+
"unexpected": "value", // This will trigger the unexpected key warning
908+
},
909+
expectError: false,
910+
expectedAuth: credentialprovider.AuthenticationSourceSecret,
911+
},
912+
{
913+
name: "invalid access_key format",
914+
secretData: map[string]string{
915+
"key_id": "AKIA123456789ABC",
916+
// This will trigger the invalid access_key warning
917+
"access_key": "SECRET!@#$%^&*()",
918+
},
919+
expectError: true,
920+
},
921+
{
922+
name: "access_key exceeds max length",
923+
secretData: map[string]string{
924+
"key_id": "AKIA123456789ABC",
925+
// This will trigger the max length warning for access_key
926+
"access_key": "SECRET123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ",
927+
},
928+
expectError: true,
929+
},
930+
}
931+
932+
for _, tt := range tests {
933+
t.Run(tt.name, func(t *testing.T) {
934+
provider := credentialprovider.New(nil, nil)
935+
936+
provideCtx := credentialprovider.ProvideContext{
937+
VolumeID: "test-volume-id",
938+
AuthenticationSource: credentialprovider.AuthenticationSourceSecret,
939+
SecretData: tt.secretData,
940+
}
941+
942+
env, authSource, err := provider.Provide(context.Background(), provideCtx)
943+
944+
if tt.expectError {
945+
if err == nil {
946+
t.Errorf("Expected error but got nil")
947+
}
948+
} else {
949+
assert.NoError(t, err)
950+
assert.Equals(t, tt.expectedAuth, authSource)
951+
if env == nil {
952+
t.Errorf("Expected environment to be not nil")
953+
}
954+
}
955+
})
956+
}
957+
}
958+
959+
func TestProvideWithUnknownAuthSource(t *testing.T) {
960+
provider := credentialprovider.New(nil, dummyRegionProvider)
961+
962+
writePath := t.TempDir()
963+
provideCtx := credentialprovider.ProvideContext{
964+
AuthenticationSource: "unknown-source", // Using an unknown authentication source
965+
WritePath: writePath,
966+
EnvPath: testEnvPath,
967+
PodID: testPodID,
968+
VolumeID: testVolumeID,
969+
}
970+
971+
env, source, err := provider.Provide(context.Background(), provideCtx)
972+
973+
// Verify error was returned
974+
if err == nil {
975+
t.Errorf("Expected error for unknown authentication source, got nil")
976+
}
977+
978+
// Verify error message contains all supported auth sources
979+
expectedErrMsg := "unknown `authenticationSource`: unknown-source, only `driver` (default option if not specified), `pod`, and `secret` supported"
980+
if err.Error() != expectedErrMsg {
981+
t.Errorf("Expected error message %q, got %q", expectedErrMsg, err.Error())
982+
}
983+
984+
// Verify returned values
985+
assert.Equals(t, credentialprovider.AuthenticationSourceUnspecified, source)
986+
assert.Equals(t, envprovider.Environment(nil), env)
987+
}

‎pkg/driver/node/log_request_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package node_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/container-storage-interface/spec/lib/go/csi"
7+
"github.com/scality/mountpoint-s3-csi-driver/pkg/util/testutil/assert"
8+
9+
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node"
10+
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/volumecontext"
11+
)
12+
13+
func TestLogSafeNodePublishVolumeRequestCoverage(t *testing.T) {
14+
tests := []struct {
15+
name string
16+
secrets map[string]string
17+
expectedOutput map[string]string
18+
}{
19+
{
20+
name: "redact access_key only",
21+
secrets: map[string]string{
22+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
23+
"access_key": "secret-that-should-be-redacted",
24+
},
25+
expectedOutput: map[string]string{
26+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
27+
"access_key": "[REDACTED]",
28+
},
29+
},
30+
{
31+
name: "keep other values",
32+
secrets: map[string]string{
33+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
34+
"access_key": "secret-that-should-be-redacted",
35+
"other_key": "other-value",
36+
},
37+
expectedOutput: map[string]string{
38+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
39+
"access_key": "[REDACTED]",
40+
"other_key": "other-value",
41+
},
42+
},
43+
{
44+
name: "empty secrets",
45+
secrets: map[string]string{},
46+
expectedOutput: map[string]string{},
47+
},
48+
}
49+
50+
for _, tt := range tests {
51+
t.Run(tt.name, func(t *testing.T) {
52+
req := &csi.NodePublishVolumeRequest{
53+
VolumeId: "test-volume",
54+
VolumeContext: map[string]string{
55+
"bucketName": "test-bucket",
56+
volumecontext.CSIServiceAccountTokens: "some-token-value",
57+
},
58+
Secrets: tt.secrets,
59+
}
60+
61+
// Call the function through the exported accessor
62+
safeReq := node.LogSafeNodePublishVolumeRequestForTest(req)
63+
64+
// Verify secrets are properly redacted
65+
assert.Equals(t, tt.expectedOutput, safeReq.Secrets)
66+
67+
// Verify sensitive service account tokens are removed
68+
_, hasTokens := safeReq.VolumeContext[volumecontext.CSIServiceAccountTokens]
69+
if hasTokens {
70+
t.Errorf("Expected tokens to be removed from VolumeContext")
71+
}
72+
73+
// Verify other fields are preserved
74+
assert.Equals(t, req.VolumeId, safeReq.VolumeId)
75+
if safeReq.VolumeContext == nil {
76+
t.Errorf("Expected VolumeContext to be not nil")
77+
}
78+
assert.Equals(t, "test-bucket", safeReq.VolumeContext["bucketName"])
79+
})
80+
}
81+
}

‎pkg/driver/node/node.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,3 +317,8 @@ func logSafeNodePublishVolumeRequest(req *csi.NodePublishVolumeRequest) *csi.Nod
317317
Secrets: redactedCredential,
318318
}
319319
}
320+
321+
// LogSafeNodePublishVolumeRequestForTest exports logSafeNodePublishVolumeRequest for testing
322+
func LogSafeNodePublishVolumeRequestForTest(req *csi.NodePublishVolumeRequest) *csi.NodePublishVolumeRequest {
323+
return logSafeNodePublishVolumeRequest(req)
324+
}

‎pkg/driver/node/node_test.go

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
package node_test
22

33
import (
4+
"context"
45
"errors"
56
"io/fs"
67
"testing"
78

89
csi "github.com/container-storage-interface/spec/lib/go/csi"
910
"github.com/golang/mock/gomock"
10-
"golang.org/x/net/context"
11+
"github.com/scality/mountpoint-s3-csi-driver/pkg/util/testutil/assert"
1112

1213
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node"
1314
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/credentialprovider"
1415
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/mounter"
1516
mock_driver "github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/mounter/mocks"
17+
"github.com/scality/mountpoint-s3-csi-driver/pkg/driver/node/volumecontext"
1618
"github.com/scality/mountpoint-s3-csi-driver/pkg/mountpoint"
17-
"github.com/scality/mountpoint-s3-csi-driver/pkg/util/testutil/assert"
1819
)
1920

2021
type nodeServerTestEnv struct {
@@ -568,6 +569,46 @@ func TestNodeGetCapabilitiesForPodMounter(t *testing.T) {
568569
nodeTestEnv.mockCtl.Finish()
569570
}
570571

572+
func TestLogSafeNodePublishVolumeRequest(t *testing.T) {
573+
tests := []struct {
574+
name string
575+
publishReq *csi.NodePublishVolumeRequest
576+
expectedPublishReq *csi.NodePublishVolumeRequest
577+
}{
578+
{
579+
name: "redacts access key and removes tokens",
580+
publishReq: &csi.NodePublishVolumeRequest{
581+
VolumeId: "test-volume-id",
582+
Secrets: map[string]string{
583+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
584+
"access_key": "SECRET-VALUE-TO-REDACT",
585+
},
586+
VolumeContext: map[string]string{
587+
"bucketName": "my-bucket",
588+
volumecontext.CSIServiceAccountTokens: "sensitive-token-value",
589+
},
590+
},
591+
expectedPublishReq: &csi.NodePublishVolumeRequest{
592+
VolumeId: "test-volume-id",
593+
Secrets: map[string]string{
594+
"key_id": "AKIAXXXXXXXXXXXXXXXX",
595+
"access_key": "[REDACTED]",
596+
},
597+
VolumeContext: map[string]string{
598+
"bucketName": "my-bucket",
599+
},
600+
},
601+
},
602+
}
603+
604+
for _, tc := range tests {
605+
t.Run(tc.name, func(t *testing.T) {
606+
safeReq := node.LogSafeNodePublishVolumeRequestForTest(tc.publishReq)
607+
assert.Equals(t, tc.expectedPublishReq, safeReq)
608+
})
609+
}
610+
}
611+
571612
var _ mounter.Mounter = &dummyMounter{}
572613

573614
type dummyMounter struct{}

0 commit comments

Comments
 (0)