⭐ AWS: 18 new resources, ~70 new fields on existing resources, 23 services#6667
⭐ AWS: 18 new resources, ~70 new fields on existing resources, 23 services#6667
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
578b97d to
9437e39
Compare
This comment has been minimized.
This comment has been minimized.
Superseded by new review
Superseded by new review
This comment has been minimized.
This comment has been minimized.
339d214 to
2d43a10
Compare
49e77f3 to
094a7c0
Compare
There was a problem hiding this comment.
Well-structured PR with comprehensive resource additions, but CodeBuild timeout defaults need correction
Additional findings (file/line not in diff):
- 🔵
providers/aws/resources/aws.lr:1057— Changed IAM instance profile display defaults fromarn instanceProfileIdtoinstanceProfileId instanceProfileName. While more user-friendly, this changes existing behavior. Consider noting in changelog.
| args["privilegedMode"] = llx.BoolData(*project.Environment.PrivilegedMode) | ||
| } else { | ||
| args["privilegedMode"] = llx.BoolData(false) | ||
| } |
There was a problem hiding this comment.
🔴 critical — Using 0 as default for timeoutInMinutes is incorrect. AWS CodeBuild default is 60 minutes. Use llx.IntDataDefault(project.TimeoutInMinutes, 60) or handle nil explicitly. Similarly, queuedTimeoutInMinutes defaults to 480 (8 hours), not 0.
| kmsKey() aws.kms.key | ||
| // Whether SageMaker provides internet access to the instance | ||
| directInternetAccess string | ||
| directInternetAccess bool |
There was a problem hiding this comment.
🟡 warning — Changing directInternetAccess from string to bool is a breaking change. Ensure this is documented in release notes and that the implementation correctly maps SDK string values ("Enabled"/"Disabled") to bool.
Superseded by new review
|
Unable to complete the code review. Please try again or reduce the PR size. |
There was a problem hiding this comment.
Well-structured additions with comprehensive resource definitions; two breaking changes need attention
Additional findings (file/line not in diff):
- 🟡
providers/aws/resources/aws.lr:1306— Breaking change:directInternetAccesschanged fromstringtobool. AWS SDK returns "Enabled"/"Disabled" strings, so the implementation must convert these. This will break existing queries expecting a string value. Confirm this is intentional for v13 and ensure the implementation handles the conversion.
| dataRepositoryAssociations []dict | ||
| // Resource tags | ||
| tags map[string]string | ||
| // Region where the cache exists |
There was a problem hiding this comment.
🟡 warning — Breaking change: The tags field was removed from aws.fsx.cache. This will break existing queries using cache.tags. Verify this removal is intentional and document in the PR if this is part of v13 breaking changes.
| "agentVersion": llx.StringDataPtr(instance.AgentVersion), | ||
| "lastPingedAt": llx.TimeDataPtr(instance.LastPingDateTime), | ||
| "computerName": llx.StringDataPtr(instance.ComputerName), |
There was a problem hiding this comment.
🔵 suggestion — Implementation looks correct using llx.*DataPtr helpers for nil-safe pointer handling. Verify these fields exist in the AWS SDK's InstanceInformation type to avoid runtime panics.
New typed resources: - aws.fsx.volume: FSx ONTAP and OpenZFS volumes with flattened config fields - aws.ec2.transitgateway: Transit gateways with flattened Options fields - aws.neptune.snapshot: Neptune DB cluster snapshots (per-cluster) - aws.redshift.snapshot: Redshift cluster snapshots (per-cluster) - aws.backup.plan: Backup plans with lazy-loaded rules - aws.backup.plan.rule: Backup plan rules with schedule, lifecycle, copy actions - aws.backup.plan.rule.copyAction: Cross-region/account backup copies - aws.backup.plan.advancedBackupSetting: Resource-specific backup options - aws.backup.lifecycle: Typed lifecycle settings (cold storage, deletion) - aws.elb.listener: ALB/NLB listeners with protocol, port, SSL policy - aws.elb.loadbalancer.attribute: Typed LB attributes (deletion protection, access logs, etc.) - aws.elb.targetgroup.attributes: Typed TG attributes (stickiness, algorithm, etc.) - aws.vpc.routetable.route: Typed VPC route table entries - aws.s3.bucket.encryptionRule: Typed S3 encryption rules - aws.s3.bucket.replicationRule: Typed S3 replication rules New fields on existing resources: - EC2: bootMode, sourceDestCheck, ipv6Address on instances; description on images - ECS: enableExecuteCommand, schedulingStrategy, platformFamily, cpu/memory on tasks - ELB: ipAddressType, listeners(), instances(), typed attributes on load balancers - RDS: licenseModel, maxAllocatedStorage, dedicatedLogVolume, storageThroughput, engineMode, earliestRestorableTime on clusters - IAM: path on users, roles, and groups - S3: typed encryptionRules() and replicationRules() on buckets - Lambda: state, codeSize, stateReason, lastUpdateStatus - CloudWatch: actionsEnabled on alarms; dataProtectionStatus, deletionProtection on log groups - CodeBuild: createdAt, modifiedAt, privilegedMode, encryptionKey, timeouts - ElastiCache: preferredMaintenanceWindow; subnets on serverless caches - SageMaker: rootAccess, minimumInstanceMetadataServiceVersion, subnet - SNS: kmsMasterKey on topics - SQS: deduplicationScope, fifoThroughputLimit on queues - SSM: agentVersion, lastPingedAt, computerName on instances - Redshift: clusterAvailabilityStatus, multiAZ, ipAddressType, snapshots() - ACM: renewalEligible, signatureAlgorithm on certificates - CloudFront: comment on distributions - DynamoDB: latestStreamLabel on tables - ECR: lifecyclePolicy, scanningFrequency on repositories - Neptune: snapshots() on clusters - FSx: volumes() on service - Backup: plans() on service Signed-off-by: Tim Smith <tsmith84@gmail.com>
SageMaker directInternetAccess/rootAccess and transit gateway autoAcceptSharedAttachments/defaultRouteTableAssociation/ defaultRouteTablePropagation/dnsSupport/multicastSupport/ vpnEcmpSupport are all two-state enums that map naturally to bool. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TimeoutInMinutes defaults to 60 and QueuedTimeoutInMinutes to 480, matching the actual AWS CodeBuild defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…S provider - Add explicit __id to transit gateway, ELB listener, ELB attributes, FSx cache/backup - Convert Redshift multiAZ from string to bool (missed in enum→bool conversion) - Add region field to aws.fsx.cache and aws.fsx.backup for multi-region queries - Remove fake tags field from aws.fsx.cache (SDK has no Tags on FileCache) - Eliminate duplicate S3 GetBucketReplication/GetBucketEncryption calls via sync.Once caching - Eliminate triplicate SNS GetTopicAttributes calls via fetchTopicAttributes helper - Replace fragile string matching with typed LifecyclePolicyNotFoundException in ECR Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the untyped dict with two new resources (aws.ecr.lifecyclePolicy and aws.ecr.lifecyclePolicy.rule) for type-safe MQL traversal. Also exposes lastEvaluatedAt from the SDK response, which was previously inaccessible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
These compile errors are pre-existing and unrelated to the PR under review. The PR only touches providers/aws/... files, while the failing tests live in:
providers/coordinator_test.goproviders/runtime_test.goproviders/os/connection/device/linux/device_manager_test.go
The root cause is that generated mock files are missing. The go:generate directives exist but the output files were never committed:
providers/coordinator.go declares:
//go:generate mockgen -source=./coordinator.go -destination=./mock_coordinator.go -package=providers
//go:generate mockgen -source=../providers-sdk/v1/plugin/interface.go -destination=./mock_plugin_interface.go -package=providers
//go:generate mockgen -source=../providers-sdk/v1/resources/schema.go -destination=./mock_schema.go -package=providersproviders/os/connection/snapshot/volumemounter.go declares:
//go:generate mockgen -source=./volumemounter.go -destination=./mock_volumemounter.go -package=snapshotThe generated files (mock_coordinator.go, mock_plugin_interface.go, mock_schema.go, mock_volumemounter.go) are absent from the repository. Running the following will fix it:
cd providers && go generate ./coordinator.go
cd providers/os/connection/snapshot && go generate ./volumemounter.goOr from the repo root:
make mql/generateThen commit the generated mock files. This issue exists independent of the current PR and should be addressed separately (or as a prerequisite before the next make test/go/plain run).
New Resources
aws.backup.lifecycleaws.backup.planaws.backup.plan.advancedBackupSettingaws.backup.plan.ruleaws.backup.plan.rule.copyActionaws.ec2.transitgatewayaws.ecr.lifecyclePolicyaws.ecr.lifecyclePolicy.ruleaws.elb.listeneraws.elb.loadbalancer.attributeaws.elb.targetgroup.attributesaws.fsx.volumeaws.iam.instanceProfileaws.neptune.snapshotaws.redshift.snapshotaws.s3.bucket.encryptionRuleaws.s3.bucket.replicationRuleaws.vpc.routetable.routeNew Fields on Existing Resources
aws.acm.certificate: renewalEligible, signatureAlgorithmaws.cloudfront.distribution: commentaws.cloudwatch.loggroup: dataProtectionStatus, deletionProtectionEnabledaws.cloudwatch.metricsalarm: actionsEnabledaws.codebuild.project: createdAt, encryptionKey, modifiedAt, privilegedMode, projectVisibility, queuedTimeoutInMinutes, serviceRole, timeoutInMinutesaws.dynamodb.table: latestStreamLabelaws.ec2.image: descriptionaws.ec2.instance: bootMode, ipv6Address, sourceDestCheckaws.ec2: transitGatewaysaws.ecr.repository: lifecyclePolicy (converted from dict to typed resource)aws.ecs.service: enableEcsManagedTags, enableExecuteCommand, healthCheckGracePeriodSeconds, platformFamily, platformVersion, schedulingStrategyaws.ecs.taskDefinition: cpu, memory, registeredAtaws.elasticache.cluster: preferredMaintenanceWindow, replicationGroupLogDeliveryEnabledaws.elasticache.serverlessCache: subnetsaws.elb.loadbalancer: instances, ipAddressType, listenersaws.elb.targetgroup: healthyThresholdCountaws.fsx.backup: regionaws.fsx.cache: regionaws.iam.group: pathaws.iam.role: pathaws.iam.user: pathaws.lambda.function: codeSize, lastUpdateStatus, state, stateReasonaws.neptune.cluster: snapshotsaws.rds.dbcluster: earliestRestorableTime, engineModeaws.rds.dbinstance: dbClusterIdentifier, dbiResourceId, dedicatedLogVolume, licenseModel, maxAllocatedStorage, storageThroughputaws.redshift.cluster: clusterAvailabilityStatus, ipAddressType, manualSnapshotRetentionPeriod, multiAZ, snapshots, totalStorageCapacityInMegaBytesaws.s3.bucket: encryptionRules, replicationRulesaws.sagemaker.notebookinstancedetails: minimumInstanceMetadataServiceVersion, rootAccess, subnetaws.sns.topic: kmsMasterKeyaws.sqs.queue: deduplicationScope, fifoThroughputLimitaws.ssm.instance: agentVersion, computerName, lastPingedAtaws.vpc.routetable: routeEntries