Skip to content

Commit 9ee823d

Browse files
ohaibbqostermanmilldr
authored
[Stores] Add support for non-string values to GSM (#1237)
* [Stores] Add support for non-string values to GSM * dry up --------- Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com> Co-authored-by: Dan Miller <miller0daniel@gmail.com>
1 parent 2325ab9 commit 9ee823d

2 files changed

Lines changed: 149 additions & 119 deletions

File tree

‎pkg/store/google_secret_manager_store.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package store
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"strings"
78
"time"
@@ -179,10 +180,12 @@ func (s *GSMStore) Set(stack string, component string, key string, value any) er
179180
ctx, cancel := context.WithTimeout(context.Background(), gsmOperationTimeout)
180181
defer cancel()
181182

182-
strValue, ok := value.(string)
183-
if !ok {
184-
return ErrValueMustBeString
183+
// Convert value to JSON string
184+
jsonValue, err := json.Marshal(value)
185+
if err != nil {
186+
return fmt.Errorf(errWrapFormat, ErrSerializeJSON, err)
185187
}
188+
strValue := string(jsonValue)
186189

187190
// Get the secret ID using getKey
188191
secretID, err := s.getKey(stack, component, key)
@@ -243,5 +246,11 @@ func (s *GSMStore) Get(stack string, component string, key string) (any, error)
243246
return nil, fmt.Errorf(errWrapFormat, ErrAccessSecret, err)
244247
}
245248

246-
return string(result.Payload.Data), nil
249+
var unmarshalled interface{}
250+
// Intentionally ignoring JSON unmarshal error to handle legacy or 3rd-party secrets that might not be JSON-encoded
251+
if err := json.Unmarshal(result.Payload.Data, &unmarshalled); err != nil {
252+
// If it's not valid JSON, return the raw string value
253+
return string(result.Payload.Data), nil
254+
}
255+
return unmarshalled, nil
247256
}

‎pkg/store/google_secret_manager_store_test.go‎

Lines changed: 136 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ import (
1818

1919
// Define test error constants.
2020
var (
21-
ErrInternalError = errors.New("internal error")
21+
ErrInternalError = errors.New("internal error")
22+
ErrTransientError = errors.New("transient error")
2223
)
2324

2425
// MockGSMClient is a mock implementation of GSMClient.
@@ -76,6 +77,45 @@ func newGSMStoreWithClient(client GSMClient, options GSMStoreOptions) *GSMStore
7677
return store
7778
}
7879

80+
func gsmClientSecretCreationMock(parent string, secretId string, secretPayload string, err error) func(m *MockGSMClient) {
81+
return func(m *MockGSMClient) {
82+
m.On("CreateSecret", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.CreateSecretRequest) bool {
83+
expectedReq := &secretmanagerpb.CreateSecretRequest{
84+
Parent: parent,
85+
SecretId: secretId,
86+
Secret: &secretmanagerpb.Secret{
87+
Replication: &secretmanagerpb.Replication{
88+
Replication: &secretmanagerpb.Replication_Automatic_{
89+
Automatic: &secretmanagerpb.Replication_Automatic{},
90+
},
91+
},
92+
},
93+
}
94+
return req.Parent == expectedReq.Parent &&
95+
req.SecretId == expectedReq.SecretId &&
96+
req.Secret.GetReplication().GetAutomatic() != nil
97+
})).Return(&secretmanagerpb.Secret{
98+
Name: fmt.Sprintf("%s/secrets/%s", parent, secretId),
99+
}, nil)
100+
101+
var ret *secretmanagerpb.SecretVersion
102+
if err == nil {
103+
ret = &secretmanagerpb.SecretVersion{}
104+
}
105+
106+
m.On("AddSecretVersion", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.AddSecretVersionRequest) bool {
107+
expectedReq := &secretmanagerpb.AddSecretVersionRequest{
108+
Parent: fmt.Sprintf("%s/secrets/%s", parent, secretId),
109+
Payload: &secretmanagerpb.SecretPayload{
110+
Data: []byte(secretPayload),
111+
},
112+
}
113+
return req.Parent == expectedReq.Parent &&
114+
string(req.Payload.Data) == string(expectedReq.Payload.Data)
115+
})).Return(ret, err)
116+
}
117+
}
118+
79119
func TestGSMStore_Set(t *testing.T) {
80120
testPrefix := "test-prefix"
81121
testDelimiter := "-"
@@ -95,37 +135,12 @@ func TestGSMStore_Set(t *testing.T) {
95135
component: "app/service",
96136
key: "config-key",
97137
value: "test-value",
98-
mockFn: func(m *MockGSMClient) {
99-
m.On("CreateSecret", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.CreateSecretRequest) bool {
100-
expectedReq := &secretmanagerpb.CreateSecretRequest{
101-
Parent: "projects/test-project",
102-
SecretId: "test-prefix_dev_usw2_app_service_config-key",
103-
Secret: &secretmanagerpb.Secret{
104-
Replication: &secretmanagerpb.Replication{
105-
Replication: &secretmanagerpb.Replication_Automatic_{
106-
Automatic: &secretmanagerpb.Replication_Automatic{},
107-
},
108-
},
109-
},
110-
}
111-
return req.Parent == expectedReq.Parent &&
112-
req.SecretId == expectedReq.SecretId &&
113-
req.Secret.GetReplication().GetAutomatic() != nil
114-
})).Return(&secretmanagerpb.Secret{
115-
Name: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key",
116-
}, nil)
117-
118-
m.On("AddSecretVersion", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.AddSecretVersionRequest) bool {
119-
expectedReq := &secretmanagerpb.AddSecretVersionRequest{
120-
Parent: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key",
121-
Payload: &secretmanagerpb.SecretPayload{
122-
Data: []byte("test-value"),
123-
},
124-
}
125-
return req.Parent == expectedReq.Parent &&
126-
string(req.Payload.Data) == string(expectedReq.Payload.Data)
127-
})).Return(&secretmanagerpb.SecretVersion{}, nil)
128-
},
138+
mockFn: gsmClientSecretCreationMock(
139+
"projects/test-project",
140+
"test-prefix_dev_usw2_app_service_config-key",
141+
`"test-value"`,
142+
nil,
143+
),
129144
wantErr: false,
130145
},
131146
{
@@ -134,35 +149,12 @@ func TestGSMStore_Set(t *testing.T) {
134149
component: "app/service",
135150
key: "config-key",
136151
value: "test-value",
137-
mockFn: func(m *MockGSMClient) {
138-
m.On("CreateSecret", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.CreateSecretRequest) bool {
139-
expectedReq := &secretmanagerpb.CreateSecretRequest{
140-
Parent: "projects/test-project",
141-
SecretId: "test-prefix_dev_usw2_app_service_config-key",
142-
Secret: &secretmanagerpb.Secret{
143-
Replication: &secretmanagerpb.Replication{
144-
Replication: &secretmanagerpb.Replication_Automatic_{
145-
Automatic: &secretmanagerpb.Replication_Automatic{},
146-
},
147-
},
148-
},
149-
}
150-
return req.Parent == expectedReq.Parent &&
151-
req.SecretId == expectedReq.SecretId &&
152-
req.Secret.GetReplication().GetAutomatic() != nil
153-
})).Return(nil, status.Error(codes.AlreadyExists, "secret already exists"))
154-
155-
m.On("AddSecretVersion", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.AddSecretVersionRequest) bool {
156-
expectedReq := &secretmanagerpb.AddSecretVersionRequest{
157-
Parent: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key",
158-
Payload: &secretmanagerpb.SecretPayload{
159-
Data: []byte("test-value"),
160-
},
161-
}
162-
return req.Parent == expectedReq.Parent &&
163-
string(req.Payload.Data) == string(expectedReq.Payload.Data)
164-
})).Return(&secretmanagerpb.SecretVersion{}, nil)
165-
},
152+
mockFn: gsmClientSecretCreationMock(
153+
"projects/test-project",
154+
"test-prefix_dev_usw2_app_service_config-key",
155+
`"test-value"`,
156+
nil,
157+
),
166158
wantErr: false,
167159
},
168160
{
@@ -171,24 +163,26 @@ func TestGSMStore_Set(t *testing.T) {
171163
component: "app/service",
172164
key: "config-key",
173165
value: "test-value",
174-
mockFn: func(m *MockGSMClient) {
175-
m.On("CreateSecret", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.CreateSecretRequest) bool {
176-
expectedReq := &secretmanagerpb.CreateSecretRequest{
177-
Parent: "projects/test-project",
178-
SecretId: "test-prefix_dev_usw2_app_service_config-key",
179-
Secret: &secretmanagerpb.Secret{
180-
Replication: &secretmanagerpb.Replication{
181-
Replication: &secretmanagerpb.Replication_Automatic_{
182-
Automatic: &secretmanagerpb.Replication_Automatic{},
183-
},
184-
},
185-
},
186-
}
187-
return req.Parent == expectedReq.Parent &&
188-
req.SecretId == expectedReq.SecretId &&
189-
req.Secret.GetReplication().GetAutomatic() != nil
190-
})).Return(nil, fmt.Errorf("internal error: %w", ErrInternalError))
191-
},
166+
mockFn: gsmClientSecretCreationMock(
167+
"projects/test-project",
168+
"test-prefix_dev_usw2_app_service_config-key",
169+
`"test-value"`,
170+
fmt.Errorf("internal error: %w", ErrInternalError)),
171+
wantErr: true,
172+
},
173+
174+
{
175+
name: "create secret - transient error",
176+
stack: "dev-usw2",
177+
component: "app/service",
178+
key: "config-key",
179+
value: "test-value",
180+
mockFn: gsmClientSecretCreationMock(
181+
"projects/test-project",
182+
"test-prefix_dev_usw2_app_service_config-key",
183+
`"test-value"`,
184+
fmt.Errorf("transient error: %w", ErrTransientError),
185+
),
192186
wantErr: true,
193187
},
194188
{
@@ -197,47 +191,54 @@ func TestGSMStore_Set(t *testing.T) {
197191
component: "app/service",
198192
key: "config-key",
199193
value: "test-value",
200-
mockFn: func(m *MockGSMClient) {
201-
m.On("CreateSecret", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.CreateSecretRequest) bool {
202-
expectedReq := &secretmanagerpb.CreateSecretRequest{
203-
Parent: "projects/test-project",
204-
SecretId: "test-prefix_dev_usw2_app_service_config-key",
205-
Secret: &secretmanagerpb.Secret{
206-
Replication: &secretmanagerpb.Replication{
207-
Replication: &secretmanagerpb.Replication_Automatic_{
208-
Automatic: &secretmanagerpb.Replication_Automatic{},
209-
},
210-
},
211-
},
212-
}
213-
return req.Parent == expectedReq.Parent &&
214-
req.SecretId == expectedReq.SecretId &&
215-
req.Secret.GetReplication().GetAutomatic() != nil
216-
})).Return(&secretmanagerpb.Secret{
217-
Name: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key",
218-
}, nil)
219-
220-
m.On("AddSecretVersion", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.AddSecretVersionRequest) bool {
221-
expectedReq := &secretmanagerpb.AddSecretVersionRequest{
222-
Parent: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key",
223-
Payload: &secretmanagerpb.SecretPayload{
224-
Data: []byte("test-value"),
225-
},
226-
}
227-
return req.Parent == expectedReq.Parent &&
228-
string(req.Payload.Data) == string(expectedReq.Payload.Data)
229-
})).Return(nil, fmt.Errorf("internal error: %w", ErrInternalError))
230-
},
194+
mockFn: gsmClientSecretCreationMock(
195+
"projects/test-project",
196+
"test-prefix_dev_usw2_app_service_config-key",
197+
`"test-value"`,
198+
fmt.Errorf("internal error: %w", ErrInternalError),
199+
),
231200
wantErr: true,
232201
},
233202
{
234-
name: "invalid value type",
203+
name: "successful set with int",
235204
stack: "dev-usw2",
236205
component: "app/service",
237206
key: "config-key",
238-
value: 123, // Not a string
239-
mockFn: func(m *MockGSMClient) {},
240-
wantErr: true,
207+
value: 123,
208+
mockFn: gsmClientSecretCreationMock(
209+
"projects/test-project",
210+
"test-prefix_dev_usw2_app_service_config-key",
211+
`123`,
212+
nil,
213+
),
214+
wantErr: false,
215+
},
216+
{
217+
name: "successful_set_with_slice",
218+
stack: "dev_usw2",
219+
component: "app/service",
220+
key: "slice-key",
221+
value: []string{"value1", "value2", "value3"},
222+
mockFn: gsmClientSecretCreationMock(
223+
"projects/test-project",
224+
"test-prefix_dev_usw2_app_service_slice-key",
225+
`["value1","value2","value3"]`,
226+
nil,
227+
),
228+
},
229+
{
230+
name: "successful_set_with_map",
231+
stack: "dev_usw2",
232+
component: "app/service",
233+
key: "map-key",
234+
value: map[string]interface{}{"key1": "value1", "key2": 42, "key3": true},
235+
236+
mockFn: gsmClientSecretCreationMock(
237+
"projects/test-project",
238+
"test-prefix_dev_usw2_app_service_map-key",
239+
`{"key1":"value1","key2":42,"key3":true}`,
240+
nil,
241+
),
241242
},
242243
{
243244
name: "empty stack",
@@ -319,7 +320,27 @@ func TestGSMStore_Get(t *testing.T) {
319320
return req.Name == expectedReq.Name
320321
})).Return(&secretmanagerpb.AccessSecretVersionResponse{
321322
Payload: &secretmanagerpb.SecretPayload{
322-
Data: []byte("test-value"),
323+
Data: []byte(`"test-value"`),
324+
},
325+
}, nil)
326+
},
327+
want: "test-value",
328+
wantErr: false,
329+
},
330+
{
331+
name: "successful get - legacy value without json",
332+
stack: "dev-usw2",
333+
component: "app/service",
334+
key: "config-key",
335+
mockFn: func(m *MockGSMClient) {
336+
m.On("AccessSecretVersion", mock.Anything, mock.MatchedBy(func(req *secretmanagerpb.AccessSecretVersionRequest) bool {
337+
expectedReq := &secretmanagerpb.AccessSecretVersionRequest{
338+
Name: "projects/test-project/secrets/test-prefix_dev_usw2_app_service_config-key/versions/latest",
339+
}
340+
return req.Name == expectedReq.Name
341+
})).Return(&secretmanagerpb.AccessSecretVersionResponse{
342+
Payload: &secretmanagerpb.SecretPayload{
343+
Data: []byte(`test-value`),
323344
},
324345
}, nil)
325346
},

0 commit comments

Comments
 (0)