Skip to content

Commit b02a762

Browse files
pa250194squaremo
authored andcommitted
Added more tests and cleaned up GCP provider logic
Signed-off-by: pa250194 <[email protected]>
1 parent a46b0f5 commit b02a762

File tree

5 files changed

+124
-112
lines changed

5 files changed

+124
-112
lines changed

controllers/bucket_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/minio/minio-go/v7"
3030
"github.com/minio/minio-go/v7/pkg/credentials"
3131
"github.com/minio/minio-go/v7/pkg/s3utils"
32+
"google.golang.org/api/option"
3233
corev1 "k8s.io/api/core/v1"
3334
apimeta "k8s.io/apimachinery/pkg/api/meta"
3435
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -417,8 +418,7 @@ func (r *BucketReconciler) authGCP(ctx context.Context, bucket sourcev1.Bucket)
417418
if err := gcp.ValidateSecret(secret.Data, secret.Name); err != nil {
418419
return nil, err
419420
}
420-
serviceAccount := gcp.InitCredentialsWithSecret(secret.Data)
421-
client, err = gcp.NewClientWithSAKey(ctx, serviceAccount)
421+
client, err = gcp.NewClient(ctx, option.WithCredentialsJSON(secret.Data["serviceaccount"]))
422422
if err != nil {
423423
return nil, err
424424
}

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ require (
2323
github.com/go-git/go-git/v5 v5.4.2
2424
github.com/go-logr/logr v0.4.0
2525
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
26-
github.com/golang/mock v1.6.0 // indirect
2726
github.com/googleapis/gax-go/v2 v2.1.0 // indirect
2827
github.com/libgit2/git2go/v31 v31.6.1
2928
github.com/minio/minio-go/v7 v7.0.10

go.sum

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt
419419
github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw=
420420
github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4=
421421
github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8=
422-
github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
423422
github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
424423
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
425424
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=

pkg/gcp/gcp.go

Lines changed: 4 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package gcp
1818

1919
import (
2020
"context"
21-
"encoding/json"
2221
"errors"
2322
"fmt"
2423
"io"
@@ -30,13 +29,6 @@ import (
3029
"google.golang.org/api/option"
3130
)
3231

33-
const (
34-
ServiceAccount = "service_account"
35-
AuthUri = "https://accounts.google.com/o/oauth2/auth"
36-
TokenUri = "https://oauth2.googleapis.com/token"
37-
AuthProviderX509CertUrl = "https://www.googleapis.com/oauth2/v1/certs"
38-
)
39-
4032
var (
4133
// IteratorDone is returned when the looping of objects/content
4234
// has reached the end of the iteration.
@@ -61,98 +53,24 @@ type GCPClient struct {
6153
EndRange int64
6254
}
6355

64-
// CredentialsFile struct representing the GCP Service Account
65-
// JSON file.
66-
type CredentialsFile struct {
67-
Type string `json:"type"`
68-
ProjectID string `json:"project_id"`
69-
PrivateKeyID string `json:"private_key_id"`
70-
PrivateKey string `json:"private_key"`
71-
ClientEmail string `json:"client_email"`
72-
ClientID string `json:"client_id"`
73-
AuthUri string `json:"auth_uri"`
74-
TokenUri string `json:"token_uri"`
75-
AuthProviderX509CertUrl string `json:"auth_provider_x509_cert_url"`
76-
ClientX509CertUrl string `json:"client_x509_cert_url"`
77-
}
78-
7956
// NewClient creates a new GCP storage client
8057
// The Google Storage Client will automatically
8158
// look for the Google Application Credential environment variable
8259
// or look for the Google Application Credential file.
83-
func NewClient(ctx context.Context) (*GCPClient, error) {
84-
client, err := gcpStorage.NewClient(ctx)
85-
if err != nil {
86-
return nil, err
87-
}
88-
89-
return &GCPClient{Client: client, StartRange: 0, EndRange: -1}, nil
90-
}
91-
92-
// NewClientWithSAKey creates a new GCP storage client
93-
// It uses the provided JSON file with service account details
94-
// To authenticate.
95-
func NewClientWithSAKey(ctx context.Context, credentials *CredentialsFile) (*GCPClient, error) {
96-
saAccount, err := credentials.credentailsToJSON()
97-
if err != nil {
98-
return nil, err
99-
}
100-
101-
client, err := gcpStorage.NewClient(ctx, option.WithCredentialsJSON(saAccount))
60+
func NewClient(ctx context.Context, opts ...option.ClientOption) (*GCPClient, error) {
61+
client, err := gcpStorage.NewClient(ctx, opts...)
10262
if err != nil {
10363
return nil, err
10464
}
10565

10666
return &GCPClient{Client: client, StartRange: 0, EndRange: -1}, nil
10767
}
10868

109-
// credentailsToJSON converts GCP service account credentials struct to JSON.
110-
func (credentials *CredentialsFile) credentailsToJSON() ([]byte, error) {
111-
credentialsJSON, err := json.Marshal(credentials)
112-
if err != nil {
113-
return nil, err
114-
}
115-
116-
return credentialsJSON, nil
117-
}
118-
119-
// InitCredentialsWithSecret creates a new credential
120-
// by initializing a new CredentialsFile struct
121-
func InitCredentialsWithSecret(secret map[string][]byte) *CredentialsFile {
122-
return &CredentialsFile{
123-
Type: ServiceAccount,
124-
ProjectID: string(secret["projectid"]),
125-
PrivateKeyID: string(secret["privatekeyid"]),
126-
PrivateKey: string(secret["privatekey"]),
127-
ClientEmail: string(secret["clientemail"]),
128-
ClientID: string(secret["clientid"]),
129-
AuthUri: AuthUri,
130-
TokenUri: TokenUri,
131-
AuthProviderX509CertUrl: AuthProviderX509CertUrl,
132-
ClientX509CertUrl: string(secret["certurl"]),
133-
}
134-
}
135-
13669
// ValidateSecret validates the credential secrets
13770
// It ensures that needed secret fields are not missing.
13871
func ValidateSecret(secret map[string][]byte, name string) error {
139-
if _, exists := secret["projectid"]; !exists {
140-
return fmt.Errorf("invalid '%s' secret data: required fields 'projectid'", name)
141-
}
142-
if _, exists := secret["privatekeyid"]; !exists {
143-
return fmt.Errorf("invalid '%s' secret data: required fields 'privatekeyid'", name)
144-
}
145-
if _, exists := secret["privatekey"]; !exists {
146-
return fmt.Errorf("invalid '%s' secret data: required fields 'privatekey'", name)
147-
}
148-
if _, exists := secret["clientemail"]; !exists {
149-
return fmt.Errorf("invalid '%s' secret data: required fields 'clientemail'", name)
150-
}
151-
if _, exists := secret["clientid"]; !exists {
152-
return fmt.Errorf("invalid '%s' secret data: required fields 'clientid'", name)
153-
}
154-
if _, exists := secret["certurl"]; !exists {
155-
return fmt.Errorf("invalid '%s' secret data: required fields 'certurl'", name)
72+
if _, exists := secret["serviceaccount"]; !exists {
73+
return fmt.Errorf("invalid '%s' secret data: required fields 'serviceaccount'", name)
15674
}
15775

15876
return nil

pkg/gcp/gcp_test.go

Lines changed: 118 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -47,54 +47,61 @@ const (
4747
)
4848

4949
var (
50-
Client *gcpStorage.Client
50+
hc *http.Client
51+
client *gcpStorage.Client
52+
close func()
5153
err error
5254
)
5355

5456
func TestMain(m *testing.M) {
55-
hc, close := newTestServer(func(w http.ResponseWriter, r *http.Request) {
57+
hc, close = newTestServer(func(w http.ResponseWriter, r *http.Request) {
5658
io.Copy(ioutil.Discard, r.Body)
57-
w.WriteHeader(200)
5859
if r.RequestURI == fmt.Sprintf("/storage/v1/b/%s?alt=json&prettyPrint=false&projection=full", bucketName) {
60+
w.WriteHeader(200)
5961
response := getBucket()
60-
jsonedResp, err := json.Marshal(response)
62+
jsonResponse, err := json.Marshal(response)
6163
if err != nil {
62-
log.Fatalf("error marshalling resp %v\n", err)
64+
log.Fatalf("error marshalling response %v\n", err)
6365
}
64-
_, err = w.Write(jsonedResp)
66+
_, err = w.Write(jsonResponse)
6567
if err != nil {
66-
log.Fatalf("error writing jsonedResp %v\n", err)
68+
log.Fatalf("error writing jsonResponse %v\n", err)
6769
}
6870
} else if r.RequestURI == fmt.Sprintf("/storage/v1/b/%s/o/%s?alt=json&prettyPrint=false&projection=full", bucketName, objectName) {
71+
w.WriteHeader(200)
6972
response := getObject()
70-
jsonedResp, err := json.Marshal(response)
73+
jsonResponse, err := json.Marshal(response)
7174
if err != nil {
72-
log.Fatalf("error marshalling resp %v\n", err)
75+
log.Fatalf("error marshalling response %v\n", err)
7376
}
74-
_, err = w.Write(jsonedResp)
77+
_, err = w.Write(jsonResponse)
7578
if err != nil {
76-
log.Fatalf("error writing jsonedResp %v\n", err)
79+
log.Fatalf("error writing jsonResponse %v\n", err)
7780
}
7881
} else if r.RequestURI == fmt.Sprintf("/storage/v1/b/%s/o?alt=json&delimiter=&endOffset=&pageToken=&prefix=&prettyPrint=false&projection=full&startOffset=&versions=false", bucketName) {
82+
w.WriteHeader(200)
7983
response := getObject()
80-
jsonedResp, err := json.Marshal(response)
84+
jsonResponse, err := json.Marshal(response)
8185
if err != nil {
82-
log.Fatalf("error marshalling resp %v\n", err)
86+
log.Fatalf("error marshalling response %v\n", err)
8387
}
84-
_, err = w.Write(jsonedResp)
88+
_, err = w.Write(jsonResponse)
8589
if err != nil {
86-
log.Fatalf("error writing jsonedResp %v\n", err)
90+
log.Fatalf("error writing jsonResponse %v\n", err)
8791
}
8892
} else if r.RequestURI == fmt.Sprintf("/%s/test.yaml", bucketName) || r.RequestURI == fmt.Sprintf("/storage/v1/b/%s/o/%s?alt=json&prettyPrint=false&projection=full", bucketName, objectName) {
93+
w.WriteHeader(200)
8994
response := getObjectFile()
9095
_, err = w.Write([]byte(response))
9196
if err != nil {
92-
log.Fatalf("error writing jsonedResp %v\n", err)
97+
log.Fatalf("error writing response %v\n", err)
9398
}
99+
} else {
100+
w.WriteHeader(404)
94101
}
95102
})
96103
ctx := context.Background()
97-
Client, err = gcpStorage.NewClient(ctx, option.WithHTTPClient(hc))
104+
client, err = gcpStorage.NewClient(ctx, option.WithHTTPClient(hc))
98105
if err != nil {
99106
log.Fatal(err)
100107
}
@@ -103,9 +110,15 @@ func TestMain(m *testing.M) {
103110
os.Exit(run)
104111
}
105112

113+
func TestNewClient(t *testing.T) {
114+
gcpClient, err := gcp.NewClient(context.Background(), option.WithHTTPClient(hc))
115+
assert.NilError(t, err)
116+
assert.Assert(t, gcpClient != nil)
117+
}
118+
106119
func TestBucketExists(t *testing.T) {
107120
gcpClient := &gcp.GCPClient{
108-
Client: Client,
121+
Client: client,
109122
StartRange: 0,
110123
EndRange: -1,
111124
}
@@ -114,9 +127,21 @@ func TestBucketExists(t *testing.T) {
114127
assert.Assert(t, exists)
115128
}
116129

130+
func TestBucketNotExists(t *testing.T) {
131+
bucket := "notexistsbucket"
132+
gcpClient := &gcp.GCPClient{
133+
Client: client,
134+
StartRange: 0,
135+
EndRange: -1,
136+
}
137+
exists, err := gcpClient.BucketExists(context.Background(), bucket)
138+
assert.NilError(t, err)
139+
assert.Assert(t, !exists)
140+
}
141+
117142
func TestObjectAttributes(t *testing.T) {
118143
gcpClient := &gcp.GCPClient{
119-
Client: Client,
144+
Client: client,
120145
StartRange: 0,
121146
EndRange: -1,
122147
}
@@ -131,7 +156,7 @@ func TestObjectAttributes(t *testing.T) {
131156

132157
func TestListObjects(t *testing.T) {
133158
gcpClient := &gcp.GCPClient{
134-
Client: Client,
159+
Client: client,
135160
StartRange: 0,
136161
EndRange: -1,
137162
}
@@ -151,7 +176,7 @@ func TestFGetObject(t *testing.T) {
151176
assert.NilError(t, err)
152177
defer os.RemoveAll(tempDir)
153178
gcpClient := &gcp.GCPClient{
154-
Client: Client,
179+
Client: client,
155180
StartRange: 0,
156181
EndRange: -1,
157182
}
@@ -162,9 +187,41 @@ func TestFGetObject(t *testing.T) {
162187
}
163188
}
164189

190+
func TestFGetObjectNotExists(t *testing.T) {
191+
object := "notexists.txt"
192+
tempDir, err := os.MkdirTemp("", bucketName)
193+
assert.NilError(t, err)
194+
defer os.RemoveAll(tempDir)
195+
gcpClient := &gcp.GCPClient{
196+
Client: client,
197+
StartRange: 0,
198+
EndRange: -1,
199+
}
200+
localPath := filepath.Join(tempDir, object)
201+
err = gcpClient.FGetObject(context.Background(), bucketName, object, localPath)
202+
if err != io.EOF {
203+
assert.Error(t, err, "storage: object doesn't exist")
204+
}
205+
}
206+
207+
func TestFGetObjectDirectoryIsFileName(t *testing.T) {
208+
tempDir, err := os.MkdirTemp("", bucketName)
209+
defer os.RemoveAll(tempDir)
210+
assert.NilError(t, err)
211+
gcpClient := &gcp.GCPClient{
212+
Client: client,
213+
StartRange: 0,
214+
EndRange: -1,
215+
}
216+
err = gcpClient.FGetObject(context.Background(), bucketName, objectName, tempDir)
217+
if err != io.EOF {
218+
assert.Error(t, err, "filename is a directory")
219+
}
220+
}
221+
165222
func TestSetRange(t *testing.T) {
166223
gcpClient := &gcp.GCPClient{
167-
Client: Client,
224+
Client: client,
168225
StartRange: 0,
169226
EndRange: -1,
170227
}
@@ -173,6 +230,45 @@ func TestSetRange(t *testing.T) {
173230
assert.Equal(t, gcpClient.EndRange, int64(5))
174231
}
175232

233+
func TestValidateSecret(t *testing.T) {
234+
t.Parallel()
235+
testCases := []struct {
236+
title string
237+
secret map[string][]byte
238+
name string
239+
error bool
240+
}{
241+
{
242+
"Test Case 1",
243+
map[string][]byte{
244+
"serviceaccount": []byte("serviceaccount"),
245+
},
246+
"Service Account",
247+
false,
248+
},
249+
{
250+
"Test Case 2",
251+
map[string][]byte{
252+
"data": []byte("data"),
253+
},
254+
"Service Account",
255+
true,
256+
},
257+
}
258+
for _, testCase := range testCases {
259+
testCase := testCase
260+
t.Run(testCase.title, func(t *testing.T) {
261+
t.Parallel()
262+
err := gcp.ValidateSecret(testCase.secret, testCase.name)
263+
if testCase.error {
264+
assert.Error(t, err, fmt.Sprintf("invalid '%v' secret data: required fields 'serviceaccount'", testCase.name))
265+
} else {
266+
assert.NilError(t, err)
267+
}
268+
})
269+
}
270+
}
271+
176272
func newTestServer(handler func(w http.ResponseWriter, r *http.Request)) (*http.Client, func()) {
177273
ts := httptest.NewTLSServer(http.HandlerFunc(handler))
178274
tlsConf := &tls.Config{InsecureSkipVerify: true}

0 commit comments

Comments
 (0)