Skip to content

Commit 56ebe02

Browse files
tas50claude
andcommitted
🧹 Address PR review: deterministic binding IDs, permission handling, and resource constants
- Use role name instead of iteration index for IAM binding IDs to ensure deterministic caching across API calls - Handle gRPC PermissionDenied errors gracefully in iamPolicy() methods so scans don't fail when caller lacks getIamPolicy permission - Replace all string literal resource types with generated constants Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 parent a67f7d9 commit 56ebe02

File tree

2 files changed

+27
-20
lines changed

2 files changed

+27
-20
lines changed

‎providers/gcp/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ require (
281281
golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect
282282
google.golang.org/genproto/googleapis/api v0.0.0-20260122232226-8e98ce8d340d // indirect
283283
google.golang.org/genproto/googleapis/rpc v0.0.0-20260209200024-4cfbd4190f57 // indirect
284-
google.golang.org/grpc v1.79.1 // indirect
284+
google.golang.org/grpc v1.79.1
285285
gopkg.in/warnings.v0 v0.1.2 // indirect
286286
gopkg.in/yaml.v3 v3.0.1 // indirect
287287
howett.net/plist v1.0.1 // indirect

‎providers/gcp/resources/pubsub.go‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"context"
88
"errors"
99
"fmt"
10-
"strconv"
1110
"time"
1211

1312
"cloud.google.com/go/pubsub"
@@ -18,6 +17,8 @@ import (
1817
"go.mondoo.com/mql/v13/types"
1918
"google.golang.org/api/iterator"
2019
"google.golang.org/api/option"
20+
"google.golang.org/grpc/codes"
21+
"google.golang.org/grpc/status"
2122
)
2223

2324
func (g *mqlGcpProjectPubsubService) id() (string, error) {
@@ -50,7 +51,7 @@ func (g *mqlGcpProject) pubsub() (*mqlGcpProjectPubsubService, error) {
5051
}
5152
projectId := g.Id.Data
5253

53-
res, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService", map[string]*llx.RawData{
54+
res, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubService, map[string]*llx.RawData{
5455
"projectId": llx.StringData(projectId),
5556
})
5657
if err != nil {
@@ -168,7 +169,7 @@ func (g *mqlGcpProjectPubsubService) topics() ([]any, error) {
168169
if err != nil {
169170
return nil, err
170171
}
171-
mqlTopic, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.topic", map[string]*llx.RawData{
172+
mqlTopic, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceTopic, map[string]*llx.RawData{
172173
"projectId": llx.StringData(projectId),
173174
"name": llx.StringData(t.ID()),
174175
})
@@ -213,19 +214,19 @@ func (g *mqlGcpProjectPubsubServiceTopic) config() (*mqlGcpProjectPubsubServiceT
213214
return nil, err
214215
}
215216

216-
messageStoragePolicy, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.topic.config.messagestoragepolicy", map[string]*llx.RawData{
217+
messageStoragePolicy, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceTopicConfigMessagestoragepolicy, map[string]*llx.RawData{
217218
"configId": llx.StringData(pubsubConfigId(projectId, t.ID())),
218219
"allowedPersistenceRegions": llx.ArrayData(convert.SliceAnyToInterface(cfg.MessageStoragePolicy.AllowedPersistenceRegions), types.String),
219220
})
220221
if err != nil {
221222
return nil, err
222223
}
223-
res, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.topic.config", map[string]*llx.RawData{
224+
res, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceTopicConfig, map[string]*llx.RawData{
224225
"projectId": llx.StringData(projectId),
225226
"topicName": llx.StringData(t.ID()),
226227
"labels": llx.MapData(convert.MapToInterfaceMap(cfg.Labels), types.String),
227228
"kmsKeyName": llx.StringData(cfg.KMSKeyName),
228-
"messageStoragePolicy": llx.ResourceData(messageStoragePolicy, "gcp.project.pubsubService.topic.config.messagestoragepolicy"),
229+
"messageStoragePolicy": llx.ResourceData(messageStoragePolicy, ResourceGcpProjectPubsubServiceTopicConfigMessagestoragepolicy),
229230
"state": llx.StringData(topicStateToString(cfg.State)),
230231
})
231232
if err != nil {
@@ -266,7 +267,7 @@ func (g *mqlGcpProjectPubsubService) subscriptions() ([]any, error) {
266267
if err != nil {
267268
return nil, err
268269
}
269-
mqlSub, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.subscription", map[string]*llx.RawData{
270+
mqlSub, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceSubscription, map[string]*llx.RawData{
270271
"projectId": llx.StringData(projectId),
271272
"name": llx.StringData(s.ID()),
272273
})
@@ -311,15 +312,15 @@ func (g *mqlGcpProjectPubsubServiceSubscription) config() (*mqlGcpProjectPubsubS
311312
return nil, err
312313
}
313314

314-
topic, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.topic", map[string]*llx.RawData{
315+
topic, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceTopic, map[string]*llx.RawData{
315316
"projectId": llx.StringData(projectId),
316317
"name": llx.StringData(cfg.Topic.ID()),
317318
})
318319
if err != nil {
319320
return nil, err
320321
}
321322

322-
pushConfig, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.subscription.config.pushconfig", map[string]*llx.RawData{
323+
pushConfig, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceSubscriptionConfigPushconfig, map[string]*llx.RawData{
323324
"configId": llx.StringData(pubsubConfigId(projectId, s.ID())),
324325
"endpoint": llx.StringData(cfg.PushConfig.Endpoint),
325326
"attributes": llx.MapData(convert.MapToInterfaceMap(cfg.PushConfig.Attributes), types.String),
@@ -331,11 +332,11 @@ func (g *mqlGcpProjectPubsubServiceSubscription) config() (*mqlGcpProjectPubsubS
331332
if exp, ok := cfg.ExpirationPolicy.(time.Duration); ok {
332333
expPolicy = llx.DurationToTime(int64(exp.Seconds()))
333334
}
334-
res, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.subscription.config", map[string]*llx.RawData{
335+
res, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceSubscriptionConfig, map[string]*llx.RawData{
335336
"projectId": llx.StringData(projectId),
336337
"subscriptionName": llx.StringData(s.ID()),
337-
"topic": llx.ResourceData(topic, "gcp.project.pubsubService.topic"),
338-
"pushConfig": llx.ResourceData(pushConfig, "gcp.project.pubsubService.subscription.config.pushconfig"),
338+
"topic": llx.ResourceData(topic, ResourceGcpProjectPubsubServiceTopic),
339+
"pushConfig": llx.ResourceData(pushConfig, ResourceGcpProjectPubsubServiceSubscriptionConfigPushconfig),
339340
"ackDeadline": llx.TimeData(llx.DurationToTime(int64(cfg.AckDeadline.Seconds()))),
340341
"retainAckedMessages": llx.BoolData(cfg.RetainAckedMessages),
341342
"retentionDuration": llx.TimeData(llx.DurationToTime(int64(cfg.RetentionDuration.Seconds()))),
@@ -386,7 +387,7 @@ func (g *mqlGcpProjectPubsubService) snapshots() ([]any, error) {
386387
return nil, err
387388
}
388389

389-
topic, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.topic", map[string]*llx.RawData{
390+
topic, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceTopic, map[string]*llx.RawData{
390391
"id": llx.StringData(s.Topic.ID()),
391392
"projectId": llx.StringData(projectId),
392393
"name": llx.StringData(s.Topic.ID()),
@@ -395,11 +396,11 @@ func (g *mqlGcpProjectPubsubService) snapshots() ([]any, error) {
395396
return nil, err
396397
}
397398

398-
mqlSub, err := CreateResource(g.MqlRuntime, "gcp.project.pubsubService.snapshot", map[string]*llx.RawData{
399+
mqlSub, err := CreateResource(g.MqlRuntime, ResourceGcpProjectPubsubServiceSnapshot, map[string]*llx.RawData{
399400
"id": llx.StringData(s.ID()),
400401
"projectId": llx.StringData(projectId),
401402
"name": llx.StringData(s.ID()),
402-
"topic": llx.ResourceData(topic, "gcp.project.pubsubService.topic"),
403+
"topic": llx.ResourceData(topic, ResourceGcpProjectPubsubServiceTopic),
403404
"expiration": llx.TimeData(s.Expiration),
404405
})
405406
if err != nil {
@@ -439,15 +440,18 @@ func (g *mqlGcpProjectPubsubServiceTopic) iamPolicy() ([]any, error) {
439440

440441
policy, err := pubsubSvc.Topic(name).IAM().Policy(ctx)
441442
if err != nil {
443+
if s, ok := status.FromError(err); ok && s.Code() == codes.PermissionDenied {
444+
return nil, nil
445+
}
442446
return nil, err
443447
}
444448

445449
bindings := policy.InternalProto.Bindings
446450
res := make([]any, 0, len(bindings))
447451
topicPath := fmt.Sprintf("projects/%s/topics/%s", projectId, name)
448-
for i, b := range bindings {
452+
for _, b := range bindings {
449453
mqlBinding, err := CreateResource(g.MqlRuntime, ResourceGcpResourcemanagerBinding, map[string]*llx.RawData{
450-
"id": llx.StringData(topicPath + "-" + strconv.Itoa(i)),
454+
"id": llx.StringData(topicPath + "/" + b.Role),
451455
"role": llx.StringData(b.Role),
452456
"members": llx.ArrayData(convert.SliceAnyToInterface(b.Members), types.String),
453457
})
@@ -487,15 +491,18 @@ func (g *mqlGcpProjectPubsubServiceSubscription) iamPolicy() ([]any, error) {
487491

488492
policy, err := pubsubSvc.Subscription(name).IAM().Policy(ctx)
489493
if err != nil {
494+
if s, ok := status.FromError(err); ok && s.Code() == codes.PermissionDenied {
495+
return nil, nil
496+
}
490497
return nil, err
491498
}
492499

493500
bindings := policy.InternalProto.Bindings
494501
res := make([]any, 0, len(bindings))
495502
subPath := fmt.Sprintf("projects/%s/subscriptions/%s", projectId, name)
496-
for i, b := range bindings {
503+
for _, b := range bindings {
497504
mqlBinding, err := CreateResource(g.MqlRuntime, ResourceGcpResourcemanagerBinding, map[string]*llx.RawData{
498-
"id": llx.StringData(subPath + "-" + strconv.Itoa(i)),
505+
"id": llx.StringData(subPath + "/" + b.Role),
499506
"role": llx.StringData(b.Role),
500507
"members": llx.ArrayData(convert.SliceAnyToInterface(b.Members), types.String),
501508
})

0 commit comments

Comments
 (0)