Skip to content

Commit 230eaad

Browse files
AlbeeSoAlbeeSo
committed
oss(credentials): support standard keys and
1 parent 042958f commit 230eaad

File tree

4 files changed

+220
-10
lines changed

4 files changed

+220
-10
lines changed

‎pkg/oss/coco.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ func (ns *nodeServer) publishDirectVolume(ctx context.Context, req *csi.NodePubl
8585
"targetPath": req.GetTargetPath(),
8686
}
8787
if opt.AkSecret != "" && strings.HasPrefix(opt.AkSecret, sealedSecretPrefix) {
88+
// `AkID` and `AkSecret` are fixed for the protocal
8889
metadata[AkID] = opt.AkID
8990
metadata[AkSecret] = opt.AkSecret
9091
}

‎pkg/oss/nodeserver.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ type nodeServer struct {
5050
}
5151

5252
const (
53-
// AkID is Ak ID
54-
AkID = "akId"
55-
// AkSecret is Ak Secret
56-
AkSecret = "akSecret"
5753
// OssFsType is the oss filesystem type
5854
OssFsType = "ossfs"
5955
// OssFs2Type is the ossfs2 filesystem type

‎pkg/oss/utils.go‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,28 @@ const (
5050
VolumeAsSubpath VolumeAsType = "subpath"
5151
)
5252

53+
const (
54+
// AkID and AkSecret are the key names we have always supported.
55+
// For compatibility with standard naming conventions, we also support customers configuring `accessKeyID` and `accessKeySecret`
56+
AkID = "akId"
57+
AkSecret = "akSecret"
58+
// AccessKeyID and AccessKeySecret are provided for compatibility with standard naming conventions.
59+
AccessKeyID = "accessKeyId"
60+
AccessKeySecret = "accessKeySecret"
61+
)
62+
63+
// parseCredentialsFromSecret retrieves long-term credentials from the `Secret` field in requset.
64+
// It prioritizes obtaining the credentials from `AkID` and `AkSecret`
65+
func parseCredentialsFromSecret(secrets map[string]string) (akID, akSecret string) {
66+
akID = strings.TrimSpace(secrets[AkID])
67+
akSecret = strings.TrimSpace(secrets[AkSecret])
68+
if akID == "" && akSecret == "" {
69+
akID = strings.TrimSpace(secrets[AccessKeyID])
70+
akSecret = strings.TrimSpace(secrets[AccessKeySecret])
71+
}
72+
return
73+
}
74+
5375
// get Options for CreateVolume and PublishVolume
5476
// volOptions: CreateVolumeRequest.GetParameters, PublishVolumeRequest.GetVolumeContext
5577
// secrets: CreateVolumeRequest / PublishVolumeRequest.GetSecrets
@@ -68,11 +90,13 @@ func parseOptions(volOptions, secrets map[string]string, volCaps []*csi.VolumeCa
6890
secrets = map[string]string{}
6991
}
7092

93+
// credientials
94+
akId, akSecret := parseCredentialsFromSecret(secrets)
7195
opts := &oss.Options{
7296
UseSharedPath: true,
7397
Path: "/",
74-
AkID: strings.TrimSpace(secrets[AkID]),
75-
AkSecret: strings.TrimSpace(secrets[AkSecret]),
98+
AkID: akId,
99+
AkSecret: akSecret,
76100
}
77101

78102
var volumeAsSubpath bool
@@ -89,10 +113,6 @@ func parseOptions(volOptions, secrets map[string]string, volCaps []*csi.VolumeCa
89113
opts.URL = value
90114
case "otheropts":
91115
opts.OtherOpts = value
92-
case "akid":
93-
opts.AkID = value
94-
case "aksecret":
95-
opts.AkSecret = value
96116
case "secretref":
97117
opts.SecretRef = value
98118
case "path":
@@ -161,6 +181,11 @@ func parseOptions(volOptions, secrets map[string]string, volCaps []*csi.VolumeCa
161181
klog.Warning(WrapOssError(ParamError, "the value(%q) of %q is invalid, only support %v, %v, %v",
162182
v, k, corev1.DNSClusterFirst, corev1.DNSClusterFirstWithHostNet, corev1.DNSDefault).Error())
163183
}
184+
// deprecated:
185+
case "akid":
186+
opts.AkID = value
187+
case "aksecret":
188+
opts.AkSecret = value
164189
}
165190
}
166191
for _, c := range volCaps {

‎pkg/oss/utils_test.go‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,194 @@ var m = metadata.FakeProvider{
3838
},
3939
}
4040

41+
func TestParseCredentialsFromSecret(t *testing.T) {
42+
tests := []struct {
43+
name string
44+
secrets map[string]string
45+
expected struct {
46+
akID string
47+
akSecret string
48+
}
49+
}{
50+
{
51+
name: "Case 1: AkID and AkSecret have valid values",
52+
secrets: map[string]string{
53+
AkID: " validAkID ",
54+
AkSecret: " validAkSecret ",
55+
},
56+
expected: struct {
57+
akID string
58+
akSecret string
59+
}{
60+
akID: "validAkID",
61+
akSecret: "validAkSecret",
62+
},
63+
},
64+
{
65+
name: "Case 2: AkID and AkSecret are empty, but AccessKeyID and AccessKeySecret have valid values",
66+
secrets: map[string]string{
67+
AkID: "",
68+
AkSecret: "",
69+
AccessKeyID: " validAccessKeyID ",
70+
AccessKeySecret: " validAccessKeySecret ",
71+
},
72+
expected: struct {
73+
akID string
74+
akSecret string
75+
}{
76+
akID: "validAccessKeyID",
77+
akSecret: "validAccessKeySecret",
78+
},
79+
},
80+
{
81+
name: "Case 3: AkSecret is empty, but AccessKeyID and AccessKeySecret have valid values",
82+
secrets: map[string]string{
83+
AkID: "",
84+
AkSecret: "AkSecret",
85+
AccessKeyID: " validAccessKeyID ",
86+
AccessKeySecret: " validAccessKeySecret ",
87+
},
88+
expected: struct {
89+
akID string
90+
akSecret string
91+
}{
92+
akID: "",
93+
akSecret: "AkSecret",
94+
},
95+
},
96+
{
97+
name: "Case 4: All fields are set, but AccessKeyID and AccessKeySecret have valid values",
98+
secrets: map[string]string{
99+
AkID: "AkID",
100+
AkSecret: "AkSecret",
101+
AccessKeyID: " validAccessKeyID ",
102+
AccessKeySecret: " validAccessKeySecret ",
103+
},
104+
expected: struct {
105+
akID string
106+
akSecret string
107+
}{
108+
akID: "AkID",
109+
akSecret: "AkSecret",
110+
},
111+
},
112+
{
113+
name: "Case 5: All keys have empty values",
114+
secrets: map[string]string{
115+
AkID: "",
116+
AkSecret: "",
117+
AccessKeyID: "",
118+
AccessKeySecret: "",
119+
},
120+
expected: struct {
121+
akID string
122+
akSecret string
123+
}{
124+
akID: "",
125+
akSecret: "",
126+
},
127+
},
128+
{
129+
name: "Case 6: Missing some keys, but others have valid values",
130+
secrets: map[string]string{
131+
AccessKeyID: " validAccessKeyID ",
132+
AccessKeySecret: " validAccessKeySecret ",
133+
},
134+
expected: struct {
135+
akID string
136+
akSecret string
137+
}{
138+
akID: "validAccessKeyID",
139+
akSecret: "validAccessKeySecret",
140+
},
141+
},
142+
}
143+
144+
for _, tt := range tests {
145+
t.Run(tt.name, func(t *testing.T) {
146+
akID, akSecret := parseCredentialsFromSecret(tt.secrets)
147+
assert.Equal(t, tt.expected.akID, akID)
148+
assert.Equal(t, tt.expected.akSecret, akSecret)
149+
})
150+
}
151+
}
152+
153+
func TestParseOptons_credentialsCompatibility(t *testing.T) {
154+
tests := []struct {
155+
name string
156+
options map[string]string
157+
secrets map[string]string
158+
akID string
159+
akSecret string
160+
}{
161+
{
162+
name: "allows obtaining the credentials with the former way",
163+
secrets: map[string]string{
164+
AkID: "validAkID",
165+
AkSecret: "validAkSecret",
166+
},
167+
akID: "validAkID",
168+
akSecret: "validAkSecret",
169+
},
170+
{
171+
name: "allows the secret is empty",
172+
},
173+
{
174+
name: "prioritizes obtaining the credentials with the former way",
175+
secrets: map[string]string{
176+
AkID: "validAkID",
177+
AkSecret: "validAkSecret",
178+
AccessKeyID: "validAccessKeyID",
179+
AccessKeySecret: "validAccessKeySecret",
180+
},
181+
akID: "validAkID",
182+
akSecret: "validAkSecret",
183+
},
184+
{
185+
name: "allows obtaining the credentials with the latter way",
186+
secrets: map[string]string{
187+
AccessKeyID: "validAccessKeyID",
188+
AccessKeySecret: "validAccessKeySecret",
189+
},
190+
akID: "validAccessKeyID",
191+
akSecret: "validAccessKeySecret",
192+
},
193+
{
194+
name: "ingnore invalid credentials",
195+
secrets: map[string]string{
196+
AkID: "validAkID",
197+
AccessKeySecret: "validAccessKeySecret",
198+
},
199+
akID: "validAkID",
200+
akSecret: "",
201+
},
202+
{
203+
name: "options overwrite the credentials",
204+
secrets: map[string]string{
205+
AkID: "validAkID",
206+
AkSecret: "validAkSecret",
207+
AccessKeyID: "validAccessKeyID",
208+
AccessKeySecret: "validAccessKeySecret",
209+
},
210+
options: map[string]string{
211+
AkID: "optionsAkID",
212+
AkSecret: "optionsAkSecret",
213+
},
214+
akID: "optionsAkID",
215+
akSecret: "optionsAkSecret",
216+
},
217+
}
218+
for _, tt := range tests {
219+
t.Run(tt.name, func(t *testing.T) {
220+
gotOptions := parseOptions(tt.options,
221+
tt.secrets, []*csi.VolumeCapability{},
222+
false, "", false, m)
223+
assert.Equal(t, tt.akID, gotOptions.AkID)
224+
assert.Equal(t, tt.akSecret, gotOptions.AkSecret)
225+
})
226+
}
227+
}
228+
41229
func Test_parseOptions(t *testing.T) {
42230
t.Setenv("ALIBABA_CLOUD_NETWORK_TYPE", "vpc")
43231

0 commit comments

Comments
 (0)