Skip to content

Commit fa50ce1

Browse files
xiangshen-dkxiangshen-dk
authored
V1.6.4 followup (#194)
* update based on submission feedback
1 parent c05324c commit fa50ce1

3 files changed

Lines changed: 39 additions & 85 deletions

File tree

‎pkg/plugin/cloudlogging/client.go‎

Lines changed: 23 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -74,56 +74,49 @@ func universeDomainOpts(universeDomain string) []option.ClientOption {
7474
return []option.ClientOption{option.WithUniverseDomain(universeDomain)}
7575
}
7676

77-
// NewClient creates a new Client using jsonCreds for authentication
78-
func NewClient(ctx context.Context, jsonCreds []byte, universeDomain string) (*Client, error) {
79-
opts := append([]option.ClientOption{
80-
option.WithCredentialsJSON(jsonCreds),
81-
option.WithUserAgent("googlecloud-logging-datasource"),
82-
}, universeDomainOpts(universeDomain)...)
83-
84-
client, err := logging.NewClient(ctx, opts...)
77+
// newClientFromOpts builds the three GCP clients that back Client. If any
78+
// constructor fails, the previously-created clients are closed so we don't
79+
// leak gRPC connections.
80+
func newClientFromOpts(ctx context.Context, opts []option.ClientOption) (*Client, error) {
81+
lClient, err := logging.NewClient(ctx, opts...)
8582
if err != nil {
8683
return nil, err
8784
}
8885
rClient, err := resourcemanager.NewProjectsClient(ctx, opts...)
8986
if err != nil {
87+
lClient.Close()
9088
return nil, err
9189
}
92-
9390
configClient, err := logging.NewConfigClient(ctx, opts...)
9491
if err != nil {
92+
rClient.Close()
93+
lClient.Close()
9594
return nil, err
9695
}
9796
return &Client{
98-
lClient: client,
97+
lClient: lClient,
9998
rClient: rClient,
10099
configClient: configClient,
101100
}, nil
102101
}
103102

103+
// NewClient creates a new Client using jsonCreds for authentication
104+
func NewClient(ctx context.Context, jsonCreds []byte, universeDomain string) (*Client, error) {
105+
opts := append([]option.ClientOption{
106+
option.WithCredentialsJSON(jsonCreds),
107+
option.WithUserAgent("googlecloud-logging-datasource"),
108+
}, universeDomainOpts(universeDomain)...)
109+
110+
return newClientFromOpts(ctx, opts)
111+
}
112+
104113
// NewClient creates a new Clients using GCE metadata for authentication
105114
func NewClientWithGCE(ctx context.Context, universeDomain string) (*Client, error) {
106115
opts := append([]option.ClientOption{
107116
option.WithUserAgent("googlecloud-logging-datasource"),
108117
}, universeDomainOpts(universeDomain)...)
109118

110-
client, err := logging.NewClient(ctx, opts...)
111-
if err != nil {
112-
return nil, err
113-
}
114-
rClient, err := resourcemanager.NewProjectsClient(ctx, opts...)
115-
if err != nil {
116-
return nil, err
117-
}
118-
configClient, err := logging.NewConfigClient(ctx, opts...)
119-
if err != nil {
120-
return nil, err
121-
}
122-
return &Client{
123-
lClient: client,
124-
rClient: rClient,
125-
configClient: configClient,
126-
}, nil
119+
return newClientFromOpts(ctx, opts)
127120
}
128121

129122
// NewClient creates a new Clients using service account impersonation
@@ -154,23 +147,7 @@ func NewClientWithImpersonation(ctx context.Context, jsonCreds []byte, impersona
154147
option.WithUserAgent("googlecloud-logging-datasource"),
155148
}, universeDomainOpts(universeDomain)...)
156149

157-
client, err := logging.NewClient(ctx, opts...)
158-
if err != nil {
159-
return nil, err
160-
}
161-
rClient, err := resourcemanager.NewProjectsClient(ctx, opts...)
162-
if err != nil {
163-
return nil, err
164-
}
165-
configClient, err := logging.NewConfigClient(ctx, opts...)
166-
if err != nil {
167-
return nil, err
168-
}
169-
return &Client{
170-
lClient: client,
171-
rClient: rClient,
172-
configClient: configClient,
173-
}, nil
150+
return newClientFromOpts(ctx, opts)
174151
}
175152

176153
// NewClientWithAccessToken creates a new Client using an access token for authentication.
@@ -183,26 +160,7 @@ func NewClientWithAccessToken(ctx context.Context, accessToken string, universeD
183160
option.WithUserAgent("googlecloud-logging-datasource"),
184161
}, universeDomainOpts(universeDomain)...)
185162

186-
client, err := logging.NewClient(ctx, opts...)
187-
if err != nil {
188-
return nil, err
189-
}
190-
191-
rClient, err := resourcemanager.NewProjectsClient(ctx, opts...)
192-
if err != nil {
193-
return nil, err
194-
}
195-
196-
configClient, err := logging.NewConfigClient(ctx, opts...)
197-
if err != nil {
198-
return nil, err
199-
}
200-
201-
return &Client{
202-
lClient: client,
203-
rClient: rClient,
204-
configClient: configClient,
205-
}, nil
163+
return newClientFromOpts(ctx, opts)
206164
}
207165

208166
// NewClientWithPassThrough creates a new Clients using Oauth browser credentials
@@ -221,23 +179,7 @@ func NewClientWithPassThrough(ctx context.Context, headers map[string]string, un
221179
option.WithUserAgent("googlecloud-logging-datasource"),
222180
}, universeDomainOpts(universeDomain)...)
223181

224-
client, err := logging.NewClient(ctx, opts...)
225-
if err != nil {
226-
return nil, err
227-
}
228-
rClient, err := resourcemanager.NewProjectsClient(ctx, opts...)
229-
if err != nil {
230-
return nil, err
231-
}
232-
configClient, err := logging.NewConfigClient(ctx, opts...)
233-
if err != nil {
234-
return nil, err
235-
}
236-
return &Client{
237-
lClient: client,
238-
rClient: rClient,
239-
configClient: configClient,
240-
}, nil
182+
return newClientFromOpts(ctx, opts)
241183
}
242184

243185
// Close closes the underlying connection to the GCP API

‎pkg/plugin/cloudlogging/cloudlogging.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ func GetLogLabels(entry *loggingpb.LogEntry) data.Labels {
9696
if strings.HasSuffix(typeUrl, "AuditLog") {
9797
var a alpb.AuditLog
9898
if err := t.ProtoPayload.UnmarshalTo(&a); err != nil {
99-
log.DefaultLogger.Error("Could not get AuditLog payload out of LogEntry: %v", err)
99+
log.DefaultLogger.Error("Could not get AuditLog payload out of LogEntry", "error", err)
100100
} else {
101101
byteArr, _ := json.Marshal(a)
102102
var inInterface map[string]*structpb.Value
@@ -108,7 +108,7 @@ func GetLogLabels(entry *loggingpb.LogEntry) data.Labels {
108108
} else if strings.HasSuffix(typeUrl, "RequestLog") {
109109
var r rlpb.RequestLog
110110
if err := t.ProtoPayload.UnmarshalTo(&r); err != nil {
111-
log.DefaultLogger.Error("Could not get RequestLog payload out of LogEntry: %v", err)
111+
log.DefaultLogger.Error("Could not get RequestLog payload out of LogEntry", "error", err)
112112
} else {
113113
byteArr, _ := json.Marshal(r)
114114
var inInterface map[string]*structpb.Value

‎pkg/plugin/plugin.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,13 @@ func (d *CloudLoggingDatasource) CallResource(ctx context.Context, req *backend.
267267
})
268268
}
269269
} else if resource == "logbuckets" {
270-
reqUrl, _ := url.Parse(req.URL)
270+
reqUrl, err := url.Parse(req.URL)
271+
if err != nil {
272+
return sender.Send(&backend.CallResourceResponse{
273+
Status: http.StatusBadRequest,
274+
Body: jsonErrorBody("Invalid request URL"),
275+
})
276+
}
271277
params, _ := url.ParseQuery(reqUrl.RawQuery)
272278

273279
if params.Get("ProjectId") == "" {
@@ -294,7 +300,13 @@ func (d *CloudLoggingDatasource) CallResource(ctx context.Context, req *backend.
294300
})
295301
}
296302
} else if resource == "logviews" {
297-
reqUrl, _ := url.Parse(req.URL)
303+
reqUrl, err := url.Parse(req.URL)
304+
if err != nil {
305+
return sender.Send(&backend.CallResourceResponse{
306+
Status: http.StatusBadRequest,
307+
Body: jsonErrorBody("Invalid request URL"),
308+
})
309+
}
298310
params, _ := url.ParseQuery(reqUrl.RawQuery)
299311

300312
if params.Get("ProjectId") == "" {

0 commit comments

Comments
 (0)