feat(internal/surfer): improve standard behaviors autogeneration#3914
feat(internal/surfer): improve standard behaviors autogeneration#3914quirogas wants to merge 18 commits intogoogleapis:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request significantly improves the autogeneration of gcloud command configurations by dynamically deriving collection paths, output formats for list commands, and async behavior for LROs, leveraging AIP annotations. A critical security concern, aligning with the rule to sanitize inputs for code generation templates, has been identified: a potential DSL injection vulnerability in the new newFormat and newTable functions due to unsanitized field names from API definitions, which could manipulate CLI output. There are also existing path traversal and code injection points in generate.go that rely on unsanitized strings. It is strongly recommended to sanitize all field names and conduct a comprehensive security audit of the entire generator for safe handling of strings derived from external API definitions. Minor code clarity improvements were also suggested.
…uirogas/librarian into feat-standard-behaviors-remaining-3649
…uirogas/librarian into feat-standard-behaviors-remaining-3649
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3914 +/- ##
=======================================
Coverage 83.35% 83.35%
=======================================
Files 69 69
Lines 6188 6188
=======================================
Hits 5158 5158
Misses 671 671
Partials 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Improve dynamic generation of gcloud command collection paths and standard command behaviors. Previously, the generator relied on hardcoded service prefixes and missed critical configurations for standard AIP patterns, leading to incorrect
request.collectionpaths for new services and incomplete command definitions.This PR addresses these issues by:
output.formattables for List commands.request.methoddirectly from the AIP-136 HTTP custom verb (e.g.,:backup) rather than the RPC name. This fixes invocation errors where gcloud would attempt to call a method likebackupInstanceinstead ofbackup.extract_resource_resultbased on whether the operation returns the resource itself (Create/Update) or just metadata.Changes
internal/surfer/gcloud/builder.goDynamic Collection Paths:
newCollectionPathto dynamically constructrequest.collectionandasync.collectionby parsing AIP-127 HTTP annotations.parallelstore.) with dynamic extraction fromservice.DefaultHost.v1) and handle complex variables (e.g.,{name=projects/*/locations/*/instances/*}) viaextractPathFromSegments.async.collectionby identifying the resource parent and appending.operations(AIP-151), replacing static defaults.Standard Command Behaviors:
newOutputConfigandnewFormatto automatically generatetable(...)output formats for List commands. It recursively inspects the response message to include all top-level scalar fields and timestamps, ensuring useful default output tables.newRequestto prioritize the custom HTTP verb (AIP-136, e.g.,:backup) over the RPC name. This ensures generatedrequest.methodvalues match the backend expectations (e.g.,backupvsbackupInstance).newAsyncwith a heuristic to setextract_resource_result: truewhen the LRO'sresponse_typematches the method's resource type, enabling polling behavior for Create/Update operations.internal/surfer/gcloud/builder_test.goTestNewCollectionPathto validate path generation across various scenarios:{name=...}).TestNewOutputConfigto verify table format generation from message fields.internal/surfer/gcloud/command.goomitemptytag fromExtractResourceResultto ensure explicit boolean values are generated in the YAML, as expected by the gcloud framework for LROs.internal/surfer/gcloud/utils/method.go&method_test.goIsStandardMethodandIsCustomMethodhelpers to support cleaner dispatch logic in the builder.Fixes #3290, #3649